Re: [Qemu-devel] [PATCH v17 0/4][WIP] block/gluster: add support for multiple gluster servers

["Daniel P. Berrange" <berrange@redhat.com>]

On Wed, Jun 29, 2016 at 10:11:31AM -0400, Jeff Cody wrote:
> On Wed, Jun 15, 2016 at 01:55:43PM +0530, Prasanna Kumar Kalever wrote:
> > This version of patches are rebased on master branch.
> > 
> > Prasanna Kumar Kalever (4):
> >   block/gluster: rename [server, volname, image] -> [host, volume, path]
> >   block/gluster: code cleanup
> >   block/gluster: using new qapi schema
> >   block/gluster: add support for multiple gluster servers
> >
> 
> I think the main criticism with this series revolves around the interface,
> and the overloading of the server hosts fields when using tcp and unix
> sockets, etc.  The idea of using flat unions for the API was floated.
> 
> Eric, does this criticism still stand, from libvirt's perspective?  Or are
> you comfortable enough with the current interface that I can go ahead and
> take this series in through my tree?

Just from a general QAPI design POV I think this overloading is undesirable.

We cared enough about not doing this overloading in the past that we
created SocketAddress which is a union of InetSocketAddress and
UnixSocketAddress. Given this historical best practice, I don't think
we should be overloading "host" for unix socket path.


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