[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On Thu, Aug 31, 2017 at 08:02:33 -0400, John Ferlan wrote:
> 
> 
> 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 veritas com>
> >>
> >> 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 veritas com>
> >> Signed-off-by: John Ferlan <jferlan redhat com>
> >> ---
> > 
> > [...]
> > 
> >>  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)...

I have a solution for this in mind. Basically the TLS object name should
be added as a property to virStorageSource. The code then sets the
secret names at first and then both the object and disk get added.

virStorageSource currently contains quite a lot qemu-isms so it won't be
an regression in code looks. I might eventually convert that to some
kind of private data, but at this point it's a convenient way to shove
things.

In that way the secret name won't depend on the disk source from the
point of view of the json struct generator (as it makes sense) and thus
the code will stay generic enough to use with qemu-img, where a
different secret name can be generated if necessary.

Attachment: signature.asc
Description: PGP signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]