Re: [PATCH v3] tools/xl: fix autoballoon regex

[Anthony PERARD]

On Thu, Sep 16, 2021 at 03:15:21PM +0300, Dmitry Isaykin wrote:
> This regex is used for auto-balloon mode detection based on Xen command line.
> 
> The case of specifying a negative size was handled incorrectly.
> From misc/xen-command-line documentation:
> 
>     dom0_mem (x86)
>     = List of ( min:<sz> | max:<sz> | <sz> )
> 
>     If a size is positive, it represents an absolute value.
>     If a size is negative, it is subtracted from the total available memory.
> 
> Also add support for [tT] granularity suffix.
> Also add support for memory fractions (i.e. '50%' or '1G+25%').
> 
> Signed-off-by: Dmitry Isaykin <isaikin-dmi...@yandex.ru>
> ---
>      ret = regcomp(&regex,
> -                  "(^| )dom0_mem=((|min:|max:)[0-9]+[bBkKmMgG]?,?)+($| )",
> +                  "(^| 
> )dom0_mem=((|min:|max:)(-?[0-9]+[bBkKmMgGtT]?)?(\+?[0-9]+%)?,?)+($| )",

It seems that by trying to match fractions, the new regex would match
too much. For example, if there is " dom0_mem= " on the command line, xl
would detect it as autoballoon=off, while it isn't the case without this
patch. I don't know if it is possible to have "dom0_mem=" on the command
line as I don't know what Xen would do in this case.

It might be better to make the regex more complicated and match
fraction like they are described in the doc, something like:
    ( <size> | (<size>\+)?<frac>% )

unless xen doesn't boot with bogus value for dom0_mem, but I haven't
checked. (we could use CPP macros to avoid duplicating the <size>
regex.)

Also, <frac> is supposed to be < 100, so [0-9]{1,2} would be better to
only match no more than 2 digit.

Thought?

Thanks,

-- 
Anthony PERARD