[libvirt] [PATCH 00/10] Add new JSON pseudo-protocol support for qemu 2.9 changes

The conversion to proper structures in qemu 2.9 lead to a change in the JSON pseudo protocol fields, which made them unknown to libvirt. This patchset fixes and refactors a few helpers and then adds the new format to the backing store string parser. Peter Krempa (10): util: storage: Output parsed network backing store string to debug log util: storage: Add missing return to virStorageSourceParseBackingJSONGluster util: storage: make virStorageSourceParseBackingJSONGlusterHost universal util: storage: Split out parsing of TCP network host from JSON pseudoprotocol util: storage: Report errors when source host data is missing util: storage: Add JSON parser for new options in iSCSI protocol util: storage: adapt to changes in JSON format for NBD util: storage: adapt to changes in JSON format for ceph/rbd util: storage: adapt to changes in JSON format for ssh util: storage: adapt to changes in JSON format for sheepdog src/util/virstoragefile.c | 256 ++++++++++++++++++++++++++++++++++++---------- tests/virstoragetest.c | 62 +++++++++++ 2 files changed, 266 insertions(+), 52 deletions(-) -- 2.12.2

--- src/util/virstoragefile.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index e82a7fb53..020c69def 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -3229,6 +3229,8 @@ virStorageSourceNewFromBackingAbsolute(const char *path) } else { ret->type = VIR_STORAGE_TYPE_NETWORK; + VIR_DEBUG("parsing backing store string: '%s'", path); + /* handle URI formatted backing stores */ if ((json = STRSKIP(path, "json:"))) rc = virStorageSourceParseBackingJSON(ret, json); -- 2.12.2

On Fri, Jun 16, 2017 at 05:29:38PM +0200, Peter Krempa wrote:
--- src/util/virstoragefile.c | 2 ++ 1 file changed, 2 insertions(+)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

If the number of servers is not expected the code would report an error but would not return failure. --- src/util/virstoragefile.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 020c69def..6b0af521f 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2888,6 +2888,8 @@ virStorageSourceParseBackingJSONGluster(virStorageSourcePtr src, virReportError(VIR_ERR_INVALID_ARG, "%s", _("at least 1 server is necessary in " "JSON backing definition for gluster volume")); + + return -1; } if (VIR_ALLOC_N(src->hosts, nservers) < 0) -- 2.12.2

On Fri, Jun 16, 2017 at 05:29:39PM +0200, Peter Krempa wrote:
If the number of servers is not expected the code would report an error but would not return failure. --- src/util/virstoragefile.c | 2 ++ 1 file changed, 2 insertions(+)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

The same json strucutre is used for NBD and sheepdog volumes for specifying of the host. Rename the function and fix up error messages to be more universal. --- src/util/virstoragefile.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 6b0af521f..92bc561a2 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2795,8 +2795,8 @@ virStorageSourceParseBackingJSONUri(virStorageSourcePtr src, static int -virStorageSourceParseBackingJSONGlusterHost(virStorageNetHostDefPtr host, - virJSONValuePtr json) +virStorageSourceParseBackingJSONSocketAddress(virStorageNetHostDefPtr host, + virJSONValuePtr json) { const char *type = virJSONValueObjectGetString(json, "type"); const char *hostname = virJSONValueObjectGetString(json, "host"); @@ -2817,7 +2817,7 @@ virStorageSourceParseBackingJSONGlusterHost(virStorageNetHostDefPtr host, if (!hostname) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("missing hostname for tcp backing server in " - "JSON backing definition for gluster volume")); + "JSON backing volume definition")); return -1; } @@ -2830,7 +2830,7 @@ virStorageSourceParseBackingJSONGlusterHost(virStorageNetHostDefPtr host, if (!socket) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("missing socket path for udp backing server in " - "JSON backing definition for gluster volume")); + "JSON backing volume definition")); return -1; } @@ -2897,8 +2897,8 @@ virStorageSourceParseBackingJSONGluster(virStorageSourcePtr src, src->nhosts = nservers; for (i = 0; i < nservers; i++) { - if (virStorageSourceParseBackingJSONGlusterHost(src->hosts + i, - virJSONValueArrayGet(server, i)) < 0) + if (virStorageSourceParseBackingJSONSocketAddress(src->hosts + i, + virJSONValueArrayGet(server, i)) < 0) return -1; } -- 2.12.2

On Fri, Jun 16, 2017 at 05:29:40PM +0200, Peter Krempa wrote:
The same json strucutre is used for NBD and sheepdog volumes for specifying of the host. Rename the function and fix up error messages to be more universal. --- src/util/virstoragefile.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

Few backing protocols support only TCP. Split out the function which will correspond to parsing qemu's InetSocketAddressBase. --- src/util/virstoragefile.c | 38 +++++++++++++++++++++++++------------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 92bc561a2..c7632808e 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2795,12 +2795,34 @@ virStorageSourceParseBackingJSONUri(virStorageSourcePtr src, static int +virStorageSourceParseBackingJSONInetSocketAddress(virStorageNetHostDefPtr host, + virJSONValuePtr json) +{ + const char *hostname = virJSONValueObjectGetString(json, "host"); + const char *port = virJSONValueObjectGetString(json, "port"); + + if (!hostname) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("missing hostname for tcp backing server in " + "JSON backing volume definition")); + return -1; + } + + if (VIR_STRDUP(host->name, hostname) < 0 || + VIR_STRDUP(host->port, port) < 0) + return -1; + + host->transport = VIR_STORAGE_NET_HOST_TRANS_TCP; + + return 0; +} + + +static int virStorageSourceParseBackingJSONSocketAddress(virStorageNetHostDefPtr host, virJSONValuePtr json) { const char *type = virJSONValueObjectGetString(json, "type"); - const char *hostname = virJSONValueObjectGetString(json, "host"); - const char *port = virJSONValueObjectGetString(json, "port"); const char *socket = virJSONValueObjectGetString(json, "socket"); int transport; @@ -2814,17 +2836,7 @@ virStorageSourceParseBackingJSONSocketAddress(virStorageNetHostDefPtr host, switch ((virStorageNetHostTransport) transport) { case VIR_STORAGE_NET_HOST_TRANS_TCP: - if (!hostname) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("missing hostname for tcp backing server in " - "JSON backing volume definition")); - return -1; - } - - if (VIR_STRDUP(host->name, hostname) < 0 || - VIR_STRDUP(host->port, port) < 0) - return -1; - break; + return virStorageSourceParseBackingJSONInetSocketAddress(host, json); case VIR_STORAGE_NET_HOST_TRANS_UNIX: if (!socket) { -- 2.12.2

On Fri, Jun 16, 2017 at 05:29:41PM +0200, Peter Krempa wrote:
Few backing protocols support only TCP. Split out the function which will correspond to parsing qemu's InetSocketAddressBase. --- src/util/virstoragefile.c | 38 +++++++++++++++++++++++++------------- 1 file changed, 25 insertions(+), 13 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

Merge the reporting of the missing source host data into the parser functions so that callers don't have to do it separately. --- src/util/virstoragefile.c | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index c7632808e..c0aa4e4c6 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2798,8 +2798,18 @@ static int virStorageSourceParseBackingJSONInetSocketAddress(virStorageNetHostDefPtr host, virJSONValuePtr json) { - const char *hostname = virJSONValueObjectGetString(json, "host"); - const char *port = virJSONValueObjectGetString(json, "port"); + const char *hostname; + const char *port; + + if (!json) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("missing remote server specification in JSON " + "backing volume definition")); + return -1; + } + + hostname = virJSONValueObjectGetString(json, "host"); + port = virJSONValueObjectGetString(json, "port"); if (!hostname) { virReportError(VIR_ERR_INVALID_ARG, "%s", @@ -2822,10 +2832,19 @@ static int virStorageSourceParseBackingJSONSocketAddress(virStorageNetHostDefPtr host, virJSONValuePtr json) { - const char *type = virJSONValueObjectGetString(json, "type"); - const char *socket = virJSONValueObjectGetString(json, "socket"); + const char *type; + const char *socket; int transport; + if (!json) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("missing remote server specification in JSON " + "backing volume definition")); + return -1; + } + + type = virJSONValueObjectGetString(json, "type"); + if ((transport = virStorageNetHostTransportTypeFromString(type)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unknown backing store transport protocol '%s'"), type); @@ -2839,14 +2858,13 @@ virStorageSourceParseBackingJSONSocketAddress(virStorageNetHostDefPtr host, return virStorageSourceParseBackingJSONInetSocketAddress(host, json); case VIR_STORAGE_NET_HOST_TRANS_UNIX: - if (!socket) { + if (!(socket = virJSONValueObjectGetString(json, "socket"))) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("missing socket path for udp backing server in " "JSON backing volume definition")); return -1; } - if (VIR_STRDUP(host->socket, socket) < 0) return -1; break; -- 2.12.2

On Fri, Jun 16, 2017 at 05:29:42PM +0200, Peter Krempa wrote:
Merge the reporting of the missing source host data into the parser functions so that callers don't have to do it separately. --- src/util/virstoragefile.c | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index c7632808e..c0aa4e4c6 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c
[...]
@@ -2839,14 +2858,13 @@ virStorageSourceParseBackingJSONSocketAddress(virStorageNetHostDefPtr host, return virStorageSourceParseBackingJSONInetSocketAddress(host, json);
case VIR_STORAGE_NET_HOST_TRANS_UNIX: - if (!socket) { + if (!(socket = virJSONValueObjectGetString(json, "socket"))) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("missing socket path for udp backing server in " "JSON backing volume definition")); return -1; }
-
Spurious line removal :)
if (VIR_STRDUP(host->socket, socket) < 0) return -1;
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

Starting from qemu 2.9, more granular options are supported. Add parser for the relevant bits. With this patch libvirt is able to parse the host and target IQN of from the JSON pseudo-protocol specification. This corresponds to BlockdevOptionsIscsi in qemu qapi. --- src/util/virstoragefile.c | 53 +++++++++++++++++++++++++++++++++++++++++++---- tests/virstoragetest.c | 10 +++++++++ 2 files changed, 59 insertions(+), 4 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index c0aa4e4c6..4f063e7f2 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2941,18 +2941,63 @@ virStorageSourceParseBackingJSONiSCSI(virStorageSourcePtr src, virJSONValuePtr json, int opaque ATTRIBUTE_UNUSED) { + const char *transport = virJSONValueObjectGetString(json, "transport"); + const char *portal = virJSONValueObjectGetString(json, "portal"); + const char *target = virJSONValueObjectGetString(json, "target"); const char *uri; + const char *lun; + char *fulltarget = NULL; + int ret = -1; /* legacy URI based syntax passed via 'filename' option */ if ((uri = virJSONValueObjectGetString(json, "filename"))) return virStorageSourceParseBackingJSONUriStr(src, uri, VIR_STORAGE_NET_PROTOCOL_ISCSI); - /* iSCSI currently supports only URI syntax passed in as filename */ - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("missing iSCSI URI in JSON backing volume definition")); + src->type = VIR_STORAGE_TYPE_NETWORK; + src->protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI; - return -1; + if (VIR_ALLOC(src->hosts) < 0) + goto cleanup; + + src->nhosts = 1; + + if (STRNEQ_NULLABLE(transport, "tcp")) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("only TCP transport is supported for iSCSI volumes")); + goto cleanup; + } + + src->hosts->transport = VIR_STORAGE_NET_HOST_TRANS_TCP; + + if (!portal) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("missing 'portal' address in iSCSI backing definition")); + goto cleanup; + } + + if (!target) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("missing 'target' in iSCSI backing definition")); + goto cleanup; + } + + if (VIR_STRDUP(src->hosts->name, portal) < 0) + goto cleanup; + + if (!(lun = virJSONValueObjectGetString(json, "lun"))) + lun = "0"; + + if (virAsprintf(&fulltarget, "%s/%s", target, lun) < 0) + goto cleanup; + + VIR_STEAL_PTR(src->path, fulltarget); + + ret = 0; + + cleanup: + VIR_FREE(fulltarget); + return ret; } diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 70e24a1b7..c31f4fc54 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1502,6 +1502,16 @@ mymain(void) "\"driver\": \"file\"," "\"filename\": \"/path/to/file\" } } }", "<source file='/path/to/file'/>\n"); + TEST_BACKING_PARSE("json:{\"file\":{\"driver\":\"iscsi\"," + "\"transport\":\"tcp\"," + "\"portal\":\"test.org\"," + "\"target\":\"iqn.2016-12.com.virttest:emulated-iscsi-noauth.target\"," + "\"lun\":\"6\"" + "}" + "}", + "<source protocol='iscsi' name='iqn.2016-12.com.virttest:emulated-iscsi-noauth.target/6'>\n" + " <host name='test.org'/>\n" + "</source>\n"); cleanup: /* Final cleanup */ -- 2.12.2

