* [Qemu-devel] [PATCH v17 1/4] block/gluster: rename [server, volname, image] -> [host, volume, path]
2016-06-15 8:25 [Qemu-devel] [PATCH v17 0/4][WIP] block/gluster: add support for multiple gluster servers Prasanna Kumar Kalever
@ 2016-06-15 8:25 ` Prasanna Kumar Kalever
2016-06-17 17:32 ` Jeff Cody
2016-06-15 8:25 ` [Qemu-devel] [PATCH v17 2/4] block/gluster: code cleanup Prasanna Kumar Kalever
` (3 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Prasanna Kumar Kalever @ 2016-06-15 8:25 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, eblake, pkrempa, jcody, bharata, berrange, rtalur,
deepakcs, Prasanna Kumar Kalever
A future patch will add support for multiple gluster servers. Existing
terminology is a bit unusual in relation to what names are used by
other networked devices, and doesn't map very well to the terminology
we expect to use for multiple servers. Therefore, rename the following
options:
'server' -> 'host'
'image' -> 'path'
'volname' -> 'volume'
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block/gluster.c | 54 +++++++++++++++++++++++++++---------------------------
1 file changed, 27 insertions(+), 27 deletions(-)
diff --git a/block/gluster.c b/block/gluster.c
index d361d8e..0c711a3 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -27,19 +27,19 @@ typedef struct BDRVGlusterState {
} BDRVGlusterState;
typedef struct GlusterConf {
- char *server;
+ char *host;
int port;
- char *volname;
- char *image;
+ char *volume;
+ char *path;
char *transport;
} GlusterConf;
static void qemu_gluster_gconf_free(GlusterConf *gconf)
{
if (gconf) {
- g_free(gconf->server);
- g_free(gconf->volname);
- g_free(gconf->image);
+ g_free(gconf->host);
+ g_free(gconf->volume);
+ g_free(gconf->path);
g_free(gconf->transport);
g_free(gconf);
}
@@ -59,19 +59,19 @@ static int parse_volume_options(GlusterConf *gconf, char *path)
if (*p == '\0') {
return -EINVAL;
}
- gconf->volname = g_strndup(q, p - q);
+ gconf->volume = g_strndup(q, p - q);
- /* image */
+ /* path */
p += strspn(p, "/");
if (*p == '\0') {
return -EINVAL;
}
- gconf->image = g_strdup(p);
+ gconf->path = g_strdup(p);
return 0;
}
/*
- * file=gluster[+transport]://[server[:port]]/volname/image[?socket=...]
+ * file=gluster[+transport]://[host[:port]]/volume/path[?socket=...]
*
* 'gluster' is the protocol.
*
@@ -80,10 +80,10 @@ static int parse_volume_options(GlusterConf *gconf, char *path)
* tcp, unix and rdma. If a transport type isn't specified, then tcp
* type is assumed.
*
- * 'server' specifies the server where the volume file specification for
+ * 'host' specifies the host where the volume file specification for
* the given volume resides. This can be either hostname, ipv4 address
* or ipv6 address. ipv6 address needs to be within square brackets [ ].
- * If transport type is 'unix', then 'server' field should not be specified.
+ * If transport type is 'unix', then 'host' field should not be specified.
* The 'socket' field needs to be populated with the path to unix domain
* socket.
*
@@ -92,9 +92,9 @@ static int parse_volume_options(GlusterConf *gconf, char *path)
* default port. If the transport type is unix, then 'port' should not be
* specified.
*
- * 'volname' is the name of the gluster volume which contains the VM image.
+ * 'volume' is the name of the gluster volume which contains the VM image.
*
- * 'image' is the path to the actual VM image that resides on gluster volume.
+ * 'path' is the path to the actual VM image that resides on gluster volume.
*
* Examples:
*
@@ -103,7 +103,7 @@ static int parse_volume_options(GlusterConf *gconf, char *path)
* file=gluster+tcp://1.2.3.4:24007/testvol/dir/a.img
* file=gluster+tcp://[1:2:3:4:5:6:7:8]/testvol/dir/a.img
* file=gluster+tcp://[1:2:3:4:5:6:7:8]:24007/testvol/dir/a.img
- * file=gluster+tcp://server.domain.com:24007/testvol/dir/a.img
+ * file=gluster+tcp://host.domain.com:24007/testvol/dir/a.img
* file=gluster+unix:///testvol/dir/a.img?socket=/tmp/glusterd.socket
* file=gluster+rdma://1.2.3.4:24007/testvol/a.img
*/
@@ -154,9 +154,9 @@ static int qemu_gluster_parseuri(GlusterConf *gconf, const char *filename)
ret = -EINVAL;
goto out;
}
- gconf->server = g_strdup(qp->p[0].value);
+ gconf->host = g_strdup(qp->p[0].value);
} else {
- gconf->server = g_strdup(uri->server ? uri->server : "localhost");
+ gconf->host = g_strdup(uri->server ? uri->server : "localhost");
gconf->port = uri->port;
}
@@ -177,18 +177,18 @@ static struct glfs *qemu_gluster_init(GlusterConf *gconf, const char *filename,
ret = qemu_gluster_parseuri(gconf, filename);
if (ret < 0) {
- error_setg(errp, "Usage: file=gluster[+transport]://[server[:port]]/"
- "volname/image[?socket=...]");
+ error_setg(errp, "Usage: file=gluster[+transport]://[host[:port]]/"
+ "volume/path[?socket=...]");
errno = -ret;
goto out;
}
- glfs = glfs_new(gconf->volname);
+ glfs = glfs_new(gconf->volume);
if (!glfs) {
goto out;
}
- ret = glfs_set_volfile_server(glfs, gconf->transport, gconf->server,
+ ret = glfs_set_volfile_server(glfs, gconf->transport, gconf->host,
gconf->port);
if (ret < 0) {
goto out;
@@ -206,9 +206,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 server=%s port=%d "
- "volume=%s image=%s transport=%s", gconf->server,
- gconf->port, gconf->volname, gconf->image,
+ "Gluster connection failed for host=%s port=%d "
+ "volume=%s path=%s transport=%s", gconf->host,
+ gconf->port, gconf->volume, gconf->path,
gconf->transport);
/* glfs_init sometimes doesn't set errno although docs suggest that */
@@ -333,7 +333,7 @@ static int qemu_gluster_open(BlockDriverState *bs, QDict *options,
qemu_gluster_parse_flags(bdrv_flags, &open_flags);
- s->fd = glfs_open(s->glfs, gconf->image, open_flags);
+ s->fd = glfs_open(s->glfs, gconf->path, open_flags);
if (!s->fd) {
ret = -errno;
}
@@ -393,7 +393,7 @@ static int qemu_gluster_reopen_prepare(BDRVReopenState *state,
}
#endif
- reop_s->fd = glfs_open(reop_s->glfs, gconf->image, open_flags);
+ reop_s->fd = glfs_open(reop_s->glfs, gconf->path, open_flags);
if (reop_s->fd == NULL) {
/* reops->glfs will be cleaned up in _abort */
ret = -errno;
@@ -533,7 +533,7 @@ static int qemu_gluster_create(const char *filename,
goto out;
}
- fd = glfs_creat(glfs, gconf->image,
+ fd = glfs_creat(glfs, gconf->path,
O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, S_IRUSR | S_IWUSR);
if (!fd) {
ret = -errno;
--
2.5.5
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [Qemu-devel] [PATCH v17 1/4] block/gluster: rename [server, volname, image] -> [host, volume, path]
2016-06-15 8:25 ` [Qemu-devel] [PATCH v17 1/4] block/gluster: rename [server, volname, image] -> [host, volume, path] Prasanna Kumar Kalever
@ 2016-06-17 17:32 ` Jeff Cody
0 siblings, 0 replies; 9+ messages in thread
From: Jeff Cody @ 2016-06-17 17:32 UTC (permalink / raw)
To: Prasanna Kumar Kalever
Cc: qemu-devel, kwolf, eblake, pkrempa, bharata, berrange, rtalur,
deepakcs
On Wed, Jun 15, 2016 at 01:55:44PM +0530, Prasanna Kumar Kalever wrote:
> A future patch will add support for multiple gluster servers. Existing
> terminology is a bit unusual in relation to what names are used by
> other networked devices, and doesn't map very well to the terminology
> we expect to use for multiple servers. Therefore, rename the following
> options:
> 'server' -> 'host'
> 'image' -> 'path'
> 'volname' -> 'volume'
>
> Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
> ---
> block/gluster.c | 54 +++++++++++++++++++++++++++---------------------------
> 1 file changed, 27 insertions(+), 27 deletions(-)
>
> diff --git a/block/gluster.c b/block/gluster.c
> index d361d8e..0c711a3 100644
> --- a/block/gluster.c
> +++ b/block/gluster.c
> @@ -27,19 +27,19 @@ typedef struct BDRVGlusterState {
> } BDRVGlusterState;
>
> typedef struct GlusterConf {
> - char *server;
> + char *host;
> int port;
> - char *volname;
> - char *image;
> + char *volume;
> + char *path;
> char *transport;
> } GlusterConf;
>
> static void qemu_gluster_gconf_free(GlusterConf *gconf)
> {
> if (gconf) {
> - g_free(gconf->server);
> - g_free(gconf->volname);
> - g_free(gconf->image);
> + g_free(gconf->host);
> + g_free(gconf->volume);
> + g_free(gconf->path);
> g_free(gconf->transport);
> g_free(gconf);
> }
> @@ -59,19 +59,19 @@ static int parse_volume_options(GlusterConf *gconf, char *path)
> if (*p == '\0') {
> return -EINVAL;
> }
> - gconf->volname = g_strndup(q, p - q);
> + gconf->volume = g_strndup(q, p - q);
>
> - /* image */
> + /* path */
> p += strspn(p, "/");
> if (*p == '\0') {
> return -EINVAL;
> }
> - gconf->image = g_strdup(p);
> + gconf->path = g_strdup(p);
> return 0;
> }
>
> /*
> - * file=gluster[+transport]://[server[:port]]/volname/image[?socket=...]
> + * file=gluster[+transport]://[host[:port]]/volume/path[?socket=...]
> *
> * 'gluster' is the protocol.
> *
> @@ -80,10 +80,10 @@ static int parse_volume_options(GlusterConf *gconf, char *path)
> * tcp, unix and rdma. If a transport type isn't specified, then tcp
> * type is assumed.
> *
> - * 'server' specifies the server where the volume file specification for
> + * 'host' specifies the host where the volume file specification for
> * the given volume resides. This can be either hostname, ipv4 address
> * or ipv6 address. ipv6 address needs to be within square brackets [ ].
> - * If transport type is 'unix', then 'server' field should not be specified.
> + * If transport type is 'unix', then 'host' field should not be specified.
> * The 'socket' field needs to be populated with the path to unix domain
> * socket.
> *
> @@ -92,9 +92,9 @@ static int parse_volume_options(GlusterConf *gconf, char *path)
> * default port. If the transport type is unix, then 'port' should not be
> * specified.
> *
> - * 'volname' is the name of the gluster volume which contains the VM image.
> + * 'volume' is the name of the gluster volume which contains the VM image.
> *
> - * 'image' is the path to the actual VM image that resides on gluster volume.
> + * 'path' is the path to the actual VM image that resides on gluster volume.
> *
> * Examples:
> *
> @@ -103,7 +103,7 @@ static int parse_volume_options(GlusterConf *gconf, char *path)
> * file=gluster+tcp://1.2.3.4:24007/testvol/dir/a.img
> * file=gluster+tcp://[1:2:3:4:5:6:7:8]/testvol/dir/a.img
> * file=gluster+tcp://[1:2:3:4:5:6:7:8]:24007/testvol/dir/a.img
> - * file=gluster+tcp://server.domain.com:24007/testvol/dir/a.img
> + * file=gluster+tcp://host.domain.com:24007/testvol/dir/a.img
> * file=gluster+unix:///testvol/dir/a.img?socket=/tmp/glusterd.socket
> * file=gluster+rdma://1.2.3.4:24007/testvol/a.img
> */
> @@ -154,9 +154,9 @@ static int qemu_gluster_parseuri(GlusterConf *gconf, const char *filename)
> ret = -EINVAL;
> goto out;
> }
> - gconf->server = g_strdup(qp->p[0].value);
> + gconf->host = g_strdup(qp->p[0].value);
> } else {
> - gconf->server = g_strdup(uri->server ? uri->server : "localhost");
> + gconf->host = g_strdup(uri->server ? uri->server : "localhost");
> gconf->port = uri->port;
> }
>
> @@ -177,18 +177,18 @@ static struct glfs *qemu_gluster_init(GlusterConf *gconf, const char *filename,
>
> ret = qemu_gluster_parseuri(gconf, filename);
> if (ret < 0) {
> - error_setg(errp, "Usage: file=gluster[+transport]://[server[:port]]/"
> - "volname/image[?socket=...]");
> + error_setg(errp, "Usage: file=gluster[+transport]://[host[:port]]/"
> + "volume/path[?socket=...]");
> errno = -ret;
> goto out;
> }
>
> - glfs = glfs_new(gconf->volname);
> + glfs = glfs_new(gconf->volume);
> if (!glfs) {
> goto out;
> }
>
> - ret = glfs_set_volfile_server(glfs, gconf->transport, gconf->server,
> + ret = glfs_set_volfile_server(glfs, gconf->transport, gconf->host,
> gconf->port);
> if (ret < 0) {
> goto out;
> @@ -206,9 +206,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 server=%s port=%d "
> - "volume=%s image=%s transport=%s", gconf->server,
> - gconf->port, gconf->volname, gconf->image,
> + "Gluster connection failed for host=%s port=%d "
> + "volume=%s path=%s transport=%s", gconf->host,
> + gconf->port, gconf->volume, gconf->path,
> gconf->transport);
>
> /* glfs_init sometimes doesn't set errno although docs suggest that */
> @@ -333,7 +333,7 @@ static int qemu_gluster_open(BlockDriverState *bs, QDict *options,
>
> qemu_gluster_parse_flags(bdrv_flags, &open_flags);
>
> - s->fd = glfs_open(s->glfs, gconf->image, open_flags);
> + s->fd = glfs_open(s->glfs, gconf->path, open_flags);
> if (!s->fd) {
> ret = -errno;
> }
> @@ -393,7 +393,7 @@ static int qemu_gluster_reopen_prepare(BDRVReopenState *state,
> }
> #endif
>
> - reop_s->fd = glfs_open(reop_s->glfs, gconf->image, open_flags);
> + reop_s->fd = glfs_open(reop_s->glfs, gconf->path, open_flags);
> if (reop_s->fd == NULL) {
> /* reops->glfs will be cleaned up in _abort */
> ret = -errno;
> @@ -533,7 +533,7 @@ static int qemu_gluster_create(const char *filename,
> goto out;
> }
>
> - fd = glfs_creat(glfs, gconf->image,
> + fd = glfs_creat(glfs, gconf->path,
> O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, S_IRUSR | S_IWUSR);
> if (!fd) {
> ret = -errno;
> --
> 2.5.5
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH v17 2/4] block/gluster: code cleanup
2016-06-15 8:25 [Qemu-devel] [PATCH v17 0/4][WIP] block/gluster: add support for multiple gluster servers Prasanna Kumar Kalever
2016-06-15 8:25 ` [Qemu-devel] [PATCH v17 1/4] block/gluster: rename [server, volname, image] -> [host, volume, path] Prasanna Kumar Kalever
@ 2016-06-15 8:25 ` Prasanna Kumar Kalever
2016-06-17 17:32 ` Jeff Cody
2016-06-15 8:25 ` [Qemu-devel] [PATCH v17 3/4] block/gluster: using new qapi schema Prasanna Kumar Kalever
` (2 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Prasanna Kumar Kalever @ 2016-06-15 8:25 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, eblake, pkrempa, jcody, bharata, berrange, rtalur,
deepakcs, Prasanna Kumar Kalever
unified coding styles of multiline function arguments and other error functions
moved random declarations of structures and other list variables
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block/gluster.c | 113 ++++++++++++++++++++++++++++++--------------------------
1 file changed, 60 insertions(+), 53 deletions(-)
diff --git a/block/gluster.c b/block/gluster.c
index 0c711a3..45072b1 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -26,6 +26,11 @@ typedef struct BDRVGlusterState {
struct glfs_fd *fd;
} BDRVGlusterState;
+typedef struct BDRVGlusterReopenState {
+ struct glfs *glfs;
+ struct glfs_fd *fd;
+} BDRVGlusterReopenState;
+
typedef struct GlusterConf {
char *host;
int port;
@@ -34,6 +39,39 @@ typedef struct GlusterConf {
char *transport;
} GlusterConf;
+
+static QemuOptsList qemu_gluster_create_opts = {
+ .name = "qemu-gluster-create-opts",
+ .head = QTAILQ_HEAD_INITIALIZER(qemu_gluster_create_opts.head),
+ .desc = {
+ {
+ .name = BLOCK_OPT_SIZE,
+ .type = QEMU_OPT_SIZE,
+ .help = "Virtual disk size"
+ },
+ {
+ .name = BLOCK_OPT_PREALLOC,
+ .type = QEMU_OPT_STRING,
+ .help = "Preallocation mode (allowed values: off, full)"
+ },
+ { /* end of list */ }
+ }
+};
+
+static QemuOptsList runtime_opts = {
+ .name = "gluster",
+ .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),
+ .desc = {
+ {
+ .name = "filename",
+ .type = QEMU_OPT_STRING,
+ .help = "URL to the gluster image",
+ },
+ { /* end of list */ }
+ },
+};
+
+
static void qemu_gluster_gconf_free(GlusterConf *gconf)
{
if (gconf) {
@@ -178,7 +216,7 @@ static struct glfs *qemu_gluster_init(GlusterConf *gconf, const char *filename,
ret = qemu_gluster_parseuri(gconf, filename);
if (ret < 0) {
error_setg(errp, "Usage: file=gluster[+transport]://[host[:port]]/"
- "volume/path[?socket=...]");
+ "volume/path[?socket=...]");
errno = -ret;
goto out;
}
@@ -256,20 +294,6 @@ static void gluster_finish_aiocb(struct glfs_fd *fd, ssize_t ret, void *arg)
qemu_bh_schedule(acb->bh);
}
-/* TODO Convert to fine grained options */
-static QemuOptsList runtime_opts = {
- .name = "gluster",
- .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),
- .desc = {
- {
- .name = "filename",
- .type = QEMU_OPT_STRING,
- .help = "URL to the gluster image",
- },
- { /* end of list */ }
- },
-};
-
static void qemu_gluster_parse_flags(int bdrv_flags, int *open_flags)
{
assert(open_flags != NULL);
@@ -287,7 +311,7 @@ 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;
@@ -353,12 +377,6 @@ out:
return ret;
}
-typedef struct BDRVGlusterReopenState {
- struct glfs *glfs;
- struct glfs_fd *fd;
-} BDRVGlusterReopenState;
-
-
static int qemu_gluster_reopen_prepare(BDRVReopenState *state,
BlockReopenQueue *queue, Error **errp)
{
@@ -455,7 +473,9 @@ static void qemu_gluster_reopen_abort(BDRVReopenState *state)
#ifdef CONFIG_GLUSTERFS_ZEROFILL
static coroutine_fn int qemu_gluster_co_pwrite_zeroes(BlockDriverState *bs,
- int64_t offset, int size, BdrvRequestFlags flags)
+ int64_t offset,
+ int size,
+ BdrvRequestFlags flags)
{
int ret;
GlusterAIOCB acb;
@@ -481,7 +501,7 @@ static inline bool gluster_supports_zerofill(void)
}
static inline int qemu_gluster_zerofill(struct glfs_fd *fd, int64_t offset,
- int64_t size)
+ int64_t size)
{
return glfs_zerofill(fd, offset, size);
}
@@ -493,7 +513,7 @@ static inline bool gluster_supports_zerofill(void)
}
static inline int qemu_gluster_zerofill(struct glfs_fd *fd, int64_t offset,
- int64_t size)
+ int64_t size)
{
return 0;
}
@@ -522,19 +542,17 @@ static int qemu_gluster_create(const char *filename,
tmp = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC);
if (!tmp || !strcmp(tmp, "off")) {
prealloc = 0;
- } else if (!strcmp(tmp, "full") &&
- gluster_supports_zerofill()) {
+ } else if (!strcmp(tmp, "full") && gluster_supports_zerofill()) {
prealloc = 1;
} else {
error_setg(errp, "Invalid preallocation mode: '%s'"
- " or GlusterFS doesn't support zerofill API",
- tmp);
+ " or GlusterFS doesn't support zerofill API", tmp);
ret = -EINVAL;
goto out;
}
fd = glfs_creat(glfs, gconf->path,
- O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, S_IRUSR | S_IWUSR);
+ O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, S_IRUSR | S_IWUSR);
if (!fd) {
ret = -errno;
} else {
@@ -560,7 +578,8 @@ out:
}
static coroutine_fn int qemu_gluster_co_rw(BlockDriverState *bs,
- int64_t sector_num, int nb_sectors, QEMUIOVector *qiov, int write)
+ int64_t sector_num, int nb_sectors,
+ QEMUIOVector *qiov, int write)
{
int ret;
GlusterAIOCB acb;
@@ -575,10 +594,10 @@ static coroutine_fn int qemu_gluster_co_rw(BlockDriverState *bs,
if (write) {
ret = glfs_pwritev_async(s->fd, qiov->iov, qiov->niov, offset, 0,
- gluster_finish_aiocb, &acb);
+ gluster_finish_aiocb, &acb);
} else {
ret = glfs_preadv_async(s->fd, qiov->iov, qiov->niov, offset, 0,
- gluster_finish_aiocb, &acb);
+ gluster_finish_aiocb, &acb);
}
if (ret < 0) {
@@ -603,13 +622,17 @@ static int qemu_gluster_truncate(BlockDriverState *bs, int64_t offset)
}
static coroutine_fn int qemu_gluster_co_readv(BlockDriverState *bs,
- int64_t sector_num, int nb_sectors, QEMUIOVector *qiov)
+ int64_t sector_num,
+ int nb_sectors,
+ QEMUIOVector *qiov)
{
return qemu_gluster_co_rw(bs, sector_num, nb_sectors, qiov, 0);
}
static coroutine_fn int qemu_gluster_co_writev(BlockDriverState *bs,
- int64_t sector_num, int nb_sectors, QEMUIOVector *qiov)
+ int64_t sector_num,
+ int nb_sectors,
+ QEMUIOVector *qiov)
{
return qemu_gluster_co_rw(bs, sector_num, nb_sectors, qiov, 1);
}
@@ -671,7 +694,8 @@ error:
#ifdef CONFIG_GLUSTERFS_DISCARD
static coroutine_fn int qemu_gluster_co_discard(BlockDriverState *bs,
- int64_t sector_num, int nb_sectors)
+ int64_t sector_num,
+ int nb_sectors)
{
int ret;
GlusterAIOCB acb;
@@ -727,23 +751,6 @@ static int qemu_gluster_has_zero_init(BlockDriverState *bs)
return 0;
}
-static QemuOptsList qemu_gluster_create_opts = {
- .name = "qemu-gluster-create-opts",
- .head = QTAILQ_HEAD_INITIALIZER(qemu_gluster_create_opts.head),
- .desc = {
- {
- .name = BLOCK_OPT_SIZE,
- .type = QEMU_OPT_SIZE,
- .help = "Virtual disk size"
- },
- {
- .name = BLOCK_OPT_PREALLOC,
- .type = QEMU_OPT_STRING,
- .help = "Preallocation mode (allowed values: off, full)"
- },
- { /* end of list */ }
- }
-};
static BlockDriver bdrv_gluster = {
.format_name = "gluster",
--
2.5.5
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [Qemu-devel] [PATCH v17 2/4] block/gluster: code cleanup
2016-06-15 8:25 ` [Qemu-devel] [PATCH v17 2/4] block/gluster: code cleanup Prasanna Kumar Kalever
@ 2016-06-17 17:32 ` Jeff Cody
0 siblings, 0 replies; 9+ messages in thread
From: Jeff Cody @ 2016-06-17 17:32 UTC (permalink / raw)
To: Prasanna Kumar Kalever
Cc: qemu-devel, kwolf, eblake, pkrempa, bharata, berrange, rtalur,
deepakcs
On Wed, Jun 15, 2016 at 01:55:45PM +0530, Prasanna Kumar Kalever wrote:
> unified coding styles of multiline function arguments and other error functions
> moved random declarations of structures and other list variables
>
> Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
> ---
> block/gluster.c | 113 ++++++++++++++++++++++++++++++--------------------------
> 1 file changed, 60 insertions(+), 53 deletions(-)
>
> diff --git a/block/gluster.c b/block/gluster.c
> index 0c711a3..45072b1 100644
> --- a/block/gluster.c
> +++ b/block/gluster.c
> @@ -26,6 +26,11 @@ typedef struct BDRVGlusterState {
> struct glfs_fd *fd;
> } BDRVGlusterState;
>
> +typedef struct BDRVGlusterReopenState {
> + struct glfs *glfs;
> + struct glfs_fd *fd;
> +} BDRVGlusterReopenState;
> +
> typedef struct GlusterConf {
> char *host;
> int port;
> @@ -34,6 +39,39 @@ typedef struct GlusterConf {
> char *transport;
> } GlusterConf;
>
> +
> +static QemuOptsList qemu_gluster_create_opts = {
> + .name = "qemu-gluster-create-opts",
> + .head = QTAILQ_HEAD_INITIALIZER(qemu_gluster_create_opts.head),
> + .desc = {
> + {
> + .name = BLOCK_OPT_SIZE,
> + .type = QEMU_OPT_SIZE,
> + .help = "Virtual disk size"
> + },
> + {
> + .name = BLOCK_OPT_PREALLOC,
> + .type = QEMU_OPT_STRING,
> + .help = "Preallocation mode (allowed values: off, full)"
> + },
> + { /* end of list */ }
> + }
> +};
> +
> +static QemuOptsList runtime_opts = {
> + .name = "gluster",
> + .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),
> + .desc = {
> + {
> + .name = "filename",
> + .type = QEMU_OPT_STRING,
> + .help = "URL to the gluster image",
> + },
> + { /* end of list */ }
> + },
> +};
> +
> +
> static void qemu_gluster_gconf_free(GlusterConf *gconf)
> {
> if (gconf) {
> @@ -178,7 +216,7 @@ static struct glfs *qemu_gluster_init(GlusterConf *gconf, const char *filename,
> ret = qemu_gluster_parseuri(gconf, filename);
> if (ret < 0) {
> error_setg(errp, "Usage: file=gluster[+transport]://[host[:port]]/"
> - "volume/path[?socket=...]");
> + "volume/path[?socket=...]");
> errno = -ret;
> goto out;
> }
> @@ -256,20 +294,6 @@ static void gluster_finish_aiocb(struct glfs_fd *fd, ssize_t ret, void *arg)
> qemu_bh_schedule(acb->bh);
> }
>
> -/* TODO Convert to fine grained options */
> -static QemuOptsList runtime_opts = {
> - .name = "gluster",
> - .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),
> - .desc = {
> - {
> - .name = "filename",
> - .type = QEMU_OPT_STRING,
> - .help = "URL to the gluster image",
> - },
> - { /* end of list */ }
> - },
> -};
> -
> static void qemu_gluster_parse_flags(int bdrv_flags, int *open_flags)
> {
> assert(open_flags != NULL);
> @@ -287,7 +311,7 @@ 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;
> @@ -353,12 +377,6 @@ out:
> return ret;
> }
>
> -typedef struct BDRVGlusterReopenState {
> - struct glfs *glfs;
> - struct glfs_fd *fd;
> -} BDRVGlusterReopenState;
> -
> -
> static int qemu_gluster_reopen_prepare(BDRVReopenState *state,
> BlockReopenQueue *queue, Error **errp)
> {
> @@ -455,7 +473,9 @@ static void qemu_gluster_reopen_abort(BDRVReopenState *state)
>
> #ifdef CONFIG_GLUSTERFS_ZEROFILL
> static coroutine_fn int qemu_gluster_co_pwrite_zeroes(BlockDriverState *bs,
> - int64_t offset, int size, BdrvRequestFlags flags)
> + int64_t offset,
> + int size,
> + BdrvRequestFlags flags)
> {
> int ret;
> GlusterAIOCB acb;
> @@ -481,7 +501,7 @@ static inline bool gluster_supports_zerofill(void)
> }
>
> static inline int qemu_gluster_zerofill(struct glfs_fd *fd, int64_t offset,
> - int64_t size)
> + int64_t size)
> {
> return glfs_zerofill(fd, offset, size);
> }
> @@ -493,7 +513,7 @@ static inline bool gluster_supports_zerofill(void)
> }
>
> static inline int qemu_gluster_zerofill(struct glfs_fd *fd, int64_t offset,
> - int64_t size)
> + int64_t size)
> {
> return 0;
> }
> @@ -522,19 +542,17 @@ static int qemu_gluster_create(const char *filename,
> tmp = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC);
> if (!tmp || !strcmp(tmp, "off")) {
> prealloc = 0;
> - } else if (!strcmp(tmp, "full") &&
> - gluster_supports_zerofill()) {
> + } else if (!strcmp(tmp, "full") && gluster_supports_zerofill()) {
> prealloc = 1;
> } else {
> error_setg(errp, "Invalid preallocation mode: '%s'"
> - " or GlusterFS doesn't support zerofill API",
> - tmp);
> + " or GlusterFS doesn't support zerofill API", tmp);
> ret = -EINVAL;
> goto out;
> }
>
> fd = glfs_creat(glfs, gconf->path,
> - O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, S_IRUSR | S_IWUSR);
> + O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, S_IRUSR | S_IWUSR);
> if (!fd) {
> ret = -errno;
> } else {
> @@ -560,7 +578,8 @@ out:
> }
>
> static coroutine_fn int qemu_gluster_co_rw(BlockDriverState *bs,
> - int64_t sector_num, int nb_sectors, QEMUIOVector *qiov, int write)
> + int64_t sector_num, int nb_sectors,
> + QEMUIOVector *qiov, int write)
> {
> int ret;
> GlusterAIOCB acb;
> @@ -575,10 +594,10 @@ static coroutine_fn int qemu_gluster_co_rw(BlockDriverState *bs,
>
> if (write) {
> ret = glfs_pwritev_async(s->fd, qiov->iov, qiov->niov, offset, 0,
> - gluster_finish_aiocb, &acb);
> + gluster_finish_aiocb, &acb);
> } else {
> ret = glfs_preadv_async(s->fd, qiov->iov, qiov->niov, offset, 0,
> - gluster_finish_aiocb, &acb);
> + gluster_finish_aiocb, &acb);
> }
>
> if (ret < 0) {
> @@ -603,13 +622,17 @@ static int qemu_gluster_truncate(BlockDriverState *bs, int64_t offset)
> }
>
> static coroutine_fn int qemu_gluster_co_readv(BlockDriverState *bs,
> - int64_t sector_num, int nb_sectors, QEMUIOVector *qiov)
> + int64_t sector_num,
> + int nb_sectors,
> + QEMUIOVector *qiov)
> {
> return qemu_gluster_co_rw(bs, sector_num, nb_sectors, qiov, 0);
> }
>
> static coroutine_fn int qemu_gluster_co_writev(BlockDriverState *bs,
> - int64_t sector_num, int nb_sectors, QEMUIOVector *qiov)
> + int64_t sector_num,
> + int nb_sectors,
> + QEMUIOVector *qiov)
> {
> return qemu_gluster_co_rw(bs, sector_num, nb_sectors, qiov, 1);
> }
> @@ -671,7 +694,8 @@ error:
>
> #ifdef CONFIG_GLUSTERFS_DISCARD
> static coroutine_fn int qemu_gluster_co_discard(BlockDriverState *bs,
> - int64_t sector_num, int nb_sectors)
> + int64_t sector_num,
> + int nb_sectors)
> {
> int ret;
> GlusterAIOCB acb;
> @@ -727,23 +751,6 @@ static int qemu_gluster_has_zero_init(BlockDriverState *bs)
> return 0;
> }
>
> -static QemuOptsList qemu_gluster_create_opts = {
> - .name = "qemu-gluster-create-opts",
> - .head = QTAILQ_HEAD_INITIALIZER(qemu_gluster_create_opts.head),
> - .desc = {
> - {
> - .name = BLOCK_OPT_SIZE,
> - .type = QEMU_OPT_SIZE,
> - .help = "Virtual disk size"
> - },
> - {
> - .name = BLOCK_OPT_PREALLOC,
> - .type = QEMU_OPT_STRING,
> - .help = "Preallocation mode (allowed values: off, full)"
> - },
> - { /* end of list */ }
> - }
> -};
>
> static BlockDriver bdrv_gluster = {
> .format_name = "gluster",
> --
> 2.5.5
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH v17 3/4] block/gluster: using new qapi schema
2016-06-15 8:25 [Qemu-devel] [PATCH v17 0/4][WIP] block/gluster: add support for multiple gluster servers Prasanna Kumar Kalever
2016-06-15 8:25 ` [Qemu-devel] [PATCH v17 1/4] block/gluster: rename [server, volname, image] -> [host, volume, path] Prasanna Kumar Kalever
2016-06-15 8:25 ` [Qemu-devel] [PATCH v17 2/4] block/gluster: code cleanup Prasanna Kumar Kalever
@ 2016-06-15 8:25 ` Prasanna Kumar Kalever
2016-06-15 8:25 ` [Qemu-devel] [PATCH v17 4/4] block/gluster: add support for multiple gluster servers Prasanna Kumar Kalever
2016-06-29 14:11 ` [Qemu-devel] [PATCH v17 0/4][WIP] " Jeff Cody
4 siblings, 0 replies; 9+ messages in thread
From: Prasanna Kumar Kalever @ 2016-06-15 8:25 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, eblake, pkrempa, jcody, bharata, berrange, rtalur,
deepakcs, Prasanna Kumar Kalever
this patch adds 'GlusterServer' related schema in qapi/block-core.json
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
---
block/gluster.c | 101 ++++++++++++++++++++++++++-------------------------
qapi/block-core.json | 64 +++++++++++++++++++++++++++++---
2 files changed, 110 insertions(+), 55 deletions(-)
diff --git a/block/gluster.c b/block/gluster.c
index 45072b1..c129b0b 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -13,6 +13,10 @@
#include "qapi/error.h"
#include "qemu/uri.h"
+#define GLUSTER_OPT_FILENAME "filename"
+#define GLUSTER_DEFAULT_PORT 24007
+
+
typedef struct GlusterAIOCB {
int64_t size;
int ret;
@@ -31,15 +35,6 @@ typedef struct BDRVGlusterReopenState {
struct glfs_fd *fd;
} BDRVGlusterReopenState;
-typedef struct GlusterConf {
- char *host;
- int port;
- char *volume;
- char *path;
- char *transport;
-} GlusterConf;
-
-
static QemuOptsList qemu_gluster_create_opts = {
.name = "qemu-gluster-create-opts",
.head = QTAILQ_HEAD_INITIALIZER(qemu_gluster_create_opts.head),
@@ -63,7 +58,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",
},
@@ -72,18 +67,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;
@@ -145,8 +129,10 @@ 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 **pgconf,
+ const char *filename)
{
+ BlockdevOptionsGluster *gconf;
URI *uri;
QueryParams *qp = NULL;
bool is_unix = false;
@@ -157,20 +143,24 @@ static int qemu_gluster_parseuri(GlusterConf *gconf, const char *filename)
return -EINVAL;
}
+ gconf = g_new0(BlockdevOptionsGluster, 1);
+ 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) {
@@ -192,13 +182,23 @@ 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;
}
+ *pgconf = gconf;
+
out:
+ if (ret < 0) {
+ qapi_free_BlockdevOptionsGluster(gconf);
+ }
if (qp) {
query_params_free(qp);
}
@@ -206,14 +206,15 @@ out:
return ret;
}
-static struct glfs *qemu_gluster_init(GlusterConf *gconf, const char *filename,
- Error **errp)
+static struct glfs *qemu_gluster_init(BlockdevOptionsGluster **pgconf,
+ const char *filename, Error **errp)
{
struct glfs *glfs = NULL;
int ret;
int old_errno;
+ BlockdevOptionsGluster *gconf = NULL;
- ret = qemu_gluster_parseuri(gconf, filename);
+ ret = qemu_gluster_parseuri(&gconf, filename);
if (ret < 0) {
error_setg(errp, "Usage: file=gluster[+transport]://[host[:port]]/"
"volume/path[?socket=...]");
@@ -226,8 +227,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;
}
@@ -245,9 +247,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)
@@ -255,9 +257,11 @@ static struct glfs *qemu_gluster_init(GlusterConf *gconf, const char *filename,
goto out;
}
+ *pgconf = gconf;
return glfs;
out:
+ qapi_free_BlockdevOptionsGluster(gconf);
if (glfs) {
old_errno = errno;
glfs_fini(glfs);
@@ -317,7 +321,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;
@@ -331,8 +335,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, errp);
if (!s->glfs) {
ret = -errno;
goto out;
@@ -364,7 +367,7 @@ static int qemu_gluster_open(BlockDriverState *bs, QDict *options,
out:
qemu_opts_del(opts);
- qemu_gluster_gconf_free(gconf);
+ qapi_free_BlockdevOptionsGluster(gconf);
if (!ret) {
return ret;
}
@@ -382,7 +385,7 @@ static int qemu_gluster_reopen_prepare(BDRVReopenState *state,
{
int ret = 0;
BDRVGlusterReopenState *reop_s;
- GlusterConf *gconf = NULL;
+ BlockdevOptionsGluster *gconf = NULL;
int open_flags = 0;
assert(state != NULL);
@@ -393,9 +396,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, errp);
if (reop_s->glfs == NULL) {
ret = -errno;
goto exit;
@@ -420,7 +421,7 @@ 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);
+ qapi_free_BlockdevOptionsGluster(gconf);
return ret;
}
@@ -522,15 +523,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 = 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, errp);
if (!glfs) {
ret = -errno;
goto out;
@@ -570,7 +571,7 @@ static int qemu_gluster_create(const char *filename,
}
out:
g_free(tmp);
- qemu_gluster_gconf_free(gconf);
+ qapi_free_BlockdevOptionsGluster(gconf);
if (glfs) {
glfs_fini(glfs);
}
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 98a20d2..93fbca3 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
@@ -2032,6 +2033,59 @@
'*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
+#
+# @servers: gluster server description
+#
+# Since: 2.7
+##
+{ 'struct': 'BlockdevOptionsGluster',
+ 'data': { 'volume': 'str',
+ 'path': 'str',
+ 'server': 'GlusterServer' } }
+
+##
# @BlockdevOptions
#
# Options for creating a block device. Many options are available for all
@@ -2094,7 +2148,7 @@
'file': 'BlockdevOptionsFile',
'ftp': 'BlockdevOptionsFile',
'ftps': 'BlockdevOptionsFile',
-# TODO gluster: Wait for structured options
+ 'gluster': 'BlockdevOptionsGluster',
'host_cdrom': 'BlockdevOptionsFile',
'host_device':'BlockdevOptionsFile',
'http': 'BlockdevOptionsFile',
--
2.5.5
^ permalink raw reply related [flat|nested] 9+ messages in thread* [Qemu-devel] [PATCH v17 4/4] block/gluster: add support for multiple gluster servers
2016-06-15 8:25 [Qemu-devel] [PATCH v17 0/4][WIP] block/gluster: add support for multiple gluster servers Prasanna Kumar Kalever
` (2 preceding siblings ...)
2016-06-15 8:25 ` [Qemu-devel] [PATCH v17 3/4] block/gluster: using new qapi schema Prasanna Kumar Kalever
@ 2016-06-15 8:25 ` Prasanna Kumar Kalever
2016-06-29 14:11 ` [Qemu-devel] [PATCH v17 0/4][WIP] " Jeff Cody
4 siblings, 0 replies; 9+ messages in thread
From: Prasanna Kumar Kalever @ 2016-06-15 8:25 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, eblake, pkrempa, jcody, bharata, berrange, rtalur,
deepakcs, 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:
Currently 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,
server.0.host=1.2.3.4,
[server.0.port=24007,]
[server.0.transport=tcp,]
server.1.host=5.6.7.8,
[server.1.port=24008,]
[server.1.transport=rdma,]
server.2.host=/var/run/glusterd.socket,
server.2.transport=unix ...
Pattern II:
'json:{"driver":"qcow2","file":{"driver":"gluster",
"volume":"testvol","path":"/path/a.qcow2",
"server":[{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/socket path)
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|unix (default 'tcp')
Examples:
1.
-drive driver=qcow2,file.driver=gluster,
file.volume=testvol,file.path=/path/a.qcow2,
file.server.0.host=1.2.3.4,
file.server.0.port=24007,
file.server.0.transport=tcp,
file.server.1.host=5.6.7.8,
file.server.1.port=24008,
file.server.1.transport=rdma,
file.server.2.host=/var/run/glusterd.socket
file.server.1.transport=unix
2.
'json:{"driver":"qcow2","file":{"driver":"gluster","volume":"testvol",
"path":"/path/a.qcow2","server":
[{"host":"1.2.3.4","port":"24007","transport":"tcp"},
{"host":"4.5.6.7","port":"24008","transport":"rdma"},
{"host":"/var/run/glusterd.socket","transport":"unix"}] } }'
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)
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>
---
block/gluster.c | 312 ++++++++++++++++++++++++++++++++++++++++++++-------
qapi/block-core.json | 4 +-
2 files changed, 272 insertions(+), 44 deletions(-)
diff --git a/block/gluster.c b/block/gluster.c
index c129b0b..68f027f 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -12,8 +12,16 @@
#include "block/block_int.h"
#include "qapi/error.h"
#include "qemu/uri.h"
+#include "qemu/error-report.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_SERVER_PATTERN "server."
+
#define GLUSTER_DEFAULT_PORT 24007
@@ -66,6 +74,46 @@ 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/socket path)",
+ },
+ {
+ .name = GLUSTER_OPT_PORT,
+ .type = QEMU_OPT_NUMBER,
+ .help = "port number on which glusterd is listening (default 24007)",
+ },
+ {
+ .name = GLUSTER_OPT_TRANSPORT,
+ .type = QEMU_OPT_STRING,
+ .help = "transport type 'tcp'|'rdma'|'unix' (default 'tcp')",
+ },
+ { /* end of list */ }
+ },
+};
static int parse_volume_options(BlockdevOptionsGluster *gconf, char *path)
{
@@ -133,6 +181,7 @@ static int qemu_gluster_parseuri(BlockdevOptionsGluster **pgconf,
const char *filename)
{
BlockdevOptionsGluster *gconf;
+ GlusterServer *gsconf;
URI *uri;
QueryParams *qp = NULL;
bool is_unix = false;
@@ -144,23 +193,24 @@ static int qemu_gluster_parseuri(BlockdevOptionsGluster **pgconf,
}
gconf = g_new0(BlockdevOptionsGluster, 1);
- gconf->server = g_new0(GlusterServer, 1);
+ gconf->server = g_new0(GlusterServerList, 1);
+ gconf->server->value = gsconf = g_new0(GlusterServer, 1);
/* transport */
if (!uri->scheme || !strcmp(uri->scheme, "gluster")) {
- gconf->server->transport = GLUSTER_TRANSPORT_TCP;
+ gsconf->transport = GLUSTER_TRANSPORT_TCP;
} else if (!strcmp(uri->scheme, "gluster+tcp")) {
- gconf->server->transport = GLUSTER_TRANSPORT_TCP;
+ gsconf->transport = GLUSTER_TRANSPORT_TCP;
} else if (!strcmp(uri->scheme, "gluster+unix")) {
- gconf->server->transport = GLUSTER_TRANSPORT_UNIX;
+ gsconf->transport = GLUSTER_TRANSPORT_UNIX;
is_unix = true;
} else if (!strcmp(uri->scheme, "gluster+rdma")) {
- gconf->server->transport = GLUSTER_TRANSPORT_RDMA;
+ gsconf->transport = GLUSTER_TRANSPORT_RDMA;
} else {
ret = -EINVAL;
goto out;
}
- gconf->server->has_transport = true;
+ gsconf->has_transport = true;
ret = parse_volume_options(gconf, uri->path);
if (ret < 0) {
@@ -182,15 +232,15 @@ static int qemu_gluster_parseuri(BlockdevOptionsGluster **pgconf,
ret = -EINVAL;
goto out;
}
- gconf->server->host = g_strdup(qp->p[0].value);
+ gsconf->host = g_strdup(qp->p[0].value);
} else {
- gconf->server->host = g_strdup(uri->server ? uri->server : "localhost");
+ gsconf->host = g_strdup(uri->server ? uri->server : "localhost");
if (uri->port) {
- gconf->server->port = uri->port;
+ gsconf->port = uri->port;
} else {
- gconf->server->port = GLUSTER_DEFAULT_PORT;
+ gsconf->port = GLUSTER_DEFAULT_PORT;
}
- gconf->server->has_port = true;
+ gsconf->has_port = true;
}
*pgconf = gconf;
@@ -206,32 +256,27 @@ out:
return ret;
}
-static struct glfs *qemu_gluster_init(BlockdevOptionsGluster **pgconf,
- const char *filename, Error **errp)
+static struct glfs *qemu_gluster_glfs_init(BlockdevOptionsGluster **pgconf,
+ Error **errp)
{
- struct glfs *glfs = NULL;
+ struct glfs *glfs;
int ret;
int old_errno;
- BlockdevOptionsGluster *gconf = NULL;
-
- 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;
- }
+ GlusterServerList *server = NULL;
+ BlockdevOptionsGluster *gconf = *pgconf;
glfs = glfs_new(gconf->volume);
if (!glfs) {
goto out;
}
- ret = glfs_set_volfile_server(glfs,
- GlusterTransport_lookup[gconf->server->transport],
- gconf->server->host, gconf->server->port);
- if (ret < 0) {
- goto out;
+ for (server = gconf->server; server; server = server->next) {
+ ret = glfs_set_volfile_server(glfs,
+ GlusterTransport_lookup[server->value->transport],
+ server->value->host, server->value->port);
+ if (ret < 0) {
+ goto out;
+ }
}
/*
@@ -245,11 +290,13 @@ static struct glfs *qemu_gluster_init(BlockdevOptionsGluster **pgconf,
ret = glfs_init(glfs);
if (ret) {
+ error_report("Gluster connection for 'volume: %s and path: %s': ",
+ gconf->volume, gconf->path);
+ for (server = gconf->server; server; server = server->next) {
+ error_report("failed for host %s", server->value->host);
+ }
error_setg_errno(errp, errno,
- "Gluster connection failed for host=%s port=%d "
- "volume=%s path=%s transport=%s", gconf->server->host,
- gconf->server->port, gconf->volume, gconf->path,
- GlusterTransport_lookup[gconf->server->transport]);
+ "Please refer to gluster logs for more info");
/* glfs_init sometimes doesn't set errno although docs suggest that */
if (errno == 0)
@@ -257,11 +304,11 @@ static struct glfs *qemu_gluster_init(BlockdevOptionsGluster **pgconf,
goto out;
}
- *pgconf = gconf;
return glfs;
out:
qapi_free_BlockdevOptionsGluster(gconf);
+ *pgconf = NULL;
if (glfs) {
old_errno = errno;
glfs_fini(glfs);
@@ -270,6 +317,181 @@ out:
return NULL;
}
+static int qapi_enum_parse(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 i;
+}
+
+/*
+ * Convert the json formatted command line into qapi.
+*/
+static int qemu_gluster_parsejson(BlockdevOptionsGluster **pgconf,
+ QDict *options)
+{
+ QemuOpts *opts;
+ BlockdevOptionsGluster *gconf = NULL;
+ GlusterServer *gsconf;
+ GlusterServerList *curr = NULL;
+ QDict *backing_options = NULL;
+ Error *local_err = NULL;
+ char *str = NULL;
+ const char *ptr;
+ size_t num_servers;
+ 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(BlockdevOptionsGluster, 1);
+
+ num_servers = qdict_array_entries(options, GLUSTER_OPT_SERVER_PATTERN);
+ if (num_servers < 1) {
+ error_setg(&local_err, "qemu_gluster: please provide 'server' "
+ "option with valid fields in array of tuples");
+ goto out;
+ }
+
+ ptr = qemu_opt_get(opts, GLUSTER_OPT_VOLUME);
+ if (!ptr) {
+ error_setg(&local_err, "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, "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 */
+ for (i = 0; i < num_servers; i++) {
+ opts = qemu_opts_create(&runtime_tuple_opts, NULL, 0, &error_abort);
+ str = g_strdup_printf(GLUSTER_OPT_SERVER_PATTERN"%d.", i);
+ 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);
+ g_free(str);
+ str = NULL;
+
+ ptr = qemu_opt_get(opts, GLUSTER_OPT_HOST);
+ if (!ptr) {
+ error_setg(&local_err, "qemu_gluster: server.{tuple.%d} "
+ "requires 'host' option", i);
+ goto out;
+ }
+
+ gsconf = g_new0(GlusterServer, 1);
+
+ gsconf->host = g_strdup(ptr);
+
+ ptr = qemu_opt_get(opts, GLUSTER_OPT_TRANSPORT);
+ /* check whether transport type specified in json command is valid */
+ gsconf->transport = qapi_enum_parse(ptr);
+ if (gsconf->transport == GLUSTER_TRANSPORT__MAX) {
+ error_setg(&local_err, "qemu_gluster: please set 'transport'"
+ " type in tuple.%d as tcp|rdma|unix", i);
+ qapi_free_GlusterServer(gsconf);
+ goto out;
+ }
+ gsconf->has_transport = true;
+
+ /* we don't need port number for unix transport */
+ if (gsconf->transport != GLUSTER_TRANSPORT_UNIX) {
+ gsconf->port = qemu_opt_get_number(opts, GLUSTER_OPT_PORT,
+ GLUSTER_DEFAULT_PORT);
+ gsconf->has_port = true;
+ }
+
+ if (gconf->server == NULL) {
+ gconf->server = g_new0(GlusterServerList, 1);
+ gconf->server->value = gsconf;
+ curr = gconf->server;
+ } else {
+ curr->next = g_new0(GlusterServerList, 1);
+ curr->next->value = gsconf;
+ curr = curr->next;
+ }
+
+ qemu_opts_del(opts);
+ }
+
+ *pgconf = gconf;
+ return 0;
+
+out:
+ error_report_err(local_err);
+ qemu_opts_del(opts);
+ qapi_free_BlockdevOptionsGluster(gconf);
+ if (str) {
+ qdict_del(backing_options, str);
+ g_free(str);
+ }
+ errno = EINVAL;
+ return -errno;
+}
+
+static struct glfs *qemu_gluster_init(BlockdevOptionsGluster **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, "Usage: "
+ "-drive driver=qcow2,file.driver=gluster,"
+ "file.volume=testvol,file.path=/path/a.qcow2,"
+ "file.server.0.host=1.2.3.4,"
+ "[file.server.0.port=24007,]"
+ "[file.server.0.transport=tcp,]"
+ "file.server.1.host=5.6.7.8,"
+ "[file.server.1.port=24008,]"
+ "[file.server.1.transport=rdma,]"
+ "file.server.2.host=/var/run/glusterd.socket,"
+ "file.server.2.transport=unix ...");
+ errno = -ret;
+ return NULL;
+ }
+ }
+
+ return qemu_gluster_glfs_init(gconf, errp);
+}
+
static void qemu_gluster_complete_aio(void *opaque)
{
GlusterAIOCB *acb = (GlusterAIOCB *)opaque;
@@ -335,7 +557,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;
@@ -367,7 +589,9 @@ static int qemu_gluster_open(BlockDriverState *bs, QDict *options,
out:
qemu_opts_del(opts);
- qapi_free_BlockdevOptionsGluster(gconf);
+ if (gconf) {
+ qapi_free_BlockdevOptionsGluster(gconf);
+ }
if (!ret) {
return ret;
}
@@ -396,7 +620,7 @@ static int qemu_gluster_reopen_prepare(BDRVReopenState *state,
qemu_gluster_parse_flags(state->flags, &open_flags);
- 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;
@@ -421,7 +645,9 @@ static int qemu_gluster_reopen_prepare(BDRVReopenState *state,
exit:
/* state->opaque will be freed in either the _abort or _commit */
- qapi_free_BlockdevOptionsGluster(gconf);
+ if (gconf) {
+ qapi_free_BlockdevOptionsGluster(gconf);
+ }
return ret;
}
@@ -531,7 +757,7 @@ static int qemu_gluster_create(const char *filename,
int64_t total_size = 0;
char *tmp = NULL;
- glfs = qemu_gluster_init(&gconf, filename, errp);
+ glfs = qemu_gluster_init(&gconf, filename, NULL, errp);
if (!glfs) {
ret = -errno;
goto out;
@@ -571,7 +797,9 @@ static int qemu_gluster_create(const char *filename,
}
out:
g_free(tmp);
- qapi_free_BlockdevOptionsGluster(gconf);
+ if (gconf) {
+ qapi_free_BlockdevOptionsGluster(gconf);
+ }
if (glfs) {
glfs_fini(glfs);
}
@@ -757,7 +985,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,
@@ -784,7 +1012,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,
@@ -838,7 +1066,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 93fbca3..2b81852 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2076,14 +2076,14 @@
#
# @path: absolute path to image file in gluster volume
#
-# @servers: gluster server description
+# @servers: one or more gluster server descriptions
#
# Since: 2.7
##
{ 'struct': 'BlockdevOptionsGluster',
'data': { 'volume': 'str',
'path': 'str',
- 'server': 'GlusterServer' } }
+ 'server': [ 'GlusterServer' ] } }
##
# @BlockdevOptions
--
2.5.5
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [Qemu-devel] [PATCH v17 0/4][WIP] block/gluster: add support for multiple gluster servers
2016-06-15 8:25 [Qemu-devel] [PATCH v17 0/4][WIP] block/gluster: add support for multiple gluster servers Prasanna Kumar Kalever
` (3 preceding siblings ...)
2016-06-15 8:25 ` [Qemu-devel] [PATCH v17 4/4] block/gluster: add support for multiple gluster servers Prasanna Kumar Kalever
@ 2016-06-29 14:11 ` Jeff Cody
2016-06-29 14:17 ` Daniel P. Berrange
4 siblings, 1 reply; 9+ messages in thread
From: Jeff Cody @ 2016-06-29 14:11 UTC (permalink / raw)
To: Prasanna Kumar Kalever
Cc: qemu-devel, kwolf, eblake, pkrempa, bharata, berrange, rtalur,
deepakcs, armbru
On Wed, Jun 15, 2016 at 01:55:43PM +0530, Prasanna Kumar Kalever wrote:
> This version of patches are rebased on master branch.
>
> Prasanna Kumar Kalever (4):
> block/gluster: rename [server, volname, image] -> [host, volume, path]
> block/gluster: code cleanup
> block/gluster: using new qapi schema
> block/gluster: add support for multiple gluster servers
>
I think the main criticism with this series revolves around the interface,
and the overloading of the server hosts fields when using tcp and unix
sockets, etc. The idea of using flat unions for the API was floated.
Eric, does this criticism still stand, from libvirt's perspective? Or are
you comfortable enough with the current interface that I can go ahead and
take this series in through my tree?
> 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\
> ?server=host2&server=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\
> [?server=host1[:port]\
> &server=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","server":
> [{"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
>
> v11:
> using qapi-types* defined structures as per "Eric Blake" <eblake@redhat.com>
> review comments.
>
> v12:
> fix crash caused in qapi_free_BlockdevOptionsGluster
>
> v13:
> address comments from "Jeff Cody" <jcody@redhat.com>
>
> v14:
> address comments from "Eric Blake" <eblake@redhat.com>
> split patch 3/3 into two
> rename input option and variable from 'servers' to 'server'
>
> v15:
> patch 1/4 changed the commit message as per Eric's comment
> patch 2/4 are unchanged
> patch 3/4 addressed Jeff's comments
> patch 4/4 concentrates on unix transport related help info,
> rename 'parse_transport_option()' to 'qapi_enum_parse()',
> address memory leaks and other comments given by Jeff and Eric
>
> v16:
> In patch 4/4 fixed segfault on glfs_init() error case, as per Jeff's comments
> other patches in this series remain unchanged
>
> v17:
> rebase of v16 on latest master
>
> block/gluster.c | 484 ++++++++++++++++++++++++++++++++++++++-------------
> qapi/block-core.json | 64 ++++++-
> 2 files changed, 419 insertions(+), 129 deletions(-)
>
> --
> 2.5.5
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [Qemu-devel] [PATCH v17 0/4][WIP] block/gluster: add support for multiple gluster servers
2016-06-29 14:11 ` [Qemu-devel] [PATCH v17 0/4][WIP] " Jeff Cody
@ 2016-06-29 14:17 ` Daniel P. Berrange
0 siblings, 0 replies; 9+ messages in thread
From: Daniel P. Berrange @ 2016-06-29 14:17 UTC (permalink / raw)
To: Jeff Cody
Cc: Prasanna Kumar Kalever, qemu-devel, kwolf, eblake, pkrempa,
bharata, rtalur, deepakcs, armbru
On Wed, Jun 29, 2016 at 10:11:31AM -0400, Jeff Cody wrote:
> On Wed, Jun 15, 2016 at 01:55:43PM +0530, Prasanna Kumar Kalever wrote:
> > This version of patches are rebased on master branch.
> >
> > Prasanna Kumar Kalever (4):
> > block/gluster: rename [server, volname, image] -> [host, volume, path]
> > block/gluster: code cleanup
> > block/gluster: using new qapi schema
> > block/gluster: add support for multiple gluster servers
> >
>
> I think the main criticism with this series revolves around the interface,
> and the overloading of the server hosts fields when using tcp and unix
> sockets, etc. The idea of using flat unions for the API was floated.
>
> Eric, does this criticism still stand, from libvirt's perspective? Or are
> you comfortable enough with the current interface that I can go ahead and
> take this series in through my tree?
Just from a general QAPI design POV I think this overloading is undesirable.
We cared enough about not doing this overloading in the past that we
created SocketAddress which is a union of InetSocketAddress and
UnixSocketAddress. Given this historical best practice, I don't think
we should be overloading "host" for unix socket path.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 9+ messages in thread