On 07/19/2016 10:57 AM, Prasanna Kumar Kalever wrote: > this patch adds 'GlusterServer' related schema in qapi/block-core.json > > Signed-off-by: Prasanna Kumar Kalever <prasanna.kale...@redhat.com> > --- > block/gluster.c | 115 > +++++++++++++++++++++++++++++---------------------- > qapi/block-core.json | 68 +++++++++++++++++++++++++++--- > 2 files changed, 128 insertions(+), 55 deletions(-) >
> @@ -256,15 +254,22 @@ static struct glfs *qemu_gluster_init(GlusterConf > *gconf, const char *filename, > > ret = glfs_init(glfs); > if (ret) { > - error_setg_errno(errp, errno, > - "Gluster connection failed for host=%s port=%d " > - "volume=%s path=%s transport=%s", gconf->host, > - gconf->port, gconf->volume, gconf->path, > - gconf->transport); > + if (gconf->server->type == GLUSTER_TRANSPORT_UNIX) { > + error_setg(errp, > + "Gluster connection for volume %s, path %s failed on " > + "socket %s ", gconf->volume, gconf->path, > + gconf->server->u.q_unix.path); > + } else { > + error_setg(errp, > + "Gluster connection for volume %s, path %s failed on " > + "host %s and port %s ", gconf->volume, gconf->path, > + gconf->server->u.tcp.host, gconf->server->u.tcp.port); > + } You are throwing away the errno information that used to be reported here. Is that intentional? > > /* glfs_init sometimes doesn't set errno although docs suggest that > */ > - if (errno == 0) > + if (errno == 0) { > errno = EINVAL; > + } Pre-existing, but this is too late for messing with errno. You are not guaranteed that errno is unchanged by error_setg_errno(). You aren't even guaranteed that errno == 0 after glfs_init() if it wasn't 0 before entry. You probably want a separate patch that reorders this to: errno = 0; ret = glfs_init(glfs); if (ret) { if (!errno) { errno = EINVAL; } error_setg... > +++ b/qapi/block-core.json > @@ -1658,13 +1658,14 @@ > # @host_device, @host_cdrom: Since 2.1 > # > # Since: 2.0 > +# @gluster: Since 2.7 I would stick this line a bit earlier: # @host_device, @host_cdrom: Since 2.1 # @gluster: Since 2.7 # # Since: 2.0 > @@ -2057,6 +2058,63 @@ > '*read-pattern': 'QuorumReadPattern' } } > > ## > +# @GlusterTransport > +# > +# An enumeration of Gluster transport type Maybe s/type/types/ > +# > +# @tcp: TCP - Transmission Control Protocol > +# > +# @unix: UNIX - Unix domain socket > +# > +# Since: 2.7 > +## > +{ 'enum': 'GlusterTransport', > + 'data': [ 'unix', 'tcp' ] } > + > + > +## > +# @GlusterServer > +# > +# Captures the address of a socket > +# > +# Details for connecting to a gluster server > +# > +# @type: Transport type used for gluster connection > +# > +# @unix: socket file > +# > +# @tcp: host address and port number > +# > +# Since: 2.7 > +## > +{ 'union': 'GlusterServer', > + 'base': { 'type': 'GlusterTransport' }, > + 'discriminator': 'type', > + 'data': { 'unix': 'UnixSocketAddress', > + 'tcp': 'InetSocketAddress' } } > + > +## > +# @BlockdevOptionsGluster > +# > +# Driver specific block device options for Gluster > +# > +# @volume: name of gluster volume where VM image resides > +# > +# @path: absolute path to image file in gluster volume > +# > +# @server: gluster server description > +# > +# @debug_level: #optional libgfapi log level (default '4' which is Error) s/debug_level/debug-level/ - all new QAPI interfaces should prefer '-' over '_' (the generated C code is the same, though) > +# > +# Since: 2.7 > +## > +{ 'struct': 'BlockdevOptionsGluster', > + 'data': { 'volume': 'str', > + 'path': 'str', > + 'server': 'GlusterServer', > + '*debug_level': 'int' } } > + > +## > # @BlockdevOptions > # > # Options for creating a block device. Many options are available for all > @@ -2119,7 +2177,7 @@ > 'file': 'BlockdevOptionsFile', > 'ftp': 'BlockdevOptionsFile', > 'ftps': 'BlockdevOptionsFile', > -# TODO gluster: Wait for structured options > + 'gluster': 'BlockdevOptionsGluster', > 'host_cdrom': 'BlockdevOptionsFile', > 'host_device':'BlockdevOptionsFile', > 'http': 'BlockdevOptionsFile', > Between my findings and Markus', it's up to the maintainer whether to take this as-is and then fix up the problems with a followup patch, or whether we need a v21. Reviewed-by: Eric Blake <ebl...@redhat.com> -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature