From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38726) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bNbRz-00011g-T1 for qemu-devel@nongnu.org; Thu, 14 Jul 2016 03:52:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bNbRw-0007aM-O5 for qemu-devel@nongnu.org; Thu, 14 Jul 2016 03:52:51 -0400 Received: from mx1.redhat.com ([209.132.183.28]:58105) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bNbRw-0007aH-Fw for qemu-devel@nongnu.org; Thu, 14 Jul 2016 03:52:48 -0400 From: Markus Armbruster References: <1468418269-13490-1-git-send-email-prasanna.kalever@redhat.com> <1468418269-13490-4-git-send-email-prasanna.kalever@redhat.com> Date: Thu, 14 Jul 2016 09:52:44 +0200 In-Reply-To: <1468418269-13490-4-git-send-email-prasanna.kalever@redhat.com> (Prasanna Kumar Kalever's message of "Wed, 13 Jul 2016 19:27:48 +0530") Message-ID: <87twfsy7ub.fsf@dusky.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v18 3/4] block/gluster: using new qapi schema List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Prasanna Kumar Kalever Cc: qemu-devel@nongnu.org, kwolf@redhat.com, pkrempa@redhat.com, jcody@redhat.com, deepakcs@redhat.com, bharata@linux.vnet.ibm.com, rtalur@redhat.com Prasanna Kumar Kalever writes: > this patch adds 'GlusterServer' related schema in qapi/block-core.json > > Signed-off-by: Prasanna Kumar Kalever QAPI/QMP interface review only. > diff --git a/qapi/block-core.json b/qapi/block-core.json > index ac8f5f6..f8b38bb 100644 > --- a/qapi/block-core.json > +++ b/qapi/block-core.json > @@ -1634,13 +1634,14 @@ > # @host_device, @host_cdrom: Since 2.1 > # > # Since: 2.0 > +# @gluster: Since 2.7 > ## > { 'enum': 'BlockdevDriver', > 'data': [ 'archipelago', 'blkdebug', 'blkverify', 'bochs', 'cloop', > - 'dmg', 'file', 'ftp', 'ftps', 'host_cdrom', 'host_device', > - 'http', 'https', 'luks', 'null-aio', 'null-co', 'parallels', > - 'qcow', 'qcow2', 'qed', 'quorum', 'raw', 'tftp', 'vdi', 'vhdx', > - 'vmdk', 'vpc', 'vvfat' ] } > + 'dmg', 'file', 'ftp', 'ftps', 'gluster', 'host_cdrom', > + 'host_device', 'http', 'https', 'luks', 'null-aio', 'null-co', > + 'parallels', 'qcow', 'qcow2', 'qed', 'quorum', 'raw', 'tftp', > + 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat' ] } > > ## > # @BlockdevOptionsFile > @@ -2033,6 +2034,62 @@ > '*read-pattern': 'QuorumReadPattern' } } > > ## > +# @GlusterTransport > +# > +# An enumeration of Gluster transport type > +# > +# @tcp: TCP - Transmission Control Protocol > +# > +# @unix: UNIX - Unix domain socket > +# > +# @rdma: RDMA - Remote direct memory access > +# > +# Since: 2.7 > +## > +{ 'enum': 'GlusterTransport', 'data': [ 'tcp', 'unix', 'rdma'] } > + > +## > +# @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.7 > +## > +{ 'struct': 'GlusterServer', > + 'data': { 'host': 'str', > + '*port': 'uint16', > + '*transport': 'GlusterTransport' } } The meaning of @host and @port is obvious enough with @transport 'tcp', and your documentation matches it. Not the case for @transport 'unix'. There is no such thing as 'host' and 'port' with Unix domain sockets. I'm afraid the interface makes no sense in its current state. I'm not familiar with RDMA, so I can't say whether your definition makes sense there. I can say that you shouldn't assume people are familiar with RDMA. Please explain what @host and @port mean there. If they're just like for 'tcp', say so. For 'tcp', GlusterTransport duplicates InetSocketAddress, except it doesn't support service names (@port is 'uint16' instead of 'str'), doesn't support port ranges (no @to; not needed here), and doesn't support controlling IPv4/IPv6 (no @ipv4, @ipv6). Why is it necessary to roll your own address type? Why can't you use SocketAddress? > + > +## > +# @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: #libgfapi log level (default '4' which is Error) > +# > +# Since: 2.7 > +## > +{ 'struct': 'BlockdevOptionsGluster', > + 'data': { 'volume': 'str', > + 'path': 'str', > + 'server': 'GlusterServer', > + '*debug_level': 'int' } } Are @volume and @path interpreted in any way in QEMU, or are they merely sent to the Gluster server? > + > +## > # @BlockdevOptions > # > # Options for creating a block device. Many options are available for all > @@ -2095,7 +2152,7 @@ > 'file': 'BlockdevOptionsFile', > 'ftp': 'BlockdevOptionsFile', > 'ftps': 'BlockdevOptionsFile', > -# TODO gluster: Wait for structured options > + 'gluster': 'BlockdevOptionsGluster', > 'host_cdrom': 'BlockdevOptionsFile', > 'host_device':'BlockdevOptionsFile', > 'http': 'BlockdevOptionsFile',