[PATCH 0/5] linux-user changes to run docker

YAMAMOTO Takashi posted 5 patches 2 years, 11 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/next-importer-push tags/patchew/20210524045412.15152-1-yamamoto@midokura.com
Maintainers: Laurent Vivier <laurent@vivier.eu>
There is a newer version of this series
linux-user/main.c    | 14 +++++++++++++-
linux-user/qemu.h    |  2 ++
linux-user/syscall.c | 43 ++++++++++++++++++++++++++++++++++++++++---
3 files changed, 55 insertions(+), 4 deletions(-)
[PATCH 0/5] linux-user changes to run docker
Posted by YAMAMOTO Takashi 2 years, 11 months ago
These patches, along with a few more hacks [1] I didn't include
in this patchset, allowed me to run arm64 and armv7 version of
dind image on amd64.

[1] https://github.com/yamt/qemu/tree/linux-user-for-docker

You can find my test setup here:
https://github.com/yamt/garbage/tree/master/binfmt-aarch64-install

YAMAMOTO Takashi (5):
  linux-user: handle /proc/self/exe for execve
  linux-uesr: make exec_path realpath
  linux-user: Fix the execfd case of /proc/self/exe open
  linux-user: dup the execfd on start up
  linux-user: Implement pivot_root

 linux-user/main.c    | 14 +++++++++++++-
 linux-user/qemu.h    |  2 ++
 linux-user/syscall.c | 43 ++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 55 insertions(+), 4 deletions(-)

-- 
2.21.1 (Apple Git-122.3)


Re: [PATCH 0/5] linux-user changes to run docker
Posted by Alex Bennée 2 years, 11 months ago
YAMAMOTO Takashi <yamamoto@midokura.com> writes:

> These patches, along with a few more hacks [1] I didn't include
> in this patchset, allowed me to run arm64 and armv7 version of
> dind image on amd64.
>
> [1] https://github.com/yamt/qemu/tree/linux-user-for-docker

Might be worth posting those patches next time (even if they have a RFC
or !MERGE in the title for now). I had a little noodle around with
testing and quickly found a few holes. It would be nice if we could have
a unit test to cover these various bits as I fear it will easily break
again. Feel free to use the following as a basis if you want:



> You can find my test setup here:
> https://github.com/yamt/garbage/tree/master/binfmt-aarch64-install
>
> YAMAMOTO Takashi (5):
>   linux-user: handle /proc/self/exe for execve
>   linux-uesr: make exec_path realpath
>   linux-user: Fix the execfd case of /proc/self/exe open
>   linux-user: dup the execfd on start up
>   linux-user: Implement pivot_root
>
>  linux-user/main.c    | 14 +++++++++++++-
>  linux-user/qemu.h    |  2 ++
>  linux-user/syscall.c | 43 ++++++++++++++++++++++++++++++++++++++++---
>  3 files changed, 55 insertions(+), 4 deletions(-)

I also had a go at cleaning up is_proc_self and Daniel greatly
simplified it.



-- 
Alex Bennée
Re: [PATCH 0/5] linux-user changes to run docker
Posted by Takashi Yamamoto 2 years, 11 months ago
On Tue, May 25, 2021 at 2:49 AM Alex Bennée <alex.bennee@linaro.org> wrote:
>
>
> YAMAMOTO Takashi <yamamoto@midokura.com> writes:
>
> > These patches, along with a few more hacks [1] I didn't include
> > in this patchset, allowed me to run arm64 and armv7 version of
> > dind image on amd64.
> >
> > [1] https://github.com/yamt/qemu/tree/linux-user-for-docker
>
> Might be worth posting those patches next time (even if they have a RFC
> or !MERGE in the title for now).

ok.

> I had a little noodle around with
> testing and quickly found a few holes. It would be nice if we could have
> a unit test to cover these various bits as I fear it will easily break
> again. Feel free to use the following as a basis if you want:

