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

On 08/31/2017 06:39 AM, Peter Krempa wrote: > On Wed, Aug 30, 2017 at 18:46:06 -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.0.type=tcp,file.server.0.host=192.168.0.1,\ >> file.server.0.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 | 48 +++++++++++++++++++++- >> src/qemu/qemu_block.h | 3 +- >> src/qemu/qemu_command.c | 12 +++++- >> src/qemu/qemu_parse_command.c | 16 +++++++- >> .../qemuxml2argv-disk-drive-network-vxhs.args | 27 ++++++++++++ >> tests/qemuxml2argvtest.c | 1 + >> 6 files changed, 101 insertions(+), 6 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 d07269f..cb765ab 100644 >> --- a/src/qemu/qemu_block.c >> +++ b/src/qemu/qemu_block.c >> @@ -482,6 +482,45 @@ qemuBlockStorageSourceGetGlusterProps(virStorageSourcePtr src) >> } >> >> >> +static virJSONValuePtr >> +qemuBlockStorageSourceGetVxHSProps(virStorageSourcePtr src, >> + virQEMUCapsPtr qemuCaps) > > Please don't drag 'caps' into the options formatter. Add a function that > validates whether the disk backend is supported (it should accept a > virStorageSourcePtr and be called somewhere in > qemuProcessStartValidate). > > The 'json' syntax may be used even for qemu-img's arguments and thus > would make this non-reusable. > > Rest looks okay. > So essentially, if the attached patch got merged? I can update the series again eventually, but figured I'd get 'buy in' first on the approach. This will cause merge conflicts in/by patch11 though where I had also added a diskAlias argument. I suppose I could go with private data approach too - adding the alias to _qemuDomainDiskPrivate during qemuDomainPrepareDiskSourceTLS (from patch10)...