[prev in list] [next in list] [prev in thread] [next in thread] 

List:       libvir-list
Subject:    Re: [libvirt] [PATCH] correct the arguments of migrate_speed
From:       Eric Blake <eblake () redhat ! com>
Date:       2010-11-29 22:26:46
Message-ID: 4CF428A6.2070704 () redhat ! com
[Download RAW message or body]


On 11/25/2010 01:38 AM, Wen Congyang wrote:
> When we set migrate_speed by json, we receive the following
> error message:
> libvirtError: internal error unable to execute QEMU command
> 'migrate_set_speed': Invalid parameter type, expected: number
> 
> The reason is that: the arguments of migrate_set_speed
> by json is json number, not json string.
> 
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> 
> ---
>  src/qemu/qemu_monitor_json.c |   10 +++-------
>  1 files changed, 3 insertions(+), 7 deletions(-)

Thanks for submitting this.

> 
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index da51a4f..2576dc5 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -1453,17 +1453,13 @@ int qemuMonitorJSONSetMigrationSpeed(qemuMonitorPtr mon,
>                                       unsigned long bandwidth)
>  {
>      int ret;
> -    char *bandwidthstr;
>      virJSONValuePtr cmd;
>      virJSONValuePtr reply = NULL;
> -    if (virAsprintf(&bandwidthstr, "%lum", bandwidth) < 0) {
> -        virReportOOMError();
> -        return -1;
> -    }
>      cmd = qemuMonitorJSONMakeCommand("migrate_set_speed",
> -                                     "s:value", bandwidthstr,
> +                                     "U:value",
> +                                     /* 1048576 = 1024 * 1024 */
> +                                     (uint64_t)bandwidth * 1048576,

U implies unsigned long long (which might not necessarily be the same
width as uint64_t, although I don't know of any counter-example systems
that violate that assumption at the moment).  Also, rather than a
comment explaining a magic number followed by use of that magic number,
I found it simpler to just use inline expansion of the constant (and
avoiding a cast is always a plus in my books):

bandwidth * 1024ULL * 1024ULL

ACK with that modification, plus a tweak to AUTHORS to keep 'make
syntax-check' happy.  I've pushed the resulting patch now.

-- 
Eric Blake   eblake@redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org


["signature.asc" (application/pgp-signature)]

[prev in list] [next in list] [prev in thread] [next in thread]