Re: [Xen-devel] [PATCH v2 3/3] tools: introduce parameter max_wp_ram_ranges.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 27.01.16 at 08:01, <yu.c.zhang@xxxxxxxxxxxxxxx> wrote:

> 
> On 1/26/2016 7:00 PM, Jan Beulich wrote:
>>>>> On 26.01.16 at 08:32, <yu.c.zhang@xxxxxxxxxxxxxxx> wrote:
>>> On 1/22/2016 4:01 PM, Jan Beulich wrote:
>>>>>>> On 22.01.16 at 04:20, <yu.c.zhang@xxxxxxxxxxxxxxx> wrote:
>>>>> --- a/xen/arch/x86/hvm/hvm.c
>>>>> +++ b/xen/arch/x86/hvm/hvm.c
>>>>> @@ -940,6 +940,10 @@ static int hvm_ioreq_server_alloc_rangesets(struct
>>>>> hvm_ioreq_server *s,
>>>>>    {
>>>>>        unsigned int i;
>>>>>        int rc;
>>>>> +    unsigned int max_wp_ram_ranges =
>>>>> +        ( s->domain->arch.hvm_domain.params[HVM_PARAM_MAX_WP_RAM_RANGES] 
>>>>> > 0 ) ?
>>>>> +        s->domain->arch.hvm_domain.params[HVM_PARAM_MAX_WP_RAM_RANGES] :
>>>>> +        MAX_NR_IO_RANGES;
>>>>
>>>> Besides this having stray blanks inside the parentheses it truncates
>>>> the value from 64 to 32 bits and would benefit from using the gcc
>>>> extension of omitting the middle operand of ?:. But even better
>>>> would imo be if you avoided the local variable and ...
>>>>
>>> After second thought, how about we define a default value for this
>>> parameter in libx.h, and initialize the parameter when creating the
>>> domain with default value if it's not configured.
>>
>> No, I don't think the tool stack should be determining the default
>> here (unless you want the default to be zero, and have zero
>> indeed mean zero).
>>
> Thank you, Jan.
> If we do not provide a default value in tool stack, the code above
> should be kept, to initialize the local variable with either the one
> set in the configuration file, or with MAX_NR_IO_RANGES. Is this OK?

Well, not exactly: For one, the original comment (still present
above) regarding truncation holds. And then another question is:
Do you expect this resource type to be useful with its number of
ranges limited to MAX_NR_IO_RANGES? I ask because if the
answer is "no", having it default to zero might be as reasonable.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel