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

2018-05-28 Thread Yi Min Zhao 



在 2018/5/25 下午5:36, Eduardo Otubo 写道:

On 05/25/2018 06:23 AM, Yi Min Zhao wrote:



在 2018/5/24 下午9:40, Paolo Bonzini 写道:

On 24/05/2018 09:53, Eduardo Otubo wrote:
Thanks! But I have not got response from Paolo.  I have added 
him to

CC list.


  I'll just wait one more ACK and will send a pull request on the
seccomp queue. Thanks for the contribution.



So... what I should do is wait?

Yes, even though I think we're safe to proceed without his explicit 
ack.

The patch is okay; however, as a follow-up, you could consider moving
all the CONFIG_SECCOMP code to qemu-seccomp.c.

This way, the only #ifdef remains the one around qemu_opts_foreach.

Paolo


Thanks for your comment! Indeed, moving to the single C file is much 
more clear.

I will do this after this patch.

@Otubo, what about next step?


If you're willing to send v3 with the changes Paolo suggested, I can 
wait to send the pull request. No worries.




OK. I will update the new version with Paolo's suggestion.

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

2018-05-25 Thread Eduardo Otubo 

On 05/25/2018 06:23 AM, Yi Min Zhao wrote:



在 2018/5/24 下午9:40, Paolo Bonzini 写道:

On 24/05/2018 09:53, Eduardo Otubo wrote:

Thanks! But I have not got response from Paolo.  I have added him to
CC list.


  I'll just wait one more ACK and will send a pull request on the
seccomp queue. Thanks for the contribution.



So... what I should do is wait?


Yes, even though I think we're safe to proceed without his explicit ack.

The patch is okay; however, as a follow-up, you could consider moving
all the CONFIG_SECCOMP code to qemu-seccomp.c.

This way, the only #ifdef remains the one around qemu_opts_foreach.

Paolo


Thanks for your comment! Indeed, moving to the single C file is much 
more clear.

I will do this after this patch.

@Otubo, what about next step?


If you're willing to send v3 with the changes Paolo suggested, I can 
wait to send the pull request. No worries.

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

2018-05-24 Thread Yi Min Zhao 



在 2018/5/24 下午9:40, Paolo Bonzini 写道:

On 24/05/2018 09:53, Eduardo Otubo wrote:

Thanks! But I have not got response from Paolo.  I have added him to
CC list.


  I'll just wait one more ACK and will send a pull request on the
seccomp queue. Thanks for the contribution.



So... what I should do is wait?


Yes, even though I think we're safe to proceed without his explicit ack.

The patch is okay; however, as a follow-up, you could consider moving
all the CONFIG_SECCOMP code to qemu-seccomp.c.

This way, the only #ifdef remains the one around qemu_opts_foreach.

Paolo


Thanks for your comment! Indeed, moving to the single C file is much 
more clear.

I will do this after this patch.

@Otubo, what about next step?

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

2018-05-24 Thread Paolo Bonzini 
On 24/05/2018 09:53, Eduardo Otubo wrote:
>
 Thanks! But I have not got response from Paolo.  I have added him to
 CC list.

>>>  I'll just wait one more ACK and will send a pull request on the
>>> seccomp queue. Thanks for the contribution.
>>>
>>>
>> So... what I should do is wait?
>>
> Yes, even though I think we're safe to proceed without his explicit ack.

The patch is okay; however, as a follow-up, you could consider moving
all the CONFIG_SECCOMP code to qemu-seccomp.c.

This way, the only #ifdef remains the one around qemu_opts_foreach.

Paolo

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

2018-05-24 Thread Eduardo Otubo 

On 05/23/2018 02:17 PM, Yi Min Zhao wrote:



在 2018/5/23 下午6:33, Eduardo Otubo 写道:

On 05/23/2018 11:16 AM, Yi Min Zhao wrote:



在 2018/5/23 下午3:47, Ján Tomko 写道:

On Sat, May 19, 2018 at 04:20:37PM +0800, Yi Min Zhao wrote:



在 2018/5/18 下午9:07, Ján Tomko 写道:

On Fri, May 18, 2018 at 11:19:16AM +0200, Eduardo Otubo wrote:

On 18/05/2018 - 09:52:12, Ján Tomko wrote:

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?



Patches proposed:
https://www.redhat.com/archives/libvir-list/2018-May/msg01430.html

Jano

Thanks for your work!


Now pushed in libvirt master:
commit b87222a90919040c12fb6d7c8dcc20f944a66495
Author: Ján Tomko 
AuthorDate: 2018-05-18 14:57:51 +0200
Commit: Ján Tomko 
CommitDate: 2018-05-23 09:45:48 +0200

   qemu: only pass -sandbox off if supported

   This way we don't rely on QEMU supplying the -sandbox option
   without CONFIG_SECCOMP.

   Signed-off-by: Ján Tomko 
   Reviewed-by: John Ferlan 

git describe: v4.3.0-258-gb87222a909
https://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=b87222a90919040c12fb6d7c8dcc20f944a66495 



Jano
Thanks! But I have not got response from Paolo.  I have added him to 
CC list.


 I'll just wait one more ACK and will send a pull request on the 
seccomp queue. Thanks for the contribution.




So... what I should do is wait?


Yes, even though I think we're safe to proceed without his explicit ack.

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

2018-05-23 Thread Yi Min Zhao 



在 2018/5/23 下午6:33, Eduardo Otubo 写道:

On 05/23/2018 11:16 AM, Yi Min Zhao wrote:



在 2018/5/23 下午3:47, Ján Tomko 写道:

On Sat, May 19, 2018 at 04:20:37PM +0800, Yi Min Zhao wrote:



在 2018/5/18 下午9:07, Ján Tomko 写道:

On Fri, May 18, 2018 at 11:19:16AM +0200, Eduardo Otubo wrote:

On 18/05/2018 - 09:52:12, Ján Tomko wrote:

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?



Patches proposed:
https://www.redhat.com/archives/libvir-list/2018-May/msg01430.html

Jano

Thanks for your work!


Now pushed in libvirt master:
commit b87222a90919040c12fb6d7c8dcc20f944a66495
Author: Ján Tomko 
AuthorDate: 2018-05-18 14:57:51 +0200
Commit: Ján Tomko 
CommitDate: 2018-05-23 09:45:48 +0200

   qemu: only pass -sandbox off if supported

   This way we don't rely on QEMU supplying the -sandbox option
   without CONFIG_SECCOMP.

   Signed-off-by: Ján Tomko 
   Reviewed-by: John Ferlan 

git describe: v4.3.0-258-gb87222a909
https://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=b87222a90919040c12fb6d7c8dcc20f944a66495 



Jano
Thanks! But I have not got response from Paolo.  I have added him to 
CC list.


 I'll just wait one more ACK and will send a pull request on the 
seccomp queue. Thanks for the contribution.




So... what I should do is wait?

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

2018-05-23 Thread Eduardo Otubo 

On 05/23/2018 11:16 AM, Yi Min Zhao wrote:



在 2018/5/23 下午3:47, Ján Tomko 写道:

On Sat, May 19, 2018 at 04:20:37PM +0800, Yi Min Zhao wrote:



在 2018/5/18 下午9:07, Ján Tomko 写道:

On Fri, May 18, 2018 at 11:19:16AM +0200, Eduardo Otubo wrote:

On 18/05/2018 - 09:52:12, Ján Tomko wrote:

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?



Patches proposed:
https://www.redhat.com/archives/libvir-list/2018-May/msg01430.html

Jano

Thanks for your work!


Now pushed in libvirt master:
commit b87222a90919040c12fb6d7c8dcc20f944a66495
Author: Ján Tomko 
AuthorDate: 2018-05-18 14:57:51 +0200
Commit: Ján Tomko 
CommitDate: 2018-05-23 09:45:48 +0200

   qemu: only pass -sandbox off if supported

   This way we don't rely on QEMU supplying the -sandbox option
   without CONFIG_SECCOMP.

   Signed-off-by: Ján Tomko 
   Reviewed-by: John Ferlan 

git describe: v4.3.0-258-gb87222a909
https://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=b87222a90919040c12fb6d7c8dcc20f944a66495 



Jano
Thanks! But I have not got response from Paolo.  I have added him to CC 
list.


 I'll just wait one more ACK and will send a pull request on the 
seccomp queue. Thanks for the contribution.

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

2018-05-23 Thread Yi Min Zhao 



在 2018/5/23 下午3:47, Ján Tomko 写道:

On Sat, May 19, 2018 at 04:20:37PM +0800, Yi Min Zhao wrote:



在 2018/5/18 下午9:07, Ján Tomko 写道:

On Fri, May 18, 2018 at 11:19:16AM +0200, Eduardo Otubo wrote:

On 18/05/2018 - 09:52:12, Ján Tomko wrote:

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?



Patches proposed:
https://www.redhat.com/archives/libvir-list/2018-May/msg01430.html

Jano

Thanks for your work!


Now pushed in libvirt master:
commit b87222a90919040c12fb6d7c8dcc20f944a66495
Author: Ján Tomko 
AuthorDate: 2018-05-18 14:57:51 +0200
Commit: Ján Tomko 
CommitDate: 2018-05-23 09:45:48 +0200

   qemu: only pass -sandbox off if supported

   This way we don't rely on QEMU supplying the -sandbox option
   without CONFIG_SECCOMP.

   Signed-off-by: Ján Tomko 
   Reviewed-by: John Ferlan 

