Re: [libvirt] [PATCH 03/13] qemu: rewrite NBD command-line builder and parser
On 03/11/2013 07:18 AM, Paolo Bonzini wrote:
>
> + h->name = strdup(host);
> + if (!h->name)
Trivial, but we prefer:
if (!(h->name = strdup(host)))
Personally, I don't care enough to rewrite it - adding parenthesis to
save a line isn't documented in HACKING as a requirement.
>> + 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?
Your choice.
A macro seems nice, so I added one:
diff --git i/src/qemu/qemu_command.c w/src/qemu/qemu_command.c
index 7232906..9e2d6a9 100644
--- i/src/qemu/qemu_command.c
+++ w/src/qemu/qemu_command.c
@@ -2473,6 +2473,8 @@ no_memory:
goto cleanup;
}
+#define QEMU_DEFAULT_NBD_PORT "10809"
+
static int
qemuBuildNBDString(virDomainDiskDefPtr disk, virBufferPtr opt)
{
@@ -2491,7 +2493,8 @@ qemuBuildNBDString(virDomainDiskDefPtr disk,
virBufferPtr opt)
if (disk->hosts->name)
virBufferEscape(opt, ',', ",", "%s",
disk->hosts->name);
virBufferEscape(opt, ',', ",", ":%s",
- disk->hosts->port ? disk->hosts->port :
"10809");
+ disk->hosts->port ? disk->hosts->port :
+ QEMU_DEFAULT_NBD_PORT);
break;
default:
transp =
virDomainDiskProtocolTransportTypeToString(disk->hosts->transport);
On 03/15/2013 08:32 AM, Daniel P. Berrange wrote:
I would have had the 'parse' method further down near the other
parse function which calls it, but no big deal.
I didn't bother with this.
> +++ 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");
This test case addition could be pushed indepedently of the rest
of this patch.
I did just that, with this commit message:
commit b96e0c18e161d80c88af354ef952fe2b6013f20f
Author: Paolo Bonzini <pbonzini(a)redhat.com>
Date: Mon Feb 25 18:44:22 2013 +0100
qemu: test NBD command-line builder and parser
Enable more testing of NBD parsing, to ensure rewrites work.
Signed-off-by: Paolo Bonzini <pbonzini(a)redhat.com>
Signed-off-by: Eric Blake <eblake(a)redhat.com>
ACK
and pushed, as two patches.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 621 bytes)