[libvirt] [PATCH] util: storagefile: Properly set transport type when parsing NBD strings
When parsing legacy NBD backing file strings such as 'nbd:unix:/tmp/sock:exportname=/' we'd fail to set the transport to VIR_STORAGE_NET_HOST_TRANS_UNIX. This started to be a problem once we actually started to generate config of the backing store on the command line with -blockdev as the JSON code would try to format it as TCP and fail with: internal error: argument key 'host' must not have null value Set the type properly and add a test. This bug was found by the libguestfs test suite in: https://bugzilla.redhat.com/show_bug.cgi?id=1791614 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virstoragefile.c | 2 +- tests/virstoragetest.c | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 1397f532fd..7a2af0ad94 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2964,7 +2964,7 @@ virStorageSourceParseNBDColonString(const char *nbdstr, } src->hosts->socket = g_strdup(backing[2]); - + src->hosts->transport = VIR_STORAGE_NET_HOST_TRANS_UNIX; } else { src->hosts->name = g_strdup(backing[1]); diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 2862758752..370e19252b 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1258,6 +1258,10 @@ mymain(void) "<source protocol='nbd' name=':test'>\n" " <host name='example.org' port='6000'/>\n" "</source>\n"); + TEST_BACKING_PARSE("nbd:unix:/tmp/sock:exportname=/", + "<source protocol='nbd' name='/'>\n" + " <host transport='unix' socket='/tmp/sock'/>\n" + "</source>\n"); TEST_BACKING_PARSE("nbd://example.org:1234", "<source protocol='nbd'>\n" " <host name='example.org' port='1234'/>\n" -- 2.24.1
On 1/16/20 5:41 AM, Peter Krempa wrote:
When parsing legacy NBD backing file strings such as 'nbd:unix:/tmp/sock:exportname=/' we'd fail to set the transport to VIR_STORAGE_NET_HOST_TRANS_UNIX. This started to be a problem once we actually started to generate config of the backing store on the command line with -blockdev as the JSON code would try to format it as TCP and fail with:
internal error: argument key 'host' must not have null value
Set the type properly and add a test.
This bug was found by the libguestfs test suite in:
https://bugzilla.redhat.com/show_bug.cgi?id=1791614
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virstoragefile.c | 2 +- tests/virstoragetest.c | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-)
Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
On Thu, Jan 16, 2020 at 12:41:37PM +0100, Peter Krempa wrote:
When parsing legacy NBD backing file strings such as 'nbd:unix:/tmp/sock:exportname=/' we'd fail to set the transport to VIR_STORAGE_NET_HOST_TRANS_UNIX. This started to be a problem once we actually started to generate config of the backing store on the command line with -blockdev as the JSON code would try to format it as TCP and fail with:
internal error: argument key 'host' must not have null value
Set the type properly and add a test.
This bug was found by the libguestfs test suite in:
The bug was found by Ming Xie of the virt-v2v QE team. The patch itself looks fine as far as I can tell. But I'm having real problems actually testing it. Can you suggest any way to test libvirt parsing these URIs? My current method (which doesn't work for reasons that I don't understand) is: (1) Compile libvirt from source. (2) Run ./run src/libvirtd & (3) Create an NBD server + overlay file: rm /tmp/sock nbdkit -U /tmp/sock memory 1G qemu-img create -f qcow2 overlay.qcow2 -b nbd:unix:/tmp/sock:exportname=/ -F raw (4) Try to boot a libvirt guest using the overlay: virt-install --import --name test --disk path=overlay.qcow2,format=raw --memory 1024 But for some reason libvirt just ignores the overlay: 2020-01-16 13:00:44.980+0000: 2378711: warning : virStorageBackendVolOpen:1527 : ignoring missing file 'nbd:unix:/tmp/sock' I verified that the overlay works in programs like qemu-img so it doesn't seem to be a problem with the overlay itself. Rich.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virstoragefile.c | 2 +- tests/virstoragetest.c | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 1397f532fd..7a2af0ad94 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2964,7 +2964,7 @@ virStorageSourceParseNBDColonString(const char *nbdstr, }
src->hosts->socket = g_strdup(backing[2]); - + src->hosts->transport = VIR_STORAGE_NET_HOST_TRANS_UNIX; } else { src->hosts->name = g_strdup(backing[1]);
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 2862758752..370e19252b 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1258,6 +1258,10 @@ mymain(void) "<source protocol='nbd' name=':test'>\n" " <host name='example.org' port='6000'/>\n" "</source>\n"); + TEST_BACKING_PARSE("nbd:unix:/tmp/sock:exportname=/", + "<source protocol='nbd' name='/'>\n" + " <host transport='unix' socket='/tmp/sock'/>\n" + "</source>\n"); TEST_BACKING_PARSE("nbd://example.org:1234", "<source protocol='nbd'>\n" " <host name='example.org' port='1234'/>\n" -- 2.24.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Jan 16, 2020 at 01:13:38PM +0000, Richard W.M. Jones wrote:
On Thu, Jan 16, 2020 at 12:41:37PM +0100, Peter Krempa wrote:
When parsing legacy NBD backing file strings such as 'nbd:unix:/tmp/sock:exportname=/' we'd fail to set the transport to VIR_STORAGE_NET_HOST_TRANS_UNIX. This started to be a problem once we actually started to generate config of the backing store on the command line with -blockdev as the JSON code would try to format it as TCP and fail with:
internal error: argument key 'host' must not have null value
Set the type properly and add a test.
This bug was found by the libguestfs test suite in:
The bug was found by Ming Xie of the virt-v2v QE team.
The patch itself looks fine as far as I can tell. But I'm having real problems actually testing it. Can you suggest any way to test libvirt parsing these URIs?
My current method (which doesn't work for reasons that I don't understand) is:
(1) Compile libvirt from source.
(2) Run ./run src/libvirtd &
(3) Create an NBD server + overlay file:
rm /tmp/sock nbdkit -U /tmp/sock memory 1G qemu-img create -f qcow2 overlay.qcow2 -b nbd:unix:/tmp/sock:exportname=/ -F raw
(4) Try to boot a libvirt guest using the overlay:
virt-install --import --name test --disk path=overlay.qcow2,format=raw --memory 1024
But for some reason libvirt just ignores the overlay:
2020-01-16 13:00:44.980+0000: 2378711: warning : virStorageBackendVolOpen:1527 : ignoring missing file 'nbd:unix:/tmp/sock'
I verified that the overlay works in programs like qemu-img so it doesn't seem to be a problem with the overlay itself.
The problem was too old qemu. With qemu 4.2 I can verify that the patch is working, so: Tested-by: Richard W.M. Jones <rjones@redhat.com> Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
On Thu, Jan 16, 2020 at 13:13:38 +0000, Richard W.M. Jones wrote:
On Thu, Jan 16, 2020 at 12:41:37PM +0100, Peter Krempa wrote:
When parsing legacy NBD backing file strings such as 'nbd:unix:/tmp/sock:exportname=/' we'd fail to set the transport to VIR_STORAGE_NET_HOST_TRANS_UNIX. This started to be a problem once we actually started to generate config of the backing store on the command line with -blockdev as the JSON code would try to format it as TCP and fail with:
internal error: argument key 'host' must not have null value
Set the type properly and add a test.
This bug was found by the libguestfs test suite in:
The bug was found by Ming Xie of the virt-v2v QE team.
I'll add a Reported-by: Ming Xie <mxie@redhat.com>
The patch itself looks fine as far as I can tell. But I'm having real problems actually testing it. Can you suggest any way to test libvirt parsing these URIs?
My current method (which doesn't work for reasons that I don't understand) is:
(1) Compile libvirt from source.
(2) Run ./run src/libvirtd &
(3) Create an NBD server + overlay file:
rm /tmp/sock nbdkit -U /tmp/sock memory 1G qemu-img create -f qcow2 overlay.qcow2 -b nbd:unix:/tmp/sock:exportname=/ -F raw
(4) Try to boot a libvirt guest using the overlay:
virt-install --import --name test --disk path=overlay.qcow2,format=raw --memory 1024
But for some reason libvirt just ignores the overlay:
Do you also use qemu-4.2? The bug manifests itself only when -blockdev is used. That's why it was dormant at least since 2014. I did not bother looking further into the history. At any rate, if it's inconvenient to install qemu-4.2 you can force-enable blockdev for testing using: <domain type='kvm' xmlns:qemu='http://libvirt.org/schemas/domain/qemu/1.0'> ... <qemu:capabilities> <qemu:add capability='blockdev'/> </qemu:capabilities> </domain>
2020-01-16 13:00:44.980+0000: 2378711: warning : virStorageBackendVolOpen:1527 : ignoring missing file 'nbd:unix:/tmp/sock'
I verified that the overlay works in programs like qemu-img so it doesn't seem to be a problem with the overlay itself.
Rich.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virstoragefile.c | 2 +- tests/virstoragetest.c | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 1397f532fd..7a2af0ad94 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2964,7 +2964,7 @@ virStorageSourceParseNBDColonString(const char *nbdstr, }
src->hosts->socket = g_strdup(backing[2]); - + src->hosts->transport = VIR_STORAGE_NET_HOST_TRANS_UNIX; } else { src->hosts->name = g_strdup(backing[1]);
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 2862758752..370e19252b 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1258,6 +1258,10 @@ mymain(void) "<source protocol='nbd' name=':test'>\n" " <host name='example.org' port='6000'/>\n" "</source>\n"); + TEST_BACKING_PARSE("nbd:unix:/tmp/sock:exportname=/", + "<source protocol='nbd' name='/'>\n" + " <host transport='unix' socket='/tmp/sock'/>\n" + "</source>\n"); TEST_BACKING_PARSE("nbd://example.org:1234", "<source protocol='nbd'>\n" " <host name='example.org' port='1234'/>\n" -- 2.24.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (3)
-
Eric Blake -
Peter Krempa -
Richard W.M. Jones