[Qemu-devel] [RFC PATCH 0/2] GlusterFS support in QEMU - v2

[Qemu-devel] [RFC PATCH 0/2] GlusterFS support in QEMU - v2

[Bharata B Rao]

Hi,

Here is the v2 patchset for supporting GlusterFS protocol from QEMU.

This set of patches enables QEMU to boot VM images from gluster volumes.
This is achieved by adding gluster as a new block backend driver in QEMU.
Its already possible to boot from VM images on gluster volumes, but this
patchset provides the ability to boot VM images from gluster volumes by
by-passing the FUSE layer in gluster. In case the image is present on the
local system, it is possible to even bypass client and server translator and
hence the RPC overhead.

The major change in this version is to not implement libglusterfs based
gluster backend within QEMU but instead use libgfapi. libgfapi library
from GlusterFS project provides APIs to access gluster volumes directly.
With the use of libgfapi, the specification of gluster backend from QEMU
matches more closely with the GlusterFS's way of specifying volumes. We now
specify the gluster backed image like this:

-drive file=gluster:server@port:volname:image

- Here 'gluster' is the protocol.
- 'server@port' specifies the server where the volume file specification for
  the given volume resides. 'port' is the port number on which gluster
  management daemon (glusterd) is listening. This is optional and if not
  specified, QEMU will send 0 which will make libgfapi to use the default
  port.
- 'volname' is the name of the gluster volume which contains the VM image.
- 'image' is the path to the actual VM image in the gluster volume.

Note that we are no longer using volfiles directly and use volume names
instead. For this to work, gluster management daemon (glusterd) needs to
be running on the QEMU node. This limits the QEMU user to access the volumes by
the default volfiles that are generated by gluster CLI. This should be
fine as long as gluster CLI provides the capability to generate or regenerate
volume files for a given volume with the xlator set that QEMU user is
interested in. GlusterFS developers tell me that this can be provided with
some enhancements to Gluster CLI/glusterd. Note that the custom volume files
is typically needed when GlusterFS server is co-located with QEMU in
which case it would  be beneficial to get rid of client-server overhead and
RPC communication overhead.

Using the patches
=================
- GlusterFS backend is enabled by specifying --enable-glusterfs with the
  configure script.
- You need to have installed GlusterFS from latest gluster git which provides
  the required libgfapi library. (git://git.gluster.com/glusterfs.git)
- As mentioned above, the VM image on gluster volume can be specified like
  this:
	-drive file=gluster:localhost:testvol:/F17,format=gluster

  Note that format=gluster is not needed ideally and its a work around I have
  until libgfapi provides a working connection cleanup routine (glfs_fini()).
  When the format isn't specified, QEMU figures out the format by doing
  find_image_format that results in one open and close before opening the
  image file long term for standard read and write. Gluster connection
  initialization is done from open and connection termination is done from
  close. But since glfs_fini() isn't working yet, I am bypassing
  find_image_format by specifying format=gluster directly which results in
  just one open and hence I am not limited by glfs_fini().

Changes for v2
==============
- Removed the libglusterfs based gluster backend implementation within
  QEMU and instead using the APIs from libgfapi.
- Change in the specification from file=gluster:volfile:image to
  file=gluster:server@port:volname:image. format=gluster is no longer
  specified.
- Passing iovectors obtained from generic block layer of QEMU directly to
  libgfapi instead of converting them to linear buffers.
- Processing the aio call back directly from the read thread of event handler
  pipe instead of scheduling a BH and delegating the processing to that BH.
- Added async flush (fsync) support.
- Refusal to handle partial read/writes in gluster block driver.
- Other minor cleanups based on review comments from v1 post. There is one
  comment that is yet to be addressed (using a local argument vs using a
  field of bs - Stefan), I will address them in the next iteration.

v1
==
lists.nongnu.org/archive/html/qemu-devel/2012-06/msg01745.html

fio benchmark numbers
=====================
Environment
-----------
Dual core x86_64 laptop
QEMU (c0958559b1a58)
GlusterFS (35810fb2a7a12)
Guest: Fedora 16 (kernel 3.1.0-7.fc16.x86_64)
Host: Fedora 16 (kernel 3.4)
fio-HEAD-47ea504

fio jobfile
-----------
# cat aio-read-direct-seq 
; Read 4 files with aio at different depths
[global]
ioengine=libaio
direct=1
rw=read
bs=128k
size=512m
directory=/data1

[file1]
iodepth=4

[file2]
iodepth=32

[file3]
iodepth=8

[file4]
iodepth=16

Scenarios
---------
Base: QEMU boots from directly from image on gluster brick.
Fuse mount: QEMU boots from VM image on gluster FUSE mount.
Fuse bypass: QEMU uses gluster protocol and uses standard client volfile.
Fuse bypass custom: QEMU uses gluster protocol and uses a minimal client
	volfile that just has client xlator.
RPC bypass: QEMU uses just posix xlator and doesn't depend on gluster server.

Numbers (aggrb, minb and maxb in kB/s. mint and maxt in msec)
-------
			aggrb	minb	maxb	mint	maxt
Base			63076	15769	17488	29979	33248
Fuse mount		29392	7348	9266	56581	71350
Fuse bypass		53609	13402	14909	35164	39119
Fuse bypass custom	62968	15742	17962	29188	33305
RPC bypass		63505	15876	18534	28287	33023

All the scenarios used if=virtio and cache=none options.

[Qemu-devel] [RFC PATCH 1/2] qemu: Add a config option for GlusterFS as block backend

[Bharata B Rao]

qemu: Add a config option for GlusterFS as block backend

From: Bharata B Rao <bharata@linux.vnet.ibm.com>

Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
---

 configure |   34 ++++++++++++++++++++++++++++++++++
 1 files changed, 34 insertions(+), 0 deletions(-)


diff --git a/configure b/configure
index 500fe24..03547b9 100755
--- a/configure
+++ b/configure
@@ -824,6 +824,10 @@ for opt do
   ;;
   --disable-guest-agent) guest_agent="no"
   ;;
+  --disable-glusterfs) glusterfs="no"
+  ;;
+  --enable-glusterfs) glusterfs="yes"
+  ;;
   *) echo "ERROR: unknown option $opt"; show_help="yes"
   ;;
   esac
@@ -1110,6 +1114,8 @@ echo "  --disable-guest-agent    disable building of the QEMU Guest Agent"
 echo "  --enable-guest-agent     enable building of the QEMU Guest Agent"
 echo "  --with-coroutine=BACKEND coroutine backend. Supported options:"
 echo "                           gthread, ucontext, sigaltstack, windows"
+echo "  --enable-glusterfs       enable GlusterFS backend"
+echo "  --disable-glusterfs      disable GlusterFS backend"
 echo ""
 echo "NOTE: The object files are built at the place where configure is launched"
 exit 1
@@ -2259,6 +2265,29 @@ EOF
   fi
 fi
 
+##########################################
+# glusterfs probe
+if test "$glusterfs" != "no" ; then
+  cat > $TMPC <<EOF
+#include <glusterfs/api/glfs.h>
+int main(void) {
+    (void) glfs_new("volume");
+    return 0;
+}
+EOF
+  glusterfs_libs="-lgfapi -lgfrpc -lgfxdr"
+  if compile_prog "" "$glusterfs_libs" ; then
+    glusterfs=yes
+    libs_tools="$glusterfs_libs $libs_tools"
+    libs_softmmu="$glusterfs_libs $libs_softmmu"
+  else
+    if test "$glusterfs" = "yes" ; then
+      feature_not_found "GlusterFS backend support"
+    fi
+    glusterfs=no
+  fi
+fi
+
 #
 # Check for xxxat() functions when we are building linux-user
 # emulator.  This is done because older glibc versions don't
@@ -3055,6 +3084,7 @@ echo "OpenGL support    $opengl"
 echo "libiscsi support  $libiscsi"
 echo "build guest agent $guest_agent"
 echo "coroutine backend $coroutine_backend"
+echo "GlusterFS support $glusterfs"
 
 if test "$sdl_too_old" = "yes"; then
 echo "-> Your SDL version is too old - please upgrade to have SDL support"
@@ -3384,6 +3414,10 @@ if test "$has_environ" = "yes" ; then
   echo "CONFIG_HAS_ENVIRON=y" >> $config_host_mak
 fi
 
+if test "$glusterfs" = "yes" ; then
+  echo "CONFIG_GLUSTERFS=y" >> $config_host_mak
+fi
+
 # USB host support
 case "$usb" in
 linux)

[Qemu-devel] [RFC PATCH 2/2] block: gluster as block backend

[Bharata B Rao]

block: gluster as block backend

From: Bharata B Rao <bharata@linux.vnet.ibm.com>

This patch adds gluster as the new block backend in QEMU. This gives QEMU
the ability to boot VM images from gluster volumes.

Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
---

 block/Makefile.objs |    1 
 block/gluster.c     |  483 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 484 insertions(+), 0 deletions(-)
 create mode 100644 block/gluster.c


diff --git a/block/Makefile.objs b/block/Makefile.objs
index b5754d3..a1ae67f 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -9,3 +9,4 @@ block-obj-$(CONFIG_POSIX) += raw-posix.o
 block-obj-$(CONFIG_LIBISCSI) += iscsi.o
 block-obj-$(CONFIG_CURL) += curl.o
 block-obj-$(CONFIG_RBD) += rbd.o