git describe: v4.3.0-258-gb87222a909
https://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=b87222a90919040c12fb6d7c8dcc20f944a66495 



Jano
Thanks! But I have not got response from Paolo.  I have added him to CC 
list.

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

2018-05-23 Thread Ján Tomko 

On Sat, May 19, 2018 at 04:20:37PM +0800, Yi Min Zhao wrote:



在 2018/5/18 下午9:07, Ján Tomko 写道:

On Fri, May 18, 2018 at 11:19:16AM +0200, Eduardo Otubo wrote:

On 18/05/2018 - 09:52:12, Ján Tomko wrote:

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?



Patches proposed:
https://www.redhat.com/archives/libvir-list/2018-May/msg01430.html

Jano

Thanks for your work!


Now pushed in libvirt master:
commit b87222a90919040c12fb6d7c8dcc20f944a66495
Author: Ján Tomko 
AuthorDate: 2018-05-18 14:57:51 +0200
Commit: Ján Tomko 
CommitDate: 2018-05-23 09:45:48 +0200

   qemu: only pass -sandbox off if supported

   This way we don't rely on QEMU supplying the -sandbox option
   without CONFIG_SECCOMP.

   Signed-off-by: Ján Tomko 
   Reviewed-by: John Ferlan 

git describe: v4.3.0-258-gb87222a909
https://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=b87222a90919040c12fb6d7c8dcc20f944a66495

Jano


signature.asc
Description: Digital signature

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

2018-05-19 Thread Yi Min Zhao 



在 2018/5/18 下午9:07, Ján Tomko 写道:

On Fri, May 18, 2018 at 11:19:16AM +0200, Eduardo Otubo wrote:

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 
> > ---
> >  vl.c | 13 -
> >  1 file changed, 8 insertions(+), 5 deletions(-)
> >



[...]


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?



Patches proposed:
https://www.redhat.com/archives/libvir-list/2018-May/msg01430.html

Jano

Thanks for your work!

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

2018-05-18 Thread Ján Tomko 

On Fri, May 18, 2018 at 11:19:16AM +0200, Eduardo Otubo wrote:

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 
> > ---
> >  vl.c | 13 -
> >  1 file changed, 8 insertions(+), 5 deletions(-)
> >



[...]


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?



Patches proposed:
https://www.redhat.com/archives/libvir-list/2018-May/msg01430.html

Jano


signature.asc
Description: Digital signature

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

2018-05-18 Thread Eric Blake 

On 05/18/2018 02:52 AM, Ján Tomko wrote:


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.


And that's acceptable - we document that libvirt must be at least as new 
as qemu.  Mixing old qemu + new libvirt should always work, but mixing 
new qemu + old libvirt may fail, and this is one of those cases.  The 
solution for anyone hitting the failure is to upgrade libvirt to match 
the fact that they upgraded qemu.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

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

2018-05-18 Thread 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 
> > > ---
> > >  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

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

2018-05-18 Thread Ján Tomko 

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 
---
 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.

Jano


signature.asc
Description: Digital signature

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

2018-05-17 Thread Yi Min Zhao 



在 2018/5/17 下午8:41, Eduardo Otubo 写道:

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.

OK. Thanks for your suggestion.



Signed-off-by: Yi Min Zhao 
---
  vl.c | 13 -
  1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/vl.c b/vl.c
index 806eec2ef6..b22d158f5f 100644
--- a/vl.c
+++ b/vl.c
@@ -257,6 +257,7 @@ static QemuOptsList qemu_rtc_opts = {
  },
  };
  
+#ifdef CONFIG_SECCOMP

  static QemuOptsList qemu_sandbox_opts = {
  .name = "sandbox",
  .implied_opt_name = "enable",
@@ -285,6 +286,7 @@ static QemuOptsList qemu_sandbox_opts = {
  { /* end of list */ }
  },
  };
+#endif
  
  static QemuOptsList qemu_option_rom_opts = {

  .name = "option-rom",
@@ -1041,10 +1043,10 @@ static int bt_parse(const char *opt)
  return 1;
  }
  
+#ifdef CONFIG_SECCOMP

  static int parse_sandbox(void *opaque, QemuOpts *opts, Error **errp)
  {
  if (qemu_opt_get_bool(opts, "enable", false)) {
-#ifdef CONFIG_SECCOMP
  uint32_t seccomp_opts = QEMU_SECCOMP_SET_DEFAULT
  | QEMU_SECCOMP_SET_OBSOLETE;
  const char *value = NULL;
@@ -1114,14 +1116,11 @@ static int parse_sandbox(void *opaque, QemuOpts *opts, 
Error **errp)
   "in the kernel");
  return -1;
  }
-#else
-error_report("seccomp support is disabled");
-return -1;
-#endif

Any reason not to keep the error message on the new #endif location?

