On 11/12/2015 03:22 AM, Prasanna Kumar Kalever wrote: > this patch adds GlusterConf to qapi/block-core.json > > Signed-off-by: Prasanna Kumar Kalever <prasanna.kale...@redhat.com> > --- > block/gluster.c | 104 > +++++++++++++++++++++++++-------------------------- > qapi/block-core.json | 60 +++++++++++++++++++++++++++-- > 2 files changed, 109 insertions(+), 55 deletions(-) >
Pointing it out here for completeness, even though I first stumbled on it when reviewing 4/4: > @@ -190,13 +180,23 @@ static int qemu_gluster_parseuri(GlusterConf *gconf, > const char *filename) > ret = -EINVAL; > goto out; > } > - gconf->host = g_strdup(qp->p[0].value); > + gconf->server->host = g_strdup(qp->p[0].value); This is abusing the 'host' field of GlusterServer to track a socket path, and ignores the fact that port is meaningless for a gluster+unix:// connection. > @@ -224,8 +225,9 @@ static struct glfs *qemu_gluster_init(GlusterConf *gconf, > const char *filename, > goto out; > } > > - ret = glfs_set_volfile_server(glfs, gconf->transport, gconf->host, > - gconf->port); > + ret = glfs_set_volfile_server(glfs, > + > GlusterTransport_lookup[gconf->server->transport], > + gconf->server->host, gconf->server->port); At least gluster itself has the same overloaded abuse of terminology; I'm hoping that a port of 0 is okay when requesting a "unix" volfile_server. [I don't know, because I didn't read the docs for glfs_set_volfile_server()] > +## > +# @GlusterServer > +# > +# Details for connecting to a gluster server > +# > +# @host: host address (hostname/ipv4/ipv6 addresses) > +# > +# @port: #optional port number on which glusterd is listening > +# (default 24007) > +# > +# @transport: #optional transport type used to connect to gluster management > +# daemon (default 'tcp') > +# > +# Since: 2.5 > +## > +{ 'struct': 'GlusterServer', > + 'data': { 'host': 'str', > + '*port': 'int', > + '*transport': 'GlusterTransport' } } And my idea on patch 4/4 was that converting this from simple struct to flat union might be a more realistic view of things (if transport is 'unix', there can't be a port; and rather than abusing the name 'host' we could use the name 'socket'; similarly for 'rdma') - but without additional qapi support, I don't know that we can have an optional 'transport' and still have a discriminated union in time for 2.5. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature