On Thu, Nov 22, 2012 at 23:40:39 +0530, Harsh Prateek Bora wrote:
> Qemu accepts gluster protocol as supported storage backend beside others.
> This patch allows users to specify disks on gluster backends like this:
>
> <disk type='network' device='disk'>
> <driver name='qemu' type='raw'/>
> <source protocol='gluster' name='Volume1/image'>
> <host name='example.org' port='6000' transport='tcp'/>
> </source>
> <target dev='vda' bus='virtio'/>
> </disk>
>
> <disk type='network' device='disk'>
> <driver name='qemu' type='raw'/>
> <source protocol='gluster' name='Volume2/image'>
> <host transport='unix' socket='/path/to/sock'/>
> </source>
> <target dev='vdb' bus='virtio'/>
> </disk>
Move these XML examples to 1/4.
>
> Signed-off-by: Harsh Prateek Bora <harsh@xxxxxxxxxxxxxxxxxx>
> ---
> src/qemu/qemu_command.c | 184 +++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 183 insertions(+), 1 deletion(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 20730a9..a377744 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -2021,6 +2021,156 @@ no_memory:
> return -1;
> }
>
> +static int
> +qemuParseGlusterString(virDomainDiskDefPtr def)
> +{
> + int ret = 0;
> + char *transp = NULL;
> + char *sock = NULL;
> + char *volimg = NULL;
> + virURIPtr uri = NULL;
> + if (!(uri = virURIParse(def->src))) {
> + return -1;
> + }
> +
> + if (VIR_ALLOC(def->hosts) < 0) {
> + ret = -1;
In libvirt, we rather initialize ret = -1 and set it to 0 at the end of the
function when we know it succeeded. That way you don't have to remember to set
ret = -1 on every error.
> + goto no_memory;
> + }
> +
> + if (STREQ(uri->scheme, "gluster")) {
> + def->hosts->transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP;
> + } else if (STRPREFIX(uri->scheme, "gluster+")) {
> + transp = strstr(uri->scheme, "+") + 1;
> + def->hosts->transport = virDomainDiskProtocolTransportTypeFromString(transp);
> + if (def->hosts->transport < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Invalid gluster transport type '%s'"), transp);
> + ret = -1;
> + goto cleanup;
> +
> + }
> + } else {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Invalid transport/scheme '%s'"), uri->scheme);
> + ret = -1;
> + goto cleanup;
> + }
> + def->nhosts = 0; /* set to 1 once everything succeeds */
> +
> + if (def->hosts->transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {
> + def->hosts->name = strdup(uri->server);
> + if (!def->hosts->name) {
> + ret = -1;
> + goto no_memory;
> + }
> +
> + if (virAsprintf(&def->hosts->port, "%d", uri->port) < 0) {
> + ret = -1;
> + goto no_memory;
> + }
> + } else {
> + def->hosts->name = NULL;
> + def->hosts->port = 0;
> + if(uri->query) {
> + if(STRPREFIX(uri->query, "socket=")) {
make syntax-check requires space between "if" and "(".
> + sock = strstr(uri->query, "=") + 1;
> + def->hosts->socket = strdup(sock);
> + if (!def->hosts->socket) {
> + ret = -1;
> + goto no_memory;
> + }
> + } else {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Invalid query parameter '%s'"), uri->query);
And this is the evidence how easy it is to forget ret = -1 :-)
> + goto cleanup;
> + }
> + }
> + }
> + volimg = uri->path + 1; /* skip the prefix slash */
> + def->src = strdup(volimg);
> + if (!def->src) {
> + ret = -1;
> + goto no_memory;
> + }
> +
> + def->nhosts = 1;
> + VIR_FREE(uri);
> + return ret;
> +
> +no_memory:
> + virReportOOMError();
> +cleanup:
"cleanup" label is generally used for code which is common to both success and
error paths. Use "error" label for error-path-only code.
> + VIR_FREE(def->hosts);
Before freeing def->hosts, all strings referenced from it need to be freed by
calling virDomainDiskHostDefFree.
> + VIR_FREE(uri);
> +
> + return ret;
> +}
> +
> +static int
> +qemuBuildGlusterString(virDomainDiskDefPtr disk, virBufferPtr opt)
> +{
> + int ret = 0, port = 0;
Initialize ret to -1.
> + char *tmpscheme = NULL;
> + char *volimg = NULL;
> + char *sock = NULL;
> + char *builturi = NULL;
> + const char *transp = NULL;
> + virURI uri = {
> + .port = port /* just to clear rest of bits */
> + };
> +
> + if (disk->nhosts != 1) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("gluster accepts only one host"));
> + return -1;
> + }
> +
> + virBufferAddLit(opt, "file=");
> + transp = virDomainDiskProtocolTransportTypeToString(disk->hosts->transport);
> +
> + if (virAsprintf(&tmpscheme, "gluster+%s", transp) < 0) {
> + ret = -1;
> + goto no_memory;
> + }
> +
> + if (virAsprintf(&volimg, "/%s", disk->src) < 0) {
> + ret = -1;
> + goto no_memory;
> + }
> +
> + if (disk->hosts->port) {
> + port = atoi(disk->hosts->port);
> + }
> +
> + if (disk->hosts->socket) {
> + if (virAsprintf(&sock, "socket=%s", disk->hosts->socket) < 0) {
> + ret = -1;
> + goto no_memory;
> + }
> + }
> +
> + uri.scheme = tmpscheme; /* gluster+<transport> */
> + uri.server = disk->hosts->name;
> + uri.port = port;
> + uri.path = volimg;
> + uri.query = sock;
> +
> + builturi = virURIFormat(&uri);
> + virBufferEscape(opt, ',', ",", "%s", builturi);
> + goto cleanup;
> +
> +no_memory:
> + virReportOOMError();
> +cleanup:
> + VIR_FREE(builturi);
> + VIR_FREE(tmpscheme);
> + VIR_FREE(volimg);
> + VIR_FREE(sock);
> +
> + return ret;
We like to keep the success path linear...
> +}
> +
> char *
> qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
> virDomainDiskDefPtr disk,
> @@ -2162,6 +2312,12 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
> goto error;
> virBufferAddChar(&opt, ',');
> break;
> + case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER:
> + if (qemuBuildGlusterString(disk, &opt) < 0)
> + goto error;
> + virBufferAddChar(&opt, ',');
> + break;
> +
> case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG:
> if (disk->nhosts == 0) {
> virBufferEscape(&opt, ',', ",", "file=sheepdog:%s,",
> @@ -5242,6 +5398,18 @@ qemuBuildCommandLine(virConnectPtr conn,
> file = virBufferContentAndReset(&opt);
> }
> break;
> + case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER:
> + {
> + virBuffer opt = VIR_BUFFER_INITIALIZER;
> + if (qemuBuildGlusterString(disk, &opt) < 0)
> + goto error;
> + if (virBufferError(&opt)) {
> + virReportOOMError();
> + goto error;
> + }
> + file = virBufferContentAndReset(&opt);
> + }
> + break;
> case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG:
> if (disk->nhosts == 0) {
> if (virAsprintf(&file, "sheepdog:%s,", disk->src) < 0) {
> @@ -6919,7 +7087,6 @@ qemuParseCommandLineDisk(virCapsPtr caps,
> virReportOOMError();
> goto cleanup;
> }
> -
> VIR_FREE(def->src);
> def->src = NULL;
> } else if (STRPREFIX(def->src, "rbd:")) {
Unrelated whitespace change.
> @@ -6937,6 +7104,12 @@ qemuParseCommandLineDisk(virCapsPtr caps,
> goto cleanup;
>
> VIR_FREE(p);
> + } else if (STRPREFIX(def->src, "gluster")) {
> + def->type = VIR_DOMAIN_DISK_TYPE_NETWORK;
> + def->protocol = VIR_DOMAIN_DISK_PROTOCOL_GLUSTER;
> +
> + if (qemuParseGlusterString(def) < 0)
> + goto cleanup;
> } else if (STRPREFIX(def->src, "sheepdog:")) {
> char *p = def->src;
> char *port, *vdi;
> @@ -6972,6 +7145,7 @@ qemuParseCommandLineDisk(virCapsPtr caps,
> virReportOOMError();
> goto cleanup;
> }
> +
> def->src = strdup(vdi);
> if (!def->src) {
> virReportOOMError();
Unrelated.
> @@ -8126,6 +8300,9 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
> disk->type = VIR_DOMAIN_DISK_TYPE_NETWORK;
> disk->protocol = VIR_DOMAIN_DISK_PROTOCOL_RBD;
> val += strlen("rbd:");
> + } else if (STRPREFIX(val, "gluster")) {
> + disk->type = VIR_DOMAIN_DISK_TYPE_NETWORK;
> + disk->protocol = VIR_DOMAIN_DISK_PROTOCOL_GLUSTER;
> } else if (STRPREFIX(val, "sheepdog:")) {
> disk->type = VIR_DOMAIN_DISK_TYPE_NETWORK;
> disk->protocol = VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG;
> @@ -8211,6 +8388,11 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
> goto no_memory;
> }
> break;
> + case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER:
> + if (qemuParseGlusterString(disk) < 0)
> + goto error;
> +
> + break;
> }
> }
>
ACK with the following patch squashed in:
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 886347c..63e187a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -346,6 +346,7 @@ virDomainDiskErrorPolicyTypeToString;
virDomainDiskFindControllerModel;
virDomainDiskGeometryTransTypeFromString;
virDomainDiskGeometryTransTypeToString;
+virDomainDiskHostDefFree;
virDomainDiskIndexByName;
virDomainDiskInsert;
virDomainDiskInsertPreAlloced;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 50693a9..e946336 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2043,93 +2043,85 @@ no_memory:
static int
qemuParseGlusterString(virDomainDiskDefPtr def)
{
- int ret = 0;
+ int ret = -1;
char *transp = NULL;
char *sock = NULL;
char *volimg = NULL;
virURIPtr uri = NULL;
+
if (!(uri = virURIParse(def->src))) {
return -1;
}
- if (VIR_ALLOC(def->hosts) < 0) {
- ret = -1;
+ if (VIR_ALLOC(def->hosts) < 0)
goto no_memory;
- }
if (STREQ(uri->scheme, "gluster")) {
def->hosts->transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP;
} else if (STRPREFIX(uri->scheme, "gluster+")) {
- transp = strstr(uri->scheme, "+") + 1;
+ transp = strchr(uri->scheme, '+') + 1;
def->hosts->transport = virDomainDiskProtocolTransportTypeFromString(transp);
if (def->hosts->transport < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Invalid gluster transport type '%s'"), transp);
- ret = -1;
- goto cleanup;
-
+ goto error;
}
} else {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Invalid transport/scheme '%s'"), uri->scheme);
- ret = -1;
- goto cleanup;
+ goto error;
}
def->nhosts = 0; /* set to 1 once everything succeeds */
if (def->hosts->transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {
def->hosts->name = strdup(uri->server);
- if (!def->hosts->name) {
- ret = -1;
+ if (!def->hosts->name)
goto no_memory;
- }
- if (virAsprintf(&def->hosts->port, "%d", uri->port) < 0) {
- ret = -1;
+ if (virAsprintf(&def->hosts->port, "%d", uri->port) < 0)
goto no_memory;
- }
} else {
def->hosts->name = NULL;
def->hosts->port = 0;
- if(uri->query) {
- if(STRPREFIX(uri->query, "socket=")) {
- sock = strstr(uri->query, "=") + 1;
+ if (uri->query) {
+ if (STRPREFIX(uri->query, "socket=")) {
+ sock = strchr(uri->query, '=') + 1;
def->hosts->socket = strdup(sock);
- if (!def->hosts->socket) {
- ret = -1;
+ if (!def->hosts->socket)
goto no_memory;
- }
} else {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Invalid query parameter '%s'"), uri->query);
- goto cleanup;
+ goto error;
}
}
}
volimg = uri->path + 1; /* skip the prefix slash */
def->src = strdup(volimg);
- if (!def->src) {
- ret = -1;
+ if (!def->src)
goto no_memory;
- }
def->nhosts = 1;
- VIR_FREE(uri);
+ ret = 0;
+
+cleanup:
+ virURIFree(uri);
+
return ret;
no_memory:
virReportOOMError();
-cleanup:
+error:
+ virDomainDiskHostDefFree(def->hosts);
VIR_FREE(def->hosts);
- VIR_FREE(uri);
-
- return ret;
+ goto cleanup;
}
static int
qemuBuildGlusterString(virDomainDiskDefPtr disk, virBufferPtr opt)
{
- int ret = 0, port = 0;
+ int ret = -1;
+ int port = 0;
char *tmpscheme = NULL;
char *volimg = NULL;
char *sock = NULL;
@@ -2148,26 +2140,19 @@ qemuBuildGlusterString(virDomainDiskDefPtr disk, virBufferPtr opt)
virBufferAddLit(opt, "file=");
transp = virDomainDiskProtocolTransportTypeToString(disk->hosts->transport);
- if (virAsprintf(&tmpscheme, "gluster+%s", transp) < 0) {
- ret = -1;
+ if (virAsprintf(&tmpscheme, "gluster+%s", transp) < 0)
goto no_memory;
- }
- if (virAsprintf(&volimg, "/%s", disk->src) < 0) {
- ret = -1;
+ if (virAsprintf(&volimg, "/%s", disk->src) < 0)
goto no_memory;
- }
if (disk->hosts->port) {
port = atoi(disk->hosts->port);
}
- if (disk->hosts->socket) {
- if (virAsprintf(&sock, "socket=%s", disk->hosts->socket) < 0) {
- ret = -1;
- goto no_memory;
- }
- }
+ if (disk->hosts->socket &&
+ virAsprintf(&sock, "socket=%s", disk->hosts->socket) < 0)
+ goto no_memory;
uri.scheme = tmpscheme; /* gluster+<transport> */
uri.server = disk->hosts->name;
@@ -2177,10 +2162,9 @@ qemuBuildGlusterString(virDomainDiskDefPtr disk, virBufferPtr opt)
builturi = virURIFormat(&uri);
virBufferEscape(opt, ',', ",", "%s", builturi);
- goto cleanup;
-no_memory:
- virReportOOMError();
+ ret = 0;
+
cleanup:
VIR_FREE(builturi);
VIR_FREE(tmpscheme);
@@ -2188,6 +2172,10 @@ cleanup:
VIR_FREE(sock);
return ret;
+
+no_memory:
+ virReportOOMError();
+ goto cleanup;
}
char *
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list