Alexander Graf <ag...@suse.de> wrote on 2014/07/14 18:00:35: > > > On 14.07.14 17:59, Joakim Tjernlund wrote: > > 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 ? > > 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