Re: [libvirt] [PATCH v8 06/11] qemu: Add qemu command line generation for a VxHS block device

On 09/19/2017 09:37 AM, Peter Krempa wrote: > On Thu, Sep 14, 2017 at 08:51:51 -0400, John Ferlan wrote: >> From: Ashish Mittal <Ashish.Mittal(a)veritas.com&gt; >> >> The VxHS block device will only use the newer formatting options and >> avoid the legacy URI syntax. >> >> An excerpt for a sample QEMU command line is: >> >> -drive file.driver=vxhs,file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc251,\ >> file.server.type=tcp,file.server.host=192.168.0.1,\ >> file.server.port=9999,format=raw,if=none,id=drive-virtio-disk0,cache=none \ >> -device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,\ >> id=virtio-disk0 >> >> Update qemuxml2argvtest with a simple test. >> >> Signed-off-by: Ashish Mittal <Ashish.Mittal(a)veritas.com&gt; >> Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; >> --- >> src/qemu/qemu_block.c | 37 +++++++++++++++++++++- >> src/qemu/qemu_command.c | 10 +++++- >> src/qemu/qemu_parse_command.c | 16 +++++++++- >> src/qemu/qemu_process.c | 29 +++++++++++++++++ >> .../qemuxml2argv-disk-drive-network-vxhs.args | 27 ++++++++++++++++ >> tests/qemuxml2argvtest.c | 1 + >> 6 files changed, 117 insertions(+), 3 deletions(-) >> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.args >> >> diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c >> index c97b787c5..ca6e213b4 100644 >> --- a/src/qemu/qemu_block.c >> +++ b/src/qemu/qemu_block.c >> @@ -516,6 +516,37 @@ qemuBlockStorageSourceGetGlusterProps(virStorageSourcePtr src) >> } >> >> >> +static virJSONValuePtr >> +qemuBlockStorageSourceGetVxHSProps(virStorageSourcePtr src) >> +{ >> + const char *protocol = virStorageNetProtocolTypeToString(src->protocol); >> + virJSONValuePtr server = NULL; >> + virJSONValuePtr ret = NULL; >> + >> + if (src->nhosts != 1) { >> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", >> + _("VxHS protocol accepts only one host")); >> + return NULL; >> + } >> + >> + if (!(server = qemuBlockStorageSourceBuildJSONSocketAddress(src->hosts, true))) >> + return NULL; > > This creates a json object ... > >> + >> + /* VxHS disk specification example: >> + * { driver:"vxhs", >> + * vdisk-id:"eb90327c-8302-4725-4e85ed4dc251", >> + * server:[{type:"tcp", host:"1.2.3.4", port:9999}]} > > ... which is appended below as the 'server' field, while this example > shows an array of objects. So, which one is correct? Is this a hack to > force the JSON->commandline generator to compy? If it's so then we need > to change that and not this, since then QMP commands like 'blockdev-add' > will not accept it and thus this will be useless for expansion. > Oh right, the [ ] is wrong as this isn't an array it's just a single server element (as seen in patch 4) I can remove the [ ] and I think that answers your concern. The VxHS qemu code only likes the "server.type, server.host, and server.port" syntax and fails on the "server.0.type", "server.0.host", and "server.0.port" syntax.