[libvirt] [libvirt RFC PATCH 00/10] Add support for qemu's JSON pseudo'protocol for backing store

Libvirt didn't handle this for a long time and VMs with such config would not start we should implement it. Using JSON is basically the only option to specify advanced configuration for a backing file. Field names were taken from qemu's source since there isn't really documentation for this. CCing qemu-block for possible comments on patches 2-8,10. Peter Krempa (10): tests: Add testing of backing store string parser util: storage: Add parser for qemu's "json" backing pseudo-protocol util: storage: Add support for host device backing specified via JSON util: storage: Add support for URI based backing volumes in qemu's JSON pseudo-protocol util: storage: Add json pseudo protocol support for gluster volumes util: storage: Add json pseudo protocol support for iSCSI volumes Add JSON backing volume parser for 'nbd' protocol util: storage: Add JSON backing store parser for 'sheepdog' protocol util: storage: Add 'ssh' network storage protocol util: storage: Add JSON backing volume parser for 'ssh' protocol src/libvirt_private.syms | 1 + src/libxl/libxl_conf.c | 1 + src/qemu/qemu_command.c | 7 + src/qemu/qemu_driver.c | 3 + src/qemu/qemu_parse_command.c | 1 + src/util/virstoragefile.c | 288 ++++++++++++++++++++++++++++++++++++++++-- src/util/virstoragefile.h | 4 + src/xenconfig/xen_xl.c | 1 + tests/virstoragetest.c | 128 +++++++++++++++++++ 9 files changed, 425 insertions(+), 9 deletions(-) -- 2.8.2