+block-obj-$(CONFIG_GLUSTERFS) += gluster.o
diff --git a/block/gluster.c b/block/gluster.c
new file mode 100644
index 0000000..c33a006
--- /dev/null
+++ b/block/gluster.c
@@ -0,0 +1,483 @@
+/*
+ * GlusterFS backend for QEMU
+ *
+ * (AIO implementation is derived from block/rbd.c)
+ *
+ * Copyright (C) 2012 Bharata B Rao <bharata@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * (at your option) any later version. See the COPYING file in the top-level
+ * directory.
+ */
+#include "block_int.h"
+#include <glusterfs/api/glfs.h>
+
+typedef struct GlusterConf {
+    char server[HOST_NAME_MAX];
+    int port;
+    char volname[128]; /* TODO: use GLUSTERD_MAX_VOLUME_NAME */
+    char image[PATH_MAX];
+} GlusterConf;
+
+typedef struct GlusterAIOCB {
+    BlockDriverAIOCB common;
+    QEMUIOVector *qiov;
+    char *bounce;
+    struct BDRVGlusterState *s;
+    int cancelled;
+} GlusterAIOCB;
+
+typedef struct GlusterCBKData {
+    GlusterAIOCB *acb;
+    struct BDRVGlusterState *s;
+    int64_t size;
+    int ret;
+} GlusterCBKData;
+
+typedef struct BDRVGlusterState {
+    struct glfs *glfs;
+    int fds[2];
+    int open_flags;
+    struct glfs_fd *fd;
+    int qemu_aio_count;
+    int event_reader_pos;
+    GlusterCBKData *event_gcbk;
+} BDRVGlusterState;
+
+#define GLUSTER_FD_READ 0
+#define GLUSTER_FD_WRITE 1
+
+static void qemu_gluster_complete_aio(GlusterCBKData *gcbk)
+{
+    GlusterAIOCB *acb = gcbk->acb;
+    int ret;
+
+    if (acb->cancelled) {
+        qemu_aio_release(acb);
+        goto done;
+    }
+
+    if (gcbk->ret == gcbk->size) {
+        ret = 0; /* Success */
+    } else if (gcbk->ret < 0) {
+        ret = gcbk->ret; /* Read/Write failed */
+    } else {
+        ret = -EINVAL; /* Partial read/write - fail it */
+    }
+    acb->common.cb(acb->common.opaque, ret);
+    qemu_aio_release(acb);
+
+done:
+    g_free(gcbk);
+}
+
+static void qemu_gluster_aio_event_reader(void *opaque)
+{
+    BDRVGlusterState *s = opaque;
+    ssize_t ret;
+
+    do {
+        char *p = (char *)&s->event_gcbk;
+
+        ret = read(s->fds[GLUSTER_FD_READ], p + s->event_reader_pos,
+                   sizeof(s->event_gcbk) - s->event_reader_pos);
+        if (ret > 0) {
+            s->event_reader_pos += ret;
+            if (s->event_reader_pos == sizeof(s->event_gcbk)) {
+                s->event_reader_pos = 0;
+                qemu_gluster_complete_aio(s->event_gcbk);
+                s->qemu_aio_count--;
+            }
+        }
+    } while (ret < 0 && errno == EINTR);
+}
+
+static int qemu_gluster_aio_flush_cb(void *opaque)
+{
+    BDRVGlusterState *s = opaque;
+
+    return (s->qemu_aio_count > 0);
+}
+
+/*
+ * file=protocol:server@port:volname:image
+ */
+static int qemu_gluster_parsename(GlusterConf *c, const char *filename)
+{
+    char *file = g_strdup(filename);
+    char *token, *next_token, *saveptr;
+    char *token_s, *next_token_s, *saveptr_s;
+    int ret = -EINVAL;
+
+    /* Discard the protocol */
+    token = strtok_r(file, ":", &saveptr);
+    if (!token) {
+        goto out;
+    }
+
+    /* server@port */
+    next_token = strtok_r(NULL, ":", &saveptr);
+    if (!next_token) {
+        goto out;
+    }
+    if (strchr(next_token, '@')) {
+        token_s = strtok_r(next_token, "@", &saveptr_s);
+        if (!token_s) {
+            goto out;
+        }
+        strncpy(c->server, token_s, HOST_NAME_MAX);
+        next_token_s = strtok_r(NULL, "@", &saveptr_s);
+        if (!next_token_s) {
+            goto out;
+        }
+        c->port = atoi(next_token_s);
+    } else {
+        strncpy(c->server, next_token, HOST_NAME_MAX);
+        c->port = 0;
+    }
+
+    /* volname */
+    next_token = strtok_r(NULL, ":", &saveptr);
+    if (!next_token) {
+        goto out;
+    }
+    strncpy(c->volname, next_token, 128);
+
+    /* image */
+    next_token = strtok_r(NULL, ":", &saveptr);
+    if (!next_token) {
+        goto out;
+    }
+    strncpy(c->image, next_token, PATH_MAX);
+    ret = 0;
+out:
+    g_free(file);
+    return ret;
+}
+
+static struct glfs *qemu_gluster_init(GlusterConf *c, const char *filename)
+{
+    struct glfs *glfs = NULL;
+    int ret;
+
+    ret = qemu_gluster_parsename(c, filename);
+    if (ret < 0) {
+        errno = -ret;
+        goto out;
+    }
+
+    glfs = glfs_new(c->volname);
+    if (!glfs) {
+        goto out;
+    }
+
+    ret = glfs_set_volfile_server(glfs, "socket", c->server, c->port);
+    if (ret < 0) {
+        goto out;
+    }
+
+    /*
+     * TODO: Logging is not necessary but instead nice to have.
+     * Can QEMU optionally log into a standard place ?
+     * Need to use defines like gf_loglevel_t:GF_LOG_INFO instead of
+     * hard coded values like 7 here.
+     */
+    ret = glfs_set_logging(glfs, "/tmp/qemu-gluster.log", 7);
+    if (ret < 0) {
+        goto out;
+    }
+
+    ret = glfs_init(glfs);
+    if (ret < 0) {
+        goto out;
+    }
+    return glfs;
+
+out:
+    if (glfs) {
+        (void)glfs_fini(glfs);
+    }
+    return NULL;
+}
+
+static int qemu_gluster_open(BlockDriverState *bs, const char *filename,
+    int bdrv_flags)
+{
+    BDRVGlusterState *s = bs->opaque;
+    GlusterConf *c = g_malloc(sizeof(GlusterConf));
+    int ret;
+
+    s->glfs = qemu_gluster_init(c, filename);
+    if (!s->glfs) {
+        ret = -errno;
+        goto out;
+    }
+
+    s->open_flags |=  O_BINARY;
+    s->open_flags &= ~O_ACCMODE;
+    if (bdrv_flags & BDRV_O_RDWR) {
+        s->open_flags |= O_RDWR;
+    } else {
+        s->open_flags |= O_RDONLY;
+    }
+
+    if ((bdrv_flags & BDRV_O_NOCACHE)) {
+        s->open_flags |= O_DIRECT;
+    }
+
+    s->fd = glfs_open(s->glfs, c->image, s->open_flags);
+    if (!s->fd) {
+        ret = -errno;
+        goto out;
+    }
+
+    ret = qemu_pipe(s->fds);
+    if (ret < 0) {
+        goto out;
+    }
+    fcntl(s->fds[0], F_SETFL, O_NONBLOCK);
+    fcntl(s->fds[1], F_SETFL, O_NONBLOCK);
+    qemu_aio_set_fd_handler(s->fds[GLUSTER_FD_READ],
+        qemu_gluster_aio_event_reader, NULL, qemu_gluster_aio_flush_cb, s);
+    g_free(c);
+    return ret;
+
+out:
+    g_free(c);
+    if (s->fd) {
+        glfs_close(s->fd);
+    }
+    if (s->glfs) {
+        (void) glfs_fini(s->glfs);
+    }
+    return ret;
+}
+
+static int qemu_gluster_create(const char *filename,
+        QEMUOptionParameter *options)
+{
+    struct glfs *glfs;
+    struct glfs_fd *fd;
+    GlusterConf *c = g_malloc(sizeof(GlusterConf));
+    int ret = 0;
+    int64_t total_size = 0;
+
+    glfs = qemu_gluster_init(c, filename);
+    if (!glfs) {
+        ret = -errno;
+        goto out;
+    }
+
+    /* Read out options */
+    while (options && options->name) {
+        if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
+            total_size = options->value.n / BDRV_SECTOR_SIZE;
+        }
+        options++;
+    }
+
+    fd = glfs_creat(glfs, c->image, O_WRONLY|O_CREAT|O_TRUNC|O_BINARY, S_IRWXU);
+    if (!fd) {
+        ret = -errno;
+    } else {
+        if (glfs_ftruncate(fd, total_size * BDRV_SECTOR_SIZE) != 0) {
+            ret = -errno;
+        }
+        if (glfs_close(fd) != 0) {
+            ret = -errno;
+        }
+    }
+out:
+    g_free(c);
+    if (glfs) {
+        (void) glfs_fini(glfs);
+    }
+    return ret;
+}
+
+static AIOPool gluster_aio_pool = {
+    .aiocb_size = sizeof(GlusterAIOCB),
+};
+
+static int qemu_gluster_send_pipe(BDRVGlusterState *s, GlusterCBKData *gcbk)
+{
+    int ret = 0;
+    while (1) {
+        fd_set wfd;
+        int fd = s->fds[GLUSTER_FD_WRITE];
+
+        ret = write(fd, (void *)&gcbk, sizeof(gcbk));
+        if (ret >= 0) {
+            break;
+        }
+        if (errno == EINTR) {
+            continue;
+        }
+        if (errno != EAGAIN) {
+            break;
+        }
+
+        FD_ZERO(&wfd);
+        FD_SET(fd, &wfd);
+        do {
+            ret = select(fd + 1, NULL, &wfd, NULL, NULL);
+        } while (ret < 0 && errno == EINTR);
+    }
+    return ret;
+}
+
+static void gluster_finish_aiocb(struct glfs_fd *fd, ssize_t ret, void *arg)
+{
+    GlusterCBKData *gcbk = (GlusterCBKData *)arg;
+    BDRVGlusterState *s = gcbk->s;
+
+    gcbk->ret = ret;
+    if (qemu_gluster_send_pipe(s, gcbk) < 0) {
+        error_report("Could not complete read/write/flush from gluster");
+        abort();
+    }
+}
+
+static BlockDriverAIOCB *qemu_gluster_aio_rw(BlockDriverState *bs,
+        int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
+        BlockDriverCompletionFunc *cb, void *opaque, int write)
+{
+    int ret;
+    GlusterAIOCB *acb;
+    GlusterCBKData *gcbk;
+    BDRVGlusterState *s = bs->opaque;
+    size_t size;
+    off_t offset;
+
+    acb = qemu_aio_get(&gluster_aio_pool, bs, cb, opaque);
+    acb->qiov = qiov;
+    acb->s = s;
+
+    offset = sector_num * BDRV_SECTOR_SIZE;
+    size = nb_sectors * BDRV_SECTOR_SIZE;
+    s->qemu_aio_count++;
+
+    gcbk = g_malloc(sizeof(GlusterCBKData));
+    gcbk->acb = acb;
+    gcbk->s = s;
+    gcbk->size = size;
+
+    if (write) {
+        ret = glfs_pwritev_async(s->fd, qiov->iov, qiov->niov, offset, 0,
+            &gluster_finish_aiocb, gcbk);
+    } else {
+        ret = glfs_preadv_async(s->fd, qiov->iov, qiov->niov, offset, 0,
+            &gluster_finish_aiocb, gcbk);
+    }
+
+    if (ret < 0) {
+        goto out;
+    }
+    return &acb->common;
+
+out:
+    g_free(gcbk);
+    s->qemu_aio_count--;
+    qemu_aio_release(acb);
+    return NULL;
+}
+
+static BlockDriverAIOCB *qemu_gluster_aio_readv(BlockDriverState *bs,
+        int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
+        BlockDriverCompletionFunc *cb, void *opaque)
+{
+    return qemu_gluster_aio_rw(bs, sector_num, qiov, nb_sectors, cb, opaque, 0);
+}
+
+static BlockDriverAIOCB *qemu_gluster_aio_writev(BlockDriverState *bs,
+        int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
+        BlockDriverCompletionFunc *cb, void *opaque)
+{
+    return qemu_gluster_aio_rw(bs, sector_num, qiov, nb_sectors, cb, opaque, 1);
+}
+
+static BlockDriverAIOCB *qemu_gluster_aio_flush(BlockDriverState *bs,
+        BlockDriverCompletionFunc *cb, void *opaque)
+{
+    int ret;
+    GlusterAIOCB *acb;
+    GlusterCBKData *gcbk;
+    BDRVGlusterState *s = bs->opaque;
+
+    acb = qemu_aio_get(&gluster_aio_pool, bs, cb, opaque);
+    acb->s = s;
+    s->qemu_aio_count++;
+
+    gcbk = g_malloc(sizeof(GlusterCBKData));
+    gcbk->acb = acb;
+    gcbk->s = s;
+    gcbk->size = 0;
+
+    ret = glfs_fsync_async(s->fd, &gluster_finish_aiocb, gcbk);
+    if (ret < 0) {
+        goto out;
+    }
+    return &acb->common;
+
+out:
+    g_free(gcbk);
+    s->qemu_aio_count--;
+    qemu_aio_release(acb);
+    return NULL;
+}
+
+static int64_t qemu_gluster_getlength(BlockDriverState *bs)
+{
+    BDRVGlusterState *s = bs->opaque;
+    struct stat st;
+    int ret;
+
+    ret = glfs_fstat(s->fd, &st);
+    if (ret < 0) {
+        return -errno;
+    } else {
+        return st.st_size;
+    }
+}
+
+static void qemu_gluster_close(BlockDriverState *bs)
+{
+    BDRVGlusterState *s = bs->opaque;
+
+    if (s->fd) {
+        glfs_close(s->fd);
+        s->fd = NULL;
+    }
+}
+
+static QEMUOptionParameter qemu_gluster_create_options[] = {
+    {
+        .name = BLOCK_OPT_SIZE,
+        .type = OPT_SIZE,
+        .help = "Virtual disk size"
+    },
+    { NULL }
+};
+
+static BlockDriver bdrv_gluster = {
+    .format_name = "gluster",
+    .protocol_name = "gluster",
+    .instance_size = sizeof(BDRVGlusterState),
+    .bdrv_file_open = qemu_gluster_open,
+    .bdrv_close = qemu_gluster_close,
+    .bdrv_create = qemu_gluster_create,
+    .bdrv_getlength = qemu_gluster_getlength,
+
+    .bdrv_aio_readv = qemu_gluster_aio_readv,
+    .bdrv_aio_writev = qemu_gluster_aio_writev,
+    .bdrv_aio_flush = qemu_gluster_aio_flush,
+
+    .create_options = qemu_gluster_create_options,
+};
+
+static void bdrv_gluster_init(void)
+{
+    bdrv_register(&bdrv_gluster);
+}
+
+block_init(bdrv_gluster_init);

