Re: [PATCH] virStorageSourceParseNBDColonString: Rewrite to match what qemu does

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 4/24/20 7:04 AM, Peter Krempa wrote:
Our implementation wasn't quite able to parse everything that qemu does.
This patch rewrites the parser to a code that semantically resembles the
combination of 'nbd_parse_filename' and 'inet_parse' methods in qemu to
be able to parse the strings in an equivalent manner.

The only thing that libvirt doesn't do is to check the lenghts of

lengths

various components in the nbd string in places where qemu uses constant
size buffers.

The test cases validate that some of the corner cases involving colons
are parsed properly.

https://bugzilla.redhat.com/show_bug.cgi?id=1826652

Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx>
---
  src/util/virstoragefile.c | 90 +++++++++++++++++++++++----------------
  tests/virstoragetest.c    | 16 +++++++
  2 files changed, 70 insertions(+), 36 deletions(-)


+++ b/tests/virstoragetest.c
@@ -1261,10 +1261,26 @@ mymain(void)
                         "<source protocol='nbd' name=':test'>\n"
                         "  <host name='example.org' port='6000'/>\n"
                         "</source>\n");
+    TEST_BACKING_PARSE("nbd:[::1]:6000:exportname=:test",
+                       "<source protocol='nbd' name=':test'>\n"
+                       "  <host name='::1' port='6000'/>\n"
+                       "</source>\n");
+    TEST_BACKING_PARSE("nbd:127.0.0.1:6000:exportname=:test",
+                       "<source protocol='nbd' name=':test'>\n"
+                       "  <host name='127.0.0.1' port='6000'/>\n"
+                       "</source>\n");

Hideous abuse of both socket name and export name, but I concur that fixing libvirt to parse the ugly strings in the same way as qemu is worthwhile. I'd highly recommend anyone thinking of actually doing this in practice (rather than just in a QE setup) to use the NBD URI syntax:

nbd://[::1]:6000/:test
nbd://127.0.0.1:6000/:test

      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:unix:/tmp/sock:",
+                       "<source protocol='nbd'>\n"
+                       "  <host transport='unix' socket='/tmp/sock:'/>\n"
+                       "</source>\n");

nbd+unix:///?socket=/tmp/sock:

+    TEST_BACKING_PARSE("nbd:unix:/tmp/sock::exportname=:",
+                       "<source protocol='nbd' name=':'>\n"
+                       "  <host transport='unix' socket='/tmp/sock:'/>\n"
+                       "</source>\n");

nbd+unix:///:?socket=/tmp/sock:

Whether you also add the proper URI counterparts to the test is not directly relevant to the fix at hand.

Reviewed-by: Eric Blake <eblake@xxxxxxxxxx>

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux