[libvirt] [PATCH v2 0/8] Add new JSON pseudo-protocol support for qemu 2.9 changes

Patches 1-3 of old series were pushed already. Patch 1 of this series is new and adapts to change in 'SocketAddress' in qemu. Patches 2 and 3 needed to be modified because of patch 1. Patch 4 fixes the 'lun' field and splits the portal into host and port. There are a few changes in the rest of the patches, mostly changes from 'tcp' to 'inet' in the test suite. Peter Krempa (8): util: storage: Add support for type 'inet' in virStorageSourceParseBackingJSONSocketAddress 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 | 270 ++++++++++++++++++++++++++++++++++++---------- tests/virstoragetest.c | 73 ++++++++++++- 2 files changed, 283 insertions(+), 60 deletions(-) -- 2.12.2

'SocketAddress' structure was changed to contain 'inet' instead of 'tcp' since qemu commit c5f1ae3ae7b. Existing entries have a backward compatibility layer. Libvirt will parse 'inet' and 'tcp' as equivalents. --- src/util/virstoragefile.c | 23 +++++++++-------------- tests/virstoragetest.c | 2 +- 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 92bc561a2..7f2a50fd1 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2802,18 +2802,17 @@ virStorageSourceParseBackingJSONSocketAddress(virStorageNetHostDefPtr host, const char *hostname = virJSONValueObjectGetString(json, "host"); const char *port = virJSONValueObjectGetString(json, "port"); const char *socket = virJSONValueObjectGetString(json, "socket"); - int transport; - if ((transport = virStorageNetHostTransportTypeFromString(type)) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unknown backing store transport protocol '%s'"), type); + if (!type) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("missing socket address type in " + "JSON backing volume definition")); return -1; } - host->transport = transport; + if (STREQ(type, "tcp") || STREQ(type, "inet")) { + host->transport = VIR_STORAGE_NET_HOST_TRANS_TCP; - 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 " @@ -2824,9 +2823,9 @@ virStorageSourceParseBackingJSONSocketAddress(virStorageNetHostDefPtr host, if (VIR_STRDUP(host->name, hostname) < 0 || VIR_STRDUP(host->port, port) < 0) return -1; - break; + } else if (STREQ(type, "unix")) { + host->transport = VIR_STORAGE_NET_HOST_TRANS_UNIX; - case VIR_STORAGE_NET_HOST_TRANS_UNIX: if (!socket) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("missing socket path for udp backing server in " @@ -2834,13 +2833,9 @@ virStorageSourceParseBackingJSONSocketAddress(virStorageNetHostDefPtr host, return -1; } - if (VIR_STRDUP(host->socket, socket) < 0) return -1; - break; - - case VIR_STORAGE_NET_HOST_TRANS_RDMA: - case VIR_STORAGE_NET_HOST_TRANS_LAST: + } else { virReportError(VIR_ERR_INTERNAL_ERROR, _("backing store protocol '%s' is not yet supported"), type); diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 70e24a1b7..117208289 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1431,7 +1431,7 @@ mymain(void) "{ \"type\":\"unix\"," "\"socket\":\"/path/socket\"" "}," - "{ \"type\":\"tcp\"," + "{ \"type\":\"inet\"," "\"host\":\"example.com\"" "}" "]" -- 2.12.2

On Mon, Jun 19, 2017 at 03:58:28PM +0200, Peter Krempa wrote:
'SocketAddress' structure was changed to contain 'inet' instead of 'tcp' since qemu commit c5f1ae3ae7b. Existing entries have a backward compatibility layer.
Libvirt will parse 'inet' and 'tcp' as equivalents. --- src/util/virstoragefile.c | 23 +++++++++-------------- tests/virstoragetest.c | 2 +- 2 files changed, 10 insertions(+), 15 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. --- Notes: v2: - resolved merge conflict after adding a patch - changed ordering of setting of the transport type 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 7f2a50fd1..9af0cdbcd 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; + } + + host->transport = VIR_STORAGE_NET_HOST_TRANS_TCP; + + if (VIR_STRDUP(host->name, hostname) < 0 || + VIR_STRDUP(host->port, port) < 0) + return -1; + + 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"); if (!type) { @@ -2811,18 +2833,8 @@ virStorageSourceParseBackingJSONSocketAddress(virStorageNetHostDefPtr host, } if (STREQ(type, "tcp") || STREQ(type, "inet")) { - host->transport = VIR_STORAGE_NET_HOST_TRANS_TCP; + return virStorageSourceParseBackingJSONInetSocketAddress(host, json); - 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; } else if (STREQ(type, "unix")) { host->transport = VIR_STORAGE_NET_HOST_TRANS_UNIX; -- 2.12.2

On Mon, Jun 19, 2017 at 03:58:29PM +0200, Peter Krempa wrote:
Few backing protocols support only TCP. Split out the function which will correspond to parsing qemu's InetSocketAddressBase. ---
Notes: v2: - resolved merge conflict after adding a patch - changed ordering of setting of the transport type
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. --- Notes: v2: - fixed merge conflicts src/util/virstoragefile.c | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 9af0cdbcd..0bbbba650 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,17 @@ 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; + + if (!json) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("missing remote server specification in JSON " + "backing volume definition")); + return -1; + } - if (!type) { + if (!(type = virJSONValueObjectGetString(json, "type"))) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("missing socket address type in " "JSON backing volume definition")); @@ -2838,7 +2855,7 @@ virStorageSourceParseBackingJSONSocketAddress(virStorageNetHostDefPtr host, } else if (STREQ(type, "unix")) { host->transport = 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")); -- 2.12.2

On Mon, Jun 19, 2017 at 03:58:30PM +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. ---
Notes: v2: - fixed merge conflicts
src/util/virstoragefile.c | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-)
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. --- Notes: v2: - parse 'port' from the portal string - treat 'lun' as an integer in json src/util/virstoragefile.c | 63 ++++++++++++++++++++++++++++++++++++++++++++--- tests/virstoragetest.c | 19 ++++++++++++++ 2 files changed, 78 insertions(+), 4 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 0bbbba650..8e6631b45 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2935,18 +2935,73 @@ 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; + char *port; + unsigned int lun = 0; + 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 ((port = strchr(src->hosts->name, ':'))) { + if (VIR_STRDUP(src->hosts->port, port + 1) < 0) + goto cleanup; + + if (strlen(src->hosts->port) == 0) + VIR_FREE(src->hosts->port); + + *port = '\0'; + } + + ignore_value(virJSONValueObjectGetNumberUint(json, "lun", &lun)); + + if (virAsprintf(&fulltarget, "%s/%u", 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 117208289..894f78ba9 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1502,6 +1502,25 @@ 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\"" + "}" + "}", + "<source protocol='iscsi' name='iqn.2016-12.com.virttest:emulated-iscsi-noauth.target/0'>\n" + " <host name='test.org'/>\n" + "</source>\n"); + TEST_BACKING_PARSE("json:{\"file\":{\"driver\":\"iscsi\"," + "\"transport\":\"tcp\"," + "\"portal\":\"test.org:1234\"," + "\"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' port='1234'/>\n" + "</source>\n"); cleanup: /* Final cleanup */ -- 2.12.2

On Mon, Jun 19, 2017 at 03:58:31PM +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. ---
Notes: v2: - parse 'port' from the portal string - treat 'lun' as an integer in json
src/util/virstoragefile.c | 63 ++++++++++++++++++++++++++++++++++++++++++++--- tests/virstoragetest.c | 19 ++++++++++++++ 2 files changed, 78 insertions(+), 4 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

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'. --- Notes: v2: - changed type 'tcp' to 'inet' according to the qemu schema 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 8e6631b45..b0dc92942 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -3014,11 +3014,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; } @@ -3032,17 +3033,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 894f78ba9..ca2601de7 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\":\"inet\"," + "\"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 Mon, Jun 19, 2017 at 03:58:32PM +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'. ---
Notes: v2: - changed type 'tcp' to 'inet' according to the qemu schema
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 b0dc92942..220a55d9b 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -3122,6 +3122,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; @@ -3130,11 +3139,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 ca2601de7..9c7314bff 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 Mon, Jun 19, 2017 at 03:58:33PM +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. --- Notes: v2: - tweaked error message src/util/virstoragefile.c | 21 +++++++++++++-------- tests/virstoragetest.c | 11 +++++++++++ 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 220a55d9b..b8a3de85c 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -3087,10 +3087,11 @@ 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 " + _("missing host/server or path of SSH JSON backing " "volume definition")); return -1; } @@ -3105,12 +3106,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 9c7314bff..08f6c9003 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 Mon, Jun 19, 2017 at 03:58:34PM +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. ---
Notes: v2: - tweaked error message
src/util/virstoragefile.c | 21 +++++++++++++-------- tests/virstoragetest.c | 11 +++++++++++ 2 files changed, 24 insertions(+), 8 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. --- Notes: v2: - fixed server type to 'inet' in test - removed spurious 'transport' value in test 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 b8a3de85c..3cbbb6c8f 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -3061,6 +3061,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"))) { @@ -3069,13 +3071,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 08f6c9003..6c1287380 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1562,6 +1562,17 @@ mymain(void) "<source protocol='iscsi' name='iqn.2016-12.com.virttest:emulated-iscsi-noauth.target/6'>\n" " <host name='test.org' port='1234'/>\n" "</source>\n"); + TEST_BACKING_PARSE("json:{\"file\":{\"driver\":\"sheepdog\"," + "\"vdi\":\"test\"," + "\"server\":{ \"type\":\"inet\"," + "\"host\":\"example.com\"," + "\"port\":\"321\"" + "}" + "}" + "}", + "<source protocol='sheepdog' name='test'>\n" + " <host name='example.com' port='321'/>\n" + "</source>\n"); cleanup: /* Final cleanup */ -- 2.12.2

On Mon, Jun 19, 2017 at 03:58:35PM +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. ---
Notes: v2: - fixed server type to 'inet' in test - removed spurious 'transport' value in test
src/util/virstoragefile.c | 28 ++++++++++++++++++++++++---- tests/virstoragetest.c | 11 +++++++++++ 2 files changed, 35 insertions(+), 4 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

On Tue, Jun 20, 2017 at 09:08:34 +0200, Pavel Hrdina wrote:
On Mon, Jun 19, 2017 at 03:58:35PM +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. ---
Notes: v2: - fixed server type to 'inet' in test - removed spurious 'transport' value in test
src/util/virstoragefile.c | 28 ++++++++++++++++++++++++---- tests/virstoragetest.c | 11 +++++++++++ 2 files changed, 35 insertions(+), 4 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Thanks! I've pushed the series.
participants (2)
-
Pavel Hrdina
-
Peter Krempa