As we already test that the extraction of the backing store string works well additional tests for the backing store string parser can be made simpler. Export virStorageSourceNewFromBackingAbsolute and use it to parse the backing store strings, format them using virDomainDiskSourceFormat and match them against expected XMLs. --- src/libvirt_private.syms | 1 + src/util/virstoragefile.c | 2 +- src/util/virstoragefile.h | 3 ++ tests/virstoragetest.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 83 insertions(+), 1 deletion(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ba718b8..100841c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2351,6 +2351,7 @@ virStorageSourceIsBlockLocal; virStorageSourceIsEmpty; virStorageSourceIsLocalStorage; virStorageSourceNewFromBacking; +virStorageSourceNewFromBackingAbsolute; virStorageSourceParseRBDColonString; virStorageSourcePoolDefFree; virStorageSourcePoolModeTypeFromString; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 16de603..0fa9681 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2514,7 +2514,7 @@ virStorageSourceParseBackingColon(virStorageSourcePtr src, } -static virStorageSourcePtr +virStorageSourcePtr virStorageSourceNewFromBackingAbsolute(const char *path) { virStorageSourcePtr ret; diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 78beaf4..1a76fad 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -380,4 +380,7 @@ int virStorageFileGetRelativeBackingPath(virStorageSourcePtr from, ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); int virStorageFileCheckCompat(const char *compat); + +virStorageSourcePtr virStorageSourceNewFromBackingAbsolute(const char *path); + #endif /* __VIR_STORAGE_FILE_H__ */ diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 580065e..e2ca0b6 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -665,6 +665,58 @@ testPathRelative(const void *args) } +struct testBackingParseData { + const char *backing; + const char *expect; +}; + +static int +testBackingParse(const void *args) +{ + const struct testBackingParseData *data = args; + virBuffer buf = VIR_BUFFER_INITIALIZER; + virStorageSourcePtr src = NULL; + char *xml = NULL; + int ret = -1; + + if (!(src = virStorageSourceNewFromBackingAbsolute(data->backing))) { + if (!data->expect) + ret = 0; + + goto cleanup; + } + + if (src && !data->expect) { + fprintf(stderr, "parsing of backing store string '%s' should " + "have failed\n", data->backing); + goto cleanup; + } + + if (virDomainDiskSourceFormat(&buf, src, 0, 0) < 0 || + !(xml = virBufferContentAndReset(&buf))) { + fprintf(stderr, "failed to format disk source xml\n"); + goto cleanup; + } + + if (!STREQ(xml, data->expect)) { + fprintf(stderr, "\n backing store string '%s'\n" + "expected storage source xml:\n%s\n" + "actual storage source xml:\n%s\n", + data->backing, data->expect, xml); + goto cleanup; + } + + ret = 0; + + cleanup: + virStorageSourceFree(src); + virBufferFreeAndReset(&buf); + VIR_FREE(xml); + + return ret; +} + + static int mymain(void) { @@ -674,6 +726,7 @@ mymain(void) struct testLookupData data2; struct testPathCanonicalizeData data3; struct testPathRelativeBacking data4; + struct testBackingParseData data5; virStorageSourcePtr chain = NULL; virStorageSourcePtr chain2; /* short for chain->backingStore */ virStorageSourcePtr chain3; /* short for chain2->backingStore */ @@ -1276,6 +1329,31 @@ mymain(void) TEST_RELATIVE_BACKING(21, backingchain[10], backingchain[11], "../../../../blah/image4"); TEST_RELATIVE_BACKING(22, backingchain[11], backingchain[11], "../blah/image4"); + +#define TEST_BACKING_PARSE(id, bck, xml) \ + do { \ + data5.backing = bck; \ + data5.expect = xml; \ + if (virTestRun("Backing store parse " #id, \ + testBackingParse, &data5) < 0) \ + ret = -1; \ + } while (0) + + TEST_BACKING_PARSE(1, "path", "<source file='path'/>\n"); + TEST_BACKING_PARSE(2, "://", NULL); + TEST_BACKING_PARSE(3, "http://example.com/file", + "<source protocol='http' name='file'>\n" + " <host name='example.com'/>\n" + "</source>\n"); + TEST_BACKING_PARSE(3, "rbd:testshare:id=asdf:mon_host=example.com", + "<source protocol='rbd' name='testshare'>\n" + " <host name='example.com'/>\n" + "</source>\n"); + TEST_BACKING_PARSE(4, "nbd:example.org:6000:exportname=blah", + "<source protocol='nbd' name='blah'>\n" + " <host name='example.org' port='6000'/>\n" + "</source>\n"); + cleanup: /* Final cleanup */ virStorageSourceFree(chain); -- 2.8.2

On 07/15/2016 07:46 AM, Peter Krempa wrote:
As we already test that the extraction of the backing store string works well additional tests for the backing store string parser can be made simpler.
Export virStorageSourceNewFromBackingAbsolute and use it to parse the backing store strings, format them using virDomainDiskSourceFormat and match them against expected XMLs. --- src/libvirt_private.syms | 1 + src/util/virstoragefile.c | 2 +- src/util/virstoragefile.h | 3 ++ tests/virstoragetest.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 83 insertions(+), 1 deletion(-)
ACK -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Add a modular parser that will allow to parse 'json' backing definitions that are supported by qemu. The initial implementation adds support for the 'file' driver. --- src/util/virstoragefile.c | 87 +++++++++++++++++++++++++++++++++++++++++++---- tests/virstoragetest.c | 8 +++++ 2 files changed, 88 insertions(+), 7 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 0fa9681..826e4ba 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -43,6 +43,7 @@ #include "viruri.h" #include "dirname.h" #include "virbuffer.h" +#include "virjson.h" #define VIR_FROM_THIS VIR_FROM_STORAGE @@ -2514,10 +2515,80 @@ virStorageSourceParseBackingColon(virStorageSourcePtr src, } +static int +virStorageSourceParseBackingJSONPath(virStorageSourcePtr src, + virJSONValuePtr json, + int type) +{ + const char *path; + + if (!(path = virJSONValueObjectGetString(json, "file.filename"))) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("missing 'filename' field in JSON backing volume " + "definition")); + return -1; + } + + if (VIR_STRDUP(src->path, path) < 0) + return -1; + + src->type = type; + return 0; +} + + +struct virStorageSourceJSONDriverParser { + const char *drvname; + int (*func)(virStorageSourcePtr src, virJSONValuePtr json, int opaque); + int opaque; +}; + +static const struct virStorageSourceJSONDriverParser jsonParsers[] = { + {"file", virStorageSourceParseBackingJSONPath, VIR_STORAGE_TYPE_FILE}, +}; + + +static int +virStorageSourceParseBackingJSON(virStorageSourcePtr src, + const char *json) +{ + virJSONValuePtr root; + const char *drvname; + size_t i; + int ret = -1; + + if (!(root = virJSONValueFromString(json))) + return -1; + + if (!(drvname = virJSONValueObjectGetString(root, "file.driver"))) { + virReportError(VIR_ERR_INVALID_ARG, _("JSON backing volume defintion " + "'%s' lacks driver name"), json); + goto cleanup; + } + + for (i = 0; i < ARRAY_CARDINALITY(jsonParsers); i++) { + if (STREQ(drvname, jsonParsers[i].drvname)) { + ret = jsonParsers[i].func(src, root, jsonParsers[i].opaque); + goto cleanup; + } + } + + virReportError(VIR_ERR_INTERNAL_ERROR, + _("missing parser implementation for JSON backing volume " + "driver '%s'"), drvname); + + cleanup: + virJSONValueFree(root); + return ret; +} + + virStorageSourcePtr virStorageSourceNewFromBackingAbsolute(const char *path) { + const char *json; virStorageSourcePtr ret; + int rc; if (VIR_ALLOC(ret) < 0) return NULL; @@ -2531,13 +2602,15 @@ virStorageSourceNewFromBackingAbsolute(const char *path) ret->type = VIR_STORAGE_TYPE_NETWORK; /* handle URI formatted backing stores */ - if (strstr(path, "://")) { - if (virStorageSourceParseBackingURI(ret, path) < 0) - goto error; - } else { - if (virStorageSourceParseBackingColon(ret, path) < 0) - goto error; - } + if ((json = STRSKIP(path, "json:"))) + rc = virStorageSourceParseBackingJSON(ret, json); + else if (strstr(path, "://")) + rc = virStorageSourceParseBackingURI(ret, path); + else + rc = virStorageSourceParseBackingColon(ret, path); + + if (rc < 0) + goto error; } return ret; diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index e2ca0b6..aa56b61 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1353,6 +1353,14 @@ mymain(void) "<source protocol='nbd' name='blah'>\n" " <host name='example.org' port='6000'/>\n" "</source>\n"); + TEST_BACKING_PARSE(5, "json:", NULL); + TEST_BACKING_PARSE(6, "json:asdgsdfg", NULL); + TEST_BACKING_PARSE(7, "json:{}", NULL); + TEST_BACKING_PARSE(8, "json: { \"file.driver\":\"blah\"}", NULL); + TEST_BACKING_PARSE(9, "json:{\"file.driver\":\"file\"}", NULL); + TEST_BACKING_PARSE(10, "json:{\"file.driver\":\"file\", " + "\"file.filename\":\"/path/to/file\"}", + "<source file='/path/to/file'/>\n"); cleanup: /* Final cleanup */ -- 2.8.2

On 07/15/2016 07:46 AM, Peter Krempa wrote:
Add a modular parser that will allow to parse 'json' backing definitions that are supported by qemu. The initial implementation adds support for the 'file' driver.
Might be nice for this commit message to show an actual json: string that it now accepts.
--- src/util/virstoragefile.c | 87 +++++++++++++++++++++++++++++++++++++++++++---- tests/virstoragetest.c | 8 +++++ 2 files changed, 88 insertions(+), 7 deletions(-)
At some point, I wonder if we can use qemu's QMP introspection to write a meta-modular parser (basically, once QMP blockdev-add is fully-functional, introspection will give a self-describing layout of all possible combinations that QMP will accept, and therefore parsing the introspection output would give a reasonable approach to all possible json: strings that a given qemu will understand). But that's definitely more work, and blockdev-add is not fully working in qemu yet, so hand-duplicating common parses for now at least gets us further than libvirt's current stance of outright choking on json:.
@@ -2514,10 +2515,80 @@ virStorageSourceParseBackingColon(virStorageSourcePtr src, }
+static int +virStorageSourceParseBackingJSONPath(virStorageSourcePtr src, + virJSONValuePtr json, + int type) +{ + const char *path; + + if (!(path = virJSONValueObjectGetString(json, "file.filename"))) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("missing 'filename' field in JSON backing volume " + "definition"));
Do we really want to require libvirt to support "file.filename":"/path/to/foo" compressed naming, or do we want a more hierarchical "file":{"filename":"/path/to/foo"} layout? Or should both be supported? I also wonder if the virJSON code should be enhanced to make it easier to parse deep information, via something similar to xpath notation (other shortcuts that might be nice is "key[0]" as shorthand for accessing element0 out of "key":[element0, element1]). Part of the problem is that qemu is doing so much gross under-the-hood conversion from pure QAPI into QemuOpts (which is completely flat, so we have hacks like "file.filename"), rather than converting from QemuOpts into QAPI, and so now libvirt has to match that grossness.
+++ b/tests/virstoragetest.c @@ -1353,6 +1353,14 @@ mymain(void) "<source protocol='nbd' name='blah'>\n" " <host name='example.org' port='6000'/>\n" "</source>\n"); + TEST_BACKING_PARSE(5, "json:", NULL); + TEST_BACKING_PARSE(6, "json:asdgsdfg", NULL); + TEST_BACKING_PARSE(7, "json:{}", NULL); + TEST_BACKING_PARSE(8, "json: { \"file.driver\":\"blah\"}", NULL); + TEST_BACKING_PARSE(9, "json:{\"file.driver\":\"file\"}", NULL); + TEST_BACKING_PARSE(10, "json:{\"file.driver\":\"file\", " + "\"file.filename\":\"/path/to/file\"}", + "<source file='/path/to/file'/>\n");
Okay, the testsuite shows what you now parse. Again, I'm wondering if qemu should first be improved, and/or whether it already allows: "json:{\"file\":{\"driver\":\"file\", \"filename\":\"/path/to/file\"}}" as equivalent to the "file.driver" and "file.filename" at the flat level. Or put another way, if qemu accepts more than one JSON representation because of the way it crumples and flattens JSON into QDict, maybe libvirt should first have a way to canonicalize into preferred form. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Tue, Jul 26, 2016 at 14:28:23 -0600, Eric Blake wrote:
On 07/15/2016 07:46 AM, Peter Krempa wrote:
Add a modular parser that will allow to parse 'json' backing definitions that are supported by qemu. The initial implementation adds support for the 'file' driver.
Might be nice for this commit message to show an actual json: string that it now accepts.
--- src/util/virstoragefile.c | 87 +++++++++++++++++++++++++++++++++++++++++++---- tests/virstoragetest.c | 8 +++++ 2 files changed, 88 insertions(+), 7 deletions(-)
At some point, I wonder if we can use qemu's QMP introspection to write a meta-modular parser (basically, once QMP blockdev-add is fully-functional, introspection will give a self-describing layout of all possible combinations that QMP will accept, and therefore parsing the introspection output would give a reasonable approach to all possible json: strings that a given qemu will understand). But that's definitely more work, and blockdev-add is not fully working in qemu yet, so hand-duplicating common parses for now at least gets us further than libvirt's current stance of outright choking on json:.
@@ -2514,10 +2515,80 @@ virStorageSourceParseBackingColon(virStorageSourcePtr src, }
+static int +virStorageSourceParseBackingJSONPath(virStorageSourcePtr src, + virJSONValuePtr json, + int type) +{ + const char *path; + + if (!(path = virJSONValueObjectGetString(json, "file.filename"))) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("missing 'filename' field in JSON backing volume " + "definition"));
Do we really want to require libvirt to support "file.filename":"/path/to/foo" compressed naming, or do we want a more
The problem is that once you allow something in qemu there are users which actually will start using it. And that happened.
hierarchical "file":{"filename":"/path/to/foo"} layout? Or should both
Yes I'll switch to this for the main parser and all the weird layouts will be mapped first to this.
be supported? I also wonder if the virJSON code should be enhanced to
So I think we will need to support both.
make it easier to parse deep information, via something similar to xpath notation (other shortcuts that might be nice is "key[0]" as shorthand for accessing element0 out of "key":[element0, element1]).
Part of the problem is that qemu is doing so much gross under-the-hood
And the other part is lack of documentation for that. This also makes users try what works and use that rather than the preferred syntax.
conversion from pure QAPI into QemuOpts (which is completely flat, so we have hacks like "file.filename"), rather than converting from QemuOpts into QAPI, and so now libvirt has to match that grossness.
And since both approaches seem to have attacted their users qemu will need to support it forever. One other weird part is that the parser in qemu(-img) parses the string but then puts it in the user supplied format into the qcow image. Any extra spaces and other stuff are included. By using the formatter for that all the weird syntax could be avoided.
+++ b/tests/virstoragetest.c @@ -1353,6 +1353,14 @@ mymain(void) "<source protocol='nbd' name='blah'>\n" " <host name='example.org' port='6000'/>\n" "</source>\n"); + TEST_BACKING_PARSE(5, "json:", NULL); + TEST_BACKING_PARSE(6, "json:asdgsdfg", NULL); + TEST_BACKING_PARSE(7, "json:{}", NULL); + TEST_BACKING_PARSE(8, "json: { \"file.driver\":\"blah\"}", NULL); + TEST_BACKING_PARSE(9, "json:{\"file.driver\":\"file\"}", NULL); + TEST_BACKING_PARSE(10, "json:{\"file.driver\":\"file\", " + "\"file.filename\":\"/path/to/file\"}", + "<source file='/path/to/file'/>\n");
Okay, the testsuite shows what you now parse. Again, I'm wondering if qemu should first be improved, and/or whether it already allows:
"json:{\"file\":{\"driver\":\"file\", \"filename\":\"/path/to/file\"}}"
as equivalent to the "file.driver" and "file.filename" at the flat level. Or put another way, if qemu accepts more than one JSON representation because of the way it crumples and flattens JSON into QDict, maybe libvirt should first have a way to canonicalize into preferred form.
Yep, the nested objects will be preferred in this case as others are weird syntax can be adapted. Since this RFC was already reused in a different series I'll fix the parser first and then re-post it. Peter