On Fri, Jun 16, 2017 at 05:29:43PM +0200, Peter Krempa wrote:
Starting from qemu 2.9, more granular options are supported. Add parser for the relevant bits.
With this patch libvirt is able to parse the host and target IQN of from the JSON pseudo-protocol specification.
This corresponds to BlockdevOptionsIscsi in qemu qapi. --- src/util/virstoragefile.c | 53 +++++++++++++++++++++++++++++++++++++++++++---- tests/virstoragetest.c | 10 +++++++++ 2 files changed, 59 insertions(+), 4 deletions(-)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index c0aa4e4c6..4f063e7f2 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2941,18 +2941,63 @@ virStorageSourceParseBackingJSONiSCSI(virStorageSourcePtr src, virJSONValuePtr json, int opaque ATTRIBUTE_UNUSED) { + const char *transport = virJSONValueObjectGetString(json, "transport"); + const char *portal = virJSONValueObjectGetString(json, "portal"); + const char *target = virJSONValueObjectGetString(json, "target"); const char *uri; + const char *lun; + char *fulltarget = NULL; + int ret = -1;
/* legacy URI based syntax passed via 'filename' option */ if ((uri = virJSONValueObjectGetString(json, "filename"))) return virStorageSourceParseBackingJSONUriStr(src, uri, VIR_STORAGE_NET_PROTOCOL_ISCSI);
- /* iSCSI currently supports only URI syntax passed in as filename */ - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("missing iSCSI URI in JSON backing volume definition")); + src->type = VIR_STORAGE_TYPE_NETWORK; + src->protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI;
- return -1; + if (VIR_ALLOC(src->hosts) < 0) + goto cleanup; + + src->nhosts = 1; + + if (STRNEQ_NULLABLE(transport, "tcp")) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("only TCP transport is supported for iSCSI volumes")); + goto cleanup; + } + + src->hosts->transport = VIR_STORAGE_NET_HOST_TRANS_TCP; + + if (!portal) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("missing 'portal' address in iSCSI backing definition")); + goto cleanup; + } + + if (!target) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("missing 'target' in iSCSI backing definition")); + goto cleanup; + } + + if (VIR_STRDUP(src->hosts->name, portal) < 0) + goto cleanup;
The @portal contains a pair of IP_ADDRESS:TCP_PORT and the structure _virStorageNetHostDef contains a name and port so it should be parsed separately.
+ + if (!(lun = virJSONValueObjectGetString(json, "lun"))) + lun = "0";
This is int in the qapi schema. Pavel
+ + if (virAsprintf(&fulltarget, "%s/%s", target, lun) < 0) + goto cleanup; + + VIR_STEAL_PTR(src->path, fulltarget); + + ret = 0; + + cleanup: + VIR_FREE(fulltarget); + return ret; }
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 70e24a1b7..c31f4fc54 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1502,6 +1502,16 @@ mymain(void) "\"driver\": \"file\"," "\"filename\": \"/path/to/file\" } } }", "<source file='/path/to/file'/>\n"); + TEST_BACKING_PARSE("json:{\"file\":{\"driver\":\"iscsi\"," + "\"transport\":\"tcp\"," + "\"portal\":\"test.org\"," + "\"target\":\"iqn.2016-12.com.virttest:emulated-iscsi-noauth.target\"," + "\"lun\":\"6\"" + "}" + "}", + "<source protocol='iscsi' name='iqn.2016-12.com.virttest:emulated-iscsi-noauth.target/6'>\n" + " <host name='test.org'/>\n" + "</source>\n");
cleanup: /* Final cleanup */ -- 2.12.2
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

