* [Qemu-devel] [PATCH v10 3/3] block/gluster: add support for multiple gluster servers
@ 2015-10-27 13:06 Prasanna Kumar Kalever
2015-10-27 15:01 ` Eric Blake
0 siblings, 1 reply; 2+ messages in thread
From: Prasanna Kumar Kalever @ 2015-10-27 13:06 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, pkrempa, stefanha, deepakcs, bharata, rtalur,
Prasanna Kumar Kalever
This patch adds a way to specify multiple volfile servers to the gluster
block backend of QEMU with tcp|rdma transport types and their port numbers.
Problem:
Currenly VM Image on gluster volume is specified like this:
file=gluster[+tcp]://host[:port]/testvol/a.img
Assuming we have three hosts in trusted pool with replica 3 volume
in action and unfortunately host (mentioned in the command above) went down
for some reason, since the volume is replica 3 we now have other 2 hosts
active from which we can boot the VM.
But currently there is no mechanism to pass the other 2 gluster host
addresses to qemu.
Solution:
New way of specifying VM Image on gluster volume with volfile servers:
(We still support old syntax to maintain backward compatibility)
Basic command line syntax looks like:
Pattern I:
-drive driver=gluster,
volume=testvol,path=/path/a.raw,
servers.0.host=1.2.3.4,
[servers.0.port=24007,]
[servers.0.transport=tcp,]
servers.1.host=5.6.7.8,
[servers.1.port=24008,]
[servers.1.transport=rdma,] ...
Pattern II:
'json:{"driver":"qcow2","file":{"driver":"gluster",
"volume":"testvol","path":"/path/a.qcow2",
"servers":[{tuple0},{tuple1}, ...{tupleN}]}}'
driver => 'gluster' (protocol name)
volume => name of gluster volume where our VM image resides
path => absolute path of image in gluster volume
{tuple} => {"host":"1.2.3.4"[,"port":"24007","transport":"tcp"]}
host => host address (hostname/ipv4/ipv6 addresses)
port => port number on which glusterd is listening. (default 24007)
transport => transport type used to connect to gluster management daemon,
it can be tcp|rdma (default 'tcp')
Examples:
1.
-drive driver=qcow2,file.driver=gluster,
file.volume=testvol,file.path=/path/a.qcow2,
file.servers.0.host=1.2.3.4,
file.servers.0.port=24007,
file.servers.0.transport=tcp,
file.servers.1.host=5.6.7.8,
file.servers.1.port=24008,
file.servers.1.transport=rdma
2.
'json:{"driver":"qcow2","file":{"driver":"gluster","volume":"testvol",
"path":"/path/a.qcow2","servers":
[{"host":"1.2.3.4","port":"24007","transport":"tcp"},
{"host":"4.5.6.7","port":"24008","transport":"rdma"}] } }'
This patch gives a mechanism to provide all the server addresses, which are in
replica set, so in case host1 is down VM can still boot from any of the
active hosts.
This is equivalent to the backup-volfile-servers option supported by
mount.glusterfs (FUSE way of mounting gluster volume)
This patch depends on a recent fix in libgfapi raised as part of this work:
http://review.gluster.org/#/c/12114/
Credits: Sincere thanks to Kevin Wolf <kwolf@redhat.com> and
"Deepak C Shetty" <deepakcs@redhat.com> for inputs and all their support
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
v1:
multiple host addresses but common port number and transport type
pattern: URI syntax with query (?) delimitor
syntax:
file=gluster[+transport-type]://host1:24007/testvol/a.img\
?servers=host2&servers=host3
v2:
multiple host addresses each have their own port number, but all use
common transport type
pattern: URI syntax with query (?) delimiter
syntax:
file=gluster[+transport-type]://[host[:port]]/testvol/a.img\
[?servers=host1[:port]\
&servers=host2[:port]]
v3:
multiple host addresses each have their own port number and transport type
pattern: changed to json
syntax:
'json:{"driver":"qcow2","file":{"driver":"gluster","volume":"testvol",
"path":"/path/a.qcow2","servers":
[{"host":"1.2.3.4","port":"24007","transport":"tcp"},
{"host":"4.5.6.7","port":"24008","transport":"rdma"}] } }'
v4, v5:
address comments from "Eric Blake" <eblake@redhat.com>
renamed:
'backup-volfile-servers' -> 'volfile-servers'
v6:
address comments from Peter Krempa <pkrempa@redhat.com>
renamed:
'volname' -> 'volume'
'image-path' -> 'path'
'server' -> 'host'
v7:
fix for v6 (initialize num_servers to 1 and other typos)
v8: split patch set v7 into series of 3 as per Peter Krempa
<pkrempa@redhat.com> review comments
v9: reorder the series of patches addressing "Eric Blake" <eblake@redhat.com>
review comments
v10: fix mem-leak as per Peter Krempa <pkrempa@redhat.com> review comments
---
block/gluster.c | 423 +++++++++++++++++++++++++++++++++++++++++++++------
qapi/block-core.json | 62 +++++++-
2 files changed, 436 insertions(+), 49 deletions(-)
diff --git a/block/gluster.c b/block/gluster.c
index ededda2..3d88108 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -11,6 +11,16 @@
#include "block/block_int.h"
#include "qemu/uri.h"
+#define GLUSTER_OPT_FILENAME "filename"
+#define GLUSTER_OPT_VOLUME "volume"
+#define GLUSTER_OPT_PATH "path"
+#define GLUSTER_OPT_HOST "host"
+#define GLUSTER_OPT_PORT "port"
+#define GLUSTER_OPT_TRANSPORT "transport"
+#define GLUSTER_OPT_SERVERS_PATTERN "servers."
+
+#define GLUSTER_DEFAULT_PORT 24007
+
typedef struct GlusterAIOCB {
int64_t size;
int ret;
@@ -29,12 +39,17 @@ typedef struct BDRVGlusterReopenState {
struct glfs_fd *fd;
} BDRVGlusterReopenState;
-typedef struct GlusterConf {
+typedef struct GlusterServerConf {
char *host;
int port;
+ char *transport;
+} GlusterServerConf;
+
+typedef struct GlusterConf {
char *volume;
char *path;
- char *transport;
+ int num_servers;
+ GlusterServerConf *gsconf;
} GlusterConf;
@@ -61,7 +76,7 @@ static QemuOptsList runtime_opts = {
.head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),
.desc = {
{
- .name = "filename",
+ .name = GLUSTER_OPT_FILENAME,
.type = QEMU_OPT_STRING,
.help = "URL to the gluster image",
},
@@ -69,14 +84,61 @@ static QemuOptsList runtime_opts = {
},
};
+static QemuOptsList runtime_json_opts = {
+ .name = "gluster_json",
+ .head = QTAILQ_HEAD_INITIALIZER(runtime_json_opts.head),
+ .desc = {
+ {
+ .name = GLUSTER_OPT_VOLUME,
+ .type = QEMU_OPT_STRING,
+ .help = "name of gluster volume where VM image resides",
+ },
+ {
+ .name = GLUSTER_OPT_PATH,
+ .type = QEMU_OPT_STRING,
+ .help = "absolute path to image file in gluster volume",
+ },
+ { /* end of list */ }
+ },
+};
+
+static QemuOptsList runtime_tuple_opts = {
+ .name = "gluster_tuple",
+ .head = QTAILQ_HEAD_INITIALIZER(runtime_tuple_opts.head),
+ .desc = {
+ {
+ .name = GLUSTER_OPT_HOST,
+ .type = QEMU_OPT_STRING,
+ .help = "host address (hostname/ipv4/ipv6 addresses)",
+ },
+ {
+ .name = GLUSTER_OPT_PORT,
+ .type = QEMU_OPT_NUMBER,
+ .help = "port number on which glusterd listen (default 24007)",
+ },
+ {
+ .name = GLUSTER_OPT_TRANSPORT,
+ .type = QEMU_OPT_STRING,
+ .help = "transport type 'tcp' or 'rdma' (default 'tcp')",
+ },
+ { /* end of list */ }
+ },
+};
+
static void qemu_gluster_gconf_free(GlusterConf *gconf)
{
+ int i;
if (gconf) {
- g_free(gconf->host);
g_free(gconf->volume);
g_free(gconf->path);
- g_free(gconf->transport);
+ if (gconf->gsconf) {
+ for (i = 0; i < gconf->num_servers; i++) {
+ g_free(gconf->gsconf[i].host);
+ g_free(gconf->gsconf[i].transport);
+ }
+ g_free(gconf->gsconf);
+ }
g_free(gconf);
}
}
@@ -143,8 +205,9 @@ 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(GlusterConf **pgconf, const char *filename)
{
+ GlusterConf *gconf;
URI *uri;
QueryParams *qp = NULL;
bool is_unix = false;
@@ -155,16 +218,20 @@ static int qemu_gluster_parseuri(GlusterConf *gconf, const char *filename)
return -EINVAL;
}
+ gconf = g_new0(GlusterConf, 1);
+ gconf->num_servers = 1;
+ gconf->gsconf = g_new0(GlusterServerConf, gconf->num_servers);
+
/* transport */
if (!uri->scheme || !strcmp(uri->scheme, "gluster")) {
- gconf->transport = g_strdup("tcp");
+ gconf->gsconf[0].transport = g_strdup("tcp");
} else if (!strcmp(uri->scheme, "gluster+tcp")) {
- gconf->transport = g_strdup("tcp");
+ gconf->gsconf[0].transport = g_strdup("tcp");
} else if (!strcmp(uri->scheme, "gluster+unix")) {
- gconf->transport = g_strdup("unix");
+ gconf->gsconf[0].transport = g_strdup("unix");
is_unix = true;
} else if (!strcmp(uri->scheme, "gluster+rdma")) {
- gconf->transport = g_strdup("rdma");
+ gconf->gsconf[0].transport = g_strdup("rdma");
} else {
ret = -EINVAL;
goto out;
@@ -190,13 +257,22 @@ static int qemu_gluster_parseuri(GlusterConf *gconf, const char *filename)
ret = -EINVAL;
goto out;
}
- gconf->host = g_strdup(qp->p[0].value);
+ gconf->gsconf[0].host = g_strdup(qp->p[0].value);
} else {
- gconf->host = g_strdup(uri->server ? uri->server : "localhost");
- gconf->port = uri->port;
+ gconf->gsconf[0].host = g_strdup(uri->server ? uri->server : "localhost");
+ if (uri->port) {
+ gconf->gsconf[0].port = uri->port;
+ } else {
+ gconf->gsconf[0].port = GLUSTER_DEFAULT_PORT;
+ }
}
+ *pgconf = gconf;
+
out:
+ if (ret < 0) {
+ qemu_gluster_gconf_free(gconf);
+ }
if (qp) {
query_params_free(qp);
}
@@ -204,30 +280,25 @@ out:
return ret;
}
-static struct glfs *qemu_gluster_init(GlusterConf *gconf, const char *filename,
- Error **errp)
+static struct glfs *qemu_gluster_glfs_init(GlusterConf *gconf, Error **errp)
{
struct glfs *glfs = NULL;
int ret;
int old_errno;
-
- ret = qemu_gluster_parseuri(gconf, filename);
- if (ret < 0) {
- error_setg(errp, "Usage: file=gluster[+transport]://[host[:port]]/"
- "volume/path[?socket=...]");
- errno = -ret;
- goto out;
- }
+ int i;
glfs = glfs_new(gconf->volume);
if (!glfs) {
goto out;
}
- ret = glfs_set_volfile_server(glfs, gconf->transport, gconf->host,
- gconf->port);
- if (ret < 0) {
- goto out;
+ for (i = 0; i < gconf->num_servers; i++) {
+ ret = glfs_set_volfile_server(glfs, gconf->gsconf[i].transport,
+ gconf->gsconf[i].host,
+ gconf->gsconf[i].port);
+ if (ret < 0) {
+ goto out;
+ }
}
/*
@@ -242,10 +313,9 @@ static struct glfs *qemu_gluster_init(GlusterConf *gconf, const char *filename,
ret = glfs_init(glfs);
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);
+ "Error: Gluster connection failed for given hosts "
+ "volume:'%s' path:'%s' host1: %s", gconf->volume,
+ gconf->path, gconf->gsconf[0].host);
/* glfs_init sometimes doesn't set errno although docs suggest that */
if (errno == 0)
@@ -264,6 +334,274 @@ out:
return NULL;
}
+static int parse_transport_option(const char *opt)
+{
+ int i;
+
+ if (!opt) {
+ /* Set tcp as default */
+ return GLUSTER_TRANSPORT_TCP;
+ }
+
+ for (i = 0; i < GLUSTER_TRANSPORT_MAX; i++) {
+ if (!strcmp(opt, GlusterTransport_lookup[i])) {
+ return i;
+ }
+ }
+
+ return -EINVAL;
+}
+
+/*
+*
+* Basic command line syntax looks like:
+*
+* Pattern I:
+* -drive driver=gluster,
+* volume=testvol,file.path=/path/a.raw,
+* servers.0.host=1.2.3.4,
+* [servers.0.port=24007,]
+* [servers.0.transport=tcp,]
+* servers.1.host=5.6.7.8,
+* [servers.1.port=24008,]
+* [servers.1.transport=rdma,] ...
+*
+* Pattern II:
+* 'json:{"driver":"qcow2","file":{"driver":"gluster",
+* "volume":"testvol","path":"/path/a.qcow2",
+* "servers":[{tuple0},{tuple1}, ...{tupleN}]}}'
+*
+*
+* driver => 'gluster' (protocol name)
+* volume => name of gluster volume where VM image resides
+* path => absolute path of image in gluster volume
+*
+* {tuple} => {"host":"1.2.3.4"[,"port":"24007","transport":"tcp"]}
+*
+* host => host address (hostname/ipv4/ipv6 addresses)
+* port => port number on which glusterd is listening (default 24007)
+* tranport => transport type used to connect to gluster management daemon,
+* it can be tcp|rdma (default 'tcp')
+*
+*
+* Examples:
+* Pattern I:
+* -drive driver=qcow2,file.driver=gluster,
+* file.volume=testvol,file.path=/path/a.qcow2,
+* file.servers.0.host=1.2.3.4,
+* file.servers.0.port=24007,
+* file.servers.0.transport=tcp,
+* file.servers.1.host=5.6.7.8,
+* file.servers.1.port=24008,
+* file.servers.1.transport=rdma, ...
+*
+* -drive driver=qcow2,file.driver=gluster,
+* file.volume=testvol,file.path=/path/a.qcow2,
+* file.servers.0.host=1.2.3.4,
+* file.servers.1.host=5.6.7.8, ...
+*
+* -drive driver=qcow2,file.driver=gluster,
+* file.volume=testvol,file.path=/path/a.qcow2,
+* file.servers.0.host=1.2.3.4,
+* file.servers.0.port=24007,
+* file.servers.1.host=5.6.7.8,
+* file.servers.1.port=24008, ...
+*
+* -drive driver=qcow2,file.driver=gluster,
+* file.volume=testvol,file.path=/path/a.qcow2,
+* file.servers.0.host=1.2.3.4,
+* file.servers.0.transport=tcp,
+* file.servers.1.host=5.6.7.8,
+* file.servers.1.transport=rdma, ...
+*
+* Pattern II:
+* 'json:{"driver":"qcow2","file":{"driver":"gluster","volume":"testvol",
+* "path":"/path/a.qcow2","servers":
+* [{"host":"1.2.3.4","port":"24007","transport":"tcp"},
+* {"host":"4.5.6.7","port":"24008","transport":"rdma"}, ...]}}'
+*
+* 'json:{"driver":"qcow2","file":{"driver":"gluster","volume":"testvol",
+* "path":"/path/a.qcow2","servers":
+* [{"host":"1.2.3.4"},
+* {"host":"4.5.6.7"}, ...]}}'
+*
+*
+* 'json:{"driver":"qcow2","file":{"driver":"gluster","volume":"testvol",
+* "path":"/path/a.qcow2","servers":
+* [{"host":"1.2.3.4","port":"24007"},
+* {"host":"4.5.6.7","port":"24008"}, ...]}}'
+*
+*
+* 'json:{"driver":"qcow2","file":{"driver":"gluster","volume":"testvol",
+* "path":"/path/a.qcow2","servers":
+* [{"host":"1.2.3.4","transport":"tcp"},
+* {"host":"4.5.6.7","transport":"rdma"}, ...]}}'
+*
+* Just for better readability pattern II is kept as:
+* json:
+* {
+* "driver":"qcow2",
+* "file":{
+* "driver":"gluster",
+* "volume":"testvol",
+* "path":"/path/a.qcow2",
+* "servers":[
+* {
+* "host":"1.2.3.4",
+* "port":"24007",
+* "transport":"tcp"
+* },
+* {
+* "host":"5.6.7.8",
+* "port":"24008",
+* "transport":"rdma"
+* }
+* ]
+* }
+* }
+*
+*/
+static int qemu_gluster_parsejson(GlusterConf **pgconf, QDict *options)
+{
+ QemuOpts *opts;
+ GlusterConf *gconf = NULL;
+ QDict *backing_options = NULL;
+ Error *local_err = NULL;
+ char *str = NULL;
+ const char *ptr = NULL;
+ int i;
+
+ /* create opts info from runtime_json_opts list */
+ opts = qemu_opts_create(&runtime_json_opts, NULL, 0, &error_abort);
+ qemu_opts_absorb_qdict(opts, options, &local_err);
+ if (local_err) {
+ goto out;
+ }
+
+ gconf = g_new0(GlusterConf, 1);
+ gconf->num_servers = qdict_array_entries(options,
+ GLUSTER_OPT_SERVERS_PATTERN);
+ if (gconf->num_servers < 1) {
+ error_setg(&local_err, "Error: qemu_gluster: please provide 'servers' "
+ "option with valid fields in array of tuples");
+ goto out;
+ }
+ gconf->gsconf = g_new0(GlusterServerConf, gconf->num_servers);
+
+ ptr = qemu_opt_get(opts, GLUSTER_OPT_VOLUME);
+ if (!ptr) {
+ error_setg(&local_err, "Error: qemu_gluster: please provide 'volume' "
+ "option");
+ goto out;
+ }
+ gconf->volume = g_strdup(ptr);
+
+ ptr = qemu_opt_get(opts, GLUSTER_OPT_PATH);
+ if (!ptr) {
+ error_setg(&local_err, "Error: qemu_gluster: please provide 'path' "
+ "option");
+ goto out;
+ }
+ gconf->path = g_strdup(ptr);
+
+ qemu_opts_del(opts);
+
+ /* create opts info from runtime_tuple_opts list */
+ str = g_malloc(40);
+ for (i = 0; i < gconf->num_servers; i++) {
+ opts = qemu_opts_create(&runtime_tuple_opts, NULL, 0, &error_abort);
+ g_assert(sprintf(str, GLUSTER_OPT_SERVERS_PATTERN"%d.", i) <= 40);
+ qdict_extract_subqdict(options, &backing_options, str);
+ qemu_opts_absorb_qdict(opts, backing_options, &local_err);
+ if (local_err) {
+ goto out;
+ }
+ qdict_del(backing_options, str);
+
+ ptr = qemu_opt_get(opts, GLUSTER_OPT_HOST);
+ if (!ptr) {
+ error_setg(&local_err, "Error: qemu_gluster: servers.{tuple.%d} "
+ "requires 'host' option", i);
+ goto out;
+ }
+ gconf->gsconf[i].host = g_strdup(ptr);
+
+ ptr = qemu_opt_get(opts, GLUSTER_OPT_TRANSPORT);
+ /* check whether transport type specified in json command is valid */
+ if (parse_transport_option(ptr) < 0) {
+ error_setg(&local_err, "Error: qemu_gluster: please set 'transport'"
+ " type in tuple.%d as tcp or rdma", i);
+ goto out;
+ }
+ /* only if valid transport i.e. either of tcp|rdma is specified */
+ gconf->gsconf[i].transport = g_strdup(ptr);
+
+ gconf->gsconf[i].port = qemu_opt_get_number(opts, GLUSTER_OPT_PORT,
+ GLUSTER_DEFAULT_PORT);
+
+ qemu_opts_del(opts);
+ }
+
+ *pgconf = gconf;
+ g_free(str);
+ return 0;
+
+out:
+ error_report_err(local_err);
+ qemu_gluster_gconf_free(gconf);
+ qemu_opts_del(opts);
+ if (str) {
+ qdict_del(backing_options, str);
+ g_free(str);
+ }
+ errno = EINVAL;
+ return -errno;
+}
+
+static struct glfs *qemu_gluster_init(GlusterConf **gconf, const char *filename,
+ QDict *options, Error **errp)
+{
+ int ret;
+
+ if (filename) {
+ ret = qemu_gluster_parseuri(gconf, filename);
+ if (ret < 0) {
+ error_setg(errp, "Usage: file=gluster[+transport]://[host[:port]]/"
+ "volume/path[?socket=...]");
+ errno = -ret;
+ return NULL;
+ }
+ } else {
+ ret = qemu_gluster_parsejson(gconf, options);
+ if (ret < 0) {
+ error_setg(errp, "Wrong Usage!");
+ error_append_hint(errp,
+ "Usage1: "
+ "-drive driver=qcow2,file.driver=gluster,"
+ "file.volume=testvol,file.path=/path/a.qcow2,"
+ "file.servers.0.host=1.2.3.4,"
+ "[file.servers.0.port=24007,]"
+ "[file.servers.0.transport=tcp,]"
+ "file.servers.1.host=5.6.7.8,"
+ "[file.servers.1.port=24008,]"
+ "[file.servers.1.transport=rdma,] ...");
+ error_append_hint(errp,
+ "\nUsage2: "
+ "'json:{\"driver\":\"qcow2\",\"file\":"
+ "{\"driver\":\"gluster\",\"volume\":\""
+ "testvol\",\"path\":\"/path/a.qcow2\","
+ "\"servers\":[{\"host\":\"1.2.3.4\","
+ "\"port\":\"24007\",\"transport\":\"tcp\"},"
+ "{\"host\":\"4.5.6.7\",\"port\":\"24007\","
+ "\"transport\":\"rdma\"}, ...]}}'");
+ errno = -ret;
+ return NULL;
+ }
+ }
+
+ return qemu_gluster_glfs_init(*gconf, errp);
+}
+
static void qemu_gluster_complete_aio(void *opaque)
{
GlusterAIOCB *acb = (GlusterAIOCB *)opaque;
@@ -309,13 +647,13 @@ static void qemu_gluster_parse_flags(int bdrv_flags, int *open_flags)
}
}
-static int qemu_gluster_open(BlockDriverState *bs, QDict *options,
+static int qemu_gluster_open(BlockDriverState *bs, QDict *options,
int bdrv_flags, Error **errp)
{
BDRVGlusterState *s = bs->opaque;
int open_flags = 0;
int ret = 0;
- GlusterConf *gconf = g_new0(GlusterConf, 1);
+ GlusterConf *gconf = NULL;
QemuOpts *opts;
Error *local_err = NULL;
const char *filename;
@@ -329,8 +667,7 @@ static int qemu_gluster_open(BlockDriverState *bs, QDict *options,
}
filename = qemu_opt_get(opts, "filename");
-
- s->glfs = qemu_gluster_init(gconf, filename, errp);
+ s->glfs = qemu_gluster_init(&gconf, filename, options, errp);
if (!s->glfs) {
ret = -errno;
goto out;
@@ -374,9 +711,7 @@ static int qemu_gluster_reopen_prepare(BDRVReopenState *state,
qemu_gluster_parse_flags(state->flags, &open_flags);
- gconf = g_new0(GlusterConf, 1);
-
- reop_s->glfs = qemu_gluster_init(gconf, state->bs->filename, errp);
+ reop_s->glfs = qemu_gluster_init(&gconf, state->bs->filename, NULL, errp);
if (reop_s->glfs == NULL) {
ret = -errno;
goto exit;
@@ -500,15 +835,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)
{
+ GlusterConf *gconf = NULL;
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);
- glfs = qemu_gluster_init(gconf, filename, errp);
+ glfs = qemu_gluster_init(&gconf, filename, NULL, errp);
if (!glfs) {
ret = -errno;
goto out;
@@ -523,7 +858,7 @@ static int qemu_gluster_create(const char *filename,
} else if (!strcmp(tmp, "full") && gluster_supports_zerofill()) {
prealloc = 1;
} else {
- error_setg(errp, "Invalid preallocation mode: '%s'"
+ error_setg(errp, "Error: Invalid preallocation mode: '%s'"
" or GlusterFS doesn't support zerofill API", tmp);
ret = -EINVAL;
goto out;
@@ -725,7 +1060,7 @@ static BlockDriver bdrv_gluster = {
.format_name = "gluster",
.protocol_name = "gluster",
.instance_size = sizeof(BDRVGlusterState),
- .bdrv_needs_filename = true,
+ .bdrv_needs_filename = false,
.bdrv_file_open = qemu_gluster_open,
.bdrv_reopen_prepare = qemu_gluster_reopen_prepare,
.bdrv_reopen_commit = qemu_gluster_reopen_commit,
@@ -752,7 +1087,7 @@ static BlockDriver bdrv_gluster_tcp = {
.format_name = "gluster",
.protocol_name = "gluster+tcp",
.instance_size = sizeof(BDRVGlusterState),
- .bdrv_needs_filename = true,
+ .bdrv_needs_filename = false,
.bdrv_file_open = qemu_gluster_open,
.bdrv_reopen_prepare = qemu_gluster_reopen_prepare,
.bdrv_reopen_commit = qemu_gluster_reopen_commit,
@@ -806,7 +1141,7 @@ static BlockDriver bdrv_gluster_rdma = {
.format_name = "gluster",
.protocol_name = "gluster+rdma",
.instance_size = sizeof(BDRVGlusterState),
- .bdrv_needs_filename = true,
+ .bdrv_needs_filename = false,
.bdrv_file_open = qemu_gluster_open,
.bdrv_reopen_prepare = qemu_gluster_reopen_prepare,
.bdrv_reopen_commit = qemu_gluster_reopen_commit,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index bb2189e..62c0551 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1375,15 +1375,16 @@
#
# @host_device, @host_cdrom, @host_floppy: Since 2.1
# @host_floppy: deprecated since 2.3
+# @gluster: Since 2.5
#
# Since: 2.0
##
{ 'enum': 'BlockdevDriver',
'data': [ 'archipelago', 'blkdebug', 'blkverify', 'bochs', 'cloop',
- 'dmg', 'file', 'ftp', 'ftps', 'host_cdrom', 'host_device',
- 'host_floppy', 'http', 'https', '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', 'host_floppy', 'http', 'https', 'null-aio',
+ 'null-co', 'parallels', 'qcow', 'qcow2', 'qed', 'quorum', 'raw',
+ 'tftp', 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat' ] }
##
# @BlockdevOptionsBase
@@ -1794,6 +1795,57 @@
'*read-pattern': 'QuorumReadPattern' } }
##
+# @GlusterTransport
+#
+# An enumeration of Gluster transport type
+#
+# @tcp: TCP - Transmission Control Protocol
+#
+# @rdma: RDMA - Remote direct memory access
+#
+# Since: 2.5
+##
+{ 'enum': 'GlusterTransport', 'data': [ 'tcp', 'rdma' ] }
+
+##
+# @GlusterTuple
+#
+# Gluster tuple set
+#
+# @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.5
+##
+{ 'struct': 'GlusterTuple',
+ 'data': { 'host': 'str',
+ '*port': 'int',
+ '*transport': 'GlusterTransport' } }
+
+##
+# @BlockdevOptionsGluster
+#
+# Driver specific block device options for Gluster
+#
+# @volume: name of gluster volume where our VM image resides
+#
+# @path: absolute path to image file in gluster volume
+#
+# @servers: one or more gluster server descriptions (host, port, and transport)
+#
+# Since: 2.5
+##
+{ 'struct': 'BlockdevOptionsGluster',
+ 'data': { 'volume': 'str',
+ 'path': 'str',
+ 'servers': [ 'GlusterTuple' ] } }
+
+##
# @BlockdevOptions
#
# Options for creating a block device.
@@ -1813,7 +1865,7 @@
'file': 'BlockdevOptionsFile',
'ftp': 'BlockdevOptionsFile',
'ftps': 'BlockdevOptionsFile',
-# TODO gluster: Wait for structured options
+ 'gluster': 'BlockdevOptionsGluster',
'host_cdrom': 'BlockdevOptionsFile',
'host_device':'BlockdevOptionsFile',
'host_floppy':'BlockdevOptionsFile',
--
2.1.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [Qemu-devel] [PATCH v10 3/3] block/gluster: add support for multiple gluster servers
2015-10-27 13:06 [Qemu-devel] [PATCH v10 3/3] block/gluster: add support for multiple gluster servers Prasanna Kumar Kalever
@ 2015-10-27 15:01 ` Eric Blake
0 siblings, 0 replies; 2+ messages in thread
From: Eric Blake @ 2015-10-27 15:01 UTC (permalink / raw)
To: Prasanna Kumar Kalever, qemu-devel
Cc: kwolf, pkrempa, stefanha, deepakcs, bharata, rtalur
[-- Attachment #1: Type: text/plain, Size: 8435 bytes --]
On 10/27/2015 07:06 AM, Prasanna Kumar Kalever wrote:
> This patch adds a way to specify multiple volfile servers to the gluster
> block backend of QEMU with tcp|rdma transport types and their port numbers.
>
No 0/3 cover letter? Even if only one patch of a series changes, it's
still nice to have a cover letter explaining where to find the rest of
the series.
> Problem:
>
> Currenly VM Image on gluster volume is specified like this:
s/Currenly/Currently/
[...]
> This patch gives a mechanism to provide all the server addresses, which are in
> replica set, so in case host1 is down VM can still boot from any of the
> active hosts.
>
> This is equivalent to the backup-volfile-servers option supported by
> mount.glusterfs (FUSE way of mounting gluster volume)
>
> This patch depends on a recent fix in libgfapi raised as part of this work:
> http://review.gluster.org/#/c/12114/
>
> Credits: Sincere thanks to Kevin Wolf <kwolf@redhat.com> and
> "Deepak C Shetty" <deepakcs@redhat.com> for inputs and all their support
>
> Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
> v1:
Need a '---' line before v1. Everything above the line is okay for the
commit message, everything after the line is informational for reviewers
but doesn't belong in qemu.git (and 'git am' will automatically strip
comments after the --- separator). A year from now, we won't care how
many versions it took you to get to the version that was committed.
> +++ b/block/gluster.c
> -typedef struct GlusterConf {
> +typedef struct GlusterServerConf {
> char *host;
> int port;
> + char *transport;
> +} GlusterServerConf;
Why are you defining this type, instead of reusing 'GlusterTuple' that
gets auto-defined for you by your changes to block-core.json?
>
> +typedef struct GlusterConf {
> char *volume;
> char *path;
> - char *transport;
> + int num_servers;
size_t is better than int when counting items in an array.
> + GlusterServerConf *gsconf;
Naming it 'num_servers' vs. 'gsconf' is confusing. In fact, why are you
even defining this type? Why not just directly use
'BlockdevOptionsGluster' that you defined by your changes to
block-core.json? Of course, the generated type uses a single
GlusterTupleList (a linked list with NULL terminator) rather than a pair
of size and GlusterTuple[] for tracking the multiple servers, but
directly using the qapi types now will save you some conversion efforts
down the road.
> +static QemuOptsList runtime_tuple_opts = {
> + .name = "gluster_tuple",
> + .head = QTAILQ_HEAD_INITIALIZER(runtime_tuple_opts.head),
> + .desc = {
> + {
> + .name = GLUSTER_OPT_HOST,
> + .type = QEMU_OPT_STRING,
> + .help = "host address (hostname/ipv4/ipv6 addresses)",
> + },
> + {
> + .name = GLUSTER_OPT_PORT,
> + .type = QEMU_OPT_NUMBER,
> + .help = "port number on which glusterd listen (default 24007)",
s/listen/is listening/
> + },
> + {
> + .name = GLUSTER_OPT_TRANSPORT,
> + .type = QEMU_OPT_STRING,
> + .help = "transport type 'tcp' or 'rdma' (default 'tcp')",
> + },
> + { /* end of list */ }
> + },
> +};
> +
>
> static void qemu_gluster_gconf_free(GlusterConf *gconf)
> {
Unneeded. Just use the auto-generated
qapi_BlockdevOptionsGluster_free() that you got by adding and using the
appropriate types in block-core.json.
> @@ -155,16 +218,20 @@ static int qemu_gluster_parseuri(GlusterConf *gconf, const char *filename)
> return -EINVAL;
> }
>
> + gconf = g_new0(GlusterConf, 1);
> + gconf->num_servers = 1;
> + gconf->gsconf = g_new0(GlusterServerConf, gconf->num_servers);
If you use the qapi types, as I've suggested above, then you will need
to be working with the list of servers as a linked list rather than an
array.
> +
> /* transport */
> if (!uri->scheme || !strcmp(uri->scheme, "gluster")) {
> - gconf->transport = g_strdup("tcp");
> + gconf->gsconf[0].transport = g_strdup("tcp");
> } else if (!strcmp(uri->scheme, "gluster+tcp")) {
> - gconf->transport = g_strdup("tcp");
> + gconf->gsconf[0].transport = g_strdup("tcp");
> } else if (!strcmp(uri->scheme, "gluster+unix")) {
> - gconf->transport = g_strdup("unix");
> + gconf->gsconf[0].transport = g_strdup("unix");
> is_unix = true;
> } else if (!strcmp(uri->scheme, "gluster+rdma")) {
> - gconf->transport = g_strdup("rdma");
> + gconf->gsconf[0].transport = g_strdup("rdma");
The generated qapi code also contains convenience functions from mapping
from a finite set of strings to a set of enum values, although I don't
see "unix" listed in your set of values for 'GlusterTransport'.
> +/*
> +*
> +* Basic command line syntax looks like:
> +*
> +* Pattern I:
> +* Pattern II:
> +*
> +* Examples:
> +* Pattern I:
> +*
> +* Pattern II:
> +* 'json:{"driver":"qcow2","file":{"driver":"gluster","volume":"testvol",
> +*
> +*/
> +static int qemu_gluster_parsejson(GlusterConf **pgconf, QDict *options)
> +{
Wow. Overly long comment. It was fine in the commit message, but I don't
think you need quite so much filler here.
> + } else {
> + ret = qemu_gluster_parsejson(gconf, options);
> + if (ret < 0) {
> + error_setg(errp, "Wrong Usage!");
Drop the '!'. None of our other error messages need that much emphasis.
> +++ b/qapi/block-core.json
> @@ -1375,15 +1375,16 @@
> #
> # @host_device, @host_cdrom, @host_floppy: Since 2.1
> # @host_floppy: deprecated since 2.3
> +# @gluster: Since 2.5
> #
> # Since: 2.0
> ##
> { 'enum': 'BlockdevDriver',
> 'data': [ 'archipelago', 'blkdebug', 'blkverify', 'bochs', 'cloop',
> - 'dmg', 'file', 'ftp', 'ftps', 'host_cdrom', 'host_device',
> - 'host_floppy', 'http', 'https', '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', 'host_floppy', 'http', 'https', 'null-aio',
> + 'null-co', 'parallels', 'qcow', 'qcow2', 'qed', 'quorum', 'raw',
> + 'tftp', 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat' ] }
>
> ##
> # @BlockdevOptionsBase
> @@ -1794,6 +1795,57 @@
> '*read-pattern': 'QuorumReadPattern' } }
>
> ##
> +# @GlusterTransport
> +#
> +# An enumeration of Gluster transport type
> +#
> +# @tcp: TCP - Transmission Control Protocol
> +#
> +# @rdma: RDMA - Remote direct memory access
> +#
> +# Since: 2.5
> +##
> +{ 'enum': 'GlusterTransport', 'data': [ 'tcp', 'rdma' ] }
As mentioned above, isn't this missing 'unix'?
> +
> +##
> +# @GlusterTuple
> +#
> +# Gluster tuple set
> +#
> +# @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.5
> +##
> +{ 'struct': 'GlusterTuple',
> + 'data': { 'host': 'str',
> + '*port': 'int',
> + '*transport': 'GlusterTransport' } }
I'm still not sold that 'GlusterTuple' is the best name for this type,
but it doesn't affect ABI and I don't have any better suggestions off-hand.
> +
> +##
> +# @BlockdevOptionsGluster
> +#
> +# Driver specific block device options for Gluster
> +#
> +# @volume: name of gluster volume where our VM image resides
s/our //
> +#
> +# @path: absolute path to image file in gluster volume
> +#
> +# @servers: one or more gluster server descriptions (host, port, and transport)
> +#
> +# Since: 2.5
> +##
> +{ 'struct': 'BlockdevOptionsGluster',
> + 'data': { 'volume': 'str',
> + 'path': 'str',
> + 'servers': [ 'GlusterTuple' ] } }
This one looks okay, as far as I can tell.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2015-10-27 15:01 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-27 13:06 [Qemu-devel] [PATCH v10 3/3] block/gluster: add support for multiple gluster servers Prasanna Kumar Kalever
2015-10-27 15:01 ` Eric Blake
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).