Re: [Qemu-devel] [PATCH v1] block/vxhs: Add Veritas HyperScale VxHS block device support

[Daniel P. Berrange]

On Wed, Oct 26, 2016 at 09:33:30PM +0000, Buddhi Madhav wrote:
> 
> 
> On 10/25/16, 9:41 PM, "Jeff Cody" <address@hidden> wrote:
> 
> >On Tue, Oct 25, 2016 at 03:02:07PM -0700, Ashish Mittal wrote:
> >> This patch adds support for a new block device type called "vxhs".
> >> Source code for the library that this code loads can be downloaded from:
> >> https://github.com/MittalAshish/libqnio.git
> >>
> >
> >I grabbed the latest of libqnio, compiled it (had to disable -Werror), and
> >tried it out.  I was able to do a qemu-img info on a raw file, but it
> >would
> >just hang when trying a format such as qcow2.  I am assuming
> >this is a limitation of test_server, and not libqnio.
> 
> On my build I did not get any build errors.
> 
> The qcow2 issue is to do with the limitation in the test server, which we
> will fix in a seperate patch.
> 
> >
> >This will make qemu-iotests more difficult however.
> >
> >I haven't looked at the latest qnio code yet (other than compiling the
> >test-server to test), so the rest of this review is on the qemu driver.
> >
> >> Sample command line using JSON syntax:
> >> ./qemu-system-x86_64 -name instance-00000008 -S -vnc 0.0.0.0:0 -k en-us
> >>-vga cirrus -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5
> >>-msg timestamp=on
> >>'json:{"driver":"vxhs","vdisk_id":"{c3e9095a-a5ee-4dce-afeb-2a59fb387410}
> >>","server":[{"host":"172.172.17.4","port":"9999"}]}'
> >> 
> >> Sample command line using URI syntax:
> >> qemu-img convert -f raw -O raw -n
> >>/var/lib/nova/instances/_base/0c5eacd5ebea5ed914b6a3e7b18f1ce734c386ad
> >>vxhs://192.168.0.1:9999/%7Bc6718f6b-0401-441d-a8c3-1f0064d75ee0%7D
> >> 
> >> Signed-off-by: Ashish Mittal <address@hidden>
> >> ---
> >>  block/Makefile.objs |   2 +
> >>  block/trace-events  |  22 ++
> >>  block/vxhs.c        | 736
> >>++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>  configure           |  41 +++jj
> >>  4 files changed, 801 insertions(+)
> >>  create mode 100644 block/vxhs.c
> >
> >I think this version still does not address Daniel's concerns regarding a
> >QAPI schema for vxhs.
> 
> We are working on QAPI schema changes and will submit them in separate
> patch.

They should be part of the main patch, not a separate patch as they are
a pre-requisite for any new block driver.


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|