From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43175) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bPZuj-0004Ue-Ji for qemu-devel@nongnu.org; Tue, 19 Jul 2016 14:38:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bPZuf-0003Pb-HP for qemu-devel@nongnu.org; Tue, 19 Jul 2016 14:38:40 -0400 Received: from mx1.redhat.com ([209.132.183.28]:58077) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bPZuf-0003PV-88 for qemu-devel@nongnu.org; Tue, 19 Jul 2016 14:38:37 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id BFFE5C0467E6 for ; Tue, 19 Jul 2016 18:38:36 +0000 (UTC) References: <1468947453-5433-1-git-send-email-prasanna.kalever@redhat.com> <1468947453-5433-5-git-send-email-prasanna.kalever@redhat.com> From: Eric Blake Message-ID: <578E73A6.7090803@redhat.com> Date: Tue, 19 Jul 2016 12:38:30 -0600 MIME-Version: 1.0 In-Reply-To: <1468947453-5433-5-git-send-email-prasanna.kalever@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="d3E8EB3otKWt5c5drq5rhMMpndDVQddGs" Subject: Re: [Qemu-devel] [PATCH v20 4/5] block/gluster: using new qapi schema List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Prasanna Kumar Kalever , qemu-devel@nongnu.org Cc: armbru@redhat.com, kwolf@redhat.com, pkrempa@redhat.com, jcody@redhat.com, rtalur@redhat.com, vbellur@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --d3E8EB3otKWt5c5drq5rhMMpndDVQddGs From: Eric Blake To: Prasanna Kumar Kalever , qemu-devel@nongnu.org Cc: armbru@redhat.com, kwolf@redhat.com, pkrempa@redhat.com, jcody@redhat.com, rtalur@redhat.com, vbellur@redhat.com Message-ID: <578E73A6.7090803@redhat.com> Subject: Re: [PATCH v20 4/5] block/gluster: using new qapi schema References: <1468947453-5433-1-git-send-email-prasanna.kalever@redhat.com> <1468947453-5433-5-git-send-email-prasanna.kalever@redhat.com> In-Reply-To: <1468947453-5433-5-git-send-email-prasanna.kalever@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 07/19/2016 10:57 AM, Prasanna Kumar Kalever wrote: > this patch adds 'GlusterServer' related schema in qapi/block-core.json >=20 > Signed-off-by: Prasanna Kumar Kalever > --- > block/gluster.c | 115 +++++++++++++++++++++++++++++--------------= -------- > qapi/block-core.json | 68 +++++++++++++++++++++++++++--- > 2 files changed, 128 insertions(+), 55 deletions(-) >=20 > @@ -256,15 +254,22 @@ static struct glfs *qemu_gluster_init(GlusterConf= *gconf, const char *filename, > =20 > ret =3D glfs_init(glfs); > if (ret) { > - error_setg_errno(errp, errno, > - "Gluster connection failed for host=3D%s port= =3D%d " > - "volume=3D%s path=3D%s transport=3D%s", gconf= ->host, > - gconf->port, gconf->volume, gconf->path, > - gconf->transport); > + if (gconf->server->type =3D=3D GLUSTER_TRANSPORT_UNIX) { > + error_setg(errp, > + "Gluster connection for volume %s, path %s fail= ed on " > + "socket %s ", gconf->volume, gconf->path, > + gconf->server->u.q_unix.path); > + } else { > + error_setg(errp, > + "Gluster connection for volume %s, path %s fail= ed on " > + "host %s and port %s ", gconf->volume, gconf->= path, > + gconf->server->u.tcp.host, gconf->server->u.tcp= =2Eport); > + } You are throwing away the errno information that used to be reported here. Is that intentional? > =20 > /* glfs_init sometimes doesn't set errno although docs suggest= that */ > - if (errno =3D=3D 0) > + if (errno =3D=3D 0) { > errno =3D 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 =3D=3D 0 after glfs_init() if it wasn't 0 befo= re entry. You probably want a separate patch that reorders this to: errno =3D 0; ret =3D glfs_init(glfs); if (ret) { if (!errno) { errno =3D 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' } } > =20 > ## > +# @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 Err= or) 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', >=20 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 --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --d3E8EB3otKWt5c5drq5rhMMpndDVQddGs Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJXjnOmAAoJEKeha0olJ0NqOUUH/A5weAUqwGksBaj9KQ9elLnL /wjBYRe1NvRbM5ESVD9VKlnO5WOHmfdrpggsW31OM48I79ePx+6sOoc2dkHFRwsT mAN5/x4mNZ7bCyHIeE5eaFVwG1IWpzIi/fPa71vEvN0leIrAnQZFH7luklCrKXr8 blePpSBr6LmONX/0PL9tBqq2lMH4AM2L6ErDcpNDbo0oEEfQV6Xe1SzI7rRMeLfn MALvGV3US74KS3sJUP14lSlBczFI6te3xrKIXNLKJp1V4lWCc95oFNk78oxhgCL8 xOHoGQWcSfu6+OFDOhJBuzQOdQJzjbrE0s8u7KsMhPGVYTW4zLXfPhttQ8UxYiM= =JJiX -----END PGP SIGNATURE----- --d3E8EB3otKWt5c5drq5rhMMpndDVQddGs--