[Qemu-devel] [PATCH v8 0/4] Introduce strtosz and make use of it

[Qemu-devel] [PATCH v8 0/4] Introduce strtosz and make use of it

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

This patch introduces cutils.c: strtosz() and gets rid of the
multiple custom hacks for parsing byte sizes. In addition it adds
supports for specifying human style sizes such as 1.5G. Last it
eliminates the horrible abuse of a float to store the byte size for
migrate_set_speed in the monitor.

Note, this is tested on Linux and build tested for win32 using
mingw32.

v8 per Markus' suggestion, always write endptr, even on error.

This time I hope it's there :)

Cheers,
Jes


Jes Sorensen (4):
  Introduce strtosz() library function to convert a string to a byte
    count.
  Add support for 'o' octet (bytes) format as monitor parameter.
  Switch migrate_set_speed() to take an 'o' argument rather than a
    float.
  Remove obsolete 'f' double parameter type

 cutils.c        |   79 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hmp-commands.hx |    5 ++-
 migration.c     |    4 +-
 monitor.c       |   47 +++++++++++++++++++++------------
 qemu-common.h   |    1 +
 vl.c            |   31 +++++++--------------
 6 files changed, 125 insertions(+), 42 deletions(-)

-- 
1.7.2.3

[Qemu-devel] [PATCH 1/4] Introduce strtosz() library function to convert a string to a byte count.

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

strtosz() returns -1 on error. It now supports human unit formats in
eg. 1.0G, with better error handling.

The following suffixes are supported:
B/b = bytes
K/k = KB
M/m = MB
G/g = GB
T/t = TB

This patch changes -numa and -m input to use strtosz().

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 cutils.c      |   79 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 qemu-common.h |    1 +
 vl.c          |   31 +++++++---------------
 3 files changed, 90 insertions(+), 21 deletions(-)

diff --git a/cutils.c b/cutils.c
index 5883737..4bb459d 100644
--- a/cutils.c
+++ b/cutils.c
@@ -23,6 +23,7 @@
  */
 #include "qemu-common.h"
 #include "host-utils.h"
+#include <math.h>
 
 void pstrcpy(char *buf, int buf_size, const char *str)
 {
@@ -283,3 +284,81 @@ int fcntl_setfl(int fd, int flag)
 }
 #endif
 
+/*
+ * Convert string to bytes, allowing either B/b for bytes, K/k for KB,
+ * M/m for MB, G/g for GB or T/t for TB. Default without any postfix
+ * is MB. End pointer will be returned in *end, if not NULL.
+ * Return -1 on error.
+ */
+ssize_t strtosz(const char *nptr, char **end)
+{
+    ssize_t retval = -1;
+    char *endptr, c;
+    int mul_required = 0;
+    double val, mul, integral, fraction;
+
+    errno = 0;
+    val = strtod(nptr, &endptr);
+    if (isnan(val) || endptr == nptr || errno != 0 || val < 0 ||
+        val >= HUGE_VAL) {
+        goto fail;
+    }
+    integral = modf(val, &fraction);
+    if (integral != 0) {
+        mul_required = 1;
+    }
+    /*
+     * Any whitespace character is fine for terminating the number,
+     * in addition we accept ',' to handle strings where the size is
+     * part of a multi token argument. Otherwise check that the suffix
+     * is just one character.
+     */
+    c = *endptr++;
+    if (isspace(c) || c == '\0' || c == ',') {
+        c = 0;
+    } else if (!isspace(*endptr) && *endptr != 0) {
+        goto fail;
+    }
+    switch (c) {
+    case 'B':
+    case 'b':
+        mul = 1;
+        if (mul_required) {
+            goto fail;
+        }
+        break;
+    case 'K':
+    case 'k':
+        mul = 1 << 10;
+        break;
+    case 0:
+        if (mul_required) {
+            goto fail;
+        }
+    case 'M':
+    case 'm':
+        mul = 1ULL << 20;
+        break;
+    case 'G':
+    case 'g':
+        mul = 1ULL << 30;
+        break;
+    case 'T':
+    case 't':
+        mul = 1ULL << 40;
+        break;
+    default:
+        goto fail;
+    }
+    if (val * mul >= ~(size_t)0) {
+        goto fail;
+    }
+    retval = (val * mul);
+
+fail:
+    if (end) {
+        *end = endptr;
+    }
+
+    return retval;
+}
diff --git a/qemu-common.h b/qemu-common.h
index 81aafa0..0a062d4 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -153,6 +153,7 @@ time_t mktimegm(struct tm *tm);
 int qemu_fls(int i);
 int qemu_fdatasync(int fd);
 int fcntl_setfl(int fd, int flag);