Re: [Qemu-devel] [RFC PATCH 0/2] GlusterFS support in QEMU - v2

[Vijay Bellur]

On 07/21/2012 01:59 PM, Bharata B Rao wrote:
> We now
> specify the gluster backed image like this:
>
> -drive file=gluster:server@port:volname:image
>
> - Here 'gluster' is the protocol.
> - 'server@port' specifies the server where the volume file specification for
>    the given volume resides. 'port' is the port number on which gluster
>    management daemon (glusterd) is listening. This is optional and if not
>    specified, QEMU will send 0 which will make libgfapi to use the default
>    port.
> - 'volname' is the name of the gluster volume which contains the VM image.
> - 'image' is the path to the actual VM image in the gluster volume.
>
> Note that we are no longer using volfiles directly and use volume names
> instead. For this to work, gluster management daemon (glusterd) needs to
> be running on the QEMU node.


glusterd needs to be running on the server that is specified in 
'server@port' option.



> Scenarios
> ---------
> Base: QEMU boots from directly from image on gluster brick.
> Fuse mount: QEMU boots from VM image on gluster FUSE mount.
> Fuse bypass: QEMU uses gluster protocol and uses standard client volfile.
> Fuse bypass custom: QEMU uses gluster protocol and uses a minimal client
> 	volfile that just has client xlator.
> RPC bypass: QEMU uses just posix xlator and doesn't depend on gluster server.
>

For the Fuse mount, Fuse bypass and Fuse bypass custom scenarios, I 
assume a single local gluster brick is being used. Is this correct?

Thanks,
Vijay

Re: [Qemu-devel] [RFC PATCH 0/2] GlusterFS support in QEMU - v2

[Bharata B Rao]

On Sat, Jul 21, 2012 at 05:52:59PM +0530, Vijay Bellur wrote:
> On 07/21/2012 01:59 PM, Bharata B Rao wrote:
> >We now
> >specify the gluster backed image like this:
> >
> >-drive file=gluster:server@port:volname:image
> >
> >- Here 'gluster' is the protocol.
> >- 'server@port' specifies the server where the volume file specification for
> >   the given volume resides. 'port' is the port number on which gluster
> >   management daemon (glusterd) is listening. This is optional and if not
> >   specified, QEMU will send 0 which will make libgfapi to use the default
> >   port.
> >- 'volname' is the name of the gluster volume which contains the VM image.
> >- 'image' is the path to the actual VM image in the gluster volume.
> >
> >Note that we are no longer using volfiles directly and use volume names
> >instead. For this to work, gluster management daemon (glusterd) needs to
> >be running on the QEMU node.
> 
> 
> glusterd needs to be running on the server that is specified in
> 'server@port' option.

oh yes, thanks.

> 
> 
> 
> >Scenarios
> >---------
> >Base: QEMU boots from directly from image on gluster brick.
> >Fuse mount: QEMU boots from VM image on gluster FUSE mount.
> >Fuse bypass: QEMU uses gluster protocol and uses standard client volfile.
> >Fuse bypass custom: QEMU uses gluster protocol and uses a minimal client
> >	volfile that just has client xlator.
> >RPC bypass: QEMU uses just posix xlator and doesn't depend on gluster server.
> >
> 
> For the Fuse mount, Fuse bypass and Fuse bypass custom scenarios, I
> assume a single local gluster brick is being used. Is this correct?

Right, fio numbers are for single local gluster brick scenario.

Regards,
Bharata.

Re: [Qemu-devel] [RFC PATCH 0/2] GlusterFS support in QEMU - v2

[Stefan Hajnoczi]

On Sat, Jul 21, 2012 at 9:29 AM, Bharata B Rao
<bharata@linux.vnet.ibm.com> wrote:
> -drive file=gluster:server@port:volname:image
>
> - Here 'gluster' is the protocol.
> - 'server@port' specifies the server where the volume file specification for
>   the given volume resides. 'port' is the port number on which gluster
>   management daemon (glusterd) is listening. This is optional and if not
>   specified, QEMU will send 0 which will make libgfapi to use the default
>   port.

'server@port' is weird notation.  Normally it is 'server:port' (e.g.
URLs).  Can you change it?

What about the other transports supported by libgfapi: UNIX domain
sockets and RDMA?  My reading of glfs.h is that there are 3 connection
options:
1. 'transport': 'socket' (default), 'unix', 'rdma'
2. 'host': server hostname for 'socket', path to UNIX domain socket
for 'unix', or something else for 'rdma'
3. 'port': TCP port when 'socket' is used.  Ignored otherwise.

Unfortunately QEMU block drivers cannot take custom options yet.  That
would make it possible to cleanly map these connection options and
save you from inventing syntax which doesn't expose all options.

In the meantime it would be nice if the syntax exposed all options.

> Note that we are no longer using volfiles directly and use volume names
> instead. For this to work, gluster management daemon (glusterd) needs to
> be running on the QEMU node. This limits the QEMU user to access the volumes by
> the default volfiles that are generated by gluster CLI. This should be
> fine as long as gluster CLI provides the capability to generate or regenerate
> volume files for a given volume with the xlator set that QEMU user is
> interested in. GlusterFS developers tell me that this can be provided with
> some enhancements to Gluster CLI/glusterd. Note that the custom volume files
> is typically needed when GlusterFS server is co-located with QEMU in
> which case it would  be beneficial to get rid of client-server overhead and
> RPC communication overhead.

My knowledge of GlusterFS is limited.  Here is what I am thinking:

1. The user cannot specify a local configuration file, you require
that there is a glusterd running which provides configuration
information.
2. It is currently not possible to bypass RPC because the glusterd
managed configuration file doesn't support that.

I'm not sure if these statements are true?

Would you support local volfiles in the future again?  Why force users
to run glusterd?

> - As mentioned above, the VM image on gluster volume can be specified like
>   this:
>         -drive file=gluster:localhost:testvol:/F17,format=gluster
>
>   Note that format=gluster is not needed ideally and its a work around I have
>   until libgfapi provides a working connection cleanup routine (glfs_fini()).
>   When the format isn't specified, QEMU figures out the format by doing
>   find_image_format that results in one open and close before opening the
>   image file long term for standard read and write. Gluster connection
>   initialization is done from open and connection termination is done from
>   close. But since glfs_fini() isn't working yet, I am bypassing
>   find_image_format by specifying format=gluster directly which results in
>   just one open and hence I am not limited by glfs_fini().

Has libgfapi been released yet?  Does it have versioning which will
allow the QEMU GlusterFS block driver to build against different
versions?  I'm just wondering how the pieces will fit together once
distros start shipping them.

Stefan

Re: [Qemu-devel] [RFC PATCH 2/2] block: gluster as block backend

[Stefan Hajnoczi]

On Sat, Jul 21, 2012 at 9:31 AM, Bharata B Rao
<bharata@linux.vnet.ibm.com> wrote:
> +typedef struct GlusterAIOCB {
> +    BlockDriverAIOCB common;
> +    QEMUIOVector *qiov;

The qiov field is unused.

> +    char *bounce;

Unused.

> +    struct BDRVGlusterState *s;

You can get this through common.bs->opaque, but if you like having a
shortcut, that's fine.

> +    int cancelled;

bool

> +} GlusterAIOCB;
> +
> +typedef struct GlusterCBKData {
> +    GlusterAIOCB *acb;
> +    struct BDRVGlusterState *s;
> +    int64_t size;
> +    int ret;
> +} GlusterCBKData;

I think GlusterCBKData could just be part of GlusterAIOCB.  That would
simplify the code a little and avoid some malloc/free.

