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

[Dmitry Isaykin <isaikin-dmitry@xxxxxxxxx>]

Thanks! That's a good idea. I will do it in v4 patch.

It seems that "dom0_mem=" is a correct setting. It means "give all unused by hypervisor memory to dom0".

15:36, 28 сентября 2021 г., Anthony PERARD <anthony.perard@xxxxxxxxxx>:

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-dmitry@xxxxxxxxx>
 ---
      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

--
Отправлено из мобильного приложения Яндекс.Почты