| Message ID | 1468418269-13490-4-git-send-email-prasanna.kalever@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Prasanna Kumar Kalever <prasanna.kalever@redhat.com> writes: > this patch adds 'GlusterServer' related schema in qapi/block-core.json > > Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@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. 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',
On Thu, Jul 14, 2016 at 1:22 PM, Markus Armbruster <armbru@redhat.com> wrote: > Prasanna Kumar Kalever <prasanna.kalever@redhat.com> writes: > >> this patch adds 'GlusterServer' related schema in qapi/block-core.json >> >> Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@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',
Prasanna Kalever <pkalever@redhat.com> writes: > On Thu, Jul 14, 2016 at 1:22 PM, Markus Armbruster <armbru@redhat.com> wrote: >> Prasanna Kumar Kalever <prasanna.kalever@redhat.com> writes: >> >>> this patch adds 'GlusterServer' related schema in qapi/block-core.json >>> >>> Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@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! Even closer to SocketAddress now. > 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 If you have questions, ask Eric or me in e-mail or on IRC. >> 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. If you want to prepare for future features, don't guess how they'll look like (we all guess wrong more often than not). Make your interface extensible instead. QAPI unions are a good tool for that. >> 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 After discussing this on IRC, I understand that: * @server specifies a Gluster server * @volume is effectively the name of a file name space on that server * @path is a name in that name space * Together, they specify an image file stored on Gluster. If that's correct, the design makes sense for QMP. Is the legacy syntax involving a gluster URI accessible via QMP? > Thanks Markus You're welcome!
On 07/14/2016 04:52 AM, Markus Armbruster wrote: >>>> + >>>> +## >>>> +# @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' } } >>> GlusterServer is very similar to the existing SocketAddress; the questions at hand are whether it should be a flat union instead of a simple union, and whether 'port' on the 'inet' branch should accept integers instead of (or in addition to) strings. Also, if you're going to immediately convert it to an array of servers in the next patch, it may be better to do that up front; I guess it boils down to how much churn there is in converting the rest of the code. If it is intentionally different from the final version, at least explicitly call that out in the commit message. >>> 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 > > After discussing this on IRC, I understand that: > > * @server specifies a Gluster server > > * @volume is effectively the name of a file name space on that server > > * @path is a name in that name space > > * Together, they specify an image file stored on Gluster. > > If that's correct, the design makes sense for QMP. > > Is the legacy syntax involving a gluster URI accessible via QMP? As far as I know, 'blockdev-add' doesn't yet support gluster, so the only way to hotplug a gluster volume at the moment via QMP is to resort to HMP command passthrough. At least libvirt is still using HMP passthrough for all block device hotplugs, for two reasons: 1. blockdev-add is incomplete, 2. libvirt hasn't been taught to use node names everywhere
diff --git a/block/gluster.c b/block/gluster.c index 40ee852..40dcbc1 100644 --- a/block/gluster.c +++ b/block/gluster.c @@ -15,6 +15,7 @@ #define GLUSTER_OPT_FILENAME "filename" #define GLUSTER_OPT_DEBUG "debug" +#define GLUSTER_DEFAULT_PORT 24007 #define GLUSTER_DEBUG_DEFAULT 4 #define GLUSTER_DEBUG_MAX 9 @@ -39,15 +40,6 @@ typedef struct BDRVGlusterReopenState { struct glfs_fd *fd; } BDRVGlusterReopenState; -typedef struct GlusterConf { - char *host; - int port; - char *volume; - char *path; - char *transport; - int debug_level; -} GlusterConf; - static QemuOptsList qemu_gluster_create_opts = { .name = "qemu-gluster-create-opts", @@ -91,18 +83,7 @@ static QemuOptsList runtime_opts = { }; -static void qemu_gluster_gconf_free(GlusterConf *gconf) -{ - if (gconf) { - g_free(gconf->host); - g_free(gconf->volume); - g_free(gconf->path); - g_free(gconf->transport); - g_free(gconf); - } -} - -static int parse_volume_options(GlusterConf *gconf, char *path) +static int parse_volume_options(BlockdevOptionsGluster *gconf, char *path) { char *p, *q; @@ -164,7 +145,8 @@ static int parse_volume_options(GlusterConf *gconf, char *path) * file=gluster+unix:///testvol/dir/a.img?socket=/tmp/glusterd.socket * file=gluster+rdma://1.2.3.4:24007/testvol/a.img */ -static int qemu_gluster_parseuri(GlusterConf *gconf, const char *filename) +static int qemu_gluster_parseuri(BlockdevOptionsGluster *gconf, + const char *filename) { URI *uri; QueryParams *qp = NULL; @@ -176,20 +158,23 @@ static int qemu_gluster_parseuri(GlusterConf *gconf, const char *filename) return -EINVAL; } + gconf->server = g_new0(GlusterServer, 1); + /* transport */ if (!uri->scheme || !strcmp(uri->scheme, "gluster")) { - gconf->transport = g_strdup("tcp"); + gconf->server->transport = GLUSTER_TRANSPORT_TCP; } else if (!strcmp(uri->scheme, "gluster+tcp")) { - gconf->transport = g_strdup("tcp"); + gconf->server->transport = GLUSTER_TRANSPORT_TCP; } else if (!strcmp(uri->scheme, "gluster+unix")) { - gconf->transport = g_strdup("unix"); + gconf->server->transport = GLUSTER_TRANSPORT_UNIX; is_unix = true; } else if (!strcmp(uri->scheme, "gluster+rdma")) { - gconf->transport = g_strdup("rdma"); + gconf->server->transport = GLUSTER_TRANSPORT_RDMA; } else { ret = -EINVAL; goto out; } + gconf->server->has_transport = true; ret = parse_volume_options(gconf, uri->path); if (ret < 0) { @@ -211,10 +196,15 @@ static int qemu_gluster_parseuri(GlusterConf *gconf, const char *filename) ret = -EINVAL; goto out; } - gconf->host = g_strdup(qp->p[0].value); + gconf->server->host = g_strdup(qp->p[0].value); } else { - gconf->host = g_strdup(uri->server ? uri->server : "localhost"); - gconf->port = uri->port; + gconf->server->host = g_strdup(uri->server ? uri->server : "localhost"); + if (uri->port) { + gconf->server->port = uri->port; + } else { + gconf->server->port = GLUSTER_DEFAULT_PORT; + } + gconf->server->has_port = true; } out: @@ -225,8 +215,8 @@ out: return ret; } -static struct glfs *qemu_gluster_init(GlusterConf *gconf, const char *filename, - Error **errp) +static struct glfs *qemu_gluster_init(BlockdevOptionsGluster *gconf, + const char *filename, Error **errp) { struct glfs *glfs = NULL; int ret; @@ -245,8 +235,9 @@ static struct glfs *qemu_gluster_init(GlusterConf *gconf, const char *filename, goto out; } - ret = glfs_set_volfile_server(glfs, gconf->transport, gconf->host, - gconf->port); + ret = glfs_set_volfile_server(glfs, + GlusterTransport_lookup[gconf->server->transport], + gconf->server->host, gconf->server->port); if (ret < 0) { goto out; } @@ -260,9 +251,9 @@ static struct glfs *qemu_gluster_init(GlusterConf *gconf, const char *filename, 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); + "volume=%s path=%s transport=%s", gconf->server->host, + gconf->server->port, gconf->volume, gconf->path, + GlusterTransport_lookup[gconf->server->transport]); /* glfs_init sometimes doesn't set errno although docs suggest that */ if (errno == 0) @@ -354,7 +345,7 @@ static int qemu_gluster_open(BlockDriverState *bs, QDict *options, BDRVGlusterState *s = bs->opaque; int open_flags = 0; int ret = 0; - GlusterConf *gconf = g_new0(GlusterConf, 1); + BlockdevOptionsGluster *gconf = NULL; QemuOpts *opts; Error *local_err = NULL; const char *filename; @@ -377,7 +368,9 @@ static int qemu_gluster_open(BlockDriverState *bs, QDict *options, s->debug_level = GLUSTER_DEBUG_MAX; } + gconf = g_new0(BlockdevOptionsGluster, 1); gconf->debug_level = s->debug_level; + gconf->has_debug_level = true; s->glfs = qemu_gluster_init(gconf, filename, errp); if (!s->glfs) { ret = -errno; @@ -412,7 +405,9 @@ static int qemu_gluster_open(BlockDriverState *bs, QDict *options, out: qemu_opts_del(opts); - qemu_gluster_gconf_free(gconf); + if (gconf) { + qapi_free_BlockdevOptionsGluster(gconf); + } if (!ret) { return ret; } @@ -431,7 +426,7 @@ static int qemu_gluster_reopen_prepare(BDRVReopenState *state, int ret = 0; BDRVGlusterState *s; BDRVGlusterReopenState *reop_s; - GlusterConf *gconf = NULL; + BlockdevOptionsGluster *gconf; int open_flags = 0; assert(state != NULL); @@ -444,9 +439,9 @@ static int qemu_gluster_reopen_prepare(BDRVReopenState *state, qemu_gluster_parse_flags(state->flags, &open_flags); - gconf = g_new0(GlusterConf, 1); - + gconf = g_new0(BlockdevOptionsGluster, 1); gconf->debug_level = s->debug_level; + gconf->has_debug_level = true; reop_s->glfs = qemu_gluster_init(gconf, state->bs->filename, errp); if (reop_s->glfs == NULL) { ret = -errno; @@ -472,7 +467,9 @@ static int qemu_gluster_reopen_prepare(BDRVReopenState *state, exit: /* state->opaque will be freed in either the _abort or _commit */ - qemu_gluster_gconf_free(gconf); + if (gconf) { + qapi_free_BlockdevOptionsGluster(gconf); + } return ret; } @@ -574,14 +571,15 @@ static inline int qemu_gluster_zerofill(struct glfs_fd *fd, int64_t offset, static int qemu_gluster_create(const char *filename, QemuOpts *opts, Error **errp) { + BlockdevOptionsGluster *gconf; struct glfs *glfs; struct glfs_fd *fd; int ret = 0; int prealloc = 0; int64_t total_size = 0; char *tmp = NULL; - GlusterConf *gconf = g_new0(GlusterConf, 1); + gconf = g_new0(BlockdevOptionsGluster, 1); gconf->debug_level = qemu_opt_get_number_del(opts, GLUSTER_OPT_DEBUG, GLUSTER_DEBUG_DEFAULT); if (gconf->debug_level < 0) { @@ -589,6 +587,7 @@ static int qemu_gluster_create(const char *filename, } else if (gconf->debug_level > GLUSTER_DEBUG_MAX) { gconf->debug_level = GLUSTER_DEBUG_MAX; } + gconf->has_debug_level = true; glfs = qemu_gluster_init(gconf, filename, errp); if (!glfs) { @@ -630,7 +629,9 @@ static int qemu_gluster_create(const char *filename, } out: g_free(tmp); - qemu_gluster_gconf_free(gconf); + if (gconf) { + qapi_free_BlockdevOptionsGluster(gconf); + } if (glfs) { glfs_fini(glfs); } 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' } } + +## +# @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' } } + +## # @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',
this patch adds 'GlusterServer' related schema in qapi/block-core.json Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com> --- block/gluster.c | 89 ++++++++++++++++++++++++++-------------------------- qapi/block-core.json | 67 ++++++++++++++++++++++++++++++++++++--- 2 files changed, 107 insertions(+), 49 deletions(-)