On Thu, Jul 14, 2016 at 1:22 PM, Markus Armbruster <arm...@redhat.com> wrote: > Prasanna Kumar Kalever <prasanna.kale...@redhat.com> writes: > >> this patch adds 'GlusterServer' related schema in qapi/block-core.json >> >> Signed-off-by: Prasanna Kumar Kalever <prasanna.kale...@redhat.com> > > 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.
Yes, agreed I am about do something like + { 'struct': 'UnixAddress', + 'data': { + 'socket': 'str', + } } + + { 'struct': 'TcpAddress', + 'data': { + 'host': 'str', + 'port': 'uint16', + } } + + { 'union': 'GlusterServer', + 'data': { + 'unix': 'UnixAddress', + 'Tcp': 'TcpAddress' + } } No rdma! But I need to tweak them to fit flat union patches @ git://repo.or.cz/qemu/armbru.git qapi-not-next Currently I am reading the design changes, help in this area is appreciable > 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. I shall remove the rdma part because the gluster volfile fetch doesn't support this, Sorry for involving the rdma type in all my previous patches, I just included that in case gluster supports that in future, but it doesn't seems to be. To confirm we don't need rdma type here. > > 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? Sure, I shall take a look at InetSocketAddress and 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? have a look at libglfsapi (IMO which is poorly designed) glfs_set_volfile_server (struct glfs *fs, const char *transport, const char *host, int port) So the @volume and @path are parsed from the command line args and filled in 'struct glfs' object Thanks Markus -- prasanna > >> + >> +## >> # @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',