Re: [Qemu-devel] [PATCH 1/1] sandbox: avoid to compile options if CONFIG_SECCOMP undefined

[Yi Min Zhao <zyimin@linux.ibm.com>]

在 2018/5/8 下午6:37, Daniel P. Berrangé 写道:
> On Mon, May 07, 2018 at 01:04:17PM -0500, Eric Blake wrote:
>> On 05/06/2018 10:32 PM, Yi Min Zhao wrote:
>>
>> In the subject line: s/avoid to compile/avoid compiling/
>>
>>> If CONFIG_SECCOMP is undefined, the option 'elevatorprivileges' remains
>> s/elevator/elevate/
>>
>>> complied. This would make libvirt set the corresponding capability and
>> s/complied/compiled/
>>
>>> then trigger the guest startup fails. So let's wrap the options with
>>> CONFIG_SECCOMP.
>>>
>>> Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com>
>>> ---
>>>    vl.c | 2 ++
>>>    1 file changed, 2 insertions(+)
>>>
>>> diff --git a/vl.c b/vl.c
>>> index fce1fd12d8..cb07b19c02 100644
>>> --- a/vl.c
>>> +++ b/vl.c
>>> @@ -268,6 +268,7 @@ static QemuOptsList qemu_sandbox_opts = {
>>>                .name = "enable",
>>>                .type = QEMU_OPT_BOOL,
>>>            },
>>> +#ifdef CONFIG_SECCOMP
>>>            {
>>>                .name = "obsolete",
>>>                .type = QEMU_OPT_STRING,
>>> @@ -284,6 +285,7 @@ static QemuOptsList qemu_sandbox_opts = {
>>>                .name = "resourcecontrol",
>>>                .type = QEMU_OPT_STRING,
>>>            },
>>> +#endif
>> The commit message mentions only 'elevateprivileges' (once the typo is
>> fixed), but you are also crippling 'obsolete', 'spawn', and
>> 'resourcecontrol'.  Perhaps the commit message should call that out better?
>> Or, since libvirt is looking at just 'elevateprivileges', per this line in
>> libvirt's qemu_capabilities.c:
>>
>> src/qemu/qemu_capabilities.c:    { "sandbox", "elevateprivileges",
>> QEMU_CAPS_SECCOMP_BLACKLIST },
>>
>> is it sufficient to just mask out that one option?
> If seccomp is disabled, we should really disable the entire -sandbox
> argument, not merly the options to it.
I think it would bring a lot of changes if disable the entire -sandbox 
argument.
Looking from current code, sandbox is a default qemu option group, and 
sandbox.enable is false by default unless you obviously define it with true.
So, this patch is an easier way to fixup.
>
> Regards,
> Daniel