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@xxxxxxxxxx>
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@xxxxxxxxxx>
Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>
>
> 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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list