9:20 a.m.
Fix the naming mistake, add tests and remove useless comment.
Peter Krempa (3):
storage: Fix formatting and parsing of qemu type 'UnixSocketAddress'
virstoragetest: Add test case for NBD over unix socket with new syntax
qemu: block: Remove misleading part of comment in
qemuBlockStorageSourceBuildJSONSocketAddress
src/qemu/qemu_block.c | 7 +------
src/util/virstoragefile.c | 2 +-
tests/virstoragetest.c | 13 +++++++++++--
3 files changed, 13 insertions(+), 9 deletions(-)
--
2.15.0
9:20 a.m.
New subject: [libvirt] [PATCH 1/3] storage: Fix formatting and parsing of qemu type 'UnixSocketAddress'
The documentation for the JSON/qapi type 'UnixSocketAddress' states that
the unix socket path field is named 'path'. We used 'socket' by
mistake. Fix both the formatter and parser and test suite.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1544325
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_block.c | 2 +-
src/util/virstoragefile.c | 2 +-
tests/virstoragetest.c | 4 ++--
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 585f0255ee..eb63139ca0 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -509,7 +509,7 @@ qemuBlockStorageSourceBuildJSONSocketAddress(virStorageNetHostDefPtr
host,
case VIR_STORAGE_NET_HOST_TRANS_UNIX:
if (virJSONValueObjectCreate(&server,
"s:type", "unix",
- "s:socket", host->socket,
+ "s:path", host->socket,
NULL) < 0)
goto cleanup;
break;
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 7f878039ba..5705bb055b 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -2893,7 +2893,7 @@
virStorageSourceParseBackingJSONSocketAddress(virStorageNetHostDefPtr host,
} else if (STREQ(type, "unix")) {
host->transport = VIR_STORAGE_NET_HOST_TRANS_UNIX;
- if (!(socket = virJSONValueObjectGetString(json, "socket"))) {
+ if (!(socket = virJSONValueObjectGetString(json, "path"))) {
virReportError(VIR_ERR_INVALID_ARG, "%s",
_("missing socket path for udp backing server in "
"JSON backing volume definition"));
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 6eed7134ed..ea3d2833dd 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -1420,7 +1420,7 @@ mymain(void)
"\"port\":\"1234\""
"},"
"{
\"type\":\"unix\","
-
"\"socket\":\"/path/socket\""
+
"\"path\":\"/path/socket\""
"},"
"{
\"type\":\"tcp\","
"\"host\":\"example.com\""
@@ -1441,7 +1441,7 @@ mymain(void)
"\"port\":\"1234\""
"},"
"{
\"type\":\"unix\","
-
"\"socket\":\"/path/socket\""
+
"\"path\":\"/path/socket\""
"},"
"{
\"type\":\"inet\","
"\"host\":\"example.com\""
--
2.15.0
10:33 a.m.
New subject: [libvirt] [PATCH 1/3] storage: Fix formatting and parsing of qemu type 'UnixSocketAddress'
On Mon, Feb 12, 2018 at 04:20:58PM +0100, Peter Krempa wrote:
The documentation for the JSON/qapi type 'UnixSocketAddress'
states that
the unix socket path field is named 'path'. We used 'socket' by
mistake. Fix both the formatter and parser and test suite.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1544325
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_block.c | 2 +-
src/util/virstoragefile.c | 2 +-
tests/virstoragetest.c | 4 ++--
3 files changed, 4 insertions(+), 4 deletions(-)
ACK with this squashed in:
--- a/tests/qemuxml2argvdata/disk-drive-network-gluster.args
+++ b/tests/qemuxml2argvdata/disk-drive-network-gluster.args
@@ -30,7 +30,7 @@ id=virtio-disk1 \
-drive file.driver=gluster,file.volume=Volume3,file.path=Image.qcow2,\
file.server.0.type=tcp,file.server.0.host=example.org,file.server.0.port=6000,\
file.server.1.type=tcp,file.server.1.host=example.org,file.server.1.port=24007,\
-file.server.2.type=unix,file.server.2.socket=/path/to/sock,file.debug=4,\
+file.server.2.type=unix,file.server.2.path=/path/to/sock,file.debug=4,\
format=qcow2,if=none,id=drive-virtio-disk2 \
-device virtio-blk-pci,bus=pci.0,addr=0x5,drive=drive-virtio-disk2,\
id=virtio-disk2
Jan
3:17 a.m.
New subject: [libvirt] [PATCH 1/3] storage: Fix formatting and parsing of qemu type 'UnixSocketAddress'
On Tue, Feb 13, 2018 at 17:33:00 +0100, Ján Tomko wrote:
On Mon, Feb 12, 2018 at 04:20:58PM +0100, Peter Krempa wrote:
> The documentation for the JSON/qapi type 'UnixSocketAddress' states that
> the unix socket path field is named 'path'. We used 'socket' by
> mistake. Fix both the formatter and parser and test suite.
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1544325
>
> Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
> ---
> src/qemu/qemu_block.c | 2 +-
> src/util/virstoragefile.c | 2 +-
> tests/virstoragetest.c | 4 ++--
> 3 files changed, 4 insertions(+), 4 deletions(-)
>
ACK with this squashed in:
--- a/tests/qemuxml2argvdata/disk-drive-network-gluster.args
+++ b/tests/qemuxml2argvdata/disk-drive-network-gluster.args
@@ -30,7 +30,7 @@ id=virtio-disk1 \
-drive file.driver=gluster,file.volume=Volume3,file.path=Image.qcow2,\
file.server.0.type=tcp,file.server.0.host=example.org,file.server.0.port=6000,\
file.server.1.type=tcp,file.server.1.host=example.org,file.server.1.port=24007,\
-file.server.2.type=unix,file.server.2.socket=/path/to/sock,file.debug=4,\
+file.server.2.type=unix,file.server.2.path=/path/to/sock,file.debug=4,\
format=qcow2,if=none,id=drive-virtio-disk2 \
-device virtio-blk-pci,bus=pci.0,addr=0x5,drive=drive-virtio-disk2,\
id=virtio-disk2
This prompted me to have another look down the qemu rabbit hole on why
we actually formatted this as 'socket' since the archeology expedition
into qapi didn't ever show usage of 'socket'.
It seems that gluster _only_ accepts 'socket' rather than 'path'
contrary to the documented type.
I'll post a V2 and complain loudly at qemu.
9:20 a.m.
New subject: [libvirt] [PATCH 2/3] virstoragetest: Add test case for NBD over unix socket with new syntax
Use the new syntax which uses the 'UnixSocket' type in qemu.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
tests/virstoragetest.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index ea3d2833dd..87519495f0 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -1494,6 +1494,15 @@ mymain(void)
"<source protocol='nbd'
name='blah'>\n"
" <host name='example.org'
port='6000'/>\n"
"</source>\n");
+
TEST_BACKING_PARSE("json:{\"file\":{\"driver\":\"nbd\","
+ "\"server\": {
\"type\":\"unix\","
+
"\"path\":\"/path/socket\""
+ "}"
+ "}"
+ "}",
+ "<source protocol='nbd'>\n"
+ " <host transport='unix'
socket='/path/socket'/>\n"
+ "</source>\n");
TEST_BACKING_PARSE("json:{\"file\":{\"driver\":\"ssh\","
"\"host\":\"example.org\","
"\"port\":\"6000\","
--
2.15.0
10:33 a.m.
New subject: [libvirt] [PATCH 2/3] virstoragetest: Add test case for NBD over unix socket with new syntax
On Mon, Feb 12, 2018 at 04:20:59PM +0100, Peter Krempa wrote:
Use the new syntax which uses the 'UnixSocket' type in qemu.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
tests/virstoragetest.c | 9 +++++++++
1 file changed, 9 insertions(+)
ACK
Jan
9:21 a.m.
New subject: [libvirt] [PATCH 3/3] qemu: block: Remove misleading part of comment in qemuBlockStorageSourceBuildJSONSocketAddress
The array indexes are formatted if the JSON->commandline translator is
translating an array type. It does not at all depend on this function.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_block.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index eb63139ca0..3f37483c1e 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -472,11 +472,6 @@ qemuBlockStorageSourceGetURI(virStorageSourcePtr src)
* Formats @hosts into a json object conforming to the 'SocketAddress' type
* in qemu.
*
- * This function can be used when only 1 src->nhosts is expected in order
- * to build a command without the array indices after "server.". That is
- * to see "server.type", "server.host", and "server.port"
instead of
- * "server.#.type", "server.#.host", and "server.#.port".
- *
* Returns a virJSONValuePtr for a single server.
*/
static virJSONValuePtr
--
2.15.0
10:35 a.m.
New subject: [libvirt] [PATCH 3/3] qemu: block: Remove misleading part of comment in qemuBlockStorageSourceBuildJSONSocketAddress
On Mon, Feb 12, 2018 at 04:21:00PM +0100, Peter Krempa wrote:
The array indexes are formatted if the JSON->commandline translator
is
translating an array type. It does not at all depend on this function.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_block.c | 5 -----
1 file changed, 5 deletions(-)
ACK
Jan
2473
days inactive
2475
days old
7 comments
2 participants
participants (2)
-
Ján Tomko -
Peter Krempa