Patchwork Re: [PATCH 5/6] Switch migrate_set_speed() to take an 'o' argument rather than a float.

login
register
mail settings
Submitter Paolo Bonzini
Date Oct. 7, 2010, 4:03 p.m.
Message ID <4CADEF6D.3010706@redhat.com>
Download mbox | patch
Permalink /patch/67073/
State New
Headers show

Comments

Paolo Bonzini - Oct. 7, 2010, 4:03 p.m.
On 10/07/2010 05:01 PM, Jes.Sorensen@redhat.com wrote:
> From: Jes Sorensen<Jes.Sorensen@redhat.com>
>
> Signed-off-by: Jes Sorensen<Jes.Sorensen@redhat.com>
> ---
>   hmp-commands.hx |    2 +-
>   migration.c     |    4 ++--
>   2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 81999aa..95bdb91 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -754,7 +754,7 @@ ETEXI
>
>       {
>           .name       = "migrate_set_speed",
> -        .args_type  = "value:f",
> +        .args_type  = "value:o",
>           .params     = "value",
>           .help       = "set maximum speed (in bytes) for migrations",
                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Is this still accurate?

Alternatively, you said it's a mess to distinguish the defaults on the 
command line and the monitor, but is it harder than the attached patch?

Paolo
Jes Sorensen - Oct. 8, 2010, 8:50 a.m.
On 10/07/10 18:03, Paolo Bonzini wrote:
> On 10/07/2010 05:01 PM, Jes.Sorensen@redhat.com wrote:
>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>> index 81999aa..95bdb91 100644
>> --- a/hmp-commands.hx
>> +++ b/hmp-commands.hx
>> @@ -754,7 +754,7 @@ ETEXI
>>
>>       {
>>           .name       = "migrate_set_speed",
>> -        .args_type  = "value:f",
>> +        .args_type  = "value:o",
>>           .params     = "value",
>>           .help       = "set maximum speed (in bytes) for migrations",
>                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> Is this still accurate?
> 
> Alternatively, you said it's a mess to distinguish the defaults on the
> command line and the monitor, but is it harder than the attached patch?

I guess that depends on how you look at it, but I guess I could improve
the error message to make it clearer.

Cheers,
Jes

Patch

diff --git a/cutils.c b/cutils.c
index 012eb11..4a9ce0f 100644
--- a/cutils.c
+++ b/cutils.c
@@ -289,13 +289,13 @@  int fcntl_setfl(int fd, int flag)
  * End pointer will be returned in *end, if end is valid.
  * Return -1 on error.
  */
-ssize_t strtosz(const char *nptr, char **end)
+ssize_t strtosz(const char *nptr, char **end, int default_shift)
 {
     ssize_t retval = -1;
-    int64_t tmpval;
+    int64_t tmpval, mul;
     char *endptr;
     int mul_required = 0;
-    double val, mul = 1;
+    double val;
 
     endptr = (char *)nptr + strspn(nptr, " 0123456789");
     if (*endptr == '.') {
@@ -308,15 +308,14 @@  ssize_t strtosz(const char *nptr, char **end)
         goto fail;
 
     switch (*endptr++) {
+    case 0:
+    case ' ':
+        mul = (1ULL << default_shift);
+        break;
     case 'K':
     case 'k':
         mul = 1 << 10;
         break;
-    case 0:
-    case ' ':
-        if (mul_required) {
-            goto fail;
-        }
     case 'M':
     case 'm':
         mul = 1ULL << 20;
@@ -333,6 +332,9 @@  ssize_t strtosz(const char *nptr, char **end)
         goto fail;
     }
 
+    if (mul == 1 && mul_required) {
+        goto fail;
+    }
     tmpval = (val * mul);
     if (tmpval > ~(size_t)0)
         goto fail;
diff --git a/monitor.c b/monitor.c
index 6678fb5..0d04b27 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3713,7 +3713,7 @@  static const mon_cmd_t *monitor_parse_command(Monitor *mon,
                         break;
                     }
                 }
-                val = strtosz(p, &end);
+                val = strtosz(p, &end, 0);
                 if (val < 0) {
                     monitor_printf(mon, "invalid size\n");
                     goto fail;
diff --git a/qemu-common.h b/qemu-common.h
index 0a062d4..cabd8f8 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -153,7 +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);
+ssize_t strtosz(const char *nptr, char **end, int default_shift);
 
 /* path.c */
 void init_paths(const char *prefix);
diff --git a/vl.c b/vl.c
index 6043fa2..ae4d6f3 100644
--- a/vl.c
+++ b/vl.c
@@ -735,7 +735,7 @@  static void numa_add(const char *optarg)
             node_mem[nodenr] = 0;
         } else {
             ssize_t sval;
-            sval = strtosz(option, NULL);
+            sval = strtosz(option, NULL, 20);
             if (sval < 0) {
                 fprintf(stderr, "qemu: invalid numa mem size: %s\n", optarg);
                 exit(1);
@@ -2162,7 +2162,7 @@  int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_m: {
                 ssize_t value;
 
-                value = strtosz(optarg, NULL);
+                value = strtosz(optarg, NULL, 20);
                 if (value < 0) {
                     fprintf(stderr, "qemu: invalid ram size: %s\n", optarg);
                     exit(1);