frankly, i feel it's enough to cover the cases which are actually used
by real apps.
is "/proc/./self/exe" etc used in the field?

>
>
>
> > You can find my test setup here:
> > https://github.com/yamt/garbage/tree/master/binfmt-aarch64-install
> >
> > YAMAMOTO Takashi (5):
> >   linux-user: handle /proc/self/exe for execve
> >   linux-uesr: make exec_path realpath
> >   linux-user: Fix the execfd case of /proc/self/exe open
> >   linux-user: dup the execfd on start up
> >   linux-user: Implement pivot_root
> >
> >  linux-user/main.c    | 14 +++++++++++++-
> >  linux-user/qemu.h    |  2 ++
> >  linux-user/syscall.c | 43 ++++++++++++++++++++++++++++++++++++++++---
> >  3 files changed, 55 insertions(+), 4 deletions(-)
>
> I also had a go at cleaning up is_proc_self and Daniel greatly
> simplified it.

thank you for the info.
unfortunately the approach seems incompatible with what i want to do
eventually. (handle non-self cases as well)

>
>
>
> --
> Alex Bennée

Re: [PATCH 0/5] linux-user changes to run docker
Posted by Takashi Yamamoto 2 years, 11 months ago
On Tue, May 25, 2021 at 8:22 AM Takashi Yamamoto <yamamoto@midokura.com> wrote:
>
> On Tue, May 25, 2021 at 2:49 AM Alex Bennée <alex.bennee@linaro.org> wrote:
> >
> >
> > YAMAMOTO Takashi <yamamoto@midokura.com> writes:
> >
> > > These patches, along with a few more hacks [1] I didn't include
> > > in this patchset, allowed me to run arm64 and armv7 version of
> > > dind image on amd64.
> > >
> > > [1] https://github.com/yamt/qemu/tree/linux-user-for-docker
> >
> > Might be worth posting those patches next time (even if they have a RFC
> > or !MERGE in the title for now).
>
> ok.

while RFC is mentioned in eg. git format-patch --help,
i couldn't find what !MERGE is.
can you provide a reference?

is there a nice way to express that some patches in a post are meant
for application and the others are RFC?

>
> > I had a little noodle around with
> > testing and quickly found a few holes. It would be nice if we could have
> > a unit test to cover these various bits as I fear it will easily break
> > again. Feel free to use the following as a basis if you want:
>
> frankly, i feel it's enough to cover the cases which are actually used
> by real apps.
> is "/proc/./self/exe" etc used in the field?
>
> >
> >
> >
> > > You can find my test setup here:
> > > https://github.com/yamt/garbage/tree/master/binfmt-aarch64-install
> > >
> > > YAMAMOTO Takashi (5):
> > >   linux-user: handle /proc/self/exe for execve
> > >   linux-uesr: make exec_path realpath
> > >   linux-user: Fix the execfd case of /proc/self/exe open
> > >   linux-user: dup the execfd on start up
> > >   linux-user: Implement pivot_root
> > >
> > >  linux-user/main.c    | 14 +++++++++++++-
> > >  linux-user/qemu.h    |  2 ++
> > >  linux-user/syscall.c | 43 ++++++++++++++++++++++++++++++++++++++++---
> > >  3 files changed, 55 insertions(+), 4 deletions(-)
> >
> > I also had a go at cleaning up is_proc_self and Daniel greatly
> > simplified it.
>
> thank you for the info.
> unfortunately the approach seems incompatible with what i want to do
> eventually. (handle non-self cases as well)
>
> >
> >
> >
> > --
> > Alex Bennée

Re: [PATCH 0/5] linux-user changes to run docker
Posted by Alex Bennée 2 years, 11 months ago
Takashi Yamamoto <yamamoto@midokura.com> writes:

> On Tue, May 25, 2021 at 8:22 AM Takashi Yamamoto <yamamoto@midokura.com> wrote:
>>
>> On Tue, May 25, 2021 at 2:49 AM Alex Bennée <alex.bennee@linaro.org> wrote:
>> >
>> >
>> > YAMAMOTO Takashi <yamamoto@midokura.com> writes:
>> >
>> > > These patches, along with a few more hacks [1] I didn't include
>> > > in this patchset, allowed me to run arm64 and armv7 version of
>> > > dind image on amd64.
>> > >
>> > > [1] https://github.com/yamt/qemu/tree/linux-user-for-docker
>> >
>> > Might be worth posting those patches next time (even if they have a RFC
>> > or !MERGE in the title for now).
>>
>> ok.
>
> while RFC is mentioned in eg. git format-patch --help,
> i couldn't find what !MERGE is.
> can you provide a reference?

It's usually just an annotation to the subject line of the commit, e.g:

  foo/bar: hacky fix to frobulator (!MERGE)

  rest of commit message

or something like:

  baz/quack: invert the tachyon beam (WIP)

  reason for the fix.

  [AJB: still WIP as this breaks foo]

AFAIK the only subject lines supported by the tooling are the squash:
and fixup: prefixes.

> is there a nice way to express that some patches in a post are meant
> for application and the others are RFC?

Aside from a description in the cover letter not really. The main reason
to include patches that aren't ready for merging is to show where your
work is going so the full context of earlier changes can be seen. Having
an ALL CAPS tag in the subject line is just handy for the maintainer
when scanning what might get cherry picked. Obviously if a patch totally
breaks the build it's not worth including as it just makes review harder
when giving the patches a spin so you should exercise your judgement.

-- 
Alex Bennée

Re: [PATCH 0/5] linux-user changes to run docker
Posted by Takashi Yamamoto 2 years, 11 months ago
On Thu, May 27, 2021 at 10:25 PM Alex Bennée <alex.bennee@linaro.org> wrote:
>
>
> Takashi Yamamoto <yamamoto@midokura.com> writes:
>
> > On Tue, May 25, 2021 at 8:22 AM Takashi Yamamoto <yamamoto@midokura.com> wrote:
> >>
> >> On Tue, May 25, 2021 at 2:49 AM Alex Bennée <alex.bennee@linaro.org> wrote:
> >> >
> >> >
> >> > YAMAMOTO Takashi <yamamoto@midokura.com> writes:
> >> >
> >> > > These patches, along with a few more hacks [1] I didn't include
> >> > > in this patchset, allowed me to run arm64 and armv7 version of
> >> > > dind image on amd64.
> >> > >
> >> > > [1] https://github.com/yamt/qemu/tree/linux-user-for-docker
> >> >
> >> > Might be worth posting those patches next time (even if they have a RFC
> >> > or !MERGE in the title for now).
> >>
> >> ok.
> >
> > while RFC is mentioned in eg. git format-patch --help,
> > i couldn't find what !MERGE is.
> > can you provide a reference?
>
> It's usually just an annotation to the subject line of the commit, e.g:
>
>   foo/bar: hacky fix to frobulator (!MERGE)
>
>   rest of commit message
>
> or something like:
>
>   baz/quack: invert the tachyon beam (WIP)
>
>   reason for the fix.
>
>   [AJB: still WIP as this breaks foo]
>
> AFAIK the only subject lines supported by the tooling are the squash:
> and fixup: prefixes.
>
> > is there a nice way to express that some patches in a post are meant
> > for application and the others are RFC?
>
> Aside from a description in the cover letter not really. The main reason
> to include patches that aren't ready for merging is to show where your
> work is going so the full context of earlier changes can be seen. Having
> an ALL CAPS tag in the subject line is just handy for the maintainer
> when scanning what might get cherry picked. Obviously if a patch totally
> breaks the build it's not worth including as it just makes review harder
> when giving the patches a spin so you should exercise your judgement.

ok. thank you for the explanation.

>
> --
> Alex Bennée