JSON pseudo protocol for qemu allows to explicitly specify devices. Add convertor to the internal type. --- src/util/virstoragefile.c | 2 ++ tests/virstoragetest.c | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 826e4ba..2e47bdb 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2545,6 +2545,8 @@ struct virStorageSourceJSONDriverParser { static const struct virStorageSourceJSONDriverParser jsonParsers[] = { {"file", virStorageSourceParseBackingJSONPath, VIR_STORAGE_TYPE_FILE}, + {"host_device", virStorageSourceParseBackingJSONPath, VIR_STORAGE_TYPE_BLOCK}, + {"host_cdrom", virStorageSourceParseBackingJSONPath, VIR_STORAGE_TYPE_BLOCK}, }; diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index aa56b61..24bb419 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1361,6 +1361,12 @@ mymain(void) TEST_BACKING_PARSE(10, "json:{\"file.driver\":\"file\", " "\"file.filename\":\"/path/to/file\"}", "<source file='/path/to/file'/>\n"); + TEST_BACKING_PARSE(11, "json:{\"file.driver\":\"host_device\", " + "\"file.filename\":\"/path/to/dev\"}", + "<source dev='/path/to/dev'/>\n"); + TEST_BACKING_PARSE(12, "json:{\"file.driver\":\"host_cdrom\", " + "\"file.filename\":\"/path/to/cdrom\"}", + "<source dev='/path/to/cdrom'/>\n"); cleanup: /* Final cleanup */ -- 2.8.2

On 07/15/2016 07:46 AM, Peter Krempa wrote:
JSON pseudo protocol for qemu allows to explicitly specify devices. Add convertor to the internal type.
s/convertor/converter/
--- src/util/virstoragefile.c | 2 ++ tests/virstoragetest.c | 6 ++++++ 2 files changed, 8 insertions(+)
ACK for matching the style in 2/10. Any design changes that affect that one will have obvious ripple effects into this one. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

http(s), ftp(s) and tftp use URIs for volume definitions in the JSON pseudo protocol so it's pretty straightforward to add support for them. --- src/util/virstoragefile.c | 34 ++++++++++++++++++++++++++++++++++ tests/virstoragetest.c | 8 ++++++++ 2 files changed, 42 insertions(+) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 2e47bdb..f8c5f64 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2537,6 +2537,35 @@ virStorageSourceParseBackingJSONPath(virStorageSourcePtr src, } +static int +virStorageSourceParseBackingJSONUri(virStorageSourcePtr src, + virJSONValuePtr json, + int protocol) +{ + const char *uri; + + if (!(uri = virJSONValueObjectGetString(json, "file.uri"))) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("missing URI in JSON backing volume definition")); + return -1; + } + + if (virStorageSourceParseBackingURI(src, uri) < 0) + return -1; + + if (src->protocol != protocol) { + virReportError(VIR_ERR_INVALID_ARG, + _("expected protocol '%s' but got '%s' in URI JSON volume " + "definition"), + virStorageNetProtocolTypeToString(protocol), + virStorageNetProtocolTypeToString(src->protocol)); + return -1; + } + + return 0; +} + + struct virStorageSourceJSONDriverParser { const char *drvname; int (*func)(virStorageSourcePtr src, virJSONValuePtr json, int opaque); @@ -2547,6 +2576,11 @@ static const struct virStorageSourceJSONDriverParser jsonParsers[] = { {"file", virStorageSourceParseBackingJSONPath, VIR_STORAGE_TYPE_FILE}, {"host_device", virStorageSourceParseBackingJSONPath, VIR_STORAGE_TYPE_BLOCK}, {"host_cdrom", virStorageSourceParseBackingJSONPath, VIR_STORAGE_TYPE_BLOCK}, + {"http", virStorageSourceParseBackingJSONUri, VIR_STORAGE_NET_PROTOCOL_HTTP}, + {"https", virStorageSourceParseBackingJSONUri, VIR_STORAGE_NET_PROTOCOL_HTTPS}, + {"ftp", virStorageSourceParseBackingJSONUri, VIR_STORAGE_NET_PROTOCOL_FTP}, + {"ftps", virStorageSourceParseBackingJSONUri, VIR_STORAGE_NET_PROTOCOL_FTPS}, + {"tftp", virStorageSourceParseBackingJSONUri, VIR_STORAGE_NET_PROTOCOL_TFTP}, }; diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 24bb419..955e4db 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1367,6 +1367,14 @@ mymain(void) TEST_BACKING_PARSE(12, "json:{\"file.driver\":\"host_cdrom\", " "\"file.filename\":\"/path/to/cdrom\"}", "<source dev='/path/to/cdrom'/>\n"); + TEST_BACKING_PARSE(13, "json:{\"file.driver\":\"http\", " + "\"file.uri\":\"http://example.com/file\"}", + "<source protocol='http' name='file'>\n" + " <host name='example.com'/>\n" + "</source>\n"); + TEST_BACKING_PARSE(14, "json:{\"file.driver\":\"ftp\", " + "\"file.uri\":\"http://example.com/file\"}", + NULL); cleanup: /* Final cleanup */ -- 2.8.2

On 07/15/2016 07:46 AM, Peter Krempa wrote:
http(s), ftp(s) and tftp use URIs for volume definitions in the JSON pseudo protocol so it's pretty straightforward to add support for them. --- src/util/virstoragefile.c | 34 ++++++++++++++++++++++++++++++++++ tests/virstoragetest.c | 8 ++++++++ 2 files changed, 42 insertions(+)
Still might be nice to include an example in the commit message, not just the testsuite addition. I know that qemu is hoping to move away from URI towards more structured layouts as part of adding blockdev-add support for these drivers (back to your observation that once qemu supports two ways, it will have to continue to support both types of users). That means we may need more code to parse non-URI (but better-structured) uses, but we'd still need this uri parsing.
+++ b/tests/virstoragetest.c @@ -1367,6 +1367,14 @@ mymain(void) TEST_BACKING_PARSE(12, "json:{\"file.driver\":\"host_cdrom\", " "\"file.filename\":\"/path/to/cdrom\"}", "<source dev='/path/to/cdrom'/>\n"); + TEST_BACKING_PARSE(13, "json:{\"file.driver\":\"http\", " + "\"file.uri\":\"http://example.com/file\"}", + "<source protocol='http' name='file'>\n" + " <host name='example.com'/>\n" + "</source>\n"); + TEST_BACKING_PARSE(14, "json:{\"file.driver\":\"ftp\", " + "\"file.uri\":\"http://example.com/file\"}", + NULL);
On the bright side, this patch made it look fairly simple, which means we have good reuse of code. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Extract the common code of the URI parser and reuse it with gluster volumes. The gluster code has a separate function as multi-host support will add an alternative to the URI syntax. --- src/util/virstoragefile.c | 52 +++++++++++++++++++++++++++++++++++++---------- tests/virstoragetest.c | 5 +++++ 2 files changed, 46 insertions(+), 11 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index f8c5f64..6aaf3ff 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2538,18 +2538,10 @@ virStorageSourceParseBackingJSONPath(virStorageSourcePtr src, static int -virStorageSourceParseBackingJSONUri(virStorageSourcePtr src, - virJSONValuePtr json, - int protocol) +virStorageSourceParseBackingJSONUriStr(virStorageSourcePtr src, + const char *uri, + int protocol) { - const char *uri; - - if (!(uri = virJSONValueObjectGetString(json, "file.uri"))) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("missing URI in JSON backing volume definition")); - return -1; - } - if (virStorageSourceParseBackingURI(src, uri) < 0) return -1; @@ -2566,6 +2558,43 @@ virStorageSourceParseBackingJSONUri(virStorageSourcePtr src, } +static int +virStorageSourceParseBackingJSONUri(virStorageSourcePtr src, + virJSONValuePtr json, + int protocol) +{ + const char *uri; + + if (!(uri = virJSONValueObjectGetString(json, "file.uri"))) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("missing URI in JSON backing volume definition")); + return -1; + } + + return virStorageSourceParseBackingJSONUriStr(src, uri, protocol); +} + + +static int +virStorageSourceParseBackingJSONGluster(virStorageSourcePtr src, + virJSONValuePtr json, + int opaque ATTRIBUTE_UNUSED) +{ + const char *uri; + + /* legacy URI based syntax passed via 'filename' option */ + if ((uri = virJSONValueObjectGetString(json, "file.filename"))) + return virStorageSourceParseBackingJSONUriStr(src, uri, + VIR_STORAGE_NET_PROTOCOL_GLUSTER); + + /* gluster currently supports only URI syntax passed in as filename */ + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("missing gluster URI in JSON backing volume definition")); + + return -1; +} + + struct virStorageSourceJSONDriverParser { const char *drvname; int (*func)(virStorageSourcePtr src, virJSONValuePtr json, int opaque); @@ -2581,6 +2610,7 @@ static const struct virStorageSourceJSONDriverParser jsonParsers[] = { {"ftp", virStorageSourceParseBackingJSONUri, VIR_STORAGE_NET_PROTOCOL_FTP}, {"ftps", virStorageSourceParseBackingJSONUri, VIR_STORAGE_NET_PROTOCOL_FTPS}, {"tftp", virStorageSourceParseBackingJSONUri, VIR_STORAGE_NET_PROTOCOL_TFTP}, + {"gluster", virStorageSourceParseBackingJSONGluster, 0}, }; diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 955e4db..4250a2f 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1375,6 +1375,11 @@ mymain(void) TEST_BACKING_PARSE(14, "json:{\"file.driver\":\"ftp\", " "\"file.uri\":\"http://example.com/file\"}", NULL); + TEST_BACKING_PARSE(15, "json:{\"file.driver\":\"gluster\", " + "\"file.filename\":\"gluster://example.com/vol/file\"}", + "<source protocol='gluster' name='vol/file'>\n" + " <host name='example.com'/>\n" + "</source>\n"); cleanup: /* Final cleanup */ -- 2.8.2

iSCSI is a bit odd in this aspect since it only supports URIs but using the 'filename' property and does not have any alternative syntax. --- src/util/virstoragefile.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 6aaf3ff..0679824 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2595,6 +2595,26 @@ virStorageSourceParseBackingJSONGluster(virStorageSourcePtr src, } +static int +virStorageSourceParseBackingJSONiSCSI(virStorageSourcePtr src, + virJSONValuePtr json, + int opaque ATTRIBUTE_UNUSED) +{ + const char *uri; + + /* legacy URI based syntax passed via 'filename' option */ + if ((uri = virJSONValueObjectGetString(json, "file.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")); + + return -1; +} + + struct virStorageSourceJSONDriverParser { const char *drvname; int (*func)(virStorageSourcePtr src, virJSONValuePtr json, int opaque); @@ -2611,6 +2631,7 @@ static const struct virStorageSourceJSONDriverParser jsonParsers[] = { {"ftps", virStorageSourceParseBackingJSONUri, VIR_STORAGE_NET_PROTOCOL_FTPS}, {"tftp", virStorageSourceParseBackingJSONUri, VIR_STORAGE_NET_PROTOCOL_TFTP}, {"gluster", virStorageSourceParseBackingJSONGluster, 0}, + {"iscsi", virStorageSourceParseBackingJSONiSCSI, 0}, }; -- 2.8.2

--- src/util/virstoragefile.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ tests/virstoragetest.c | 14 ++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 0679824..06f9737 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2615,6 +2615,50 @@ virStorageSourceParseBackingJSONiSCSI(virStorageSourcePtr src, } +static int +virStorageSourceParseBackingJSONNbd(virStorageSourcePtr src, + virJSONValuePtr json, + int opaque ATTRIBUTE_UNUSED) +{ + const char *path = virJSONValueObjectGetString(json, "file.path"); + const char *host = virJSONValueObjectGetString(json, "file.host"); + const char *port = virJSONValueObjectGetString(json, "file.port"); + const char *export = virJSONValueObjectGetString(json, "file.export"); + + if (!path && !host) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("missing path or host of NBD server in JSON backing " + "volume definition")); + return -1; + } + + src->type = VIR_STORAGE_TYPE_NETWORK; + src->protocol = VIR_STORAGE_NET_PROTOCOL_NBD; + + if (VIR_STRDUP(src->path, export) < 0) + return -1; + + if (VIR_ALLOC_N(src->hosts, 1) < 0) + 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) + 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; + } + + return 0; +} + + struct virStorageSourceJSONDriverParser { const char *drvname; int (*func)(virStorageSourcePtr src, virJSONValuePtr json, int opaque); @@ -2632,6 +2676,7 @@ static const struct virStorageSourceJSONDriverParser jsonParsers[] = { {"tftp", virStorageSourceParseBackingJSONUri, VIR_STORAGE_NET_PROTOCOL_TFTP}, {"gluster", virStorageSourceParseBackingJSONGluster, 0}, {"iscsi", virStorageSourceParseBackingJSONiSCSI, 0}, + {"nbd", virStorageSourceParseBackingJSONNbd, 0}, }; diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 4250a2f..fcb6750 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1380,6 +1380,20 @@ mymain(void) "<source protocol='gluster' name='vol/file'>\n" " <host name='example.com'/>\n" "</source>\n"); + TEST_BACKING_PARSE(16, "json:{\"file.driver\":\"nbd\"," + "\"file.path\":\"/path/to/socket\"" + "}", + "<source protocol='nbd'>\n" + " <host transport='unix' socket='/path/to/socket'/>\n" + "</source>\n"); + TEST_BACKING_PARSE(17, "json:{\"file.driver\":\"nbd\"," + "\"file.export\":\"blah\"," + "\"file.host\":\"example.org\"," + "\"file.port\":\"6000\"" + "}", + "<source protocol='nbd' name='blah'>\n" + " <host name='example.org' port='6000'/>\n" + "</source>\n"); cleanup: /* Final cleanup */ -- 2.8.2