+ssize_t strtosz(const char *nptr, char **end);
 
 /* path.c */
 void init_paths(const char *prefix);
diff --git a/vl.c b/vl.c
index df414ef..6043fa2 100644
--- a/vl.c
+++ b/vl.c
@@ -734,16 +734,13 @@ static void numa_add(const char *optarg)
         if (get_param_value(option, 128, "mem", optarg) == 0) {
             node_mem[nodenr] = 0;
         } else {
-            value = strtoull(option, &endptr, 0);
-            switch (*endptr) {
-            case 0: case 'M': case 'm':
-                value <<= 20;
-                break;
-            case 'G': case 'g':
-                value <<= 30;
-                break;
+            ssize_t sval;
+            sval = strtosz(option, NULL);
+            if (sval < 0) {
+                fprintf(stderr, "qemu: invalid numa mem size: %s\n", optarg);
+                exit(1);
             }
-            node_mem[nodenr] = value;
+            node_mem[nodenr] = sval;
         }
         if (get_param_value(option, 128, "cpus", optarg) == 0) {
             node_cpumask[nodenr] = 0;
@@ -2163,18 +2160,10 @@ int main(int argc, char **argv, char **envp)
                 exit(0);
                 break;
             case QEMU_OPTION_m: {
-                uint64_t value;
-                char *ptr;
+                ssize_t value;
 
-                value = strtoul(optarg, &ptr, 10);
-                switch (*ptr) {
-                case 0: case 'M': case 'm':
-                    value <<= 20;
-                    break;
-                case 'G': case 'g':
-                    value <<= 30;
-                    break;
-                default:
+                value = strtosz(optarg, NULL);
+                if (value < 0) {
                     fprintf(stderr, "qemu: invalid ram size: %s\n", optarg);
                     exit(1);
                 }
-- 
1.7.2.3

Re: [Qemu-devel] [PATCH 1/4] Introduce strtosz() library function to convert a string to a byte count.

[Markus Armbruster]

Jes.Sorensen@redhat.com writes:

> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>
> strtosz() returns -1 on error. It now supports human unit formats in
> eg. 1.0G, with better error handling.
>
> The following suffixes are supported:
> B/b = bytes
> K/k = KB
> M/m = MB
> G/g = GB
> T/t = TB
>
> This patch changes -numa and -m input to use strtosz().
>
> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> ---
>  cutils.c      |   79 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  qemu-common.h |    1 +
>  vl.c          |   31 +++++++---------------
>  3 files changed, 90 insertions(+), 21 deletions(-)
>
> diff --git a/cutils.c b/cutils.c
> index 5883737..4bb459d 100644
> --- a/cutils.c
> +++ b/cutils.c
> @@ -23,6 +23,7 @@
>   */
>  #include "qemu-common.h"
>  #include "host-utils.h"
> +#include <math.h>
>  
>  void pstrcpy(char *buf, int buf_size, const char *str)
>  {
> @@ -283,3 +284,81 @@ int fcntl_setfl(int fd, int flag)
>  }
>  #endif
>  
> +/*
> + * Convert string to bytes, allowing either B/b for bytes, K/k for KB,
> + * M/m for MB, G/g for GB or T/t for TB. Default without any postfix
> + * is MB. End pointer will be returned in *end, if not NULL.
> + * Return -1 on error.
> + */
> +ssize_t strtosz(const char *nptr, char **end)
> +{
> +    ssize_t retval = -1;
> +    char *endptr, c;
> +    int mul_required = 0;
> +    double val, mul, integral, fraction;
> +
> +    errno = 0;
> +    val = strtod(nptr, &endptr);
> +    if (isnan(val) || endptr == nptr || errno != 0 || val < 0 ||
> +        val >= HUGE_VAL) {
> +        goto fail;
> +    }
> +    integral = modf(val, &fraction);
> +    if (integral != 0) {
> +        mul_required = 1;
> +    }
> +    /*
> +     * Any whitespace character is fine for terminating the number,
> +     * in addition we accept ',' to handle strings where the size is
> +     * part of a multi token argument. Otherwise check that the suffix
> +     * is just one character.
> +     */
> +    c = *endptr++;

Uh-oh...

> +    if (isspace(c) || c == '\0' || c == ',') {
> +        c = 0;
> +    } else if (!isspace(*endptr) && *endptr != 0) {
> +        goto fail;
> +    }
> +    switch (c) {
> +    case 'B':
> +    case 'b':
> +        mul = 1;
> +        if (mul_required) {
> +            goto fail;
> +        }
> +        break;
> +    case 'K':
> +    case 'k':
> +        mul = 1 << 10;
> +        break;
> +    case 0:
> +        if (mul_required) {
> +            goto fail;
> +        }
> +    case 'M':
> +    case 'm':
> +        mul = 1ULL << 20;
> +        break;
> +    case 'G':
> +    case 'g':
> +        mul = 1ULL << 30;
> +        break;
> +    case 'T':
> +    case 't':
> +        mul = 1ULL << 40;
> +        break;
> +    default:
> +        goto fail;
> +    }

I'm afraid strtosz("1.0x", &end) makes end point behind the 'x' instead
of to the 'x'.

Solution: increment endptr when the unit character is consumed,
i.e. after the switch.  Finish consuming the unit character before
checking the suffix.

> +    if (val * mul >= ~(size_t)0) {
> +        goto fail;
> +    }
> +    retval = (val * mul);
> +
> +fail:
> +    if (end) {
> +        *end = endptr;
> +    }
> +
> +    return retval;
> +}
[...]

[Qemu-devel] [PATCH 2/4] Add support for 'o' octet (bytes) format as monitor parameter.

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Octet format relies on strtosz which supports K/k, M/m, G/g, T/t
suffixes and unit support for humans, like 1.3G

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 monitor.c |   29 +++++++++++++++++++++++++++++
 1 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/monitor.c b/monitor.c
index fbb678d..b0ea5a3 100644
--- a/monitor.c
+++ b/monitor.c
@@ -78,6 +78,11 @@
  * 'l'          target long (32 or 64 bit)
  * 'M'          just like 'l', except in user mode the value is
  *              multiplied by 2^20 (think Mebibyte)
+ * 'o'          octets (aka bytes)
+ *              user mode accepts an optional T, t, G, g, M, m, K, k
+ *              suffix, which multiplies the value by 2^40 for
+ *              suffixes T and t, 2^30 for suffixes G and g, 2^20 for
+ *              M and m, 2^10 for K and k
  * 'f'          double
  *              user mode accepts an optional G, g, M, m, K, k suffix,
  *              which multiplies the value by 2^30 for suffixes G and
@@ -3699,6 +3704,29 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
                 qdict_put(qdict, key, qint_from_int(val));
             }
             break;
+        case 'o':
+            {
+                ssize_t val;
+                char *end;
+
+                while (qemu_isspace(*p)) {
+                    p++;
+                }
+                if (*typestr == '?') {
+                    typestr++;
+                    if (*p == '\0') {
+                        break;
+                    }
+                }
+                val = strtosz(p, &end);
+                if (val < 0) {
+                    monitor_printf(mon, "invalid size\n");
+                    goto fail;
+                }
+                qdict_put(qdict, key, qint_from_int(val));
+                p = end;
+            }
+            break;
         case 'f':
         case 'T':
             {
@@ -4196,6 +4224,7 @@ static int check_client_args_type(const QDict *client_args,
         case 'i':
         case 'l':
         case 'M':
+        case 'o':
             if (qobject_type(client_arg) != QTYPE_QINT) {
                 qerror_report(QERR_INVALID_PARAMETER_TYPE, client_arg_name,
                               "int");
-- 
1.7.2.3

[Qemu-devel] [PATCH 3/4] Switch migrate_set_speed() to take an 'o' argument rather than a float.

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Clarify default value of MB in migration speed argument in monitor, if
no suffix is specified. This differ from previous default of bytes,
but is consistent with the rest of the places where we accept a size
argument.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 hmp-commands.hx |    5 +++--
 migration.c     |    4 ++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 81999aa..e5585ba 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -754,9 +754,10 @@ ETEXI
 
     {
         .name       = "migrate_set_speed",
-        .args_type  = "value:f",
+        .args_type  = "value:o",
         .params     = "value",
-        .help       = "set maximum speed (in bytes) for migrations",
+        .help       = "set maximum speed (in bytes) for migrations. "
+	"Defaults to MB if no size suffix is specified, ie. B/K/M/G/T",
         .user_print = monitor_user_noop,
         .mhandler.cmd_new = do_migrate_set_speed,
     },
diff --git a/migration.c b/migration.c
index 468d517..9ee8b17 100644
--- a/migration.c
+++ b/migration.c
@@ -132,10 +132,10 @@ int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
 
 int do_migrate_set_speed(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
-    double d;
+    int64_t d;
     FdMigrationState *s;
 
-    d = qdict_get_double(qdict, "value");
+    d = qdict_get_int(qdict, "value");
     d = MAX(0, MIN(UINT32_MAX, d));
     max_throttle = d;
 
-- 
1.7.2.3

[Qemu-devel] [PATCH 4/4] Remove obsolete 'f' double parameter type

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

'f' double is no longer used, and we should be using floating point
variables to store byte sizes. Remove it.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 monitor.c |   18 +-----------------
 1 files changed, 1 insertions(+), 17 deletions(-)

diff --git a/monitor.c b/monitor.c
index b0ea5a3..078a66e 100644
--- a/monitor.c
+++ b/monitor.c
@@ -83,10 +83,6 @@
  *              suffix, which multiplies the value by 2^40 for
  *              suffixes T and t, 2^30 for suffixes G and g, 2^20 for
  *              M and m, 2^10 for K and k
- * 'f'          double
- *              user mode accepts an optional G, g, M, m, K, k suffix,
- *              which multiplies the value by 2^30 for suffixes G and
- *              g, 2^20 for M and m, 2^10 for K and k
  * 'T'          double
  *              user mode accepts an optional ms, us, ns suffix,
  *              which divides the value by 1e3, 1e6, 1e9, respectively
@@ -3727,7 +3723,6 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
                 p = end;
             }
             break;
-        case 'f':
         case 'T':
             {
                 double val;
@@ -3743,17 +3738,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
                 if (get_double(mon, &val, &p) < 0) {
                     goto fail;
                 }
-                if (c == 'f' && *p) {
-                    switch (*p) {
-                    case 'K': case 'k':
-                        val *= 1 << 10; p++; break;
-                    case 'M': case 'm':
-                        val *= 1 << 20; p++; break;
-                    case 'G': case 'g':
-                        val *= 1 << 30; p++; break;
-                    }
-                }
-                if (c == 'T' && p[0] && p[1] == 's') {
+                if (p[0] && p[1] == 's') {
                     switch (*p) {
                     case 'm':
                         val /= 1e3; p += 2; break;
@@ -4231,7 +4216,6 @@ static int check_client_args_type(const QDict *client_args,
                 return -1; 
             }
             break;
-        case 'f':
         case 'T':
             if (qobject_type(client_arg) != QTYPE_QINT &&
                 qobject_type(client_arg) != QTYPE_QFLOAT) {
-- 
1.7.2.3