> +
> +typedef struct BDRVGlusterState {
> +    struct glfs *glfs;
> +    int fds[2];
> +    int open_flags;
> +    struct glfs_fd *fd;
> +    int qemu_aio_count;
> +    int event_reader_pos;
> +    GlusterCBKData *event_gcbk;
> +} BDRVGlusterState;
> +
> +#define GLUSTER_FD_READ 0
> +#define GLUSTER_FD_WRITE 1
> +
> +static void qemu_gluster_complete_aio(GlusterCBKData *gcbk)
> +{
> +    GlusterAIOCB *acb = gcbk->acb;
> +    int ret;
> +
> +    if (acb->cancelled) {

Where does cancelled get set?

> +        qemu_aio_release(acb);
> +        goto done;
> +    }
> +
> +    if (gcbk->ret == gcbk->size) {
> +        ret = 0; /* Success */
> +    } else if (gcbk->ret < 0) {
> +        ret = gcbk->ret; /* Read/Write failed */
> +    } else {
> +        ret = -EINVAL; /* Partial read/write - fail it */

EINVAL is for invalid arguments.  EIO would be better.

> +/*
> + * file=protocol:server@port:volname:image
> + */
> +static int qemu_gluster_parsename(GlusterConf *c, const char *filename)
> +{
> +    char *file = g_strdup(filename);
> +    char *token, *next_token, *saveptr;
> +    char *token_s, *next_token_s, *saveptr_s;
> +    int ret = -EINVAL;
> +
> +    /* Discard the protocol */
> +    token = strtok_r(file, ":", &saveptr);
> +    if (!token) {
> +        goto out;
> +    }
> +
> +    /* server@port */
> +    next_token = strtok_r(NULL, ":", &saveptr);
> +    if (!next_token) {
> +        goto out;
> +    }
> +    if (strchr(next_token, '@')) {
> +        token_s = strtok_r(next_token, "@", &saveptr_s);
> +        if (!token_s) {
> +            goto out;
> +        }
> +        strncpy(c->server, token_s, HOST_NAME_MAX);

strncpy(3) will not NUL-terminate when token_s is HOST_NAME_MAX
characters long.  QEMU has cutils.c:pstrcpy().

When the argument is too long we should probably report an error
instead of truncating.

Same below.

> +        next_token_s = strtok_r(NULL, "@", &saveptr_s);
> +        if (!next_token_s) {
> +            goto out;
> +        }
> +        c->port = atoi(next_token_s);

No error checking.  If the input is invalid an error message would
help the user here.

> +static struct glfs *qemu_gluster_init(GlusterConf *c, const char *filename)
> +{
> +    struct glfs *glfs = NULL;
> +    int ret;
> +
> +    ret = qemu_gluster_parsename(c, filename);
> +    if (ret < 0) {
> +        errno = -ret;
> +        goto out;
> +    }
> +
> +    glfs = glfs_new(c->volname);
> +    if (!glfs) {
> +        goto out;
> +    }
> +
> +    ret = glfs_set_volfile_server(glfs, "socket", c->server, c->port);
> +    if (ret < 0) {
> +        goto out;
> +    }
> +
> +    /*
> +     * TODO: Logging is not necessary but instead nice to have.
> +     * Can QEMU optionally log into a standard place ?

QEMU prints to stderr, can you do that here too?  The global log file
is not okay, especially when multiple QEMU instances are running.

> +     * Need to use defines like gf_loglevel_t:GF_LOG_INFO instead of
> +     * hard coded values like 7 here.
> +     */
> +    ret = glfs_set_logging(glfs, "/tmp/qemu-gluster.log", 7);
> +    if (ret < 0) {
> +        goto out;
> +    }
> +
> +    ret = glfs_init(glfs);
> +    if (ret < 0) {
> +        goto out;
> +    }
> +    return glfs;
> +
> +out:
> +    if (glfs) {
> +        (void)glfs_fini(glfs);
> +    }
> +    return NULL;
> +}
> +
> +static int qemu_gluster_open(BlockDriverState *bs, const char *filename,
> +    int bdrv_flags)
> +{
> +    BDRVGlusterState *s = bs->opaque;
> +    GlusterConf *c = g_malloc(sizeof(GlusterConf));

Can this be allocated on the stack?

> +    int ret;
> +
> +    s->glfs = qemu_gluster_init(c, filename);
> +    if (!s->glfs) {
> +        ret = -errno;
> +        goto out;
> +    }
> +
> +    s->open_flags |=  O_BINARY;

Can open_flags be a local variable?

> +static int qemu_gluster_create(const char *filename,
> +        QEMUOptionParameter *options)
> +{
> +    struct glfs *glfs;
> +    struct glfs_fd *fd;
> +    GlusterConf *c = g_malloc(sizeof(GlusterConf));
> +    int ret = 0;
> +    int64_t total_size = 0;
> +
> +    glfs = qemu_gluster_init(c, filename);
> +    if (!glfs) {
> +        ret = -errno;
> +        goto out;
> +    }
> +
> +    /* Read out options */
> +    while (options && options->name) {
> +        if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
> +            total_size = options->value.n / BDRV_SECTOR_SIZE;
> +        }
> +        options++;
> +    }
> +
> +    fd = glfs_creat(glfs, c->image, O_WRONLY|O_CREAT|O_TRUNC|O_BINARY, S_IRWXU);

Why set the execute permission bit?

> +static void qemu_gluster_close(BlockDriverState *bs)
> +{
> +    BDRVGlusterState *s = bs->opaque;
> +
> +    if (s->fd) {
> +        glfs_close(s->fd);
> +        s->fd = NULL;
> +    }

Why not call glfs_fini() here?

Re: [Qemu-devel] [RFC PATCH 2/2] block: gluster as block backend

[Bharata B Rao]

On Sun, Jul 22, 2012 at 04:38:00PM +0100, Stefan Hajnoczi wrote:
> On Sat, Jul 21, 2012 at 9:31 AM, Bharata B Rao
> <bharata@linux.vnet.ibm.com> wrote:
> > +typedef struct GlusterAIOCB {
> > +    BlockDriverAIOCB common;
> > +    QEMUIOVector *qiov;
> 
> The qiov field is unused.
> 
> > +    char *bounce;
> 
> Unused.

Yes, removed these two.

> 
> > +    struct BDRVGlusterState *s;
> 
> You can get this through common.bs->opaque, but if you like having a
> shortcut, that's fine.
> 
> > +    int cancelled;
> 
> bool

Ok.

> 
> > +} GlusterAIOCB;
> > +
> > +typedef struct GlusterCBKData {
> > +    GlusterAIOCB *acb;
> > +    struct BDRVGlusterState *s;
> > +    int64_t size;
> > +    int ret;
> > +} GlusterCBKData;
> 
> I think GlusterCBKData could just be part of GlusterAIOCB.  That would
> simplify the code a little and avoid some malloc/free.

Are you suggesting to put a field

GlusterCBKData gcbk;

inside GlusterAIOCB and use gcbk from there or

Are you suggesting that I make the fields of GlusterCBKData part of
GlusterAIOCB and get rid of GlusterCBKData altogether ? This means I would
have to pass the GlusterAIOCB to gluster async calls and update its fields from
gluster callback routine. I can do this, but I am not sure if you can touch
the fields of GlusterAIOCB in non-QEMU threads (gluster callback thread).

> 
> > +
> > +typedef struct BDRVGlusterState {
> > +    struct glfs *glfs;
> > +    int fds[2];
> > +    int open_flags;
> > +    struct glfs_fd *fd;
> > +    int qemu_aio_count;
> > +    int event_reader_pos;
> > +    GlusterCBKData *event_gcbk;
> > +} BDRVGlusterState;
> > +
> > +#define GLUSTER_FD_READ 0
> > +#define GLUSTER_FD_WRITE 1
> > +
> > +static void qemu_gluster_complete_aio(GlusterCBKData *gcbk)
> > +{
> > +    GlusterAIOCB *acb = gcbk->acb;
> > +    int ret;
> > +
> > +    if (acb->cancelled) {
> 
> Where does cancelled get set?

I realised that I am not supporting bdrv_aio_cancel(). I guess I will have
to add support for this in next version.

> 
> > +        qemu_aio_release(acb);
> > +        goto done;
> > +    }
> > +
> > +    if (gcbk->ret == gcbk->size) {
> > +        ret = 0; /* Success */
> > +    } else if (gcbk->ret < 0) {
> > +        ret = gcbk->ret; /* Read/Write failed */
> > +    } else {
> > +        ret = -EINVAL; /* Partial read/write - fail it */
> 
> EINVAL is for invalid arguments.  EIO would be better.

Ok.

> 
> > +/*
> > + * file=protocol:server@port:volname:image
> > + */
> > +static int qemu_gluster_parsename(GlusterConf *c, const char *filename)
> > +{
> > +    char *file = g_strdup(filename);
> > +    char *token, *next_token, *saveptr;
> > +    char *token_s, *next_token_s, *saveptr_s;
> > +    int ret = -EINVAL;
> > +
> > +    /* Discard the protocol */
> > +    token = strtok_r(file, ":", &saveptr);
> > +    if (!token) {
> > +        goto out;
> > +    }
> > +
> > +    /* server@port */
> > +    next_token = strtok_r(NULL, ":", &saveptr);
> > +    if (!next_token) {
> > +        goto out;
> > +    }
> > +    if (strchr(next_token, '@')) {
> > +        token_s = strtok_r(next_token, "@", &saveptr_s);
> > +        if (!token_s) {
> > +            goto out;
> > +        }
> > +        strncpy(c->server, token_s, HOST_NAME_MAX);
> 
> strncpy(3) will not NUL-terminate when token_s is HOST_NAME_MAX
> characters long.  QEMU has cutils.c:pstrcpy().

Will use pstrcpy.

> 
> When the argument is too long we should probably report an error
> instead of truncating.

Or should we let gluster APIs to flag an error with truncated
server and volume names ?

> 
> Same below.
> 
> > +        next_token_s = strtok_r(NULL, "@", &saveptr_s);
> > +        if (!next_token_s) {
> > +            goto out;
> > +        }
> > +        c->port = atoi(next_token_s);
> 
> No error checking.  If the input is invalid an error message would
> help the user here.

Fixed.

> 
> > +static struct glfs *qemu_gluster_init(GlusterConf *c, const char *filename)
> > +{
> > +    struct glfs *glfs = NULL;
> > +    int ret;
> > +
> > +    ret = qemu_gluster_parsename(c, filename);
> > +    if (ret < 0) {
> > +        errno = -ret;
> > +        goto out;
> > +    }
> > +
> > +    glfs = glfs_new(c->volname);
> > +    if (!glfs) {
> > +        goto out;
> > +    }
> > +
> > +    ret = glfs_set_volfile_server(glfs, "socket", c->server, c->port);
> > +    if (ret < 0) {
> > +        goto out;
> > +    }
> > +
> > +    /*
> > +     * TODO: Logging is not necessary but instead nice to have.
> > +     * Can QEMU optionally log into a standard place ?
> 
> QEMU prints to stderr, can you do that here too?  The global log file
> is not okay, especially when multiple QEMU instances are running.

Ok, I can do glfs_set_logging(glfs, "/dev/stderr", loglevel);

> 
> > +     * Need to use defines like gf_loglevel_t:GF_LOG_INFO instead of
> > +     * hard coded values like 7 here.
> > +     */
> > +    ret = glfs_set_logging(glfs, "/tmp/qemu-gluster.log", 7);
> > +    if (ret < 0) {
> > +        goto out;
> > +    }
> > +
> > +    ret = glfs_init(glfs);
> > +    if (ret < 0) {
> > +        goto out;
> > +    }
> > +    return glfs;
> > +
> > +out:
> > +    if (glfs) {
> > +        (void)glfs_fini(glfs);
> > +    }
> > +    return NULL;
> > +}
> > +
> > +static int qemu_gluster_open(BlockDriverState *bs, const char *filename,
> > +    int bdrv_flags)
> > +{
> > +    BDRVGlusterState *s = bs->opaque;
> > +    GlusterConf *c = g_malloc(sizeof(GlusterConf));
> 
> Can this be allocated on the stack?

It consists of PATH_MAX(4096), HOST_NAME_MAX(255) and GLUSTERD_MAX_VOLUME_NAME
(1000). A bit heavy to be on stack ?

> 
> > +    int ret;
> > +
> > +    s->glfs = qemu_gluster_init(c, filename);
> > +    if (!s->glfs) {
> > +        ret = -errno;
> > +        goto out;
> > +    }
> > +
> > +    s->open_flags |=  O_BINARY;
> 
> Can open_flags be a local variable?

Yes, fixed.

> 
> > +static int qemu_gluster_create(const char *filename,
> > +        QEMUOptionParameter *options)
> > +{
> > +    struct glfs *glfs;
> > +    struct glfs_fd *fd;
> > +    GlusterConf *c = g_malloc(sizeof(GlusterConf));
> > +    int ret = 0;
> > +    int64_t total_size = 0;
> > +
> > +    glfs = qemu_gluster_init(c, filename);
> > +    if (!glfs) {
> > +        ret = -errno;
> > +        goto out;
> > +    }
> > +
> > +    /* Read out options */
> > +    while (options && options->name) {
> > +        if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
> > +            total_size = options->value.n / BDRV_SECTOR_SIZE;
> > +        }
> > +        options++;
> > +    }
> > +
> > +    fd = glfs_creat(glfs, c->image, O_WRONLY|O_CREAT|O_TRUNC|O_BINARY, S_IRWXU);
> 
> Why set the execute permission bit?

Changed to read and write only.

> 
> > +static void qemu_gluster_close(BlockDriverState *bs)
> > +{
> > +    BDRVGlusterState *s = bs->opaque;
> > +
> > +    if (s->fd) {
> > +        glfs_close(s->fd);
> > +        s->fd = NULL;
> > +    }
> 
> Why not call glfs_fini() here?

Missed that, fixed now.

Thanks for your comments.

Regards,
Bharata.

Re: [Qemu-devel] [RFC PATCH 0/2] GlusterFS support in QEMU - v2

[Bharata B Rao]

On Sun, Jul 22, 2012 at 03:42:28PM +0100, Stefan Hajnoczi wrote:
> On Sat, Jul 21, 2012 at 9:29 AM, Bharata B Rao
> <bharata@linux.vnet.ibm.com> wrote:
> > -drive file=gluster:server@port:volname:image
> >
> > - Here 'gluster' is the protocol.
> > - 'server@port' specifies the server where the volume file specification for
> >   the given volume resides. 'port' is the port number on which gluster
> >   management daemon (glusterd) is listening. This is optional and if not
> >   specified, QEMU will send 0 which will make libgfapi to use the default
> >   port.
> 
> 'server@port' is weird notation.  Normally it is 'server:port' (e.g.
> URLs).  Can you change it?

I don't like but, but settled for it since port was optional and : was
being used as separator here.

> 
> What about the other transports supported by libgfapi: UNIX domain
> sockets and RDMA?  My reading of glfs.h is that there are 3 connection
> options:
> 1. 'transport': 'socket' (default), 'unix', 'rdma'
> 2. 'host': server hostname for 'socket', path to UNIX domain socket
> for 'unix', or something else for 'rdma'
> 3. 'port': TCP port when 'socket' is used.  Ignored otherwise.
> 
> Unfortunately QEMU block drivers cannot take custom options yet.  That
> would make it possible to cleanly map these connection options and
> save you from inventing syntax which doesn't expose all options.
> 
> In the meantime it would be nice if the syntax exposed all options.

So without the capability to pass custom options to block drivers, am I forced
to keep extending the file= with more and more options ?

file=gluster:transport:server:port:volname:image ?

Looks ugly and not easy to make any particular option optional. If needed I can
support this from GlusterFS backend.

> 
> > Note that we are no longer using volfiles directly and use volume names
> > instead. For this to work, gluster management daemon (glusterd) needs to
> > be running on the QEMU node. This limits the QEMU user to access the volumes by
> > the default volfiles that are generated by gluster CLI. This should be
> > fine as long as gluster CLI provides the capability to generate or regenerate
> > volume files for a given volume with the xlator set that QEMU user is
> > interested in. GlusterFS developers tell me that this can be provided with
> > some enhancements to Gluster CLI/glusterd. Note that the custom volume files
> > is typically needed when GlusterFS server is co-located with QEMU in
> > which case it would  be beneficial to get rid of client-server overhead and
> > RPC communication overhead.
> 
> My knowledge of GlusterFS is limited.  Here is what I am thinking:
> 
> 1. The user cannot specify a local configuration file, you require
> that there is a glusterd running which provides configuration
> information.

Yes. User only specifies a volume name and glusterd is used to fetch
the right volume file for that volume name.

> 2. It is currently not possible to bypass RPC because the glusterd
> managed configuration file doesn't support that.

It is possible. Gluster already supports custom extensions
to volume names and it is possible to use the required volfile by specifying
this custom volname extension.

For eg, if I have a volume named test, by default the volfile used for
it will be test-fuse.vol. Currently I can put my own custom volfile into
the standard location and get glusterd pick that up. I can specify
test.rpcbypass as volname and glusterd will pick test.rpcbypass.vol.

What is currently not supported is the ability to create test.rpcbypass.vol
from gluster CLI. I believe that gluster developers are ok with enhancing
gluster CLI to support generating/regenerating volfiles for a given volume
with custom translator set.

> 
> I'm not sure if these statements are true?
> 
> Would you support local volfiles in the future again?  Why force users
> to run glusterd?

I will let gluster folks on CC to answer this and let us know the benefits
of always depending on glusterd.

I guess running glusterd would be beneficial when supporting migration. QEMU
working from a local volume (with volname=test.rpcbypass) can be easily
restarted on a different node by just changing volname to test. glusterd will
take care of fetching the right volfile automatically for us.

> 
> > - As mentioned above, the VM image on gluster volume can be specified like
> >   this:
> >         -drive file=gluster:localhost:testvol:/F17,format=gluster
> >
> >   Note that format=gluster is not needed ideally and its a work around I have
> >   until libgfapi provides a working connection cleanup routine (glfs_fini()).
> >   When the format isn't specified, QEMU figures out the format by doing
> >   find_image_format that results in one open and close before opening the
> >   image file long term for standard read and write. Gluster connection
> >   initialization is done from open and connection termination is done from
> >   close. But since glfs_fini() isn't working yet, I am bypassing
> >   find_image_format by specifying format=gluster directly which results in
> >   just one open and hence I am not limited by glfs_fini().
> 
> Has libgfapi been released yet?

Its part of gluster mainline now.

> Does it have versioning which will
> allow the QEMU GlusterFS block driver to build against different
> versions?  I'm just wondering how the pieces will fit together once
> distros start shipping them.

I request gluster folks on CC to comment about version and shipping
information.

Regards,
Bharata.

Re: [Qemu-devel] [RFC PATCH 2/2] block: gluster as block backend

[Stefan Hajnoczi]

On Mon, Jul 23, 2012 at 9:32 AM, Bharata B Rao
<bharata@linux.vnet.ibm.com> wrote:
> On Sun, Jul 22, 2012 at 04:38:00PM +0100, Stefan Hajnoczi wrote:
>> On Sat, Jul 21, 2012 at 9:31 AM, Bharata B Rao
>> <bharata@linux.vnet.ibm.com> wrote:
>> > +} GlusterAIOCB;
>> > +
>> > +typedef struct GlusterCBKData {
>> > +    GlusterAIOCB *acb;
>> > +    struct BDRVGlusterState *s;
>> > +    int64_t size;
>> > +    int ret;
>> > +} GlusterCBKData;
>>
>> I think GlusterCBKData could just be part of GlusterAIOCB.  That would
>> simplify the code a little and avoid some malloc/free.
>
> Are you suggesting to put a field
>
> GlusterCBKData gcbk;
>
> inside GlusterAIOCB and use gcbk from there or
>
> Are you suggesting that I make the fields of GlusterCBKData part of
> GlusterAIOCB and get rid of GlusterCBKData altogether ? This means I would
> have to pass the GlusterAIOCB to gluster async calls and update its fields from
> gluster callback routine. I can do this, but I am not sure if you can touch
> the fields of GlusterAIOCB in non-QEMU threads (gluster callback thread).

The fields in GlusterCBKData could become part of GlusterAIOCB.
Different threads can access fields in a struct, they just need to
ensure access is synchronized if they touch the same fields.  In the
case of this code I think there is nothing that requires
synchronization beyond the pipe mechanism that you already use to
complete processing in a QEMU thread.

>> When the argument is too long we should probably report an error
>> instead of truncating.
>
> Or should we let gluster APIs to flag an error with truncated
> server and volume names ?

What if the truncated name is a valid but different object?  For example:
Max chars = 5
Objects:
"helloworld"
"hello"

If "helloworld" is truncated to "hello" we get no error back because
it's a valid object!

We need to either check sizes explicitly without truncating or use a
g_strdup() approach without any size limits and let the gfapi
functions error out if the input string is too long.

>> > +static struct glfs *qemu_gluster_init(GlusterConf *c, const char *filename)
>> > +{
>> > +    struct glfs *glfs = NULL;
>> > +    int ret;
>> > +
>> > +    ret = qemu_gluster_parsename(c, filename);
>> > +    if (ret < 0) {
>> > +        errno = -ret;
>> > +        goto out;
>> > +    }
>> > +
>> > +    glfs = glfs_new(c->volname);
>> > +    if (!glfs) {
>> > +        goto out;
>> > +    }
>> > +
>> > +    ret = glfs_set_volfile_server(glfs, "socket", c->server, c->port);
>> > +    if (ret < 0) {
>> > +        goto out;
>> > +    }
>> > +
>> > +    /*
>> > +     * TODO: Logging is not necessary but instead nice to have.
>> > +     * Can QEMU optionally log into a standard place ?
>>
>> QEMU prints to stderr, can you do that here too?  The global log file
>> is not okay, especially when multiple QEMU instances are running.
>
> Ok, I can do glfs_set_logging(glfs, "/dev/stderr", loglevel);

Yes.  I think "-" is best since it is supported by gfapi
(libglusterfs/src/logging.c:gf_log_init).  /dev/stderr is not POSIX.

>> > +     * Need to use defines like gf_loglevel_t:GF_LOG_INFO instead of
>> > +     * hard coded values like 7 here.
>> > +     */
>> > +    ret = glfs_set_logging(glfs, "/tmp/qemu-gluster.log", 7);
>> > +    if (ret < 0) {
>> > +        goto out;
>> > +    }
>> > +
>> > +    ret = glfs_init(glfs);
>> > +    if (ret < 0) {
>> > +        goto out;
>> > +    }
>> > +    return glfs;
>> > +
>> > +out:
>> > +    if (glfs) {
>> > +        (void)glfs_fini(glfs);
>> > +    }
>> > +    return NULL;
>> > +}
>> > +
>> > +static int qemu_gluster_open(BlockDriverState *bs, const char *filename,
>> > +    int bdrv_flags)
>> > +{
>> > +    BDRVGlusterState *s = bs->opaque;
>> > +    GlusterConf *c = g_malloc(sizeof(GlusterConf));
>>
>> Can this be allocated on the stack?
>
> It consists of PATH_MAX(4096), HOST_NAME_MAX(255) and GLUSTERD_MAX_VOLUME_NAME
> (1000). A bit heavy to be on stack ?

This is userspace, stacks are big but it's up to you.

Stefan

Re: [Qemu-devel] [RFC PATCH 0/2] GlusterFS support in QEMU - v2

[Daniel P. Berrange]

On Sat, Jul 21, 2012 at 01:59:17PM +0530, Bharata B Rao wrote:
> Hi,
> 
> Here is the v2 patchset for supporting GlusterFS protocol from QEMU.
> 
> This set of patches enables QEMU to boot VM images from gluster volumes.
> This is achieved by adding gluster as a new block backend driver in QEMU.
> Its already possible to boot from VM images on gluster volumes, but this
> patchset provides the ability to boot VM images from gluster volumes by
> by-passing the FUSE layer in gluster. In case the image is present on the
> local system, it is possible to even bypass client and server translator and
> hence the RPC overhead.
> 
> The major change in this version is to not implement libglusterfs based
> gluster backend within QEMU but instead use libgfapi. libgfapi library
> from GlusterFS project provides APIs to access gluster volumes directly.
> With the use of libgfapi, the specification of gluster backend from QEMU
> matches more closely with the GlusterFS's way of specifying volumes. We now
> specify the gluster backed image like this:
> 
> -drive file=gluster:server@port:volname:image
> 
> - Here 'gluster' is the protocol.
> - 'server@port' specifies the server where the volume file specification for
>   the given volume resides. 'port' is the port number on which gluster
>   management daemon (glusterd) is listening. This is optional and if not
>   specified, QEMU will send 0 which will make libgfapi to use the default
>   port.
> - 'volname' is the name of the gluster volume which contains the VM image.
> - 'image' is the path to the actual VM image in the gluster volume.

I don't think we should be using '@' as a field separator here, when ":"
can do that job just fine. In addition we already have a precendent set
with the sheepdog driver for using ':' for separating all fields:

  -drive file=sheepdog:example.org:6000:imagename

If you want to allow for port number to be omitted, this can be handled
thus:

  -drive file=sheepdog:example.org::imagename

which is how -chardev deals with omitted port numbers

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 :|

Re: [Qemu-devel] [RFC PATCH 0/2] GlusterFS support in QEMU - v2

[Stefan Hajnoczi]

On Mon, Jul 23, 2012 at 9:50 AM, Bharata B Rao
<bharata@linux.vnet.ibm.com> wrote:
> On Sun, Jul 22, 2012 at 03:42:28PM +0100, Stefan Hajnoczi wrote:
>> On Sat, Jul 21, 2012 at 9:29 AM, Bharata B Rao
>> <bharata@linux.vnet.ibm.com> wrote:
>> > -drive file=gluster:server@port:volname:image
>> >
>> > - Here 'gluster' is the protocol.
>> > - 'server@port' specifies the server where the volume file specification for
>> >   the given volume resides. 'port' is the port number on which gluster
>> >   management daemon (glusterd) is listening. This is optional and if not
>> >   specified, QEMU will send 0 which will make libgfapi to use the default
>> >   port.
>>
>> 'server@port' is weird notation.  Normally it is 'server:port' (e.g.
>> URLs).  Can you change it?
>
> I don't like but, but settled for it since port was optional and : was
> being used as separator here.
>
>>
>> What about the other transports supported by libgfapi: UNIX domain
>> sockets and RDMA?  My reading of glfs.h is that there are 3 connection
>> options:
>> 1. 'transport': 'socket' (default), 'unix', 'rdma'
>> 2. 'host': server hostname for 'socket', path to UNIX domain socket
>> for 'unix', or something else for 'rdma'
>> 3. 'port': TCP port when 'socket' is used.  Ignored otherwise.
>>
>> Unfortunately QEMU block drivers cannot take custom options yet.  That
>> would make it possible to cleanly map these connection options and
>> save you from inventing syntax which doesn't expose all options.
>>
>> In the meantime it would be nice if the syntax exposed all options.
>
> So without the capability to pass custom options to block drivers, am I forced
> to keep extending the file= with more and more options ?
>
> file=gluster:transport:server:port:volname:image ?
>
> Looks ugly and not easy to make any particular option optional. If needed I can
> support this from GlusterFS backend.

Kevin, Markus: Any thoughts on passing options to block drivers?
Encoding GlusterFS options into a "filename" string is pretty
cumbersome.

Re: [Qemu-devel] [RFC PATCH 0/2] GlusterFS support in QEMU - v2

[ronnie sahlberg]

Why not use

-drive file=gluster://server[:port]/volname/image

A great many protocols today use the form

<protocol>://<server>:<port>]/<path>

so this would make it consistent with a lot of other naming schemes
out there, and imho make
the url more intuitive.


FTP looks like this :   ftp://user:password@host:port/path
NFS looks like this :   nfs://<host>:<port><url-path>
CIFS looks like this :
smb://[[[authdomain;]user@]host[:port][/share[/path][/name]]][?context]

For iSCSI we use  :   iscsi://<server>[:<port>]/<target>/<lun>


(The iscsi syntax was picked explicitely to be consistent with the
de-facto url naming scheme.)


I would argue that this is the de-facto way to create a url for
different protocols,   so it would imho be natural to specify a
glusterfs url in a similar format.


ronnie sahlberg


On Mon, Jul 23, 2012 at 7:16 PM, Daniel P. Berrange <berrange@redhat.com> wrote:
> On Sat, Jul 21, 2012 at 01:59:17PM +0530, Bharata B Rao wrote:
>> Hi,
>>
>> Here is the v2 patchset for supporting GlusterFS protocol from QEMU.
>>
>> This set of patches enables QEMU to boot VM images from gluster volumes.
>> This is achieved by adding gluster as a new block backend driver in QEMU.
>> Its already possible to boot from VM images on gluster volumes, but this
>> patchset provides the ability to boot VM images from gluster volumes by
>> by-passing the FUSE layer in gluster. In case the image is present on the
>> local system, it is possible to even bypass client and server translator and
>> hence the RPC overhead.
>>
>> The major change in this version is to not implement libglusterfs based
>> gluster backend within QEMU but instead use libgfapi. libgfapi library
>> from GlusterFS project provides APIs to access gluster volumes directly.
>> With the use of libgfapi, the specification of gluster backend from QEMU
>> matches more closely with the GlusterFS's way of specifying volumes. We now
>> specify the gluster backed image like this:
>>
>> -drive file=gluster:server@port:volname:image
>>
>> - Here 'gluster' is the protocol.
>> - 'server@port' specifies the server where the volume file specification for
>>   the given volume resides. 'port' is the port number on which gluster
>>   management daemon (glusterd) is listening. This is optional and if not
>>   specified, QEMU will send 0 which will make libgfapi to use the default
>>   port.
>> - 'volname' is the name of the gluster volume which contains the VM image.
>> - 'image' is the path to the actual VM image in the gluster volume.
>
> I don't think we should be using '@' as a field separator here, when ":"
> can do that job just fine. In addition we already have a precendent set
> with the sheepdog driver for using ':' for separating all fields:
>
>   -drive file=sheepdog:example.org:6000:imagename
>
> If you want to allow for port number to be omitted, this can be handled
> thus:
>
>   -drive file=sheepdog:example.org::imagename
>
> which is how -chardev deals with omitted port numbers
>
> 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 :|
>

Re: [Qemu-devel] [RFC PATCH 0/2] GlusterFS support in QEMU - v2

[ronnie sahlberg]

Stefan,

in iscsi,  i just specify those extra arguments that are required that
are not part of the url itself as just command line options :


qemu-system-i386 -iscsi initiator-name=iqn.qemu.test:my-initiator \
    -boot d -drive file=iscsi://127.0.0.1/iqn.qemu.test/1 \
    -cdrom iscsi://127.0.0.1/iqn.qemu.test/2

Here  initiator-name is a custom option to the iscsi layer to tell it
which name to use when identifying/logging in to the target.

Similar concept could be used by glusterfs ?

regards
ronnie sahlberg



On Mon, Jul 23, 2012 at 7:20 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Mon, Jul 23, 2012 at 9:50 AM, Bharata B Rao
> <bharata@linux.vnet.ibm.com> wrote:
>> On Sun, Jul 22, 2012 at 03:42:28PM +0100, Stefan Hajnoczi wrote:
>>> On Sat, Jul 21, 2012 at 9:29 AM, Bharata B Rao
>>> <bharata@linux.vnet.ibm.com> wrote:
>>> > -drive file=gluster:server@port:volname:image
>>> >
>>> > - Here 'gluster' is the protocol.
>>> > - 'server@port' specifies the server where the volume file specification for
>>> >   the given volume resides. 'port' is the port number on which gluster
>>> >   management daemon (glusterd) is listening. This is optional and if not
>>> >   specified, QEMU will send 0 which will make libgfapi to use the default
>>> >   port.
>>>
>>> 'server@port' is weird notation.  Normally it is 'server:port' (e.g.
>>> URLs).  Can you change it?
>>
>> I don't like but, but settled for it since port was optional and : was
>> being used as separator here.
>>
>>>
>>> What about the other transports supported by libgfapi: UNIX domain
>>> sockets and RDMA?  My reading of glfs.h is that there are 3 connection
>>> options:
>>> 1. 'transport': 'socket' (default), 'unix', 'rdma'
>>> 2. 'host': server hostname for 'socket', path to UNIX domain socket
>>> for 'unix', or something else for 'rdma'
>>> 3. 'port': TCP port when 'socket' is used.  Ignored otherwise.
>>>
>>> Unfortunately QEMU block drivers cannot take custom options yet.  That
>>> would make it possible to cleanly map these connection options and
>>> save you from inventing syntax which doesn't expose all options.
>>>
>>> In the meantime it would be nice if the syntax exposed all options.
>>
>> So without the capability to pass custom options to block drivers, am I forced
>> to keep extending the file= with more and more options ?
>>
>> file=gluster:transport:server:port:volname:image ?
>>
>> Looks ugly and not easy to make any particular option optional. If needed I can
>> support this from GlusterFS backend.
>
> Kevin, Markus: Any thoughts on passing options to block drivers?
> Encoding GlusterFS options into a "filename" string is pretty
> cumbersome.
>

Re: [Qemu-devel] [RFC PATCH 0/2] GlusterFS support in QEMU - v2

[Stefan Hajnoczi]

On Mon, Jul 23, 2012 at 10:34 AM, ronnie sahlberg
<ronniesahlberg@gmail.com> wrote:
> in iscsi,  i just specify those extra arguments that are required that
> are not part of the url itself as just command line options :
>
>
> qemu-system-i386 -iscsi initiator-name=iqn.qemu.test:my-initiator \
>     -boot d -drive file=iscsi://127.0.0.1/iqn.qemu.test/1 \
>     -cdrom iscsi://127.0.0.1/iqn.qemu.test/2
>
> Here  initiator-name is a custom option to the iscsi layer to tell it
> which name to use when identifying/logging in to the target.
>
> Similar concept could be used by glusterfs ?

That works for global options only.  If it's per-drive then this
approach can't be used because different drives might need different
option values.

Stefan

Re: [Qemu-devel] [RFC PATCH 0/2] GlusterFS support in QEMU - v2

[Vijay Bellur]

On 07/23/2012 02:20 PM, Bharata B Rao wrote:

>
>> 2. It is currently not possible to bypass RPC because the glusterd
>> managed configuration file doesn't support that.
>
> It is possible. Gluster already supports custom extensions
> to volume names and it is possible to use the required volfile by specifying
> this custom volname extension.
>
> For eg, if I have a volume named test, by default the volfile used for
> it will be test-fuse.vol. Currently I can put my own custom volfile into
> the standard location and get glusterd pick that up. I can specify
> test.rpcbypass as volname and glusterd will pick test.rpcbypass.vol.
>
> What is currently not supported is the ability to create test.rpcbypass.vol
> from gluster CLI. I believe that gluster developers are ok with enhancing
> gluster CLI to support generating/regenerating volfiles for a given volume
> with custom translator set.


Yes, this would be the preferred approach. We can tune the volume file 
generation to evolve the desired configuration file.

>
>>
>> I'm not sure if these statements are true?
>>
>> Would you support local volfiles in the future again?  Why force users
>> to run glusterd?
>
> I will let gluster folks on CC to answer this and let us know the benefits
> of always depending on glusterd.
>
> I guess running glusterd would be beneficial when supporting migration. QEMU
> working from a local volume (with volname=test.rpcbypass) can be easily
> restarted on a different node by just changing volname to test. glusterd will
> take care of fetching the right volfile automatically for us.

Yes, running glusterd would be beneficial in migration. Without deriving 
the file from glusterd features like volume tuning, client monitoring 
etc. would not be available to to clients that talk to a gluster volume. 
Additionally, driving configuration generation and management through 
glusterd helps in standardizing and stabilizing gluster configurations.

>

>>
>> Has libgfapi been released yet?
>
> Its part of gluster mainline now.
>
>> Does it have versioning which will
>> allow the QEMU GlusterFS block driver to build against different
>> versions?  I'm just wondering how the pieces will fit together once
>> distros start shipping them.
>
> I request gluster folks on CC to comment about version and shipping
> information.
>

There is no release that contains libgfapi as yet. Once that is done, we 
can probably have the dependency specified in QEMU as well.

Regards,
Vijay

Re: [Qemu-devel] [RFC PATCH 0/2] GlusterFS support in QEMU - v2

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 982 bytes --]

On 07/23/2012 03:20 AM, Stefan Hajnoczi wrote:
>>
>> So without the capability to pass custom options to block drivers, am I forced
>> to keep extending the file= with more and more options ?
>>
>> file=gluster:transport:server:port:volname:image ?
>>
>> Looks ugly and not easy to make any particular option optional. If needed I can
>> support this from GlusterFS backend.
> 
> Kevin, Markus: Any thoughts on passing options to block drivers?
> Encoding GlusterFS options into a "filename" string is pretty
> cumbersome.

On 07/23/2012 03:28 AM, ronnie sahlberg wrote:> Why not use
>
> -drive file=gluster://server[:port]/volname/image

At which point, options can fit into this URI scheme:

-drive file=gluster://server:port/volname/image?option1=foo&option2=bar

where anything after the ? of the URI can introduce whichever options
you need.

-- 
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: 620 bytes --]

Re: [Qemu-devel] [RFC PATCH 0/2] GlusterFS support in QEMU - v2

[Bharata B Rao]

On Mon, Jul 23, 2012 at 08:34:30AM -0600, Eric Blake wrote:
> On 07/23/2012 03:20 AM, Stefan Hajnoczi wrote:
> >>
> >> So without the capability to pass custom options to block drivers, am I forced
> >> to keep extending the file= with more and more options ?
> >>
> >> file=gluster:transport:server:port:volname:image ?
> >>
> >> Looks ugly and not easy to make any particular option optional. If needed I can
> >> support this from GlusterFS backend.
> > 
> > Kevin, Markus: Any thoughts on passing options to block drivers?
> > Encoding GlusterFS options into a "filename" string is pretty
> > cumbersome.
> 
> On 07/23/2012 03:28 AM, ronnie sahlberg wrote:> Why not use
> >
> > -drive file=gluster://server[:port]/volname/image
> 
> At which point, options can fit into this URI scheme:
> 
> -drive file=gluster://server:port/volname/image?option1=foo&option2=bar
> 
> where anything after the ? of the URI can introduce whichever options
> you need.

The URI covered everything and left only transport as the option, which could
be made part of the URI itself ?

So looks like we have two options:

gluster://server[:port]/[transport]/volname/image

vs

gluster:server:[port]:[transport]:volname:image

Unless there is a strong preference on one over the other, I am inclined
to go with the latter (colon based) approach and expect user to provide
double colons (::) wherever any default value needs to be specified.

Eg 1. gluster:localhost:::test:/a.img
Eg 2. gluster:localhost:0:socket:test:/a.img

Regards,
Bharata.

Re: [Qemu-devel] [RFC PATCH 0/2] GlusterFS support in QEMU - v2

[Kevin Wolf]

Am 23.07.2012 11:20, schrieb Stefan Hajnoczi:
> On Mon, Jul 23, 2012 at 9:50 AM, Bharata B Rao
> <bharata@linux.vnet.ibm.com> wrote:
>> On Sun, Jul 22, 2012 at 03:42:28PM +0100, Stefan Hajnoczi wrote:
>>> What about the other transports supported by libgfapi: UNIX domain
>>> sockets and RDMA?  My reading of glfs.h is that there are 3 connection
>>> options:
>>> 1. 'transport': 'socket' (default), 'unix', 'rdma'
>>> 2. 'host': server hostname for 'socket', path to UNIX domain socket
>>> for 'unix', or something else for 'rdma'
>>> 3. 'port': TCP port when 'socket' is used.  Ignored otherwise.
>>>
>>> Unfortunately QEMU block drivers cannot take custom options yet.  That
>>> would make it possible to cleanly map these connection options and
>>> save you from inventing syntax which doesn't expose all options.
>>>
>>> In the meantime it would be nice if the syntax exposed all options.
>>
>> So without the capability to pass custom options to block drivers, am I forced
>> to keep extending the file= with more and more options ?
>>
>> file=gluster:transport:server:port:volname:image ?
>>
>> Looks ugly and not easy to make any particular option optional. If needed I can
>> support this from GlusterFS backend.
> 
> Kevin, Markus: Any thoughts on passing options to block drivers?
> Encoding GlusterFS options into a "filename" string is pretty
> cumbersome.

This is the way it is without -blockdev. *shrug*

Kevin

Re: [Qemu-devel] [RFC PATCH 0/2] GlusterFS support in QEMU - v2

[Markus Armbruster]

Stefan Hajnoczi <stefanha@gmail.com> writes:

> On Mon, Jul 23, 2012 at 9:50 AM, Bharata B Rao
> <bharata@linux.vnet.ibm.com> wrote:
>> On Sun, Jul 22, 2012 at 03:42:28PM +0100, Stefan Hajnoczi wrote:
>>> On Sat, Jul 21, 2012 at 9:29 AM, Bharata B Rao
>>> <bharata@linux.vnet.ibm.com> wrote:
>>> > -drive file=gluster:server@port:volname:image
>>> >
>>> > - Here 'gluster' is the protocol.
>>> > - 'server@port' specifies the server where the volume file
>>> > specification for
>>> >   the given volume resides. 'port' is the port number on which gluster
>>> >   management daemon (glusterd) is listening. This is optional and if not
>>> >   specified, QEMU will send 0 which will make libgfapi to use the default
>>> >   port.
>>>
>>> 'server@port' is weird notation.  Normally it is 'server:port' (e.g.
>>> URLs).  Can you change it?
>>
>> I don't like but, but settled for it since port was optional and : was
>> being used as separator here.

For what it's worth, icsi.c uses

iscsi://[<username>%<password>@]<host>[:<port>]/<targetname>/<lun>

>>> What about the other transports supported by libgfapi: UNIX domain
>>> sockets and RDMA?  My reading of glfs.h is that there are 3 connection
>>> options:
>>> 1. 'transport': 'socket' (default), 'unix', 'rdma'
>>> 2. 'host': server hostname for 'socket', path to UNIX domain socket
>>> for 'unix', or something else for 'rdma'
>>> 3. 'port': TCP port when 'socket' is used.  Ignored otherwise.
>>>
>>> Unfortunately QEMU block drivers cannot take custom options yet.  That
>>> would make it possible to cleanly map these connection options and
>>> save you from inventing syntax which doesn't expose all options.
>>>
>>> In the meantime it would be nice if the syntax exposed all options.
>>
>> So without the capability to pass custom options to block drivers, am I forced
>> to keep extending the file= with more and more options ?
>>
>> file=gluster:transport:server:port:volname:image ?
>>
>> Looks ugly and not easy to make any particular option optional. If
>> needed I can
>> support this from GlusterFS backend.
>
> Kevin, Markus: Any thoughts on passing options to block drivers?
> Encoding GlusterFS options into a "filename" string is pretty
> cumbersome.

Yes, it is.  Every block driver with special parameters has its own ad
hoc parser, and some of them are badly designed.

I'm working on a sane way to configure block drivers.  Until we got
that, encoding parameters into the "filename" is what you have to do.

Once we got it, block drivers shouldn't be required to expose *all*
their parameters via -drive's encoded "filename".