Re: [Qemu-devel] [PATCH] linux-user: Add binfmt wrapper

[Alexander Graf]

On 14.07.14 17:38, Joakim Tjernlund wrote:
> Alexander Graf <address@hidden> wrote on 2014/07/14 17:21:33:
>
> > From: Alexander Graf <address@hidden>
> > To: Joakim Tjernlund <address@hidden>,
> > Cc: address@hidden
> > Date: 2014/07/14 17:21
> > Subject: Re: [PATCH] linux-user: Add binfmt wrapper
> >
> >
> > On 14.07.14 16:38, Joakim Tjernlund wrote:
> > > The popular binfmt-wrapper patch adds an additional
> > > executable which mangle argv suitable for binfmt flag P.
> > > In a chroot you need the both (statically linked) qemu-$arch
> > > and qemu-$arch-binfmt-wrapper. This is sub optimal and a
> > > better approach is to recognize the -binfmt-wrapper extension
> > > within linux-user(qemu-$arch) and mangle argv there.
> > > This just produces on executable which can be either copied to
> > > the chroot or bind mounted with the appropriate -binfmt-wrapper
> > > suffix.
> > >
> > > Signed-off-by: Joakim Tjernlund <address@hidden>
> > Please make sure to CC Riku on patches like this - he is the linux-user
> > maintainer.
> Doesn't he read the devel list? Anyhow CC:ed

He may or may not. Qemu-devel can be pretty high volume :).

> > > ---
> > >    linux-user/main.c | 13 +++++++++++++
> > >    1 file changed, 13 insertions(+)
> > >
> > > diff --git a/linux-user/main.c b/linux-user/main.c
> > > index 71a33c7..212067a 100644
> > > --- a/linux-user/main.c
> > > +++ b/linux-user/main.c
> > > @@ -3828,6 +3828,19 @@ int main(int argc, char **argv, char **envp)
> > >        int i;
> > >        int ret;
> > >        int execfd;
> > > +    char *binfmt;
> > > +
> > > +    i = strlen( argv[0] ) - strlen ( "-binfmt-wrapper" );
> > The spaces are odd. Did this patch pass checkpatch.pl? Same comment goes
> > for almost all function invocations.
> ehh, didn't run it through checkpatch.pl. Easy to fix next time.
>
> > > +    binfmt = argv[0] + i;
> > > +    if (i > 0 && strcmp ( binfmt, "-binfmt-wrapper" ) == 0) {
> > This magic needs to be documented somewhere. In fact, I find it pretty
> > hard to use in real world scenarios. Imagine a distribution - should it
> > package every target binary twice? Should it create hardlinks all over?
> How does dists. handle your original binfmt-wrapper? This is not much
> different I think. Here you got a choice to create a hardlink or a copy.
> Any chroot will only have to bind mount binfmt-wrapper into the chroot or
> lxc container.

Yeah, and there are reasons my original approach isn't upstream :).

> > I think we should try and find better magic :). Looking at the
> > binfmt_misc loading code, I think we can cheat a bit. If we pass the 'O'
> > flag (open target binary for handler), binfmt_misc will tell us the
> > binary fd in AT_EXECFD:
> >
> >                   NEW_AUX_ENT(AT_EXECFD, bprm->interp_data);
> >
> > We could then use this as a hint that we were spawned by binfmt_misc
> > rather than directly and interpret the first argv as target_argv[0].
> >
> > Then we can also add the P and O flags to scripts/qemu-binfmt-conf.sh
> > and have a solution that works well for everyone.
> What to do with P only then? Seems like most dists uses only P

If a distro uses the P flag it's not using upstream code, so they have 
to deal with their own breakage :). Fortunately the binfmt install 
scripts are usually part of a package too, so they can be updated easily.

If a distro cares a lot about backwards compatibility with their old 
name space, they can still compile the old -binfmt wrapper code and ship it.


Alex