On 2013年02月26日 01:44, Paolo Bonzini wrote:
Move the code to an external function, and structure it to prepare
the addition of new features in the next few patches.
Signed-off-by: Paolo Bonzini<pbonzini@xxxxxxxxxx>
---
src/qemu/qemu_command.c | 128 ++++++++++++++++++++++++++++--------------------
tests/qemuxml2xmltest.c | 1 +
2 files changed, 76 insertions(+), 53 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index a3c5a4e..beb7cfe 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2128,6 +2128,45 @@ error:
}
static int
+qemuParseNBDString(virDomainDiskDefPtr disk)
+{
+ virDomainDiskHostDefPtr h = NULL;
+ char *host, *port;
+
+ if (VIR_ALLOC(h)< 0)
+ goto no_memory;
+
+ host = disk->src + strlen("nbd:");
+ port = strchr(host, ':');
+ if (!port) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("cannot parse nbd filename '%s'"), disk->src);
+ goto error;
+ }
+
+ *port++ = '\0';
+ h->name = strdup(host);
+ if (!h->name)
Trivial, but we prefer:
if (!(h->name = strdup(host)))
+ goto no_memory;
+
+ h->port = strdup(port);
+ if (!h->port)
Likewise.
+ goto no_memory;
+
+ VIR_FREE(disk->src);
You lost setting defaults when refactoring:
- def->hosts->transport =
VIR_DOMAIN_DISK_PROTO_TRANS_TCP;
- def->hosts->socket = NULL;
+ disk->nhosts = 1;
+ disk->hosts = h;
+ return 0;
+
+no_memory:
+ virReportOOMError();
+error:
+ virDomainDiskHostDefFree(h);
+ VIR_FREE(h);
+ return -1;
+}
+
+static int
qemuBuildGlusterString(virDomainDiskDefPtr disk, virBufferPtr opt)
{
int ret = -1;
@@ -2188,6 +2227,36 @@ no_memory:
goto cleanup;
}
+static int
+qemuBuildNBDString(virDomainDiskDefPtr disk, virBufferPtr opt)
+{
+ const char *transp;
+
+ if (disk->nhosts != 1) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("nbd accepts only one host"));
+ return -1;
+ }
+
+ virBufferAddLit(opt, "file=nbd:");
+
+ switch (disk->hosts->transport) {
+ case VIR_DOMAIN_DISK_PROTO_TRANS_TCP:
+ if (disk->hosts->name)
+ virBufferEscape(opt, ',', ",", "%s", disk->hosts->name);
+ virBufferEscape(opt, ',', ",", ":%s",
+ disk->hosts->port ? disk->hosts->port : "10809");
What's the magic (10809)? It's not in the old code, I guess it's the
default port, but should we have a macro for it instead?
+ break;
+ default:
+ transp = virDomainDiskProtocolTransportTypeToString(disk->hosts->transport);
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("nbd does not support transport '%s'"), transp);
+ break;
+ }
+
+ return 0;
+}
+
char *
qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
virDomainDiskDefPtr disk,
@@ -2314,13 +2383,9 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
} else if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) {
switch (disk->protocol) {
case VIR_DOMAIN_DISK_PROTOCOL_NBD:
- if (disk->nhosts != 1) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("NBD accepts only one host"));
+ if (qemuBuildNBDString(disk,&opt)< 0)
goto error;
- }
- virBufferAsprintf(&opt, "file=nbd:%s:%s,",
- disk->hosts->name, disk->hosts->port);
+ virBufferAddChar(&opt, ',');
break;
case VIR_DOMAIN_DISK_PROTOCOL_RBD:
virBufferAddLit(&opt, "file=");
@@ -7337,39 +7402,11 @@ qemuParseCommandLineDisk(virCapsPtr qemuCaps,
if (STRPREFIX(def->src, "/dev/"))
def->type = VIR_DOMAIN_DISK_TYPE_BLOCK;
else if (STRPREFIX(def->src, "nbd:")) {
- char *host, *port;
-
def->type = VIR_DOMAIN_DISK_TYPE_NETWORK;
def->protocol = VIR_DOMAIN_DISK_PROTOCOL_NBD;
- host = def->src + strlen("nbd:");
- port = strchr(host, ':');
- if (!port) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("cannot parse nbd filename '%s'"),
- def->src);
- goto error;
- }
- *port++ = '\0';
- if (VIR_ALLOC(def->hosts)< 0) {
- virReportOOMError();
- goto error;
- }
- def->nhosts = 1;
- def->hosts->name = strdup(host);
- if (!def->hosts->name) {
- virReportOOMError();
- goto error;
- }
- def->hosts->port = strdup(port);
- if (!def->hosts->port) {
- virReportOOMError();
- goto error;
- }
- def->hosts->transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP;
- def->hosts->socket = NULL;
- VIR_FREE(def->src);
- def->src = NULL;
+ if (qemuParseNBDString(def)< 0)
+ goto error;
} else if (STRPREFIX(def->src, "rbd:")) {
char *p = def->src;
@@ -8599,7 +8636,6 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps,
else if (STRPREFIX(val, "nbd:")) {
disk->type = VIR_DOMAIN_DISK_TYPE_NETWORK;
disk->protocol = VIR_DOMAIN_DISK_PROTOCOL_NBD;
- val += strlen("nbd:");
} else if (STRPREFIX(val, "rbd:")) {
disk->type = VIR_DOMAIN_DISK_TYPE_NETWORK;
disk->protocol = VIR_DOMAIN_DISK_PROTOCOL_RBD;
@@ -8639,26 +8675,12 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps,
goto no_memory;
if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) {
- char *host, *port;
+ char *port;
switch (disk->protocol) {
case VIR_DOMAIN_DISK_PROTOCOL_NBD:
- host = disk->src;
- port = strchr(host, ':');
- if (!port) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("cannot parse nbd filename '%s'"), disk->src);
+ if (qemuParseNBDString(disk)< 0)
goto error;
- }
- *port++ = '\0';
- if (VIR_ALLOC(disk->hosts)< 0)
- goto no_memory;
- disk->nhosts = 1;
- disk->hosts->name = host;
- disk->hosts->port = strdup(port);
- if (!disk->hosts->port)
- goto no_memory;
- disk->src = NULL;
break;
case VIR_DOMAIN_DISK_PROTOCOL_RBD:
/* old-style CEPH_ARGS env variable is parsed later */
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 3f36896..2fa93a9 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -166,6 +166,7 @@ mymain(void)
DO_TEST("disk-drive-cache-v1-wt");
DO_TEST("disk-drive-cache-v1-wb");
DO_TEST("disk-drive-cache-v1-none");
+ DO_TEST("disk-drive-network-nbd");
DO_TEST("disk-scsi-device");
DO_TEST("disk-scsi-vscsi");
DO_TEST("disk-scsi-virtio-scsi");
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list