Since 2.9 the host and port for NBD are no longer directly under the json pseudo-protocol object, but rather belong to a sub-object called 'server'. --- src/util/virstoragefile.c | 28 +++++++++++++++++----------- tests/virstoragetest.c | 11 +++++++++++ 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 4f063e7f2..36e7ca759 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -3010,11 +3010,12 @@ virStorageSourceParseBackingJSONNbd(virStorageSourcePtr src, const char *host = virJSONValueObjectGetString(json, "host"); const char *port = virJSONValueObjectGetString(json, "port"); const char *export = virJSONValueObjectGetString(json, "export"); + virJSONValuePtr server = virJSONValueObjectGetObject(json, "server"); - if (!path && !host) { + if (!path && !host && !server) { virReportError(VIR_ERR_INVALID_ARG, "%s", - _("missing path or host of NBD server in JSON backing " - "volume definition")); + _("missing host specification of NBD server in JSON " + "backing volume definition")); return -1; } @@ -3028,17 +3029,22 @@ virStorageSourceParseBackingJSONNbd(virStorageSourcePtr src, return -1; src->nhosts = 1; - if (path) { - src->hosts[0].transport = VIR_STORAGE_NET_HOST_TRANS_UNIX; - if (VIR_STRDUP(src->hosts[0].socket, path) < 0) + if (server) { + if (virStorageSourceParseBackingJSONSocketAddress(src->hosts, server) < 0) return -1; } else { - src->hosts[0].transport = VIR_STORAGE_NET_HOST_TRANS_TCP; - if (VIR_STRDUP(src->hosts[0].name, host) < 0) - return -1; + if (path) { + src->hosts[0].transport = VIR_STORAGE_NET_HOST_TRANS_UNIX; + if (VIR_STRDUP(src->hosts[0].socket, path) < 0) + return -1; + } else { + src->hosts[0].transport = VIR_STORAGE_NET_HOST_TRANS_TCP; + if (VIR_STRDUP(src->hosts[0].name, host) < 0) + return -1; - if (VIR_STRDUP(src->hosts[0].port, port) < 0) - return -1; + if (VIR_STRDUP(src->hosts[0].port, port) < 0) + return -1; + } } return 0; diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index c31f4fc54..443d9c6ec 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1471,6 +1471,17 @@ mymain(void) "<source protocol='nbd' name='blah'>\n" " <host name='example.org' port='6000'/>\n" "</source>\n"); + TEST_BACKING_PARSE("json:{\"file\":{\"driver\":\"nbd\"," + "\"export\":\"blah\"," + "\"server\": { \"type\":\"tcp\"," + "\"host\":\"example.org\"," + "\"port\":\"6000\"" + "}" + "}" + "}", + "<source protocol='nbd' name='blah'>\n" + " <host name='example.org' port='6000'/>\n" + "</source>\n"); TEST_BACKING_PARSE("json:{\"file\":{\"driver\":\"ssh\"," "\"host\":\"example.org\"," "\"port\":\"6000\"," -- 2.12.2

On Fri, Jun 16, 2017 at 05:29:44PM +0200, Peter Krempa wrote:
Since 2.9 the host and port for NBD are no longer directly under the json pseudo-protocol object, but rather belong to a sub-object called 'server'. --- src/util/virstoragefile.c | 28 +++++++++++++++++----------- tests/virstoragetest.c | 11 +++++++++++ 2 files changed, 28 insertions(+), 11 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

Since qemu 2.9 the options changed from a monolithic string into fine grained options for the json pseudo-protocol object. --- src/util/virstoragefile.c | 50 +++++++++++++++++++++++++++++++++++++++++++---- tests/virstoragetest.c | 19 ++++++++++++++++++ 2 files changed, 65 insertions(+), 4 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 36e7ca759..2526bac0b 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -3118,6 +3118,15 @@ virStorageSourceParseBackingJSONRBD(virStorageSourcePtr src, int opaque ATTRIBUTE_UNUSED) { const char *filename; + const char *pool = virJSONValueObjectGetString(json, "pool"); + const char *image = virJSONValueObjectGetString(json, "image"); + const char *conf = virJSONValueObjectGetString(json, "conf"); + const char *snapshot = virJSONValueObjectGetString(json, "snapshot"); + virJSONValuePtr servers = virJSONValueObjectGetArray(json, "server"); + char *fullname = NULL; + size_t nservers; + size_t i; + int ret = -1; src->type = VIR_STORAGE_TYPE_NETWORK; src->protocol = VIR_STORAGE_NET_PROTOCOL_RBD; @@ -3126,11 +3135,44 @@ virStorageSourceParseBackingJSONRBD(virStorageSourcePtr src, if ((filename = virJSONValueObjectGetString(json, "filename"))) return virStorageSourceParseRBDColonString(filename, src); - /* RBD currently supports only URI syntax passed in as filename */ - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("missing RBD filename in JSON backing volume definition")); + if (!pool || !image) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("missing pool or image name in ceph backing volume " + "JSON specification")); + return -1; + } - return -1; + /* currently we need to store the pool name and image name together, since + * the rest of the code is not prepared for it */ + if (virAsprintf(&fullname, "%s/%s", pool, image) < 0) + return -1; + + if (VIR_STRDUP(src->snapshot, snapshot) < 0 || + VIR_STRDUP(src->configFile, conf) < 0) + goto cleanup; + + VIR_STEAL_PTR(src->path, fullname); + + if (servers) { + nservers = virJSONValueArraySize(servers); + + if (VIR_ALLOC_N(src->hosts, nservers) < 0) + goto cleanup; + + src->nhosts = nservers; + + for (i = 0; i < nservers; i++) { + if (virStorageSourceParseBackingJSONInetSocketAddress(src->hosts + i, + virJSONValueArrayGet(servers, i)) < 0) + goto cleanup; + } + } + + ret = 0; + cleanup: + VIR_FREE(fullname); + + return ret; } static int diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 443d9c6ec..890b5c37c 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1507,6 +1507,25 @@ mymain(void) "<source protocol='rbd' name='testshare'>\n" " <host name='example.com'/>\n" "</source>\n"); + TEST_BACKING_PARSE("json:{\"file\":{\"driver\":\"rbd\"," + "\"image\":\"test\"," + "\"pool\":\"libvirt\"," + "\"conf\":\"/path/to/conf\"," + "\"snapshot\":\"snapshotname\"," + "\"server\":[ {\"host\":\"example.com\"," + "\"port\":\"1234\"" + "}," + "{\"host\":\"example2.com\"" + "}" + "]" + "}" + "}", + "<source protocol='rbd' name='libvirt/test'>\n" + " <host name='example.com' port='1234'/>\n" + " <host name='example2.com'/>\n" + " <snapshot name='snapshotname'/>\n" + " <config file='/path/to/conf'/>\n" + "</source>\n"); TEST_BACKING_PARSE("json:{ \"file\": { " "\"driver\": \"raw\"," "\"file\": {" -- 2.12.2

On Fri, Jun 16, 2017 at 05:29:45PM +0200, Peter Krempa wrote:
Since qemu 2.9 the options changed from a monolithic string into fine grained options for the json pseudo-protocol object. --- src/util/virstoragefile.c | 50 +++++++++++++++++++++++++++++++++++++++++++---- tests/virstoragetest.c | 19 ++++++++++++++++++ 2 files changed, 65 insertions(+), 4 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

Since qemu 2.9 the options changed from a monolithic string into fine grained options for the json pseudo-protocol object. --- src/util/virstoragefile.c | 19 ++++++++++++------- tests/virstoragetest.c | 11 +++++++++++ 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 2526bac0b..dc06bec94 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -3083,8 +3083,9 @@ virStorageSourceParseBackingJSONSSH(virStorageSourcePtr src, const char *path = virJSONValueObjectGetString(json, "path"); const char *host = virJSONValueObjectGetString(json, "host"); const char *port = virJSONValueObjectGetString(json, "port"); + virJSONValuePtr server = virJSONValueObjectGetObject(json, "server"); - if (!host || !path) { + if (!(host || server) || !path) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("missing host or path of SSH JSON backing " "volume definition")); @@ -3101,12 +3102,16 @@ virStorageSourceParseBackingJSONSSH(virStorageSourcePtr src, return -1; src->nhosts = 1; - src->hosts[0].transport = VIR_STORAGE_NET_HOST_TRANS_TCP; - if (VIR_STRDUP(src->hosts[0].name, host) < 0) - return -1; - - if (VIR_STRDUP(src->hosts[0].port, port) < 0) - return -1; + if (server) { + if (virStorageSourceParseBackingJSONInetSocketAddress(src->hosts, + server) < 0) + return -1; + } else { + src->hosts[0].transport = VIR_STORAGE_NET_HOST_TRANS_TCP; + if (VIR_STRDUP(src->hosts[0].name, host) < 0 || + VIR_STRDUP(src->hosts[0].port, port) < 0) + return -1; + } return 0; } diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 890b5c37c..7abd30c55 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1501,6 +1501,17 @@ mymain(void) "<source protocol='ssh' name='blah'>\n" " <host name='example.org' port='6000'/>\n" "</source>\n"); + TEST_BACKING_PARSE("json:{\"file\":{\"driver\":\"ssh\"," + "\"path\":\"blah\"," + "\"server\":{ \"host\":\"example.org\"," + "\"port\":\"6000\"" + "}," + "\"user\":\"user\"" + "}" + "}", + "<source protocol='ssh' name='blah'>\n" + " <host name='example.org' port='6000'/>\n" + "</source>\n"); TEST_BACKING_PARSE("json:{\"file.driver\":\"rbd\"," "\"file.filename\":\"rbd:testshare:id=asdf:mon_host=example.com\"" "}", -- 2.12.2

On Fri, Jun 16, 2017 at 05:29:46PM +0200, Peter Krempa wrote:
Since qemu 2.9 the options changed from a monolithic string into fine grained options for the json pseudo-protocol object. --- src/util/virstoragefile.c | 19 ++++++++++++------- tests/virstoragetest.c | 11 +++++++++++ 2 files changed, 23 insertions(+), 7 deletions(-)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 2526bac0b..dc06bec94 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -3083,8 +3083,9 @@ virStorageSourceParseBackingJSONSSH(virStorageSourcePtr src, const char *path = virJSONValueObjectGetString(json, "path"); const char *host = virJSONValueObjectGetString(json, "host"); const char *port = virJSONValueObjectGetString(json, "port"); + virJSONValuePtr server = virJSONValueObjectGetObject(json, "server");
- if (!host || !path) { + if (!(host || server) || !path) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("missing host or path of SSH JSON backing " "volume definition"));
The error message could be updated as well. Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

Since qemu 2.9 the options changed from a monolithic string into fine grained options for the json pseudo-protocol object. --- src/util/virstoragefile.c | 28 ++++++++++++++++++++++++---- tests/virstoragetest.c | 11 +++++++++++ 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index dc06bec94..629908b7a 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -3057,6 +3057,8 @@ virStorageSourceParseBackingJSONSheepdog(virStorageSourcePtr src, int opaque ATTRIBUTE_UNUSED) { const char *filename; + const char *vdi = virJSONValueObjectGetString(json, "vdi"); + virJSONValuePtr server = virJSONValueObjectGetObject(json, "server"); /* legacy URI based syntax passed via 'filename' option */ if ((filename = virJSONValueObjectGetString(json, "filename"))) { @@ -3065,13 +3067,31 @@ virStorageSourceParseBackingJSONSheepdog(virStorageSourcePtr src, VIR_STORAGE_NET_PROTOCOL_SHEEPDOG); /* libvirt doesn't implement a parser for the legacy non-URI syntax */ + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("missing sheepdog URI in JSON backing volume definition")); + return -1; } - /* Sheepdog currently supports only URI and legacy syntax passed in as filename */ - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("missing sheepdog URI in JSON backing volume definition")); + src->type = VIR_STORAGE_TYPE_NETWORK; + src->protocol = VIR_STORAGE_NET_PROTOCOL_SHEEPDOG; - return -1; + if (!vdi) { + virReportError(VIR_ERR_INVALID_ARG, "%s", _("missing sheepdog vdi name")); + return -1; + } + + if (VIR_STRDUP(src->path, vdi) < 0) + return -1; + + if (VIR_ALLOC(src->hosts) < 0) + return -1; + + src->nhosts = 1; + + if (virStorageSourceParseBackingJSONSocketAddress(src->hosts, server) < 0) + return -1; + + return 0; } diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 7abd30c55..cbde02523 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1553,6 +1553,17 @@ mymain(void) "<source protocol='iscsi' name='iqn.2016-12.com.virttest:emulated-iscsi-noauth.target/6'>\n" " <host name='test.org'/>\n" "</source>\n"); + TEST_BACKING_PARSE("json:{\"file\":{\"driver\":\"sheepdog\"," + "\"transport\":\"tcp\"," + "\"vdi\":\"test\"," + "\"server\":{ \"type\":\"tcp\"," + "\"host\":\"example.com\"" + "}" + "}" + "}", + "<source protocol='sheepdog' name='test'>\n" + " <host name='example.com'/>\n" + "</source>\n"); cleanup: /* Final cleanup */ -- 2.12.2

On Fri, Jun 16, 2017 at 05:29:47PM +0200, Peter Krempa wrote:
Since qemu 2.9 the options changed from a monolithic string into fine grained options for the json pseudo-protocol object. --- src/util/virstoragefile.c | 28 ++++++++++++++++++++++++---- tests/virstoragetest.c | 11 +++++++++++ 2 files changed, 35 insertions(+), 4 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
participants (2)
-
Pavel Hrdina
-
Peter Krempa