[libvirt] [PATCH 03/13] qemu: rewrite NBD command-line builder and parser
Osier Yang
jyang at redhat.com
Mon Mar 11 03:38:50 UTC 2013
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 at redhat.com>
> ---
> 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");
More information about the libvir-list
mailing list