Re: [Xen-devel] [PATCH v2 2/3] xen/x86: add dom0 memory sizing variants

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 06.12.18 at 12:20, <jgross@xxxxxxxx> wrote:
> On 06/12/2018 12:08, Jan Beulich wrote:
>>>>> On 06.12.18 at 09:06, <jgross@xxxxxxxx> wrote:
>>> Today the memory size of dom0 can be specified only in terms of bytes
>>> (either an absolute value or "host-mem - value"). When dom0 shouldn't
>>> be auto-ballooned this requires nearly always a manual adaption of the
>>> Xen boot parameters to reflect the actual host memory size.
>>>
>>> Add more possibilities to specify memory sizes. Today we have:
>>>
>>> dom0_mem= List of ( min:<size> | max:<size> | <size> )
>>>
>>> with <size> being a positive or negative size value (e.g. 1G).
>>>
>>> Modify that to:
>>>
>>> dom0_mem= List of ( min:<sz> | max:<sz> | <sz> )
>>> <sz>: <size> | [<size>+]<frac>%
>>> <frac>: integer value < 100
>>>
>>> With the following semantics:
>>>
>>> <frac>% specifies a fraction of host memory size in percent.
>>> <sz> is a percentage of host memory plus an offset.
>>>
>>> So <sz> being 1G+25% on a 256G host would result in 65G.
>>>
>>> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
>> 
>> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
>> 
>> I notice though that ...
>> 
>>> -static long __init parse_amt(const char *s, const char **ps)
>>> +static int __init parse_amt(const char *s, const char **ps, struct memsize 
>>> *sz)
>>>  {
>>> -    long pages = parse_size_and_unit((*s == '-') ? s+1 : s, ps) >> 
>>> PAGE_SHIFT;
>>> -    return (*s == '-') ? -pages : pages;
>>> +    unsigned long val;
>>> +    struct memsize tmp = { };
>>> +
>>> +    tmp.minus = (*s == '-');
>>> +    if ( tmp.minus )
>>> +        s++;
>>> +
>>> +    /* Avoid accessing s[-1] in case value starts with '%'. */
>>> +    if ( *s == '%' )
>>> +        return -EINVAL;
>>> +
>>> +    while ( isdigit(*s) )
>>> +    {
>>> +        val = parse_size_and_unit_or_int(s, ps, '%');
>>> +        s = *ps;
>>> +        if ( *s == '%' )
>>> +        {
>>> +            if ( !isdigit(s[-1]) || val >= 100 )
>>> +                return -EINVAL;
>>> +            tmp.percent = val;
>>> +            s++;
>>> +        }
>>> +        else
>>> +            tmp.nr_pages = val >> PAGE_SHIFT;
>>> +        if ( *s == '+' )
>>> +            s++;
>>> +    }
>> 
>> ... you allow more flexibility here than you document (i.e. also
>> <percentage>+<basesize>). You may want to consider
>> refusing something like 1G+10%+10%, though.
> 
> Okay, should be fairly easy.
> 
> Can I keep your R-b: with adding something like:
> 
> +    bool percent = false;
> ...
> -    while ( isdigit(*s) )
> +    while ( isdigit(*s) && !percent )
> ...
>          if ( *s == '%' )
>          {
> +            percent = true;
> ...

Something like this, yes. The double percent value was just an
example though, "1G+10%+1G" then too would better either work
as written, or be refused.

Jan



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