[libvirt] [PATCH v3 0/3] qemu: add support for multiple gluster hosts/servers
These series of patches are rebased on latest master; The dependent QEMU patches are now merged on master targeting for 2.7 Prasanna Kumar Kalever (3): storage: add gluster backend initialization support for multiple servers qemu: add support for multiple gluster hosts/servers util: add backing store parser support for gluster protocol v3: Since now there is qemu support available refreshed the patches on master, also addressed most of the comments on v2 changes on patch 1/3: moved to the new JSON format of options changes on patch 2/3: moved to the new JSON format of options changes on patch 3/3: resolved most of the comments except moving the code from utils to qemu specific, I need suggestions here to find what is the best file to fit this in. v2: Split patches addressing "Peter Krempa" <pkrempa redhat com> comments patch 1/3: Support to initialization of gluster backend with multiple servers patch 2/3: JSON formatter patch 3/3: JSON parser v1: Initial Patches add support to gluster json formatter src/qemu/qemu_command.c | 219 +++++++++++++++++++++++++++------- src/storage/storage_backend_gluster.c | 100 +++++++++++----- src/util/virstoragefile.c | 208 ++++++++++++++++++++++++++++++++ 3 files changed, 453 insertions(+), 74 deletions(-) -- 2.7.4
This patch adds support for initialization of gluster backend with multiple servers which acts as gluster volfile servers for the gluster storage backend. This will help in achieving high availability of gluster backend connectivity via libgfapi i.e. when the first volfile server fails, then other servers specified are used as volfile server to re-establish the backend gluster connectivity. Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com> --- src/storage/storage_backend_gluster.c | 100 ++++++++++++++++++++++++---------- 1 file changed, 71 insertions(+), 29 deletions(-) diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index eda060d..ab61619 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -32,6 +32,8 @@ #include "virstring.h" #include "viruri.h" +#define QEMU_DEFAULT_GLUSTER_PORT 24007 + #define VIR_FROM_THIS VIR_FROM_STORAGE VIR_LOG_INIT("storage.storage_backend_gluster"); @@ -573,20 +575,18 @@ static int virStorageFileBackendGlusterInit(virStorageSourcePtr src) { virStorageFileBackendGlusterPrivPtr priv = NULL; - virStorageNetHostDefPtr host = &(src->hosts[0]); - const char *hostname; + const char *transport; + bool is_unix = false; int port = 0; + size_t i = 0; - if (src->nhosts != 1) { + if (src->nhosts < 1) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Expected exactly 1 host for the gluster volume")); + _("Expected atleast 1 host for the gluster volume")); return -1; } - hostname = host->name; - - VIR_DEBUG("initializing gluster storage file %p (gluster://%s:%s/%s%s)[%u:%u]", - src, hostname, host->port ? host->port : "0", + VIR_DEBUG("Initializing gluster volume=%s path=%s with uid=%u gid=%u", NULLSTR(src->volume), src->path, (unsigned int)src->drv->uid, (unsigned int)src->drv->gid); @@ -600,35 +600,77 @@ virStorageFileBackendGlusterInit(virStorageSourcePtr src) if (VIR_ALLOC(priv) < 0) return -1; - if (host->port && - virStrToLong_i(host->port, NULL, 10, &port) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("failed to parse port number '%s'"), - host->port); - goto error; - } - - if (host->transport == VIR_STORAGE_NET_HOST_TRANS_UNIX) - hostname = host->socket; - if (!(priv->vol = glfs_new(src->volume))) { virReportOOMError(); goto error; } - if (glfs_set_volfile_server(priv->vol, - virStorageNetHostTransportTypeToString(host->transport), - hostname, port) < 0) { - virReportSystemError(errno, - _("failed to set gluster volfile server '%s'"), - hostname); - goto error; + for (i = 0; i < src->nhosts; i++) { + + if ((src->hosts[i].transport == VIR_STORAGE_NET_HOST_TRANS_TCP) || + (src->hosts[i].transport == VIR_STORAGE_NET_HOST_TRANS_UNIX)) { + transport = virStorageNetHostTransportTypeToString(src->hosts[i].transport); + } else { + virReportSystemError(errno, + _("gluster only supports tcp|unix not '%s'"), + virStorageNetHostTransportTypeToString(src->hosts[i].transport)); + goto error; + } + + + if (src->hosts[i].transport == VIR_STORAGE_NET_HOST_TRANS_UNIX) { + is_unix = true; + } else if (!src->hosts[i].port) { + port = QEMU_DEFAULT_GLUSTER_PORT; + } else { + if (virStrToLong_i(src->hosts[i].port, NULL, 10, &port) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to parse port number '%s'"), + src->hosts[i].port); + goto error; + } + } + if (glfs_set_volfile_server(priv->vol, transport, + is_unix ? (src->hosts[i].socket) : (src->hosts[i].name), + is_unix ? 0 : port) < 0) { + if (is_unix) { + virReportSystemError(errno, + _("failed to set gluster volfile server with " + "socket '%s'"), src->hosts[i].socket); + } else { + virReportSystemError(errno, + _("failed to set gluster volfile server with " + "host '%s' and port '%s'"), src->hosts[i].name, + src->hosts[i].port); + } + goto error; + } else { + if (is_unix) { + VIR_DEBUG("Successfully set volfile server with socket:'%s' ", + src->hosts[i].socket); + } else { + VIR_DEBUG("Successfully set volfile server with host:'%s' " + "port:'%d' transport:'%s'", src->hosts[i].name, + port, transport); + } + } + is_unix = false; } if (glfs_init(priv->vol) < 0) { - virReportSystemError(errno, - _("failed to initialize gluster connection to " - "server: '%s'"), hostname); + virReportSystemError(errno, + _("failed to initialize gluster connection" + " for volume %s, path %s with"), + NULLSTR(src->volume), src->path); + for (i = 0; i < src->nhosts; i++) { + if (src->hosts[i].transport == VIR_STORAGE_NET_HOST_TRANS_TCP) { + virReportSystemError(errno, _("host: %s and port %s"), + src->hosts[i].name, src->hosts[i].port); + } else { + virReportSystemError(errno, _("socket %s"), + src->hosts[i].socket); + } + } goto error; } -- 2.7.4
On Fri, Jul 22, 2016 at 01:50:29PM +0530, Prasanna Kumar Kalever wrote:
This patch adds support for initialization of gluster backend with multiple servers which acts as gluster volfile servers for the gluster storage backend.
This will help in achieving high availability of gluster backend connectivity via libgfapi i.e. when the first volfile server fails, then other servers specified are used as volfile server to re-establish the backend gluster connectivity.
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com> --- src/storage/storage_backend_gluster.c | 100 ++++++++++++++++++++++++---------- 1 file changed, 71 insertions(+), 29 deletions(-)
diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index eda060d..ab61619 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -32,6 +32,8 @@ #include "virstring.h" #include "viruri.h"
+#define QEMU_DEFAULT_GLUSTER_PORT 24007 + #define VIR_FROM_THIS VIR_FROM_STORAGE
VIR_LOG_INIT("storage.storage_backend_gluster"); @@ -573,20 +575,18 @@ static int virStorageFileBackendGlusterInit(virStorageSourcePtr src) { virStorageFileBackendGlusterPrivPtr priv = NULL; - virStorageNetHostDefPtr host = &(src->hosts[0]); - const char *hostname; + const char *transport; + bool is_unix = false; int port = 0; + size_t i = 0;
- if (src->nhosts != 1) { + if (src->nhosts < 1) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Expected exactly 1 host for the gluster volume")); + _("Expected atleast 1 host for the gluster volume"));
s/atleast/at least/
return -1; }
- hostname = host->name; - - VIR_DEBUG("initializing gluster storage file %p (gluster://%s:%s/%s%s)[%u:%u]", - src, hostname, host->port ? host->port : "0", + VIR_DEBUG("Initializing gluster volume=%s path=%s with uid=%u gid=%u", NULLSTR(src->volume), src->path, (unsigned int)src->drv->uid, (unsigned int)src->drv->gid);
@@ -600,35 +600,77 @@ virStorageFileBackendGlusterInit(virStorageSourcePtr src) if (VIR_ALLOC(priv) < 0) return -1;
- if (host->port && - virStrToLong_i(host->port, NULL, 10, &port) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("failed to parse port number '%s'"), - host->port); - goto error; - } - - if (host->transport == VIR_STORAGE_NET_HOST_TRANS_UNIX) - hostname = host->socket; - if (!(priv->vol = glfs_new(src->volume))) { virReportOOMError(); goto error; }
- if (glfs_set_volfile_server(priv->vol, - virStorageNetHostTransportTypeToString(host->transport), - hostname, port) < 0) { - virReportSystemError(errno, - _("failed to set gluster volfile server '%s'"), - hostname); - goto error; + for (i = 0; i < src->nhosts; i++) { + + if ((src->hosts[i].transport == VIR_STORAGE_NET_HOST_TRANS_TCP) || + (src->hosts[i].transport == VIR_STORAGE_NET_HOST_TRANS_UNIX)) { + transport = virStorageNetHostTransportTypeToString(src->hosts[i].transport); + } else { + virReportSystemError(errno, + _("gluster only supports tcp|unix not '%s'"), + virStorageNetHostTransportTypeToString(src->hosts[i].transport)); + goto error; + } + + + if (src->hosts[i].transport == VIR_STORAGE_NET_HOST_TRANS_UNIX) { + is_unix = true; + } else if (!src->hosts[i].port) { + port = QEMU_DEFAULT_GLUSTER_PORT; + } else { + if (virStrToLong_i(src->hosts[i].port, NULL, 10, &port) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to parse port number '%s'"), + src->hosts[i].port); + goto error; + }
Indentation is screwed up here.
+ } + if (glfs_set_volfile_server(priv->vol, transport, + is_unix ? (src->hosts[i].socket) : (src->hosts[i].name),
The () used here serve no useful purpose.
+ is_unix ? 0 : port) < 0) { + if (is_unix) { + virReportSystemError(errno, + _("failed to set gluster volfile server with " + "socket '%s'"), src->hosts[i].socket); + } else { + virReportSystemError(errno, + _("failed to set gluster volfile server with " + "host '%s' and port '%s'"), src->hosts[i].name, + src->hosts[i].port); + } + goto error; + } else { + if (is_unix) { + VIR_DEBUG("Successfully set volfile server with socket:'%s' ", + src->hosts[i].socket); + } else { + VIR_DEBUG("Successfully set volfile server with host:'%s' " + "port:'%d' transport:'%s'", src->hosts[i].name, + port, transport); + } + } + is_unix = false; }
if (glfs_init(priv->vol) < 0) { - virReportSystemError(errno, - _("failed to initialize gluster connection to " - "server: '%s'"), hostname); + virReportSystemError(errno, + _("failed to initialize gluster connection" + " for volume %s, path %s with"), + NULLSTR(src->volume), src->path); + for (i = 0; i < src->nhosts; i++) { + if (src->hosts[i].transport == VIR_STORAGE_NET_HOST_TRANS_TCP) { + virReportSystemError(errno, _("host: %s and port %s"), + src->hosts[i].name, src->hosts[i].port); + } else { + virReportSystemError(errno, _("socket %s"), + src->hosts[i].socket); + } + }
This code looks really messed up - any codepath should only call the virReportXXXXXXError functions *once*, certainly never report errors in a loop. 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 :|
This patch adds support for gluster specific JSON formatter functionality currently libvirt has the capability to parse only one host and convert that into URI formatted string, with the help of this patch libvirt will be able to parse multiple hosts from the domain xml and can convert that into JSON formatted string if the number of hosts supplied is only one, then we use existing URI format for backward compatibility and if number of hosts is greater than one we switch to the new JSON format before: ------ example.xml: ... <disk type='network' device='disk'> <driver name='qemu' type='qcow2' cache='none'/> <source protocol='gluster' name='testvol/a.qcow2'> <host name='1.2.3.4' port='24007' transport="tcp"/> </source> <target dev='vda' bus='virtio'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </disk> ... resultant string: file=gluster://1.2.3.4:24007/testvol/a.qcow2, \ if=none,id=drive-virtio-disk0,format=qcow2,cache=none after: ----- example.xml: ... <disk type='network' device='disk'> <driver name='qemu' type='qcow2' cache='none'/> <source protocol='gluster' name='testvol/a.qcow2'> <host name='1.2.3.4' port='24009' transport="tcp"/> <host socket='/var/run/glusterd.socket' transport="unix"/> </source> <target dev='vda' bus='virtio'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </disk> ... resultant string: -drive file=json:{ "file": { "driver": "gluster",, "volume": "testvol",, "path": "/a.qcow2",, "server": [ { "type": "tcp" "host": "1.2.3.4",, "port": 24009,, },, { "transport": "unix" "socket": "/var/run/glusterd.socket", } ] },, "driver": "qcow2" } ,if=none,id=drive-virtio-disk0,cache=none credits: sincere thanks to all the supporters Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com> --- src/qemu/qemu_command.c | 219 ++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 174 insertions(+), 45 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 4558b9f..fc38ecf 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -481,10 +481,12 @@ qemuNetworkDriveGetPort(int protocol, return 10809; case VIR_STORAGE_NET_PROTOCOL_ISCSI: - case VIR_STORAGE_NET_PROTOCOL_GLUSTER: /* no default port specified */ return 0; + case VIR_STORAGE_NET_PROTOCOL_GLUSTER: + return 24007; + case VIR_STORAGE_NET_PROTOCOL_RBD: case VIR_STORAGE_NET_PROTOCOL_LAST: case VIR_STORAGE_NET_PROTOCOL_NONE: @@ -684,16 +686,172 @@ qemuBuildRBDSecinfoURI(virBufferPtr buf, return 0; } +static char* +qemuBuildNetworkDriveURI(virStorageSourcePtr src, + qemuDomainSecretInfoPtr secinfo) +{ + virURIPtr uri = NULL; + char *str = NULL; + + if (VIR_ALLOC(uri) < 0) + goto cleanup; + + if (src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_TCP) { + if (VIR_STRDUP(uri->scheme, + virStorageNetProtocolTypeToString(src->protocol)) < 0) + goto cleanup; + } else { + if (virAsprintf(&uri->scheme, "%s+%s", + virStorageNetProtocolTypeToString(src->protocol), + virStorageNetHostTransportTypeToString(src->hosts->transport)) < 0) + goto cleanup; + } + + if (src->path) { + if (src->volume) { + if (virAsprintf(&uri->path, "/%s%s", + src->volume, src->path) < 0) + goto cleanup; + } else { + if (virAsprintf(&uri->path, "%s%s", + src->path[0] == '/' ? "" : "/", + src->path) < 0) + goto cleanup; + } + } + + if (src->hosts->transport != VIR_STORAGE_NET_HOST_TRANS_UNIX) { + if (VIR_STRDUP(uri->server, src->hosts->name) < 0) + goto cleanup; + + if ((uri->port = qemuNetworkDriveGetPort(src->protocol, src->hosts->port)) < 0) + goto cleanup; + } else { + if (src->hosts->socket && + virAsprintf(&uri->query, "socket=%s", src->hosts->socket) < 0) + goto cleanup; + } + + if (qemuBuildGeneralSecinfoURI(uri, secinfo) < 0) + goto cleanup; + + str = virURIFormat(uri); + + cleanup: + virURIFree(uri); + + return str; +} + +static char * +qemuBuildGlusterDriveJSON(virStorageSourcePtr src) +{ + virJSONValuePtr file = NULL; + virJSONValuePtr object = NULL; + virJSONValuePtr server = NULL; + char *ret = NULL; + size_t i; + + if (!(file = virJSONValueNewObject())) + return NULL; + + /* 1. prepare { driver:"gluster", volume:"testvol", path:"/a.img"} */ + if (virJSONValueObjectAdd(file, + "s:driver", virStorageNetProtocolTypeToString(src->protocol), + "s:volume", src->volume, + "s:path", src->path, + NULL) < 0) + goto cleanup; + + if (!(server = virJSONValueNewArray())) + goto cleanup; + + /* 2. prepare array [ {type:"tcp", host:"1.2.3.4", port:24007}, + * {type:"unix", socket:"/tmp/glusterd.socket"}, + * {}, ... ] */ + for (i = 0; i < src->nhosts; i++) { + if (!(object = virJSONValueNewObject())) + goto cleanup; + if (src->hosts[i].transport == 0) { + if (virJSONValueObjectAdd(object, + "s:type", + virStorageNetHostTransportTypeToString(src->hosts[i].transport), + "s:host", src->hosts[i].name, + "i:port", + qemuNetworkDriveGetPort(src->protocol, src->hosts[i].port), + NULL) < 0) + goto cleanup; + } else if (src->hosts[i].transport == 1) { + if (virJSONValueObjectAdd(object, + "s:type", + virStorageNetHostTransportTypeToString(src->hosts[i].transport), + "s:socket", src->hosts[i].socket, + NULL) < 0) + goto cleanup; + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("protocol '%s' accepts only tranport tcp|unix"), + virStorageNetProtocolTypeToString(src->protocol)); + goto cleanup; + } + + if (virJSONValueArrayAppend(server, object) < 0) + goto cleanup; + } + + /* 3. append 1 and 2 + * { driver:"gluster", volume:"testvol", path:"/a.img", + * server :[ {type:"tcp", host:"1.2.3.4", port:24007}, + * {type:"unix", socket:"/tmp/glusterd.socket"}, + * {}, ... ] } + */ + if (virJSONValueObjectAppend(file, "server", server) < 0) + goto cleanup; + + /* 4. assign key to 3 + * { file: { driver:"gluster", volume:"testvol", path:"/a.img", + * server :[ {type:"tcp", host:"1.2.3.4", port:24007}, + * {type:"unix", socket:"/tmp/glusterd.socket"}, + * {}, ... ] } } + */ + if (!(object = virJSONValueNewObject())) + goto cleanup; + if (virJSONValueObjectAppend(object, "file", file) < 0) + goto cleanup; + + /* 5. append format to 4 + * { file: { driver:"gluster", volume:"testvol", path:"/a.img", + * server :[ {type:"tcp", host:"1.2.3.4", port:24007}, + * {type:"unix", socket:"/tmp/glusterd.socket"}, + * {}, ... ] }, driver:"qcow2" } + */ + if (virJSONValueObjectAdd(object, + "s:driver", virStorageFileFormatTypeToString(src->format), + NULL) < 0) + goto cleanup; + + if (virAsprintf(&ret, "json:%s", virJSONValueToString(object, false)) < 0) + ret = NULL; + + return ret; + + cleanup: + virJSONValueFree(server); + virJSONValueFree(file); + virJSONValueFree(object); + + return ret; +} + #define QEMU_DEFAULT_NBD_PORT "10809" static char * -qemuBuildNetworkDriveURI(virStorageSourcePtr src, +qemuBuildNetworkDriveStr(virStorageSourcePtr src, qemuDomainSecretInfoPtr secinfo) { char *ret = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; - virURIPtr uri = NULL; size_t i; switch ((virStorageNetProtocol) src->protocol) { @@ -758,57 +916,29 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src, case VIR_STORAGE_NET_PROTOCOL_FTPS: case VIR_STORAGE_NET_PROTOCOL_TFTP: case VIR_STORAGE_NET_PROTOCOL_ISCSI: - case VIR_STORAGE_NET_PROTOCOL_GLUSTER: - if (src->nhosts != 1) { + if (src->nhosts == 1) { + if (!(ret = qemuBuildNetworkDriveURI(src, secinfo))) + goto cleanup; + } else { virReportError(VIR_ERR_INTERNAL_ERROR, _("protocol '%s' accepts only one host"), virStorageNetProtocolTypeToString(src->protocol)); goto cleanup; } - if (VIR_ALLOC(uri) < 0) - goto cleanup; + break; - if (src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_TCP) { - if (VIR_STRDUP(uri->scheme, - virStorageNetProtocolTypeToString(src->protocol)) < 0) + case VIR_STORAGE_NET_PROTOCOL_GLUSTER: + if (src->nhosts == 1) { + /* URI formatted syntax */ + if (!(ret = qemuBuildNetworkDriveURI(src, secinfo))) goto cleanup; } else { - if (virAsprintf(&uri->scheme, "%s+%s", - virStorageNetProtocolTypeToString(src->protocol), - virStorageNetHostTransportTypeToString(src->hosts->transport)) < 0) + /* switch to new json formatted syntax */ + if (!(ret = qemuBuildGlusterDriveJSON(src))) goto cleanup; } - if ((uri->port = qemuNetworkDriveGetPort(src->protocol, - src->hosts->port)) < 0) - goto cleanup; - - if (src->path) { - if (src->volume) { - if (virAsprintf(&uri->path, "/%s%s", - src->volume, src->path) < 0) - goto cleanup; - } else { - if (virAsprintf(&uri->path, "%s%s", - src->path[0] == '/' ? "" : "/", - src->path) < 0) - goto cleanup; - } - } - - if (src->hosts->socket && - virAsprintf(&uri->query, "socket=%s", src->hosts->socket) < 0) - goto cleanup; - - if (qemuBuildGeneralSecinfoURI(uri, secinfo) < 0) - goto cleanup; - - if (VIR_STRDUP(uri->server, src->hosts->name) < 0) - goto cleanup; - - ret = virURIFormat(uri); - break; case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: @@ -889,7 +1019,6 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src, cleanup: virBufferFreeAndReset(&buf); - virURIFree(uri); return ret; } @@ -919,7 +1048,7 @@ qemuGetDriveSourceString(virStorageSourcePtr src, break; case VIR_STORAGE_TYPE_NETWORK: - if (!(*source = qemuBuildNetworkDriveURI(src, secinfo))) + if (!(*source = qemuBuildNetworkDriveStr(src, secinfo))) goto cleanup; break; @@ -4527,7 +4656,7 @@ qemuBuildSCSIiSCSIHostdevDrvStr(virDomainHostdevDefPtr dev) src.nhosts = iscsisrc->nhosts; /* Rather than pull what we think we want - use the network disk code */ - source = qemuBuildNetworkDriveURI(&src, hostdevPriv->secinfo); + source = qemuBuildNetworkDriveStr(&src, hostdevPriv->secinfo); return source; } -- 2.7.4
On Fri, Jul 22, 2016 at 01:50:30PM +0530, Prasanna Kumar Kalever wrote:
This patch adds support for gluster specific JSON formatter functionality
currently libvirt has the capability to parse only one host and convert that into URI formatted string, with the help of this patch libvirt will be able to parse multiple hosts from the domain xml and can convert that into JSON formatted string
if the number of hosts supplied is only one, then we use existing URI format for backward compatibility and if number of hosts is greater than one we switch to the new JSON format
before: ------ example.xml: ... <disk type='network' device='disk'> <driver name='qemu' type='qcow2' cache='none'/> <source protocol='gluster' name='testvol/a.qcow2'> <host name='1.2.3.4' port='24007' transport="tcp"/> </source> <target dev='vda' bus='virtio'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </disk> ...
resultant string: file=gluster://1.2.3.4:24007/testvol/a.qcow2, \ if=none,id=drive-virtio-disk0,format=qcow2,cache=none
after: ----- example.xml: ... <disk type='network' device='disk'> <driver name='qemu' type='qcow2' cache='none'/> <source protocol='gluster' name='testvol/a.qcow2'> <host name='1.2.3.4' port='24009' transport="tcp"/> <host socket='/var/run/glusterd.socket' transport="unix"/> </source> <target dev='vda' bus='virtio'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </disk> ...
resultant string: -drive file=json:{ "file": { "driver": "gluster",, "volume": "testvol",, "path": "/a.qcow2",, "server": [ { "type": "tcp" "host": "1.2.3.4",, "port": 24009,, },, { "transport": "unix" "socket": "/var/run/glusterd.socket", } ] },, "driver": "qcow2" } ,if=none,id=drive-virtio-disk0,cache=none
Ewww, we really don't want to be using the json syntax on the command line. Please use the normal property syntax instead. The JSON syntax is only appropriate for use with QMPP for hotplug.
credits: sincere thanks to all the supporters
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com> --- src/qemu/qemu_command.c | 219 ++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 174 insertions(+), 45 deletions(-)
This is missing test cases to validate the new syntax. See tests/qemuxml2argvtest.c & related data files. 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 :|
This patch adds support for gluster specific JSON parser functionality This will help in parsing the backing store which uses JSON syntax and update the meta-data in the domain specific objects while taking snapshots which inturn helps in successful creation/updation of backing store information in domain xml Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com> --- src/util/virstoragefile.c | 208 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 208 insertions(+) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 16de603..033d6a2 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -41,10 +41,12 @@ #include "virstring.h" #include "virutil.h" #include "viruri.h" +#include "virjson.h" #include "dirname.h" #include "virbuffer.h" #define VIR_FROM_THIS VIR_FROM_STORAGE +#define DEFAULT_GLUSTER_PORT 24007 VIR_LOG_INIT("util.storagefile"); @@ -2126,6 +2128,209 @@ virStorageSourceNewFromBackingRelative(virStorageSourcePtr parent, goto cleanup; } +#if WITH_STORAGE_GLUSTER +static int +virStorageSourceParseBackingJSON(virStorageSourcePtr src, + const char *path) +{ + virJSONValuePtr json = NULL; + virJSONValuePtr file = NULL; + virJSONValuePtr server = NULL; + virJSONValuePtr object = NULL; + const char *str = NULL; + int port = 0; + size_t i = 0; + int ret = -1; + + /* An example of gluster json formatted string containing two servers: + * + * json:{ file: { driver:"gluster", volume:"testvol", path:"/a.img", + * server :[ {type:"tcp", host:"1.2.3.4", port:24007}, + * {type:"unix", socket:"/tmp/glusterd.socket"}, + * ] }, driver:"qcow2" } + */ + + /* Get 'json:' object */ + json = virJSONValueFromString(path+strlen("json:")); + + /* Get Image file format */ + if (virJSONValueObjectHasKey(json, "driver")) { + if (!(str = virJSONValueObjectGetString(json, "driver"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to get String from key 'driver'")); + goto error; + } + if ((src->format = virStorageFileFormatTypeFromString(str)) <= 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to get Format type of key 'driver'")); + goto error; + } + } else { + src->format = 1; /* default to raw */ + } + + /* Get 'file:' object */ + if (virJSONValueObjectHasKey(json, "file")) { + if (!(file = virJSONValueObjectGet(json, "file"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to get Object from key 'file'")); + goto error; + } + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("No Key 'file'")); + goto error; + } + + /* Get 'protocol' Info */ + if (virJSONValueObjectHasKey(file, "driver")) { + if (!(str = virJSONValueObjectGetString(file, "driver"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to get String from key 'driver'")); + goto error; + } + if ((src->protocol = virStorageNetProtocolTypeFromString(str)) <= 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to get Protocol type of key 'driver'")); + goto error; + } + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("No protocol Key 'driver'")); + goto error; + } + + /* Get 'volume' name Info */ + if (virJSONValueObjectHasKey(file, "volume")) { + if (!(str = virJSONValueObjectGetString(file, "volume"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to get String from key 'volume'")); + goto error; + } + if (VIR_STRDUP(src->volume, str) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Falied to duplicate value of key 'volume'")); + goto error; + } + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("No Key 'volume'")); + goto error; + } + + /* Get 'path' info of Image*/ + if (virJSONValueObjectHasKey(file, "path")) { + if (!(str = virJSONValueObjectGetString(file, "path"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to get String from key 'path'")); + goto error; + } + if (VIR_STRDUP(src->path, str) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Falied to duplicate value of key 'path'")); + goto error; + } + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("No Key 'path'")); + goto error; + } + + /* Get 'server:' object */ + if (virJSONValueObjectHasKey(file, "server")) { + if (!(server = virJSONValueObjectGetArray(file, "server"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to get Array Object from key 'file'")); + goto error; + } + if (!(src->nhosts = virJSONValueArraySize(server))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to get Size of Array from key 'server'")); + goto error; + } + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("No Key 'server'")); + goto error; + } + + /* null-terminated list */ + if (VIR_ALLOC_N(src->hosts, src->nhosts + 1) < 0) + goto error; + + /* Parse 'server' object for { type, host, port, socket} */ + for (i = 0; i < src->nhosts; i++) { + object = virJSONValueArrayGet(server, i); + if (virJSONValueObjectHasKey(object, "type")) { + if (!(str = virJSONValueObjectGetString(object, "type"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to get String from key 'type'")); + goto error; + } + if ((src->hosts[i].transport = virStorageNetHostTransportTypeFromString(str)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to get Transport from key 'type'")); + } + } else { + src->hosts[i].transport = VIR_STORAGE_NET_HOST_TRANS_TCP; + } + + if (src->hosts[i].transport == VIR_STORAGE_NET_HOST_TRANS_UNIX) { + if (virJSONValueObjectHasKey(object, "socket")) { + if (!(str = virJSONValueObjectGetString(object, "socket"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to get String from key 'socket'")); + goto error; + } + if (VIR_STRDUP(src->hosts[i].socket, str) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Falied to duplicate value of key 'socket'")); + goto error; + } + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("No Key 'socket'")); + goto error; + } + } else { /* tcp */ + if (virJSONValueObjectHasKey(object, "host")) { + if (!(str = virJSONValueObjectGetString(object, "host"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to get String from key 'host'")); + goto error; + } + if (VIR_STRDUP(src->hosts[i].name, str) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Falied to duplicate value of key 'host'")); + goto error; + } + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("No Key 'host'")); + goto error; + } + + if (virJSONValueObjectHasKey(object, "port")) { + if (virJSONValueObjectGetNumberInt(object, "port", &port) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to get NumberInt from key 'port'")); + goto error; + } + if (virAsprintf(&src->hosts[i].port, "%d", port) < 0) + goto error; + } else { + if (virAsprintf(&src->hosts[i].port, "%d", DEFAULT_GLUSTER_PORT) < 0) + goto error; + } + } + } + + ret = 0; + + error: + virJSONValueFree(object); + virJSONValueFree(server); + virJSONValueFree(file); + virJSONValueFree(json); + + return ret; +} + +#endif /* WITH_STORAGE_GLUSTER */ static int virStorageSourceParseBackingURI(virStorageSourcePtr src, @@ -2534,6 +2739,9 @@ virStorageSourceNewFromBackingAbsolute(const char *path) if (strstr(path, "://")) { if (virStorageSourceParseBackingURI(ret, path) < 0) goto error; + } else if (strstr(path, "json:")) { + if (virStorageSourceParseBackingJSON(ret, path) < 0) + goto error; } else { if (virStorageSourceParseBackingColon(ret, path) < 0) goto error; -- 2.7.4
On Fri, Jul 22, 2016 at 13:50:28 +0530, Prasanna Kumar Kalever wrote:
These series of patches are rebased on latest master; The dependent QEMU patches are now merged on master targeting for 2.7
I actually started implementing this already. The precursor for this series is the JSON backing store parser [1]. Since the backing store parser you've posted here is incomplete and lacking some changes that I've requested in the review of the last version (mostly that it will fail to compile without gluster enabled) I will post my own implementation of that. I'll try to adapt your series on top of the code that I've posted earlier and a few refactors that I've not yet posted. Peter [1] https://www.redhat.com/archives/libvir-list/2016-July/msg00563.html
On Fri, Jul 22, 2016 at 2:03 PM, Peter Krempa <pkrempa@redhat.com> wrote:
On Fri, Jul 22, 2016 at 13:50:28 +0530, Prasanna Kumar Kalever wrote:
These series of patches are rebased on latest master; The dependent QEMU patches are now merged on master targeting for 2.7
I actually started implementing this already. The precursor for this series is the JSON backing store parser [1].
I remember you comment on my v2 series saying "we should wait until qemu design settles ?" So I have focused on them first, since they got merged on master yesterday, I have resumed this patches :)
Since the backing store parser you've posted here is incomplete and lacking some changes that I've requested in the review of the last version (mostly that it will fail to compile without gluster enabled) I will post my own implementation of that.
I'll try to adapt your series on top of the code that I've posted earlier and a few refactors that I've not yet posted.
From your patches provided at [1] I have noticed,
+static int +virStorageSourceParseBackingJSONGluster(virStorageSourcePtr src, + virJSONValuePtr json, [...] + + /* gluster currently supports only URI syntax passed in as filename */ + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("missing gluster URI in JSON backing volume definition")); + + return -1; +} So this just has the skeleton atleast for now, I think here our v3 patches should fit-in and do their job of JSON parsing and formatting IMO we already have most of the gluster work done here. Let me know when you are expecting [1] go in, I shall wait for your patch series. And will be happy to help you with a rebase of gluster patches on top of your pseudo JSON code. What do you think ? [1] https://www.redhat.com/archives/libvir-list/2016-July/msg00563.html Thanks Peter! -- Prasanna
Peter
[1] https://www.redhat.com/archives/libvir-list/2016-July/msg00563.html
On Fri, Jul 22, 2016 at 15:14:47 +0530, Prasanna Kalever wrote:
On Fri, Jul 22, 2016 at 2:03 PM, Peter Krempa <pkrempa@redhat.com> wrote:
On Fri, Jul 22, 2016 at 13:50:28 +0530, Prasanna Kumar Kalever wrote:
These series of patches are rebased on latest master; The dependent QEMU patches are now merged on master targeting for 2.7
I actually started implementing this already. The precursor for this series is the JSON backing store parser [1].
I remember you comment on my v2 series saying "we should wait until qemu design settles ?" So I have focused on them first, since they got merged on master yesterday, I have resumed this patches :)
Since the backing store parser you've posted here is incomplete and lacking some changes that I've requested in the review of the last version (mostly that it will fail to compile without gluster enabled) I will post my own implementation of that.
I'll try to adapt your series on top of the code that I've posted earlier and a few refactors that I've not yet posted.
From your patches provided at [1] I have noticed,
+static int +virStorageSourceParseBackingJSONGluster(virStorageSourcePtr src, + virJSONValuePtr json,
[...]
+ + /* gluster currently supports only URI syntax passed in as filename */ + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("missing gluster URI in JSON backing volume definition")); + + return -1; +}
So this just has the skeleton atleast for now,
Yep. I've posted that before the qemu code settled.
I think here our v3 patches should fit-in and do their job of JSON parsing and formatting IMO we already have most of the gluster work done here.
I wrote the patch on top of that series adding the JSON parser for qemu already yesterday.
Let me know when you are expecting [1] go in, I shall wait for your patch series.
I'll actually take some bits of your series and try to integrate it.
And will be happy to help you with a rebase of gluster patches on top of your pseudo JSON code.
It won't be necessary as I've said since I've already done a different patch for that. I'm planing of taking parts of 2/3 and using some stuff since as DanPB pointed out the command line format isn't really what we should use. I already wrote a json to commandline formatter for memory hotplug so I plan to fix it for use for disk commands too. I'll cc you on the stuff. Peter
participants (4)
-
Daniel P. Berrange -
Peter Krempa -
Prasanna Kalever -
Prasanna Kumar Kalever