[v3] tools/xl: fix autoballoon regex

Commit Message

Dmitry Isaykin Sept. 16, 2021, 12:15 p.m. UTC

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 <[e-mail]>
---
Changes in v3:
- add support for [tT] granularity suffix
- add support for memory fractions

Changes in v2:
- add missing Signed-off-by tag
---
 tools/xl/xl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Anthony PERARD Sept. 28, 2021, 12:36 p.m. UTC | #1

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 <[e-mail]>
> ---
>      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,

Patch

diff --git a/tools/xl/xl.c b/tools/xl/xl.c
index 4107d10fd4..8500b3ac57 100644
--- a/tools/xl/xl.c
+++ b/tools/xl/xl.c
@@ -81,7 +81,7 @@  static int auto_autoballoon(void)
         return 1; /* default to on */
 
     ret = regcomp(&regex,
-                  "(^| )dom0_mem=((|min:|max:)[0-9]+[bBkKmMgG]?,?)+($| )",
+                  "(^| )dom0_mem=((|min:|max:)(-?[0-9]+[bBkKmMgGtT]?)?(\+?[0-9]+%)?,?)+($| )",
                   REG_NOSUB | REG_EXTENDED);
     if (ret)
         return 1;