Re: [Qemu-devel] [PATCH v2 1/1] sandbox: disable -sandbox if CONFIG_SECCOMP undefined

[Eduardo Otubo]

On 18/05/2018 - 09:52:12, Ján Tomko wrote:
> On Thu, May 17, 2018 at 02:41:09PM +0200, Eduardo Otubo wrote:
> > On 15/05/2018 - 19:33:48, Yi Min Zhao wrote:
> > > If CONFIG_SECCOMP is undefined, the option 'elevateprivileges' remains
> > > compiled. This would make libvirt set the corresponding capability and
> > > then trigger the guest startup fails. So this patch excludes the code
> > > regarding seccomp staff if CONFIG_SECCOMP is undefined.
> > 
> > Just a sugestion for the next patch you send: If it's a single patch, you 
> > don't
> > need to format it with a cover-letter. Just put all the description in the 
> > body,
> > or if you need to add a text that shouldn't be included in the commit 
> > message,
> > just add it after the "---" after Signed-off-by.
> > 
> > > 
> > > Signed-off-by: Yi Min Zhao <address@hidden>
> > > ---
> > >  vl.c | 13 ++++++++-----
> > >  1 file changed, 8 insertions(+), 5 deletions(-)
> > > 
> 
> > > @@ -4071,10 +4072,12 @@ int main(int argc, char **argv, char **envp)
> > >          exit(1);
> > >      }
> > > 
> > > +#ifdef CONFIG_SECCOMP
> > >      if (qemu_opts_foreach(qemu_find_opts("sandbox"),
> > >                            parse_sandbox, NULL, NULL)) {
> > >          exit(1);
> > >      }
> > > +#endif
> > > 
> > >      if (qemu_opts_foreach(qemu_find_opts("name"),
> > >                            parse_name, NULL, NULL)) {
> > > --
> > > Yi Min
> > > 
> > 
> > I just wanted a review from Ján, since he is the author of the original 
> > libvirt
> > patch. Does this breaks libvirt logic in any way? If not, ACK on this patch.
> > 
> 
> Current libvirt logic assumes the -sandbox option is always present.
> (IIRC it was introduced in QEMU 1.1 and when we switched from help
> scraping to capability probing via QMP for QEMU 1.2, there was no
> way to detect it)
> 
> This patch fixes the usage of QEMU new enough for seccomp blacklist
> (where libvirt enables the sandbox by default),
> but breaks the usage of QEMU with compiled out sandbox and
> setting
>  seccomp_sandbox = 0
> in libvirt's qemu.conf:
> 
> error: internal error: process exited while connecting to monitor:
> qemu-git: -sandbox off: There is no option group 'sandbox'
> 
> 
> But now libvirt requires QEMU >= 1.5.0 which already supports
> query-command-line-options, so if you want the option gone completely
> --without-seccomp, I can add the code that probes for it and
> make seccomp_sandbox = 0 a no-op if it's compiled out.

This looks like a good solution for the libvirt side. Can you add this support
so we can merge this fix?

Thanks a lot,

-- 
Eduardo Otubo