[Qemu-devel] [PATCH 0/5] Introduce strtobytes and make use of it

[Qemu-devel] [PATCH 0/5] Introduce strtobytes and make use of it

[Jes.Sorensen]

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

This patch introduces cutils.c: strtobytes() and gets rid of the
multiple custom hacks for parsing byte sizes. In addition it add
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.

Jes

Jes Sorensen (5):
  Introduce strtobytes() library function to convert string to byte
    count.
  Support human unit formats in strtobytes, eg. 1.0G
  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        |   64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 migration.c     |    4 +-
 monitor.c       |   44 ++++++++++++++++++++++++-------------
 qemu-common.h   |    1 +
 qemu-monitor.hx |    2 +-
 vl.c            |   26 ++++++----------------
 6 files changed, 103 insertions(+), 38 deletions(-)

-- 
1.7.2.2

[Qemu-devel] [PATCH 1/5] Introduce strtobytes() library function to convert string to byte count.

[Jes.Sorensen]

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

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 cutils.c      |   39 +++++++++++++++++++++++++++++++++++++++
 qemu-common.h |    1 +
 vl.c          |   26 +++++++-------------------
 3 files changed, 47 insertions(+), 19 deletions(-)

diff --git a/cutils.c b/cutils.c
index 036ae3c..a3087fe 100644
--- a/cutils.c
+++ b/cutils.c
@@ -251,3 +251,42 @@ int fcntl_setfl(int fd, int flag)
 }
 #endif
 
+/*
+ * Convert string to bytes, allowing either K/k for KB, M/m for MB,
+ * G/b for GB or T/t for TB. Default without any postfix is MB.
+ * End pointer will be returned in *end, if end is valid.
+ * Return 0 on error.
+ */
+uint64_t strtobytes(const char *nptr, char **end)
+{
+    uint64_t value;
+    char *endptr;
+
+    value = strtoll(nptr, &endptr, 0);
+    switch (*endptr++) {
+    case 'K':
+    case 'k':
+        value <<= 10;
+        break;
+    case 0:
+    case 'M':
+    case 'm':
+        value <<= 20;
+        break;
+    case 'G':
+    case 'g':
+        value <<= 30;
+        break;
+    case 'T':
+    case 't':
+        value <<= 40;
+        break;
+    default:
+        value = 0;
+    }
+
+    if (end)
+        *end = endptr;
+
+    return value;
+}
diff --git a/qemu-common.h b/qemu-common.h
index dfd3dc0..01d7dcb 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -137,6 +137,7 @@ time_t mktimegm(struct tm *tm);
 int qemu_fls(int i);
 int qemu_fdatasync(int fd);
 int fcntl_setfl(int fd, int flag);
+uint64_t strtobytes(const char *nptr, char **end);
 
 /* path.c */
 void init_paths(const char *prefix);
diff --git a/vl.c b/vl.c
index 3f45aa9..0cdd94a 100644
--- a/vl.c
+++ b/vl.c
@@ -734,14 +734,10 @@ 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;
+            value = strtobytes(option, NULL);
+            if (!value) {
+                fprintf(stderr, "qemu: invalid numa mem size: %s\n", optarg);
+                exit(1);
             }
             node_mem[nodenr] = value;
         }
@@ -2166,17 +2162,9 @@ int main(int argc, char **argv, char **envp)
                 break;
             case QEMU_OPTION_m: {
                 uint64_t value;
-                char *ptr;
 
-                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 = strtobytes(optarg, NULL);
+                if (!value) {
                     fprintf(stderr, "qemu: invalid ram size: %s\n", optarg);
                     exit(1);
                 }
-- 
1.7.2.2

[Qemu-devel] [PATCH 2/5] Support human unit formats in strtobytes, eg. 1.0G

[Jes.Sorensen]

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

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

diff --git a/cutils.c b/cutils.c
index a3087fe..d34ed08 100644
--- a/cutils.c
+++ b/cutils.c
@@ -259,16 +259,38 @@ int fcntl_setfl(int fd, int flag)
  */
 uint64_t strtobytes(const char *nptr, char **end)
 {
-    uint64_t value;
+    uint64_t value, value2;
     char *endptr;
+    int divider = 0;
 
     value = strtoll(nptr, &endptr, 0);
+    if (endptr[0] == '.') {
+        endptr++;
+        value2 = 0;
+        divider = 10;
+        while ((endptr[0] == '0') && (endptr[1] >= '0') && (endptr[1] <= '9')) {
+            divider = divider * 10;
+            endptr++;
+        }
+
+        if ((endptr[0] >= '0') && (endptr[0] <= '9')) {
+            value2 = strtoll(endptr, &endptr, 0);
+            value = value * divider + value2;
+        } else {
+            value = 0;
+            goto fail;
+        }
+    }
     switch (*endptr++) {
     case 'K':
     case 'k':
         value <<= 10;
         break;
     case 0:
+        if (divider) {
+            value = 0;
+            break;
+        }
     case 'M':
     case 'm':
         value <<= 20;
@@ -284,9 +306,12 @@ uint64_t strtobytes(const char *nptr, char **end)
     default:
         value = 0;
     }
+    if (divider)
+        value /= divider;
 
     if (end)
         *end = endptr;
 
+fail:
     return value;
 }
-- 
1.7.2.2

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

[Jes.Sorensen]

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

Octet format relies on strtobytes 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 |   27 +++++++++++++++++++++++++++
 1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/monitor.c b/monitor.c
index e602480..3630061 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
@@ -3594,6 +3599,28 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
                 qdict_put(qdict, key, qint_from_int(val));
             }
             break;
