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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list