Re: [Xen-devel] [PATCH v2] Rationalize max_grant_frames and max_maptrack_frames handling

[Jan Beulich <jbeulich@xxxxxxxx>]

On 28.11.2019 11:26,  Durrant, Paul  wrote:
>> From: George Dunlap <george.dunlap@xxxxxxxxxx>
>> Sent: 27 November 2019 16:52
>>
>> On 11/27/19 4:43 PM, Durrant, Paul wrote:
>>>> From: George Dunlap <george.dunlap@xxxxxxxxxx>
>>>> Sent: 27 November 2019 16:34
>>>>
>>>> TBH I'd be tempted to define XENSOMETHING_MAX_DEFAULT as (unsigned
>>>> long)(-1) or something, and explicitly compare to that.  That leaves
>>>> open the possibility of having more sentinel values if we decided we
>>>> wanted them.
>>>
>>> I'm extremely confused now. What do you want me to compare and where?
>>>
>>> I assume we're talking about the opt_XXX values. Am I supposed to stop
>>> INT_MAX being assigned to them? Or should I define local unsigned values
>> for max_maptrack/grant_frames and simply initialize them to the passed-in
>> arg (if >= 0) or the opt_XXX value otherwise.
>>
>> In this version of the patch, you change the domctl arguments from
>> uint32_t to int32_t.  I would leave them uint32_t, and if (
>> max_grant_frames == XENSOMETHING_MAX_DEFAULT ) max_grant_frames = opt_&c.
>>
>> Then the only invalid value we have to worry about is checking for
>> XENSOMETHING_MAX_DEFAULT.
>>
>> This is a suggestion, and I wouldn't argue strongly if someone thought
>> it was a bad idea, but it seems like the most straightforward option to
>> me.
> 
> AFAICT the definition of that invalid value is going to be needed by both
> the grant table code and the user-space toolstack code so I guess the
> logical place for the definition would be a tools-only section of the
> public grant table header? TBH I prefer the idea of any negative value
> being default though.

FWIW I agree, as I can't really see what other purposes we might need
sentinel values for down the road.

> As long as the xl/libxl parts don't allow a *specified* value > INT_MAX
> then that should be fine, although for the full story a custom parser
> for the command line values should also be added to ensure the same
> semantics there.

Right. While going a little far, I can't right now see easy alternatives
to a custom parser.

Jan

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