[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v18 3/4] block/gluster: using new qapi schema
From: |
Prasanna Kalever |
Subject: |
Re: [Qemu-devel] [PATCH v18 3/4] block/gluster: using new qapi schema |
Date: |
Thu, 14 Jul 2016 13:48:54 +0530 |
On Thu, Jul 14, 2016 at 1:22 PM, Markus Armbruster <address@hidden> wrote:
> Prasanna Kumar Kalever <address@hidden> writes:
>
>> this patch adds 'GlusterServer' related schema in qapi/block-core.json
>>
>> Signed-off-by: Prasanna Kumar Kalever <address@hidden>
>
> 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',
- [Qemu-devel] [PATCH v18 0/4] block/gluster: add support for multiple gluster servers, Prasanna Kumar Kalever, 2016/07/13
- [Qemu-devel] [PATCH v18 1/4] block/gluster: rename [server, volname, image] -> [host, volume, path], Prasanna Kumar Kalever, 2016/07/13
- [Qemu-devel] [PATCH v18 2/4] block/gluster: code cleanup, Prasanna Kumar Kalever, 2016/07/13
- [Qemu-devel] [PATCH v18 3/4] block/gluster: using new qapi schema, Prasanna Kumar Kalever, 2016/07/13
[Qemu-devel] [PATCH v18 4/4] block/gluster: add support for multiple gluster servers, Prasanna Kumar Kalever, 2016/07/13