--- src/util/virstoragefile.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 06f9737..7c6d507 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2659,6 +2659,30 @@ virStorageSourceParseBackingJSONNbd(virStorageSourcePtr src, } +static int +virStorageSourceParseBackingJSONSheepdog(virStorageSourcePtr src, + virJSONValuePtr json, + int opaque ATTRIBUTE_UNUSED) +{ + const char *filename; + + /* legacy URI based syntax passed via 'filename' option */ + if ((filename = virJSONValueObjectGetString(json, "file.filename"))) { + if (strstr(filename, "://")) + return virStorageSourceParseBackingJSONUriStr(src, filename, + VIR_STORAGE_NET_PROTOCOL_SHEEPDOG); + + /* libvirt doesn't implement a parser for the legacy non-URI syntax */ + } + + /* 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")); + + return -1; +} + + struct virStorageSourceJSONDriverParser { const char *drvname; int (*func)(virStorageSourcePtr src, virJSONValuePtr json, int opaque); @@ -2677,6 +2701,7 @@ static const struct virStorageSourceJSONDriverParser jsonParsers[] = { {"gluster", virStorageSourceParseBackingJSONGluster, 0}, {"iscsi", virStorageSourceParseBackingJSONiSCSI, 0}, {"nbd", virStorageSourceParseBackingJSONNbd, 0}, + {"sheepdog", virStorageSourceParseBackingJSONSheepdog, 0}, }; -- 2.8.2

Allow using 'ssh' protocol in backing chains and later for disks themselves. --- src/libxl/libxl_conf.c | 1 + src/qemu/qemu_command.c | 7 +++++++ src/qemu/qemu_driver.c | 3 +++ src/qemu/qemu_parse_command.c | 1 + src/util/virstoragefile.c | 4 +++- src/util/virstoragefile.h | 1 + src/xenconfig/xen_xl.c | 1 + 7 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 146e08a..2d6d5da 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -581,6 +581,7 @@ libxlMakeNetworkDiskSrcStr(virStorageSourcePtr src, case VIR_STORAGE_NET_PROTOCOL_ISCSI: case VIR_STORAGE_NET_PROTOCOL_GLUSTER: case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: + case VIR_STORAGE_NET_PROTOCOL_SSH: case VIR_STORAGE_NET_PROTOCOL_LAST: case VIR_STORAGE_NET_PROTOCOL_NONE: virReportError(VIR_ERR_NO_SUPPORT, diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index fe4bb88..16f1277 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -491,6 +491,9 @@ qemuNetworkDriveGetPort(int protocol, case VIR_STORAGE_NET_PROTOCOL_NBD: return 10809; + case VIR_STORAGE_NET_PROTOCOL_SSH: + return 22; + case VIR_STORAGE_NET_PROTOCOL_ISCSI: case VIR_STORAGE_NET_PROTOCOL_GLUSTER: /* no default port specified */ @@ -889,6 +892,10 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src, ret = virBufferContentAndReset(&buf); break; + case VIR_STORAGE_NET_PROTOCOL_SSH: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("'ssh' protocol is not yet supported")); + goto cleanup; case VIR_STORAGE_NET_PROTOCOL_LAST: case VIR_STORAGE_NET_PROTOCOL_NONE: diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index cda85f6..d07a53c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13359,6 +13359,7 @@ qemuDomainSnapshotPrepareDiskExternalBackingInactive(virDomainDiskDefPtr disk) case VIR_STORAGE_NET_PROTOCOL_FTP: case VIR_STORAGE_NET_PROTOCOL_FTPS: case VIR_STORAGE_NET_PROTOCOL_TFTP: + case VIR_STORAGE_NET_PROTOCOL_SSH: case VIR_STORAGE_NET_PROTOCOL_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, _("external inactive snapshots are not supported on " @@ -13421,6 +13422,7 @@ qemuDomainSnapshotPrepareDiskExternalOverlayActive(virDomainSnapshotDiskDefPtr d case VIR_STORAGE_NET_PROTOCOL_FTP: case VIR_STORAGE_NET_PROTOCOL_FTPS: case VIR_STORAGE_NET_PROTOCOL_TFTP: + case VIR_STORAGE_NET_PROTOCOL_SSH: case VIR_STORAGE_NET_PROTOCOL_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, _("external active snapshots are not supported on " @@ -13565,6 +13567,7 @@ qemuDomainSnapshotPrepareDiskInternal(virConnectPtr conn, case VIR_STORAGE_NET_PROTOCOL_FTP: case VIR_STORAGE_NET_PROTOCOL_FTPS: case VIR_STORAGE_NET_PROTOCOL_TFTP: + case VIR_STORAGE_NET_PROTOCOL_SSH: case VIR_STORAGE_NET_PROTOCOL_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, _("internal inactive snapshots are not supported on " diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c index 3f7e445..82d1621 100644 --- a/src/qemu/qemu_parse_command.c +++ b/src/qemu/qemu_parse_command.c @@ -2014,6 +2014,7 @@ qemuParseCommandLine(virCapsPtr caps, case VIR_STORAGE_NET_PROTOCOL_FTP: case VIR_STORAGE_NET_PROTOCOL_FTPS: case VIR_STORAGE_NET_PROTOCOL_TFTP: + case VIR_STORAGE_NET_PROTOCOL_SSH: case VIR_STORAGE_NET_PROTOCOL_LAST: case VIR_STORAGE_NET_PROTOCOL_NONE: /* ignored for now */ diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 7c6d507..4aa1a9b 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -84,7 +84,8 @@ VIR_ENUM_IMPL(virStorageNetProtocol, VIR_STORAGE_NET_PROTOCOL_LAST, "https", "ftp", "ftps", - "tftp") + "tftp", + "ssh") VIR_ENUM_IMPL(virStorageNetHostTransport, VIR_STORAGE_NET_HOST_TRANS_LAST, "tcp", @@ -2501,6 +2502,7 @@ virStorageSourceParseBackingColon(virStorageSourcePtr src, case VIR_STORAGE_NET_PROTOCOL_TFTP: case VIR_STORAGE_NET_PROTOCOL_ISCSI: case VIR_STORAGE_NET_PROTOCOL_GLUSTER: + case VIR_STORAGE_NET_PROTOCOL_SSH: virReportError(VIR_ERR_INTERNAL_ERROR, _("malformed backing store path for protocol %s"), protocol); diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 1a76fad..3ea3a60 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -132,6 +132,7 @@ typedef enum { VIR_STORAGE_NET_PROTOCOL_FTP, VIR_STORAGE_NET_PROTOCOL_FTPS, VIR_STORAGE_NET_PROTOCOL_TFTP, + VIR_STORAGE_NET_PROTOCOL_SSH, VIR_STORAGE_NET_PROTOCOL_LAST } virStorageNetProtocol; diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c index dcd4849..25a3621 100644 --- a/src/xenconfig/xen_xl.c +++ b/src/xenconfig/xen_xl.c @@ -727,6 +727,7 @@ xenFormatXLDiskSrcNet(virStorageSourcePtr src) case VIR_STORAGE_NET_PROTOCOL_ISCSI: case VIR_STORAGE_NET_PROTOCOL_GLUSTER: case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: + case VIR_STORAGE_NET_PROTOCOL_SSH: case VIR_STORAGE_NET_PROTOCOL_LAST: case VIR_STORAGE_NET_PROTOCOL_NONE: virReportError(VIR_ERR_NO_SUPPORT, -- 2.8.2

--- src/util/virstoragefile.c | 38 ++++++++++++++++++++++++++++++++++++++ tests/virstoragetest.c | 9 +++++++++ 2 files changed, 47 insertions(+) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 4aa1a9b..9c7b5a4 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2685,6 +2685,43 @@ virStorageSourceParseBackingJSONSheepdog(virStorageSourcePtr src, } +static int +virStorageSourceParseBackingJSONSSH(virStorageSourcePtr src, + virJSONValuePtr json, + int opaque ATTRIBUTE_UNUSED) +{ + const char *path = virJSONValueObjectGetString(json, "file.path"); + const char *host = virJSONValueObjectGetString(json, "file.host"); + const char *port = virJSONValueObjectGetString(json, "file.port"); + + if (!host) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("missing host of SSH server in JSON backing " + "volume definition")); + return -1; + } + + src->type = VIR_STORAGE_TYPE_NETWORK; + src->protocol = VIR_STORAGE_NET_PROTOCOL_SSH; + + if (VIR_STRDUP(src->path, path) < 0) + return -1; + + if (VIR_ALLOC_N(src->hosts, 1) < 0) + 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; + + return 0; +} + + struct virStorageSourceJSONDriverParser { const char *drvname; int (*func)(virStorageSourcePtr src, virJSONValuePtr json, int opaque); @@ -2704,6 +2741,7 @@ static const struct virStorageSourceJSONDriverParser jsonParsers[] = { {"iscsi", virStorageSourceParseBackingJSONiSCSI, 0}, {"nbd", virStorageSourceParseBackingJSONNbd, 0}, {"sheepdog", virStorageSourceParseBackingJSONSheepdog, 0}, + {"ssh", virStorageSourceParseBackingJSONSSH, 0}, }; diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index fcb6750..3479cd8 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1394,6 +1394,15 @@ mymain(void) "<source protocol='nbd' name='blah'>\n" " <host name='example.org' port='6000'/>\n" "</source>\n"); + TEST_BACKING_PARSE(17, "json:{\"file.driver\":\"ssh\"," + "\"file.host\":\"example.org\"," + "\"file.port\":\"6000\"," + "\"file.path\":\"blah\"," + "\"file.user\":\"user\"" + "}", + "<source protocol='ssh' name='blah'>\n" + " <host name='example.org' port='6000'/>\n" + "</source>\n"); cleanup: /* Final cleanup */ -- 2.8.2

On Fri, Jul 15, 2016 at 03:46:33PM +0200, Peter Krempa wrote:
Libvirt didn't handle this for a long time and VMs with such config would not start we should implement it.
Using JSON is basically the only option to specify advanced configuration for a backing file.
Field names were taken from qemu's source since there isn't really documentation for this.
Actually, this syntax is formally specified via QEMU QAPI schema language. In particular for block device options see $QEMU/qapi/block-core.json Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 07/15/2016 07:49 AM, Daniel P. Berrange wrote:
On Fri, Jul 15, 2016 at 03:46:33PM +0200, Peter Krempa wrote:
Libvirt didn't handle this for a long time and VMs with such config would not start we should implement it.
Using JSON is basically the only option to specify advanced configuration for a backing file.
Field names were taken from qemu's source since there isn't really documentation for this.
Actually, this syntax is formally specified via QEMU QAPI schema language. In particular for block device options see $QEMU/qapi/block-core.json
Except that it is incomplete; as long as BlockdevOptions doesn't cover every driver, such as NBD, gluster, and sheepdog, there are still some und(er)ocumented aspects. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Fri, Jul 15, 2016 at 13:11:16 -0600, Eric Blake wrote:
On 07/15/2016 07:49 AM, Daniel P. Berrange wrote:
On Fri, Jul 15, 2016 at 03:46:33PM +0200, Peter Krempa wrote:
Libvirt didn't handle this for a long time and VMs with such config would not start we should implement it.
Using JSON is basically the only option to specify advanced configuration for a backing file.
Field names were taken from qemu's source since there isn't really documentation for this.
Actually, this syntax is formally specified via QEMU QAPI schema language. In particular for block device options see $QEMU/qapi/block-core.json
Except that it is incomplete; as long as BlockdevOptions doesn't cover
and wrong in some cases: ftp,http and others implemented by curl use the 'url' filed but the documentation still hints that 'filename' is used while the driver does not allow it. $ qemu-img create -f qcow2 -b 'json: { "file.driver":"http", "file.filename":"http://..." }' bla.img qemu-img: bla.img: curl block driver requires an 'url' option
every driver, such as NBD, gluster, and sheepdog, there are still some
Drivers such as NBD and SSH are actually implemented but not documented.
und(er)ocumented aspects.
Mostly how this maps to the JSON pseudo-protocol as blockdev add uses nested JSON objects whereas the JSON protocol requires the fileds in the "flattened" form. The code looks like the only credible source in this case. Peter
participants (3)
-
Daniel P. Berrange
-
Eric Blake
-
Peter Krempa