+        case 'o':
+            {
+                int64_t val;
+                char *end;
+
+                while (qemu_isspace(*p))
+                    p++;
+                if (*typestr == '?') {
+                    typestr++;
+                    if (*p == '\0') {
+                        break;
+                    }
+                }
+                val = strtobytes(p, &end);
+                if (!val) {
+                    monitor_printf(mon, "invalid size\n");
+                    goto fail;
+                }
+                qdict_put(qdict, key, qint_from_int(val));
+                p = end;
+            }
+            break;
         case 'f':
         case 'T':
             {
-- 
1.7.2.2

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

[Jes.Sorensen]

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

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

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;
 
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 49bcd8d..7f58fb2 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -1093,7 +1093,7 @@ EQMP
 
     {
         .name       = "migrate_set_speed",
-        .args_type  = "value:f",
+        .args_type  = "value:o",
         .params     = "value",
         .help       = "set maximum speed (in bytes) for migrations",
         .user_print = monitor_user_noop,
-- 
1.7.2.2

[Qemu-devel] [PATCH 5/5] 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 |   17 +----------------
 1 files changed, 1 insertions(+), 16 deletions(-)

diff --git a/monitor.c b/monitor.c
index 3630061..ffeb4ee 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
@@ -3621,7 +3617,6 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
                 p = end;
             }
             break;
-        case 'f':
         case 'T':
             {
                 double val;
@@ -3637,17 +3632,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;
-- 
1.7.2.2

[Qemu-devel] Re: [PATCH 2/5] Support human unit formats in strtobytes, eg. 1.0G

[Juan Quintela]

Jes.Sorensen@redhat.com wrote:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>          value <<= 10;
>          break;
>      case 0:
> +        if (divider) {
> +            value = 0;
> +            break;

changing break by goto fail here?
1.5G and 1.0G is ok, but using 1024.00  or similar should be one error,
no?

nice cleanup for the rest of the patch series.

Later, Juan.


> +        }
>      case 'M':
>      case 'm':
>          value <<= 20;

[Qemu-devel] Re: [PATCH 2/5] Support human unit formats in strtobytes, eg. 1.0G

[Paolo Bonzini]

On 09/15/2010 02:23 PM, Jes.Sorensen@redhat.com wrote:
>       switch (*endptr++) {
>       case 'K':
>       case 'k':
>           value<<= 10;
>           break;
>       case 0:
> +        if (divider) {
> +            value = 0;
> +            break;
> +        }
>       case 'M':
>       case 'm':
>           value<<= 20;
> @@ -284,9 +306,12 @@ uint64_t strtobytes(const char *nptr, char **end)
>       default:
>           value = 0;
>       }
> +    if (divider)
> +        value /= divider;
>

This risks overflow if you do 1.00000000000000G or something similarly 
braindead.  Do we loathe floating point so much that you cannot use 
strtod, like

     endptr1 = nptr + strspn(s, "0123456789.");
     switch (*endptr1)
     {
     case 0: divider = 1; break;
     case 'm': divider = 1 << 20; break;
     ...
     default: /* error, including for 1.0e+5 and negative */
     }
     value = (uint64_t) (strtod(nptr, &endptr2) / divider);
     if (endptr1 != endptr2) /* error, e.g. 1.2.3 */

     return value;

Paolo

Re: [Qemu-devel] Re: [PATCH 2/5] Support human unit formats in strtobytes, eg. 1.0G

[Anthony Liguori]

On 09/15/2010 10:45 AM, Paolo Bonzini wrote:
> On 09/15/2010 02:23 PM, Jes.Sorensen@redhat.com wrote:
>>       switch (*endptr++) {
>>       case 'K':
>>       case 'k':
>>           value<<= 10;
>>           break;
>>       case 0:
>> +        if (divider) {
>> +            value = 0;
>> +            break;
>> +        }
>>       case 'M':
>>       case 'm':
>>           value<<= 20;
>> @@ -284,9 +306,12 @@ uint64_t strtobytes(const char *nptr, char **end)
>>       default:
>>           value = 0;
>>       }
>> +    if (divider)
>> +        value /= divider;
>>
>
> This risks overflow if you do 1.00000000000000G or something similarly 
> braindead.  Do we loathe floating point so much that you cannot use 
> strtod, like

It should be strtod.  Only badness can happen otherwise.

Regards,

Anthony Liguori

>     endptr1 = nptr + strspn(s, "0123456789.");
>     switch (*endptr1)
>     {
>     case 0: divider = 1; break;
>     case 'm': divider = 1 << 20; break;
>     ...
>     default: /* error, including for 1.0e+5 and negative */
>     }
>     value = (uint64_t) (strtod(nptr, &endptr2) / divider);
>     if (endptr1 != endptr2) /* error, e.g. 1.2.3 */
>
>     return value;
> Paolo
>

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

[Andreas Färber]

Am 15.09.2010 um 14:23 schrieb Jes.Sorensen@redhat.com:
[...]
> +/*
> + * Convert string to bytes, allowing either K/k for KB, M/m for MB,
> + * G/b for GB or T/t for TB. Default without any postfix is MB.
       ^^^ typo
> + * End pointer will be returned in *end, if end is valid.
> + * Return 0 on error.
> + */

You seem to be refactoring existing code into this function, but the  
use of such suffixes usually brings up the question whether it's  
factor 1024 or 1000.
Here you're using 1024 apparently. If you don't want the user dealing  
with (imo ugly) Ki/Mi/Gi/Ti units this should at least be documented  
accordingly: G/g for GiB, etc. or G/g for GB = 1024 MB, etc.

Andreas

> +uint64_t strtobytes(const char *nptr, char **end)
> +{
> +    uint64_t value;
> +    char *endptr;
> +
> +    value = strtoll(nptr, &endptr, 0);
> +    switch (*endptr++) {
> +    case 'K':
> +    case 'k':
> +        value <<= 10;
> +        break;
> +    case 0:
> +    case 'M':
> +    case 'm':
> +        value <<= 20;
> +        break;
> +    case 'G':
> +    case 'g':
> +        value <<= 30;
> +        break;
> +    case 'T':
> +    case 't':
> +        value <<= 40;
> +        break;
> +    default:
> +        value = 0;
> +    }
> +
> +    if (end)
> +        *end = endptr;
> +
> +    return value;
> +}
> diff --git a/qemu-common.h b/qemu-common.h
> index dfd3dc0..01d7dcb 100644
> --- a/qemu-common.h
> +++ b/qemu-common.h
> @@ -137,6 +137,7 @@ time_t mktimegm(struct tm *tm);
> int qemu_fls(int i);
> int qemu_fdatasync(int fd);
> int fcntl_setfl(int fd, int flag);
> +uint64_t strtobytes(const char *nptr, char **end);
>
> /* path.c */
> void init_paths(const char *prefix);
> diff --git a/vl.c b/vl.c
> index 3f45aa9..0cdd94a 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -734,14 +734,10 @@ 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;
> +            value = strtobytes(option, NULL);
> +            if (!value) {
> +                fprintf(stderr, "qemu: invalid numa mem size: %s 
> \n", optarg);
> +                exit(1);
>             }
>             node_mem[nodenr] = value;
>         }
> @@ -2166,17 +2162,9 @@ int main(int argc, char **argv, char **envp)
>                 break;
>             case QEMU_OPTION_m: {
>                 uint64_t value;
> -                char *ptr;
>
> -                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 = strtobytes(optarg, NULL);
> +                if (!value) {
>                     fprintf(stderr, "qemu: invalid ram size: %s\n",  
> optarg);
>                     exit(1);
>                 }
> -- 
> 1.7.2.2

[Qemu-devel] Re: [PATCH 2/5] Support human unit formats in strtobytes, eg. 1.0G

[Jes Sorensen]

On 09/15/10 16:50, Juan Quintela wrote:
> Jes.Sorensen@redhat.com wrote:
>> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>>          value <<= 10;
>>          break;
>>      case 0:
>> +        if (divider) {
>> +            value = 0;
>> +            break;
> 
> changing break by goto fail here?
> 1.5G and 1.0G is ok, but using 1024.00  or similar should be one error,
> no?
> 
> nice cleanup for the rest of the patch series.

In my testing, 1234.5 fails as expected, so I expect 1024.00 to fail as
well.

Cheers,
Jes

[Qemu-devel] Re: [PATCH 2/5] Support human unit formats in strtobytes, eg. 1.0G

[Jes Sorensen]

On 09/15/10 17:45, Paolo Bonzini wrote:
> On 09/15/2010 02:23 PM, Jes.Sorensen@redhat.com wrote:
>>       switch (*endptr++) {
>>       case 'K':
>>       case 'k':
>>           value<<= 10;
>>           break;
>>       case 0:
>> +        if (divider) {
>> +            value = 0;
>> +            break;
>> +        }
>>       case 'M':
>>       case 'm':
>>           value<<= 20;
>> @@ -284,9 +306,12 @@ uint64_t strtobytes(const char *nptr, char **end)
>>       default:
>>           value = 0;
>>       }
>> +    if (divider)
>> +        value /= divider;
>>
> 
> This risks overflow if you do 1.00000000000000G or something similarly
> braindead.  Do we loathe floating point so much that you cannot use
> strtod, like

Floating point is just plain wrong. If someone wants to do something
like in your example they really ask for an error.

Jes

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

[Stefan Weil]

Am 15.09.2010 20:46, schrieb Andreas Färber:
> Am 15.09.2010 um 14:23 schrieb Jes.Sorensen@redhat.com:
> [...]
>> +/*
>> + * Convert string to bytes, allowing either K/k for KB, M/m for MB,
>> + * G/b for GB or T/t for TB. Default without any postfix is MB.
>       ^^^ typo
>> + * End pointer will be returned in *end, if end is valid.
>> + * Return 0 on error.
>> + */
>
> You seem to be refactoring existing code into this function, but the 
> use of such suffixes usually brings up the question whether it's 
> factor 1024 or 1000.
> Here you're using 1024 apparently. If you don't want the user dealing 
> with (imo ugly) Ki/Mi/Gi/Ti units this should at least be documented 
> accordingly: G/g for GiB, etc. or G/g for GB = 1024 MB, etc.
>
> Andreas
[snip]

I'd prefer the standard prefixes: KiB, MiB, GiB for powers of 1024, KB, 
MB, GB for powers of 1000.
The standard has the big advantage of being a standard, even if not 
everybody likes it.

Existing QEMU code should be cleaned (= changed) were needed.

Stefan

[Qemu-devel] Re: [PATCH 2/5] Support human unit formats in strtobytes, eg. 1.0G

[Paolo Bonzini]

On 09/15/2010 09:31 PM, Jes Sorensen wrote:
> On 09/15/10 17:45, Paolo Bonzini wrote:
>> On 09/15/2010 02:23 PM, Jes.Sorensen@redhat.com wrote:
>>>        switch (*endptr++) {
>>>        case 'K':
>>>        case 'k':
>>>            value<<= 10;
>>>            break;
>>>        case 0:
>>> +        if (divider) {
>>> +            value = 0;
>>> +            break;
>>> +        }
>>>        case 'M':
>>>        case 'm':
>>>            value<<= 20;
>>> @@ -284,9 +306,12 @@ uint64_t strtobytes(const char *nptr, char **end)
>>>        default:
>>>            value = 0;
>>>        }
>>> +    if (divider)
>>> +        value /= divider;
>>>
>>
>> This risks overflow if you do 1.00000000000000G or something similarly
>> braindead.  Do we loathe floating point so much that you cannot use
>> strtod, like
>
> Floating point is just plain wrong. If someone wants to do something
> like in your example they really ask for an error.

An error, not an overflow.

Adding overflow checking on top of your patch is also fine.  Another 
possibility is to look ahead for the multiplier so that you correctly 
base the divider and do everything in 64.64 fixed point.  But it seems 
overkill compared to floating-point, whose 53-bit mantissa precision 
will almost always lead to exact results (large numbers usually have a 
lot of zeros at the end, both in binary and in decimal).

Paolo

[Qemu-devel] Re: [PATCH 2/5] Support human unit formats in strtobytes, eg. 1.0G

[Jes Sorensen]

On 09/16/10 09:19, Paolo Bonzini wrote:
> On 09/15/2010 09:31 PM, Jes Sorensen wrote:
>> Floating point is just plain wrong. If someone wants to do something
>> like in your example they really ask for an error.
> 
> An error, not an overflow.
> 
> Adding overflow checking on top of your patch is also fine.  Another
> possibility is to look ahead for the multiplier so that you correctly
> base the divider and do everything in 64.64 fixed point.  But it seems
> overkill compared to floating-point, whose 53-bit mantissa precision
> will almost always lead to exact results (large numbers usually have a
> lot of zeros at the end, both in binary and in decimal).

I think it would be quite reasonable not to accept anything more than
say 3-4 decimal points, since there are the t/g/m/k options as well.

Cheers,
Jes

Re: [Qemu-devel] Re: [PATCH 2/5] Support human unit formats in strtobytes, eg. 1.0G

[Avi Kivity]

  On 09/15/2010 09:31 PM, Jes Sorensen wrote:
> Floating point is just plain wrong.

Why?  If command-line processing becomes too slow, you can always buy a 
math co-processor.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] Re: [PATCH 2/5] Support human unit formats in strtobytes, eg. 1.0G

[Jes Sorensen]

On 09/16/10 12:40, Avi Kivity wrote:
>  On 09/15/2010 09:31 PM, Jes Sorensen wrote:
>> Floating point is just plain wrong.
> 
> Why?  If command-line processing becomes too slow, you can always buy a
> math co-processor.
> 

Because it's imprecise anyway and requires dealing with fp regs.
Besides, most users will probably hit their shell command line limit
before hitting the problem with the decimals.

Jes

Re: [Qemu-devel] Re: [PATCH 2/5] Support human unit formats in strtobytes, eg. 1.0G

[Avi Kivity]

  On 09/16/2010 12:42 PM, Jes Sorensen wrote:
> On 09/16/10 12:40, Avi Kivity wrote:
> >   On 09/15/2010 09:31 PM, Jes Sorensen wrote:
> >>  Floating point is just plain wrong.
> >
> >  Why?  If command-line processing becomes too slow, you can always buy a
> >  math co-processor.
> >
>
> Because it's imprecise anyway

52 bits = 4PB.  At that point some rounding will take place.

> and requires dealing with fp regs.

The compiler takes care of allocating registers.

> Besides, most users will probably hit their shell command line limit
> before hitting the problem with the decimals.
>

20 digits will overflow your divider.

-- 
error compiling committee.c: too many arguments to function

[Qemu-devel] Re: [PATCH 2/5] Support human unit formats in strtobytes, eg. 1.0G

[Paolo Bonzini]

On 09/16/2010 12:42 PM, Jes Sorensen wrote:
> On 09/16/10 12:40, Avi Kivity wrote:
>>   On 09/15/2010 09:31 PM, Jes Sorensen wrote:
>>> Floating point is just plain wrong.
>>
>> Why?  If command-line processing becomes too slow, you can always buy a
>> math co-processor.
>
> Because it's imprecise anyway

As Avi mentioned, this is only true if you need byte precision beyond 4 
PB.  But most of the time byte precision is not necessary so in practice 
floating-point will be indistinguishable: all exact powers of 10 up to 
10^22 (beyond 64-bits) can be represented correctly by an IEEE double.

There's also strtold, if you're worried about precision...

> Besides, most users will probably hit their shell command line limit
> before hitting the problem with the decimals.

Value is first shifted and then multiplied, so that 6-7 digits may 
already overflow if the unit is terabytes.

Paolo