Alexander Graf <ag...@suse.de> wrote on 2014/07/14 17:46:18:
>
>
> On 14.07.14 17:38, Joakim Tjernlund wrote:
> > Alexander Graf <ag...@suse.de> wrote on 2014/07/14 17:21:33:
> >
> >> From: Alexander Graf <ag...@suse.de>
> >> To: Joakim Tjernlund <joakim.tjernl...@transmode.se>,
> >> Cc: qemu-devel@nongnu.org
> >> 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 <joakim.tjernl...@transmode.se>
> >> 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 ?
>
> 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.