If error report is originally wrapped in CONFIG_SECCOMP undefined.
This patch excludes the entire function if CONFIG_SECCOMP is undefined.
So the error report is not needed.



  }
  
  return 0;

  }
+#endif
  
  static int parse_name(void *opaque, QemuOpts *opts, Error **errp)

  {
@@ -3074,7 +3073,9 @@ int main(int argc, char **argv, char **envp)
  qemu_add_opts(_mem_opts);
  qemu_add_opts(_smp_opts);
  qemu_add_opts(_boot_opts);
+#ifdef CONFIG_SECCOMP
  qemu_add_opts(_sandbox_opts);
+#endif
  qemu_add_opts(_add_fd_opts);
  qemu_add_opts(_object_opts);
  qemu_add_opts(_tpmdev_opts);
@@ -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.



OK.

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

2018-05-17 Thread Eduardo Otubo 
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 
> ---
>  vl.c | 13 -
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/vl.c b/vl.c
> index 806eec2ef6..b22d158f5f 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -257,6 +257,7 @@ static QemuOptsList qemu_rtc_opts = {
>  },
>  };
>  
> +#ifdef CONFIG_SECCOMP
>  static QemuOptsList qemu_sandbox_opts = {
>  .name = "sandbox",
>  .implied_opt_name = "enable",
> @@ -285,6 +286,7 @@ static QemuOptsList qemu_sandbox_opts = {
>  { /* end of list */ }
>  },
>  };
> +#endif
>  
>  static QemuOptsList qemu_option_rom_opts = {
>  .name = "option-rom",
> @@ -1041,10 +1043,10 @@ static int bt_parse(const char *opt)
>  return 1;
>  }
>  
> +#ifdef CONFIG_SECCOMP
>  static int parse_sandbox(void *opaque, QemuOpts *opts, Error **errp)
>  {
>  if (qemu_opt_get_bool(opts, "enable", false)) {
> -#ifdef CONFIG_SECCOMP
>  uint32_t seccomp_opts = QEMU_SECCOMP_SET_DEFAULT
>  | QEMU_SECCOMP_SET_OBSOLETE;
>  const char *value = NULL;
> @@ -1114,14 +1116,11 @@ static int parse_sandbox(void *opaque, QemuOpts 
> *opts, Error **errp)
>   "in the kernel");
>  return -1;
>  }
> -#else
> -error_report("seccomp support is disabled");
> -return -1;
> -#endif

Any reason not to keep the error message on the new #endif location?

>  }
>  
>  return 0;
>  }
> +#endif
>  
>  static int parse_name(void *opaque, QemuOpts *opts, Error **errp)
>  {
> @@ -3074,7 +3073,9 @@ int main(int argc, char **argv, char **envp)
>  qemu_add_opts(_mem_opts);
>  qemu_add_opts(_smp_opts);
>  qemu_add_opts(_boot_opts);
> +#ifdef CONFIG_SECCOMP
>  qemu_add_opts(_sandbox_opts);
> +#endif
>  qemu_add_opts(_add_fd_opts);
>  qemu_add_opts(_object_opts);
>  qemu_add_opts(_tpmdev_opts);
> @@ -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.

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

2018-05-17 Thread Yi Min Zhao 

Add Paolo to CC list. @Paolo, expect your comment. Thanks very much!


在 2018/5/15 下午11:25, Eric Blake 写道:

On 05/15/2018 06:33 AM, 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


s/trigger the guest startup fails/trigger failure during guest startup/


regarding seccomp staff if CONFIG_SECCOMP is undefined.


s/staff/command line options/



Signed-off-by: Yi Min Zhao 
---
  vl.c | 13 -
  1 file changed, 8 insertions(+), 5 deletions(-)



A maintainer can touch up the commit message, so:
Reviewed-by: Eric Blake

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

2018-05-15 Thread Yi Min Zhao 



在 2018/5/15 下午11:25, Eric Blake 写道:

On 05/15/2018 06:33 AM, 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


s/trigger the guest startup fails/trigger failure during guest startup/


regarding seccomp staff if CONFIG_SECCOMP is undefined.


s/staff/command line options/



Signed-off-by: Yi Min Zhao 
---
  vl.c | 13 -
  1 file changed, 8 insertions(+), 5 deletions(-)



A maintainer can touch up the commit message, so:
Reviewed-by: Eric Blake 


Thanks for your comments! Have updated commit msg.

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

2018-05-15 Thread Eric Blake 

On 05/15/2018 06:33 AM, 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


s/trigger the guest startup fails/trigger failure during guest startup/


regarding seccomp staff if CONFIG_SECCOMP is undefined.


s/staff/command line options/



Signed-off-by: Yi Min Zhao 
---
  vl.c | 13 -
  1 file changed, 8 insertions(+), 5 deletions(-)



A maintainer can touch up the commit message, so:
Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org