[libvirt] [PATCH v1 03/11] qemu: Introduce nbd-server-start command

Eric Blake eblake at redhat.com
Tue Nov 27 23:26:24 UTC 2012


----- Original Message -----
> This will be used with new migration scheme.
> This patch creates basically just monitor stub
> functions. Wiring them into something useful
> is done in later patches.

Reasonable way to break up the review.

> ---
>  src/qemu/qemu_monitor.c      |   22 ++++++++++++++++++
>  src/qemu/qemu_monitor.h      |    3 ++
>  src/qemu/qemu_monitor_json.c |   49
>  ++++++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_monitor_json.h |    3 ++
>  4 files changed, 77 insertions(+), 0 deletions(-)

> +    if (!(data = virJSONValueNewObject()) ||
> +        !(addr = virJSONValueNewObject()) ||
> +        virAsprintf(&port_str, "%d", port) < 0) {
> +        virReportOOMError();
> +        goto cleanup;
> +    }
> +
> +    if (virJSONValueObjectAppendString(data, "host", host) < 0 ||
> +        virJSONValueObjectAppendString(data, "port", port_str) < 0

Is 'port' really a string rather than a JSON integer?  (goes and checks...
yep - you really did match the JSON here to the documentation in
qemu.git:qapi-schema.json)

> ||
> +        virJSONValueObjectAppendString(addr, "type", "inet") < 0 ||
> +        virJSONValueObjectAppend(addr, "data", data) < 0) {

Hmm, you aren't supplying anything for the optional 'ipv4' and 'ipv6'
portions of the address; do we always want the defaults of always
trying both families, or are we going to need to make this configurable?
But I guess we can add that later if we find we need it.

ACK.




More information about the libvir-list mailing list