Re: [Xen-devel] [PATCH v2 3/5] xen/domain: Stricter configuration checking

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.11.18 at 16:07, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 13/11/2018 14:14, Jan Beulich wrote:
>>>>> On 12.11.18 at 17:16, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> --- a/xen/arch/x86/domain.c
>>> +++ b/xen/arch/x86/domain.c
>>> @@ -420,6 +420,46 @@ void arch_vcpu_destroy(struct vcpu *v)
>>>  
>>>  int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
>>>  {
>>> +    bool hvm;
>>> +
>>> +    if ( !IS_ENABLED(CONFIG_PV) && !(config->flags & 
>>> XEN_DOMCTL_CDF_hvm_guest) )
>>> +    {
>>> +        dprintk(XENLOG_INFO, "PV support not available\n");
>>> +        return -EINVAL;
>>> +    }
>>> +
>>> +    if ( !hvm_enabled && (config->flags & XEN_DOMCTL_CDF_hvm_guest) )
>>> +    {
>>> +        dprintk(XENLOG_INFO, "HVM support not available\n");
>>> +        return -EINVAL;
>>> +    }
>>> +
>>> +    hvm = config->flags & XEN_DOMCTL_CDF_hvm_guest;
>> Would you mind making this the initializer of the variable and using
>> the variable in the two if()-s above? Personally I also think the two
>> if()-s would better be folded, using a conditional expression as its
>> condition.
> 
> I can move the initialiser, but how do you propose folding the
> conditionals given their different contents?

    if ( hvm ? !hvm_enabled : !IS_ENABLED(CONFIG_PV) )
    {
        dprintk(XENLOG_INFO, "%s support not available\n", hvm ? "HVM" : "PV");
        return -EINVAL;
    }

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel