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

[Joakim Tjernlund]

Alexander Graf <address@hidden> wrote on 2014/07/14 18:00:35:
> 
> 
> On 14.07.14 17:59, Joakim Tjernlund wrote:
> > Alexander Graf <address@hidden> wrote on 2014/07/14 17:46:18:
> >>
> >> 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 :).
> > What are those then? Hardly just packaging problem/choise.
> >
> >>>> 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.
> >
> > scripts/qemu-binfmt-conf.sh does not use any flag currently, I don't 
think
> > that works either with current linux-user and choot/lxc
> >
> > You think everyone feel OK with new defaults like OP ?
> 
> Yes.

hmm, with current qemu it works to boot a LXC with just O flag.
Why would we then want to complicate things by adding OP which
then requires some version of my patch?

 Jocke