* [Qemu-devel] [PATCH v2 0/1] Bug: Sandbox: libvirt breakdowns qemu guest @ 2018-05-15 11:33 Yi Min Zhao 2018-05-15 11:33 ` [Qemu-devel] [PATCH v2 1/1] sandbox: disable -sandbox if CONFIG_SECCOMP undefined Yi Min Zhao 0 siblings, 1 reply; 21+ messages in thread From: Yi Min Zhao @ 2018-05-15 11:33 UTC (permalink / raw) To: qemu-devel; +Cc: jtomko, jferlan, berrange, otubo, borntraeger, fiuczy, zyimin 1. Problem Description ====================== If QEMU is built without seccomp support, 'elevateprivileges' remains compiled. This option of sandbox is treated as an indication for seccomp blacklist support in libvirt. This behavior is introduced by the libvirt commits 31ca6a5 and 3527f9d. It would make libvirt build wrong QEMU cmdline, and then the guest startup would fail. 2. Libvirt Log ============== qemu-system-s390x: -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ resourcecontrol=deny: seccomp support is disabled 3. Fixup ======== Compile the code related to sandbox only when CONFIG_SECCOMP is defined. Yi Min Zhao (1): sandbox: disable -sandbox if CONFIG_SECCOMP undefined vl.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) -- Yi Min ^ permalink raw reply [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH v2 1/1] sandbox: disable -sandbox if CONFIG_SECCOMP undefined 2018-05-15 11:33 [Qemu-devel] [PATCH v2 0/1] Bug: Sandbox: libvirt breakdowns qemu guest Yi Min Zhao @ 2018-05-15 11:33 ` Yi Min Zhao 2018-05-15 15:25 ` Eric Blake 2018-05-17 12:41 ` Eduardo Otubo 0 siblings, 2 replies; 21+ messages in thread From: Yi Min Zhao @ 2018-05-15 11:33 UTC (permalink / raw) To: qemu-devel; +Cc: jtomko, jferlan, berrange, otubo, borntraeger, fiuczy, zyimin 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. Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> --- 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 } 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(&qemu_mem_opts); qemu_add_opts(&qemu_smp_opts); qemu_add_opts(&qemu_boot_opts); +#ifdef CONFIG_SECCOMP qemu_add_opts(&qemu_sandbox_opts); +#endif qemu_add_opts(&qemu_add_fd_opts); qemu_add_opts(&qemu_object_opts); qemu_add_opts(&qemu_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 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/1] sandbox: disable -sandbox if CONFIG_SECCOMP undefined 2018-05-15 11:33 ` [Qemu-devel] [PATCH v2 1/1] sandbox: disable -sandbox if CONFIG_SECCOMP undefined Yi Min Zhao @ 2018-05-15 15:25 ` Eric Blake 2018-05-16 1:05 ` Yi Min Zhao 2018-05-17 11:33 ` Yi Min Zhao 2018-05-17 12:41 ` Eduardo Otubo 1 sibling, 2 replies; 21+ messages in thread From: Eric Blake @ 2018-05-15 15:25 UTC (permalink / raw) To: Yi Min Zhao, qemu-devel; +Cc: otubo, fiuczy, jtomko, borntraeger, jferlan 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 <zyimin@linux.ibm.com> > --- > vl.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > A maintainer can touch up the commit message, so: Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/1] sandbox: disable -sandbox if CONFIG_SECCOMP undefined 2018-05-15 15:25 ` Eric Blake @ 2018-05-16 1:05 ` Yi Min Zhao 2018-05-17 11:33 ` Yi Min Zhao 1 sibling, 0 replies; 21+ messages in thread From: Yi Min Zhao @ 2018-05-16 1:05 UTC (permalink / raw) To: Eric Blake, qemu-devel; +Cc: otubo, fiuczy, jtomko, borntraeger, jferlan 在 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 <zyimin@linux.ibm.com> >> --- >> vl.c | 13 ++++++++----- >> 1 file changed, 8 insertions(+), 5 deletions(-) >> > > A maintainer can touch up the commit message, so: > Reviewed-by: Eric Blake <eblake@redhat.com> > Thanks for your comments! Have updated commit msg. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/1] sandbox: disable -sandbox if CONFIG_SECCOMP undefined 2018-05-15 15:25 ` Eric Blake 2018-05-16 1:05 ` Yi Min Zhao @ 2018-05-17 11:33 ` Yi Min Zhao 1 sibling, 0 replies; 21+ messages in thread From: Yi Min Zhao @ 2018-05-17 11:33 UTC (permalink / raw) To: Eric Blake, qemu-devel Cc: otubo, borntraeger, fiuczy, jtomko, jferlan, pbonzini 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 <zyimin@linux.ibm.com> >> --- >> vl.c | 13 ++++++++----- >> 1 file changed, 8 insertions(+), 5 deletions(-) >> > > A maintainer can touch up the commit message, so: > Reviewed-by: Eric Blake <eblake@redhat.com> > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/1] sandbox: disable -sandbox if CONFIG_SECCOMP undefined 2018-05-15 11:33 ` [Qemu-devel] [PATCH v2 1/1] sandbox: disable -sandbox if CONFIG_SECCOMP undefined Yi Min Zhao 2018-05-15 15:25 ` Eric Blake @ 2018-05-17 12:41 ` Eduardo Otubo 2018-05-17 14:36 ` Yi Min Zhao 2018-05-18 7:52 ` Ján Tomko 1 sibling, 2 replies; 21+ messages in thread From: Eduardo Otubo @ 2018-05-17 12:41 UTC (permalink / raw) To: Yi Min Zhao; +Cc: qemu-devel, jtomko, jferlan, berrange, borntraeger, fiuczy 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 <zyimin@linux.ibm.com> > --- > 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(&qemu_mem_opts); > qemu_add_opts(&qemu_smp_opts); > qemu_add_opts(&qemu_boot_opts); > +#ifdef CONFIG_SECCOMP > qemu_add_opts(&qemu_sandbox_opts); > +#endif > qemu_add_opts(&qemu_add_fd_opts); > qemu_add_opts(&qemu_object_opts); > qemu_add_opts(&qemu_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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/1] sandbox: disable -sandbox if CONFIG_SECCOMP undefined 2018-05-17 12:41 ` Eduardo Otubo @ 2018-05-17 14:36 ` Yi Min Zhao 2018-05-18 7:52 ` Ján Tomko 1 sibling, 0 replies; 21+ messages in thread From: Yi Min Zhao @ 2018-05-17 14:36 UTC (permalink / raw) To: Eduardo Otubo; +Cc: qemu-devel, jtomko, jferlan, berrange, borntraeger, fiuczy 在 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 <zyimin@linux.ibm.com> >> --- >> 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(&qemu_mem_opts); >> qemu_add_opts(&qemu_smp_opts); >> qemu_add_opts(&qemu_boot_opts); >> +#ifdef CONFIG_SECCOMP >> qemu_add_opts(&qemu_sandbox_opts); >> +#endif >> qemu_add_opts(&qemu_add_fd_opts); >> qemu_add_opts(&qemu_object_opts); >> qemu_add_opts(&qemu_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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/1] sandbox: disable -sandbox if CONFIG_SECCOMP undefined 2018-05-17 12:41 ` Eduardo Otubo 2018-05-17 14:36 ` Yi Min Zhao @ 2018-05-18 7:52 ` Ján Tomko 2018-05-18 9:19 ` Eduardo Otubo 2018-05-18 12:08 ` Eric Blake 1 sibling, 2 replies; 21+ messages in thread From: Ján Tomko @ 2018-05-18 7:52 UTC (permalink / raw) To: Eduardo Otubo; +Cc: Yi Min Zhao, fiuczy, borntraeger, qemu-devel, jferlan [-- Attachment #1: Type: text/plain, Size: 2318 bytes --] 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 <zyimin@linux.ibm.com> >> --- >> 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 [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/1] sandbox: disable -sandbox if CONFIG_SECCOMP undefined 2018-05-18 7:52 ` Ján Tomko @ 2018-05-18 9:19 ` Eduardo Otubo 2018-05-18 13:07 ` Ján Tomko 2018-05-18 12:08 ` Eric Blake 1 sibling, 1 reply; 21+ messages in thread From: Eduardo Otubo @ 2018-05-18 9:19 UTC (permalink / raw) To: Ján Tomko; +Cc: Yi Min Zhao, fiuczy, borntraeger, qemu-devel, jferlan 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 <zyimin@linux.ibm.com> > > > --- > > > 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/1] sandbox: disable -sandbox if CONFIG_SECCOMP undefined 2018-05-18 9:19 ` Eduardo Otubo @ 2018-05-18 13:07 ` Ján Tomko 2018-05-19 8:20 ` Yi Min Zhao 0 siblings, 1 reply; 21+ messages in thread From: Ján Tomko @ 2018-05-18 13:07 UTC (permalink / raw) To: Eduardo Otubo; +Cc: Yi Min Zhao, fiuczy, borntraeger, qemu-devel [-- Attachment #1: Type: text/plain, Size: 2157 bytes --] 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 <zyimin@linux.ibm.com> >> > > --- >> > > 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 [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/1] sandbox: disable -sandbox if CONFIG_SECCOMP undefined 2018-05-18 13:07 ` Ján Tomko @ 2018-05-19 8:20 ` Yi Min Zhao 2018-05-23 7:47 ` Ján Tomko 0 siblings, 1 reply; 21+ messages in thread From: Yi Min Zhao @ 2018-05-19 8:20 UTC (permalink / raw) To: Ján Tomko, Eduardo Otubo; +Cc: fiuczy, borntraeger, qemu-devel 在 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 <zyimin@linux.ibm.com> >>> > > --- >>> > > 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! ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/1] sandbox: disable -sandbox if CONFIG_SECCOMP undefined 2018-05-19 8:20 ` Yi Min Zhao @ 2018-05-23 7:47 ` Ján Tomko 2018-05-23 9:16 ` Yi Min Zhao 0 siblings, 1 reply; 21+ messages in thread From: Ján Tomko @ 2018-05-23 7:47 UTC (permalink / raw) To: Yi Min Zhao; +Cc: Eduardo Otubo, borntraeger, fiuczy, qemu-devel [-- Attachment #1: Type: text/plain, Size: 1409 bytes --] 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 <jtomko@redhat.com> AuthorDate: 2018-05-18 14:57:51 +0200 Commit: Ján Tomko <jtomko@redhat.com> 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 <jtomko@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com> git describe: v4.3.0-258-gb87222a909 https://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=b87222a90919040c12fb6d7c8dcc20f944a66495 Jano [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/1] sandbox: disable -sandbox if CONFIG_SECCOMP undefined 2018-05-23 7:47 ` Ján Tomko @ 2018-05-23 9:16 ` Yi Min Zhao 2018-05-23 10:33 ` Eduardo Otubo 0 siblings, 1 reply; 21+ messages in thread From: Yi Min Zhao @ 2018-05-23 9:16 UTC (permalink / raw) To: Ján Tomko; +Cc: Eduardo Otubo, borntraeger, fiuczy, qemu-devel, pbonzini 在 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 <jtomko@redhat.com> > AuthorDate: 2018-05-18 14:57:51 +0200 > Commit: Ján Tomko <jtomko@redhat.com> > 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 <jtomko@redhat.com> > Reviewed-by: John Ferlan <jferlan@redhat.com> > > 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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/1] sandbox: disable -sandbox if CONFIG_SECCOMP undefined 2018-05-23 9:16 ` Yi Min Zhao @ 2018-05-23 10:33 ` Eduardo Otubo 2018-05-23 12:17 ` Yi Min Zhao 0 siblings, 1 reply; 21+ messages in thread From: Eduardo Otubo @ 2018-05-23 10:33 UTC (permalink / raw) To: Yi Min Zhao, Ján Tomko; +Cc: borntraeger, fiuczy, qemu-devel, pbonzini 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 <jtomko@redhat.com> >> AuthorDate: 2018-05-18 14:57:51 +0200 >> Commit: Ján Tomko <jtomko@redhat.com> >> 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 <jtomko@redhat.com> >> Reviewed-by: John Ferlan <jferlan@redhat.com> >> >> 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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/1] sandbox: disable -sandbox if CONFIG_SECCOMP undefined 2018-05-23 10:33 ` Eduardo Otubo @ 2018-05-23 12:17 ` Yi Min Zhao 2018-05-24 7:53 ` Eduardo Otubo 0 siblings, 1 reply; 21+ messages in thread From: Yi Min Zhao @ 2018-05-23 12:17 UTC (permalink / raw) To: otubo, Ján Tomko; +Cc: borntraeger, fiuczy, qemu-devel, pbonzini 在 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 <jtomko@redhat.com> >>> AuthorDate: 2018-05-18 14:57:51 +0200 >>> Commit: Ján Tomko <jtomko@redhat.com> >>> 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 <jtomko@redhat.com> >>> Reviewed-by: John Ferlan <jferlan@redhat.com> >>> >>> 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? ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/1] sandbox: disable -sandbox if CONFIG_SECCOMP undefined 2018-05-23 12:17 ` Yi Min Zhao @ 2018-05-24 7:53 ` Eduardo Otubo 2018-05-24 13:40 ` Paolo Bonzini 0 siblings, 1 reply; 21+ messages in thread From: Eduardo Otubo @ 2018-05-24 7:53 UTC (permalink / raw) To: Yi Min Zhao, Ján Tomko; +Cc: borntraeger, fiuczy, qemu-devel, pbonzini 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 <jtomko@redhat.com> >>>> AuthorDate: 2018-05-18 14:57:51 +0200 >>>> Commit: Ján Tomko <jtomko@redhat.com> >>>> 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 <jtomko@redhat.com> >>>> Reviewed-by: John Ferlan <jferlan@redhat.com> >>>> >>>> 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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/1] sandbox: disable -sandbox if CONFIG_SECCOMP undefined 2018-05-24 7:53 ` Eduardo Otubo @ 2018-05-24 13:40 ` Paolo Bonzini 2018-05-25 4:23 ` Yi Min Zhao 0 siblings, 1 reply; 21+ messages in thread From: Paolo Bonzini @ 2018-05-24 13:40 UTC (permalink / raw) To: otubo, Yi Min Zhao, Ján Tomko; +Cc: borntraeger, fiuczy, qemu-devel 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/1] sandbox: disable -sandbox if CONFIG_SECCOMP undefined 2018-05-24 13:40 ` Paolo Bonzini @ 2018-05-25 4:23 ` Yi Min Zhao 2018-05-25 9:36 ` Eduardo Otubo 0 siblings, 1 reply; 21+ messages in thread From: Yi Min Zhao @ 2018-05-25 4:23 UTC (permalink / raw) To: Paolo Bonzini, otubo, Ján Tomko; +Cc: borntraeger, fiuczy, qemu-devel 在 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? ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/1] sandbox: disable -sandbox if CONFIG_SECCOMP undefined 2018-05-25 4:23 ` Yi Min Zhao @ 2018-05-25 9:36 ` Eduardo Otubo 2018-05-28 12:55 ` Yi Min Zhao 0 siblings, 1 reply; 21+ messages in thread From: Eduardo Otubo @ 2018-05-25 9:36 UTC (permalink / raw) To: Yi Min Zhao, Paolo Bonzini, Ján Tomko Cc: borntraeger, fiuczy, qemu-devel 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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/1] sandbox: disable -sandbox if CONFIG_SECCOMP undefined 2018-05-25 9:36 ` Eduardo Otubo @ 2018-05-28 12:55 ` Yi Min Zhao 0 siblings, 0 replies; 21+ messages in thread From: Yi Min Zhao @ 2018-05-28 12:55 UTC (permalink / raw) To: qemu-devel 在 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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/1] sandbox: disable -sandbox if CONFIG_SECCOMP undefined 2018-05-18 7:52 ` Ján Tomko 2018-05-18 9:19 ` Eduardo Otubo @ 2018-05-18 12:08 ` Eric Blake 1 sibling, 0 replies; 21+ messages in thread From: Eric Blake @ 2018-05-18 12:08 UTC (permalink / raw) To: Ján Tomko, Eduardo Otubo Cc: borntraeger, fiuczy, qemu-devel, jferlan, Yi Min Zhao 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2018-05-28 12:56 UTC | newest] Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-05-15 11:33 [Qemu-devel] [PATCH v2 0/1] Bug: Sandbox: libvirt breakdowns qemu guest Yi Min Zhao 2018-05-15 11:33 ` [Qemu-devel] [PATCH v2 1/1] sandbox: disable -sandbox if CONFIG_SECCOMP undefined Yi Min Zhao 2018-05-15 15:25 ` Eric Blake 2018-05-16 1:05 ` Yi Min Zhao 2018-05-17 11:33 ` Yi Min Zhao 2018-05-17 12:41 ` Eduardo Otubo 2018-05-17 14:36 ` Yi Min Zhao 2018-05-18 7:52 ` Ján Tomko 2018-05-18 9:19 ` Eduardo Otubo 2018-05-18 13:07 ` Ján Tomko 2018-05-19 8:20 ` Yi Min Zhao 2018-05-23 7:47 ` Ján Tomko 2018-05-23 9:16 ` Yi Min Zhao 2018-05-23 10:33 ` Eduardo Otubo 2018-05-23 12:17 ` Yi Min Zhao 2018-05-24 7:53 ` Eduardo Otubo 2018-05-24 13:40 ` Paolo Bonzini 2018-05-25 4:23 ` Yi Min Zhao 2018-05-25 9:36 ` Eduardo Otubo 2018-05-28 12:55 ` Yi Min Zhao 2018-05-18 12:08 ` Eric Blake
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.