There are a few error messages which might want to report the original backing store string. Pass it around rather than trying to re-generate it. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/util/virstoragefile.c | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index a08705603a..bdf82b04bb 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -3051,12 +3051,14 @@ virStorageSourceParseBackingColon(virStorageSourcePtr src, static int virStorageSourceParseBackingJSONInternal(virStorageSourcePtr src, - virJSONValuePtr json); + virJSONValuePtr json, + const char *jsonstr); static int virStorageSourceParseBackingJSONPath(virStorageSourcePtr src, virJSONValuePtr json, + const char *jsonstr G_GNUC_UNUSED, int type) { const char *path; @@ -3101,6 +3103,7 @@ virStorageSourceParseBackingJSONUriStr(virStorageSourcePtr src, static int virStorageSourceParseBackingJSONUri(virStorageSourcePtr src, virJSONValuePtr json, + const char *jsonstr G_GNUC_UNUSED, int protocol) { const char *uri; @@ -3204,6 +3207,7 @@ virStorageSourceParseBackingJSONSocketAddress(virStorageNetHostDefPtr host, static int virStorageSourceParseBackingJSONGluster(virStorageSourcePtr src, virJSONValuePtr json, + const char *jsonstr G_GNUC_UNUSED, int opaque G_GNUC_UNUSED) { const char *uri = virJSONValueObjectGetString(json, "filename"); @@ -3257,6 +3261,7 @@ virStorageSourceParseBackingJSONGluster(virStorageSourcePtr src, static int virStorageSourceParseBackingJSONiSCSI(virStorageSourcePtr src, virJSONValuePtr json, + const char *jsonstr G_GNUC_UNUSED, int opaque G_GNUC_UNUSED) { const char *transport = virJSONValueObjectGetString(json, "transport"); @@ -3326,6 +3331,7 @@ virStorageSourceParseBackingJSONiSCSI(virStorageSourcePtr src, static int virStorageSourceParseBackingJSONNbd(virStorageSourcePtr src, virJSONValuePtr json, + const char *jsonstr G_GNUC_UNUSED, int opaque G_GNUC_UNUSED) { const char *path = virJSONValueObjectGetString(json, "path"); @@ -3373,6 +3379,7 @@ virStorageSourceParseBackingJSONNbd(virStorageSourcePtr src, static int virStorageSourceParseBackingJSONSheepdog(virStorageSourcePtr src, virJSONValuePtr json, + const char *jsonstr G_GNUC_UNUSED, int opaque G_GNUC_UNUSED) { const char *filename; @@ -3416,6 +3423,7 @@ virStorageSourceParseBackingJSONSheepdog(virStorageSourcePtr src, static int virStorageSourceParseBackingJSONSSH(virStorageSourcePtr src, virJSONValuePtr json, + const char *jsonstr G_GNUC_UNUSED, int opaque G_GNUC_UNUSED) { const char *path = virJSONValueObjectGetString(json, "path"); @@ -3458,6 +3466,7 @@ virStorageSourceParseBackingJSONSSH(virStorageSourcePtr src, static int virStorageSourceParseBackingJSONRBD(virStorageSourcePtr src, virJSONValuePtr json, + const char *jsonstr G_GNUC_UNUSED, int opaque G_GNUC_UNUSED) { const char *filename; @@ -3509,18 +3518,20 @@ virStorageSourceParseBackingJSONRBD(virStorageSourcePtr src, static int virStorageSourceParseBackingJSONRaw(virStorageSourcePtr src, virJSONValuePtr json, + const char *jsonstr, int opaque G_GNUC_UNUSED) { /* There are no interesting attributes in raw driver. * Treat it as pass-through. */ - return virStorageSourceParseBackingJSONInternal(src, json); + return virStorageSourceParseBackingJSONInternal(src, json, jsonstr); } static int virStorageSourceParseBackingJSONVxHS(virStorageSourcePtr src, virJSONValuePtr json, + const char *jsonstr G_GNUC_UNUSED, int opaque G_GNUC_UNUSED) { const char *vdisk_id = virJSONValueObjectGetString(json, "vdisk-id"); @@ -3559,7 +3570,7 @@ struct virStorageSourceJSONDriverParser { * can't be converted to libvirt's configuration (e.g. inline authentication * credentials are present). */ - int (*func)(virStorageSourcePtr src, virJSONValuePtr json, int opaque); + int (*func)(virStorageSourcePtr src, virJSONValuePtr json, const char *jsonstr, int opaque); int opaque; }; @@ -3586,36 +3597,34 @@ static const struct virStorageSourceJSONDriverParser jsonParsers[] = { static int virStorageSourceParseBackingJSONInternal(virStorageSourcePtr src, - virJSONValuePtr json) + virJSONValuePtr json, + const char *jsonstr) { g_autoptr(virJSONValue) deflattened = NULL; virJSONValuePtr file; const char *drvname; size_t i; - g_autofree char *str = NULL; if (!(deflattened = virJSONValueObjectDeflatten(json))) return -1; if (!(file = virJSONValueObjectGetObject(deflattened, "file"))) { - str = virJSONValueToString(json, false); virReportError(VIR_ERR_INVALID_ARG, _("JSON backing volume definition '%s' lacks 'file' object"), - NULLSTR(str)); + jsonstr); return -1; } if (!(drvname = virJSONValueObjectGetString(file, "driver"))) { - str = virJSONValueToString(json, false); virReportError(VIR_ERR_INVALID_ARG, _("JSON backing volume definition '%s' lacks driver name"), - NULLSTR(str)); + jsonstr); return -1; } for (i = 0; i < G_N_ELEMENTS(jsonParsers); i++) { if (STREQ(drvname, jsonParsers[i].drvname)) - return jsonParsers[i].func(src, file, jsonParsers[i].opaque); + return jsonParsers[i].func(src, file, jsonstr, jsonParsers[i].opaque); } virReportError(VIR_ERR_INTERNAL_ERROR, @@ -3634,7 +3643,7 @@ virStorageSourceParseBackingJSON(virStorageSourcePtr src, if (!(root = virJSONValueFromString(json))) return -1; - return virStorageSourceParseBackingJSONInternal(src, root); + return virStorageSourceParseBackingJSONInternal(src, root, json); } -- 2.24.1

Originally virStorageSourceParseBackingJSON didn't recurse, but when the 'raw' driver support was added we need to parse it's information which contains nested 'file' object. Since the deflattening helper recurses already there's no need to call it again. Move it one level up to the entry point. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- 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 bdf82b04bb..fd0069a4fa 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -3600,15 +3600,11 @@ virStorageSourceParseBackingJSONInternal(virStorageSourcePtr src, virJSONValuePtr json, const char *jsonstr) { - g_autoptr(virJSONValue) deflattened = NULL; virJSONValuePtr file; const char *drvname; size_t i; - if (!(deflattened = virJSONValueObjectDeflatten(json))) - return -1; - - if (!(file = virJSONValueObjectGetObject(deflattened, "file"))) { + if (!(file = virJSONValueObjectGetObject(json, "file"))) { virReportError(VIR_ERR_INVALID_ARG, _("JSON backing volume definition '%s' lacks 'file' object"), jsonstr); @@ -3639,11 +3635,15 @@ virStorageSourceParseBackingJSON(virStorageSourcePtr src, const char *json) { g_autoptr(virJSONValue) root = NULL; + g_autoptr(virJSONValue) deflattened = NULL; if (!(root = virJSONValueFromString(json))) return -1; - return virStorageSourceParseBackingJSONInternal(src, root, json); + if (!(deflattened = virJSONValueObjectDeflatten(root))) + return -1; + + return virStorageSourceParseBackingJSONInternal(src, deflattened, json); } -- 2.24.1

The parser was originally designed only for protocol parsers. Since we already have 'raw' format driver in the list we'll need to be able to parse it too. In later patches this will be used to prevent parsing nested format drivers. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/util/virstoragefile.c | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index fd0069a4fa..565ba08319 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -3563,6 +3563,7 @@ virStorageSourceParseBackingJSONVxHS(virStorageSourcePtr src, struct virStorageSourceJSONDriverParser { const char *drvname; + bool formatdriver; /** * The callback gets a pre-allocated storage source @src and the JSON * object to parse. The callback shall return -1 on error and report error @@ -3575,22 +3576,22 @@ 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}, - {"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}, - {"gluster", virStorageSourceParseBackingJSONGluster, 0}, - {"iscsi", virStorageSourceParseBackingJSONiSCSI, 0}, - {"nbd", virStorageSourceParseBackingJSONNbd, 0}, - {"sheepdog", virStorageSourceParseBackingJSONSheepdog, 0}, - {"ssh", virStorageSourceParseBackingJSONSSH, 0}, - {"rbd", virStorageSourceParseBackingJSONRBD, 0}, - {"raw", virStorageSourceParseBackingJSONRaw, 0}, - {"vxhs", virStorageSourceParseBackingJSONVxHS, 0}, + {"file", false, virStorageSourceParseBackingJSONPath, VIR_STORAGE_TYPE_FILE}, + {"host_device", false, virStorageSourceParseBackingJSONPath, VIR_STORAGE_TYPE_BLOCK}, + {"host_cdrom", false, virStorageSourceParseBackingJSONPath, VIR_STORAGE_TYPE_BLOCK}, + {"http", false, virStorageSourceParseBackingJSONUri, VIR_STORAGE_NET_PROTOCOL_HTTP}, + {"https", false, virStorageSourceParseBackingJSONUri, VIR_STORAGE_NET_PROTOCOL_HTTPS}, + {"ftp", false, virStorageSourceParseBackingJSONUri, VIR_STORAGE_NET_PROTOCOL_FTP}, + {"ftps", false, virStorageSourceParseBackingJSONUri, VIR_STORAGE_NET_PROTOCOL_FTPS}, + {"tftp", false, virStorageSourceParseBackingJSONUri, VIR_STORAGE_NET_PROTOCOL_TFTP}, + {"gluster", false, virStorageSourceParseBackingJSONGluster, 0}, + {"iscsi", false, virStorageSourceParseBackingJSONiSCSI, 0}, + {"nbd", false, virStorageSourceParseBackingJSONNbd, 0}, + {"sheepdog", false, virStorageSourceParseBackingJSONSheepdog, 0}, + {"ssh", false, virStorageSourceParseBackingJSONSSH, 0}, + {"rbd", false, virStorageSourceParseBackingJSONRBD, 0}, + {"raw", true, virStorageSourceParseBackingJSONRaw, 0}, + {"vxhs", false, virStorageSourceParseBackingJSONVxHS, 0}, }; -- 2.24.1

There are two possibilities: 1) json:{"file":{"driver":...}} 2) json:{"driver":...} Our code didn't work properly with the second one as it was expecting the 'file' wrapper. Conditionalize the removal to only the situation when the top level doesn't have "driver". Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/util/virstoragefile.c | 41 +++++++++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 565ba08319..ddf837c3b3 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -3521,10 +3521,17 @@ virStorageSourceParseBackingJSONRaw(virStorageSourcePtr src, const char *jsonstr, int opaque G_GNUC_UNUSED) { - /* There are no interesting attributes in raw driver. - * Treat it as pass-through. - */ - return virStorageSourceParseBackingJSONInternal(src, json, jsonstr); + virJSONValuePtr file; + + /* 'raw' is a format driver so it can have protocol driver children */ + if (!(file = virJSONValueObjectGetObject(json, "file"))) { + virReportError(VIR_ERR_INVALID_ARG, + _("JSON backing volume definition '%s' lacks 'file' object"), + jsonstr); + return -1; + } + + return virStorageSourceParseBackingJSONInternal(src, file, jsonstr); } @@ -3601,18 +3608,10 @@ virStorageSourceParseBackingJSONInternal(virStorageSourcePtr src, virJSONValuePtr json, const char *jsonstr) { - virJSONValuePtr file; const char *drvname; size_t i; - if (!(file = virJSONValueObjectGetObject(json, "file"))) { - virReportError(VIR_ERR_INVALID_ARG, - _("JSON backing volume definition '%s' lacks 'file' object"), - jsonstr); - return -1; - } - - if (!(drvname = virJSONValueObjectGetString(file, "driver"))) { + if (!(drvname = virJSONValueObjectGetString(json, "driver"))) { virReportError(VIR_ERR_INVALID_ARG, _("JSON backing volume definition '%s' lacks driver name"), jsonstr); @@ -3621,7 +3620,7 @@ virStorageSourceParseBackingJSONInternal(virStorageSourcePtr src, for (i = 0; i < G_N_ELEMENTS(jsonParsers); i++) { if (STREQ(drvname, jsonParsers[i].drvname)) - return jsonParsers[i].func(src, file, jsonstr, jsonParsers[i].opaque); + return jsonParsers[i].func(src, json, jsonstr, jsonParsers[i].opaque); } virReportError(VIR_ERR_INTERNAL_ERROR, @@ -3637,6 +3636,7 @@ virStorageSourceParseBackingJSON(virStorageSourcePtr src, { g_autoptr(virJSONValue) root = NULL; g_autoptr(virJSONValue) deflattened = NULL; + virJSONValuePtr file = NULL; if (!(root = virJSONValueFromString(json))) return -1; @@ -3644,7 +3644,18 @@ virStorageSourceParseBackingJSON(virStorageSourcePtr src, if (!(deflattened = virJSONValueObjectDeflatten(root))) return -1; - return virStorageSourceParseBackingJSONInternal(src, deflattened, json); + /* There are 2 possible syntaxes: + * 1) json:{"file":{"driver":...}} + * 2) json:{"driver":...} + * Remove the 'file' wrapper object in case 1. + */ + if (!virJSONValueObjectHasKey(deflattened, "driver")) + file = virJSONValueObjectGetObject(deflattened, "file"); + + if (!file) + file = deflattened; + + return virStorageSourceParseBackingJSONInternal(src, file, json); } -- 2.24.1

Since we parse attributes for 'raw' which is a format driver and thus has nested 'file' structure we must prevent that this isn't nested arbitrarily. Add a flag for the function which allows parsing of 'format' type drivers only on the first pass. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/util/virstoragefile.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index ddf837c3b3..7ffb2cdcf4 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -3052,7 +3052,8 @@ virStorageSourceParseBackingColon(virStorageSourcePtr src, static int virStorageSourceParseBackingJSONInternal(virStorageSourcePtr src, virJSONValuePtr json, - const char *jsonstr); + const char *jsonstr, + bool allowformat); static int @@ -3531,7 +3532,7 @@ virStorageSourceParseBackingJSONRaw(virStorageSourcePtr src, return -1; } - return virStorageSourceParseBackingJSONInternal(src, file, jsonstr); + return virStorageSourceParseBackingJSONInternal(src, file, jsonstr, false); } @@ -3606,7 +3607,8 @@ static const struct virStorageSourceJSONDriverParser jsonParsers[] = { static int virStorageSourceParseBackingJSONInternal(virStorageSourcePtr src, virJSONValuePtr json, - const char *jsonstr) + const char *jsonstr, + bool allowformat) { const char *drvname; size_t i; @@ -3619,8 +3621,17 @@ virStorageSourceParseBackingJSONInternal(virStorageSourcePtr src, } for (i = 0; i < G_N_ELEMENTS(jsonParsers); i++) { - if (STREQ(drvname, jsonParsers[i].drvname)) - return jsonParsers[i].func(src, json, jsonstr, jsonParsers[i].opaque); + if (STRNEQ(drvname, jsonParsers[i].drvname)) + continue; + + if (jsonParsers[i].formatdriver && !allowformat) { + virReportError(VIR_ERR_INVALID_ARG, + _("JSON backing volume definition '%s' must not have nested format drivers"), + jsonstr); + return -1; + } + + return jsonParsers[i].func(src, json, jsonstr, jsonParsers[i].opaque); } virReportError(VIR_ERR_INTERNAL_ERROR, @@ -3655,7 +3666,7 @@ virStorageSourceParseBackingJSON(virStorageSourcePtr src, if (!file) file = deflattened; - return virStorageSourceParseBackingJSONInternal(src, file, json); + return virStorageSourceParseBackingJSONInternal(src, file, json, true); } -- 2.24.1

Add few cases that prove the second format of "json:" pseudo-URIs. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- tests/virstoragetest.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 4341c04b1e..6d62aab654 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1315,6 +1315,10 @@ mymain(void) "}" "}", "<source file='/path/to/file'/>\n"); + TEST_BACKING_PARSE("json:{\"driver\":\"file\"," + "\"filename\":\"/path/to/file\"" + "}", + "<source file='/path/to/file'/>\n"); TEST_BACKING_PARSE("json:{\"file.driver\":\"host_device\", " "\"file.filename\":\"/path/to/dev\"}", "<source dev='/path/to/dev'/>\n"); @@ -1389,6 +1393,12 @@ mymain(void) "<source protocol='nbd'>\n" " <host transport='unix' socket='/path/to/socket'/>\n" "</source>\n"); + TEST_BACKING_PARSE("json:{\"driver\":\"nbd\"," + "\"path\":\"/path/to/socket\"" + "}", + "<source protocol='nbd'>\n" + " <host transport='unix' socket='/path/to/socket'/>\n" + "</source>\n"); TEST_BACKING_PARSE("json:{\"file.driver\":\"nbd\"," "\"file.path\":\"/path/to/socket\"" "}", -- 2.24.1

QEMU allows specifying the offset and size into a raw file to expose a sub-slice of the image to the guest with the raw driver. Libvirt currently doesn't support it but we can add test case for future reference. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- tests/virstoragetest.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 6d62aab654..25d41f0de4 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1593,6 +1593,15 @@ mymain(void) "<source protocol='vxhs' name='c6718f6b-0401-441d-a8c3-1f0064d75ee0'>\n" " <host name='example.com' port='9999'/>\n" "</source>\n"); + TEST_BACKING_PARSE_FULL("json:{ \"driver\": \"raw\"," + "\"offset\": 10752," + "\"size\": 4063232," + "\"file\": { \"driver\": \"file\"," + "\"filename\": \"/tmp/testfle\"" + "}" + "}", + "<source file='/tmp/testfle'/>\n", 0); + #endif /* WITH_YAJL */ cleanup: -- 2.24.1

Introduce virStorageSourceSlice which will store the 'offset' and 'size' of a virStorageSource and declare it as 'sliceStorage' and 'sliceFormat' attributes of a virStorageSource. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/util/virstoragefile.c | 18 ++++++++++++++++++ src/util/virstoragefile.h | 12 ++++++++++++ 2 files changed, 30 insertions(+) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 7ffb2cdcf4..30d5a2fe67 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2248,6 +2248,18 @@ virStorageSourcePoolDefCopy(const virStorageSourcePoolDef *src) } +static virStorageSourceSlicePtr +virStorageSourceSliceCopy(const virStorageSourceSlice *src) +{ + virStorageSourceSlicePtr ret = g_new0(virStorageSourceSlice, 1); + + ret->offset = src->offset; + ret->size = src->size; + + return ret; +} + + /** * virStorageSourcePtr: * @@ -2302,6 +2314,9 @@ virStorageSourceCopy(const virStorageSource *src, def->tlsAlias = g_strdup(src->tlsAlias); def->tlsCertdir = g_strdup(src->tlsCertdir); + def->sliceFormat = virStorageSourceSliceCopy(src->sliceFormat); + def->sliceStorage = virStorageSourceSliceCopy(src->sliceStorage); + if (src->nhosts) { if (!(def->hosts = virStorageNetHostDefCopy(src->nhosts, src->hosts))) return NULL; @@ -2581,6 +2596,9 @@ virStorageSourceClear(virStorageSourcePtr def) VIR_FREE(def->timestamps); VIR_FREE(def->externalDataStoreRaw); + VIR_FREE(def->sliceFormat); + VIR_FREE(def->sliceStorage); + virObjectUnref(def->externalDataStore); def->externalDataStore = NULL; diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 39e50a989d..a6b474ec51 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -242,6 +242,15 @@ struct _virStorageSourceNVMeDef { /* Don't forget to update virStorageSourceNVMeDefCopy */ }; + +typedef struct _virStorageSourceSlice virStorageSourceSlice; +typedef virStorageSourceSlice *virStorageSourceSlicePtr; +struct _virStorageSourceSlice { + unsigned long long offset; + unsigned long long size; +}; + + typedef struct _virStorageDriverData virStorageDriverData; typedef virStorageDriverData *virStorageDriverDataPtr; @@ -286,6 +295,9 @@ struct _virStorageSource { bool nocow; bool sparse; + virStorageSourceSlicePtr sliceFormat; + virStorageSourceSlicePtr sliceStorage; + virStoragePermsPtr perms; virStorageTimestampsPtr timestamps; unsigned long long capacity; /* in bytes, 0 if unknown */ -- 2.24.1

If we have a 'format' type slice for a raw driver we can directly format the values. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_block.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 63116ef5f2..5fac8c17b4 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1187,16 +1187,18 @@ qemuBlockStorageSourceGetFormatRawProps(virStorageSourcePtr src, secretalias = srcPriv->encinfo->s.aes.alias; } - /* currently unhandled properties for the 'raw' driver: - * 'offset' - * 'size' - */ - if (virJSONValueObjectAdd(props, "s:driver", driver, "S:key-secret", secretalias, NULL) < 0) return -1; + if (src->sliceFormat && + virJSONValueObjectAdd(props, + "p:offset", src->sliceFormat->offset, + "p:size", src->sliceFormat->size, + NULL) < 0) + return -1; + return 0; } -- 2.24.1

On Thu, Feb 06, 2020 at 16:39:12 +0100, Ján Tomko wrote: Oops, I wanted to actually format them always. Thus I should use U.

We will currently support slice only for the 'raw' format slice reject any other option. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_domain.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 6ebee4d5f4..8a38101bea 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6823,6 +6823,21 @@ qemuDomainValidateStorageSource(virStorageSourcePtr src, return -1; } + if (src->sliceStorage) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("storage slice is not supported for format '%s'"), + virStorageFileFormatTypeToString(src->format)); + return -1; + } + + if (src->sliceFormat && + src->format != VIR_STORAGE_FILE_RAW) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("format slice is not supported for format '%s'"), + virStorageFileFormatTypeToString(src->format)); + return -1; + } + return 0; } -- 2.24.1

Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- docs/formatdomain.html.in | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 60a103d7c6..acb604e9c7 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2881,6 +2881,7 @@ &lt;reservations managed='no'&gt; &lt;source type='unix' path='/path/to/qemu-pr-helper' mode='client'/&gt; &lt;/reservations&gt; + &lt;/source&gt; &lt;target dev='sda' bus='scsi'/&gt; &lt;address type='drive' controller='0' bus='0' target='3' unit='0'/&gt; &lt;/disk&gt; -- 2.24.1

We are going to add support for specifying offset and size attributes which will allow controling where the image and where the guest data itself starts in the source of the disk. This will be represented by a <slices> element filled with either a <slice type='storage'> for the offset of the image format itself. <slice type='format'> then controls where the guest data starts in the image. Add the XML documentation and RNG schema. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- docs/formatdomain.html.in | 11 +++++++++++ docs/schemas/domaincommon.rng | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index acb604e9c7..2143e57b11 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2878,6 +2878,9 @@ &lt;disk type='block' device='lun'&gt; &lt;driver name='qemu' type='raw'/&gt; &lt;source dev='/dev/sda'&gt; + &lt;slices&gt; + &lt;slice type='format' offset='12345' size='123'/&gt; + &lt;/slices&gt; &lt;reservations managed='no'&gt; &lt;source type='unix' path='/path/to/qemu-pr-helper' mode='client'/&gt; &lt;/reservations&gt; @@ -3360,6 +3363,14 @@ controller. <span class="since">Since 6.0.0</span> </dd> + <dt><code>slices</code></dt> + <dd>The <code>slices</code> element using its <code>slice</code> + sub-elements allows configuring offset and size of either the + location of the image format (<code>slice type='storage'</code>) or + the guest data in the image container (<code>slice type='format'</code>). + + The <code>offset</code> and <code>size</code> values are in bytes. + </dd> </dl> <p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index ea237a05e5..720e4e3da7 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1595,12 +1595,45 @@ </optional> </define> + <define name="diskSourceSlice"> + <attribute name='offset'> + <ref name="positiveInteger"/> + </attribute> + <attribute name='size'> + <ref name="positiveInteger"/> + </attribute> + </define> + <define name="diskSourceCommon"> <optional> <attribute name="index"> <ref name="positiveInteger"/> </attribute> </optional> + <optional> + <element name='slices'> + <oneOrMore> + <choice> + <group> + <element name='slice'> + <attribute name='type'> + <value>storage</value> + </attribute> + <ref name="diskSourceSlice"/> + </element> + </group> + <group> + <element name='slice'> + <attribute name='type'> + <value>format</value> + </attribute> + <ref name="diskSourceSlice"/> + </element> + </group> + </choice> + </oneOrMore> + </element> + </optional> </define> <define name="diskSource"> -- 2.24.1

Implement parsing and formatting of the 'source' and 'format' slices. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/conf/domain_conf.c | 92 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c95bd34fb5..4d1bace700 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9467,6 +9467,62 @@ virDomainStorageSourceParseBase(const char *type, } +static virStorageSourceSlicePtr +virDomainStorageSourceParseSlice(xmlNodePtr node, + xmlXPathContextPtr ctxt) +{ + VIR_XPATH_NODE_AUTORESTORE(ctxt); + g_autofree char *offset = NULL; + g_autofree char *size = NULL; + g_autofree virStorageSourceSlicePtr ret = g_new0(virStorageSourceSlice, 1); + + ctxt->node = node; + + if (!(offset = virXPathString("string(./@offset)", ctxt)) || + !(size = virXPathString("string(./@size)", ctxt))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing offset or size attribute of slice")); + return NULL; + } + + if (virStrToLong_ullp(offset, NULL, 10, &ret->offset) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("malformed value '%s' of 'offset' attribute of slice"), + offset); + return NULL; + } + + if (virStrToLong_ullp(size, NULL, 10, &ret->size) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("malformed value '%s' of 'size' attribute of slice"), + size); + return NULL; + } + + return g_steal_pointer(&ret); +} + + +static int +virDomainStorageSourceParseSlices(virStorageSourcePtr src, + xmlXPathContextPtr ctxt) +{ + xmlNodePtr node; + + if ((node = virXPathNode("./slices/slice[@type='storage']", ctxt))) { + if (!(src->sliceStorage = virDomainStorageSourceParseSlice(node, ctxt))) + return -1; + } + + if ((node = virXPathNode("./slices/slice[@type='format']", ctxt))) { + if (!(src->sliceFormat = virDomainStorageSourceParseSlice(node, ctxt))) + return -1; + } + + return 0; +} + + /** * virDomainStorageSourceParse: * @node: XML node pointing to the source element to parse @@ -9532,6 +9588,9 @@ virDomainStorageSourceParse(xmlNodePtr node, if (virDomainDiskSourcePRParse(node, ctxt, &src->pr) < 0) return -1; + if (virDomainStorageSourceParseSlices(src, ctxt) < 0) + return -1; + if (virSecurityDeviceLabelDefParseXML(&src->seclabels, &src->nseclabels, ctxt, flags) < 0) return -1; @@ -24373,6 +24432,37 @@ virDomainDiskSourceFormatPrivateData(virBufferPtr buf, } +static void +virDomainDiskSourceFormatSlice(virBufferPtr buf, + const char *slicetype, + virStorageSourceSlicePtr slice) +{ + g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; + + if (!slice) + return; + + virBufferAsprintf(&attrBuf, " type='%s'", slicetype); + virBufferAsprintf(&attrBuf, " offset='%llu'", slice->offset); + virBufferAsprintf(&attrBuf, " size='%llu'", slice->size); + + virXMLFormatElement(buf, "slice", &attrBuf, NULL); +} + + +static void +virDomainDiskSourceFormatSlices(virBufferPtr buf, + virStorageSourcePtr src) +{ + g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); + + virDomainDiskSourceFormatSlice(&childBuf, "format", src->sliceFormat); + virDomainDiskSourceFormatSlice(&childBuf, "storage", src->sliceStorage); + + virXMLFormatElement(buf, "slices", NULL, &childBuf); +} + + /** * virDomainDiskSourceFormat: * @buf: output buffer @@ -24443,6 +24533,8 @@ virDomainDiskSourceFormat(virBufferPtr buf, return -1; } + virDomainDiskSourceFormatSlices(&childBuf, src); + if (src->type != VIR_STORAGE_TYPE_NETWORK) virDomainSourceDefFormatSeclabel(&childBuf, src->nseclabels, src->seclabels, flags); -- 2.24.1

Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- .../disk-slices.x86_64-latest.args | 50 +++++++++++++++++ tests/qemuxml2argvdata/disk-slices.xml | 45 +++++++++++++++ tests/qemuxml2argvtest.c | 2 + .../disk-slices.x86_64-latest.xml | 56 +++++++++++++++++++ tests/qemuxml2xmltest.c | 2 + 5 files changed, 155 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-slices.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/disk-slices.xml create mode 100644 tests/qemuxml2xmloutdata/disk-slices.x86_64-latest.xml diff --git a/tests/qemuxml2argvdata/disk-slices.x86_64-latest.args b/tests/qemuxml2argvdata/disk-slices.x86_64-latest.args new file mode 100644 index 0000000000..9beb07798e --- /dev/null +++ b/tests/qemuxml2argvdata/disk-slices.x86_64-latest.args @@ -0,0 +1,50 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-i386 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object secret,id=masterKey0,format=raw,\ +file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off \ +-cpu qemu64 \ +-m 214 \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot strict=on \ +-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ +-blockdev '{"driver":"file","filename":"/var/lib/libvirt/images/raw.img",\ +"node-name":"libvirt-3-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-3-format","read-only":false,"driver":"raw",\ +"offset":1234,"size":321,"file":"libvirt-3-storage"}' \ +-device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x2,drive=libvirt-3-format,\ +id=virtio-disk0,bootindex=1 \ +-blockdev '{"driver":"file","filename":"/var/lib/libvirt/images/raw.img",\ +"node-name":"libvirt-2-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-2-format","read-only":true,"driver":"raw",\ +"offset":9876,"size":123456789,"file":"libvirt-2-storage"}' \ +-blockdev '{"driver":"file","filename":"/var/lib/libvirt/images/overlay.qcow2",\ +"node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"qcow2",\ +"file":"libvirt-1-storage","backing":"libvirt-2-format"}' \ +-device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x3,drive=libvirt-1-format,\ +id=virtio-disk1 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ +resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/disk-slices.xml b/tests/qemuxml2argvdata/disk-slices.xml new file mode 100644 index 0000000000..1385cc3b95 --- /dev/null +++ b/tests/qemuxml2argvdata/disk-slices.xml @@ -0,0 +1,45 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i386</emulator> + <disk type='file' device='disk'> + <driver name='qemu' type='raw'/> + <source file='/var/lib/libvirt/images/raw.img'> + <slices> + <slice type='format' offset='1234' size='321'/> + </slices> + </source> + <backingStore/> + <target dev='vda' bus='virtio'/> + </disk> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2'/> + <source file='/var/lib/libvirt/images/overlay.qcow2'/> + <backingStore type='file'> + <format type='raw'/> + <source file='/var/lib/libvirt/images/raw.img'> + <slices> + <slice type='format' offset='9876' size='123456789'/> + </slices> + </source> + <backingStore/> + </backingStore> + <target dev='vdb' bus='virtio'/> + </disk> + <controller type='usb'/> + <controller type='pci' model='pci-root'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index a36183bf34..79b2d0f435 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1162,6 +1162,8 @@ mymain(void) DO_TEST_CAPS_VER("disk-backing-chains-noindex", "2.12.0"); DO_TEST_CAPS_LATEST("disk-backing-chains-noindex"); + DO_TEST_CAPS_LATEST("disk-slices"); + DO_TEST("graphics-egl-headless", QEMU_CAPS_EGL_HEADLESS, QEMU_CAPS_DEVICE_CIRRUS_VGA); diff --git a/tests/qemuxml2xmloutdata/disk-slices.x86_64-latest.xml b/tests/qemuxml2xmloutdata/disk-slices.x86_64-latest.xml new file mode 100644 index 0000000000..d3520a5ba4 --- /dev/null +++ b/tests/qemuxml2xmloutdata/disk-slices.x86_64-latest.xml @@ -0,0 +1,56 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i386</emulator> + <disk type='file' device='disk'> + <driver name='qemu' type='raw'/> + <source file='/var/lib/libvirt/images/raw.img'> + <slices> + <slice type='format' offset='1234' size='321'/> + </slices> + </source> + <backingStore/> + <target dev='vda' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </disk> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2'/> + <source file='/var/lib/libvirt/images/overlay.qcow2'/> + <backingStore type='file'> + <format type='raw'/> + <source file='/var/lib/libvirt/images/raw.img'> + <slices> + <slice type='format' offset='9876' size='123456789'/> + </slices> + </source> + <backingStore/> + </backingStore> + <target dev='vdb' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </disk> + <controller type='usb' index='0' model='piix3-uhci'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index fa238ec339..5856d802f4 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -530,6 +530,8 @@ mymain(void) DO_TEST("pci-rom-disabled-invalid", NONE); DO_TEST("pci-serial-dev-chardev", NONE); + DO_TEST_CAPS_LATEST("disk-slices"); + DO_TEST("encrypted-disk", QEMU_CAPS_QCOW2_LUKS); DO_TEST("encrypted-disk-usage", QEMU_CAPS_QCOW2_LUKS); DO_TEST("luks-disks", NONE); -- 2.24.1

If the parsed 'raw' format JSON string has 'offset' or 'size' attributes parse them as the format slice. https://bugzilla.redhat.com/show_bug.cgi?id=1791788 Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/util/virstoragefile.c | 20 ++++++++++++++++++++ tests/virstoragetest.c | 6 +++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 30d5a2fe67..802f3d59be 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -3540,8 +3540,28 @@ virStorageSourceParseBackingJSONRaw(virStorageSourcePtr src, const char *jsonstr, int opaque G_GNUC_UNUSED) { + bool has_offset = virJSONValueObjectHasKey(json, "offset"); + bool has_size = virJSONValueObjectHasKey(json, "size"); virJSONValuePtr file; + if (has_offset || has_size) { + src->sliceFormat = g_new0(virStorageSourceSlice, 1); + + if (has_offset && + virJSONValueObjectGetNumberUlong(json, "offset", &src->sliceFormat->offset) < 0) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("malformed 'offset' property of 'raw' driver")); + return -1; + } + + if (has_size && + virJSONValueObjectGetNumberUlong(json, "size", &src->sliceFormat->size) < 0) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("malformed 'size' property of 'raw' driver")); + return -1; + } + } + /* 'raw' is a format driver so it can have protocol driver children */ if (!(file = virJSONValueObjectGetObject(json, "file"))) { virReportError(VIR_ERR_INVALID_ARG, diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 25d41f0de4..8dda349b5c 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1600,7 +1600,11 @@ mymain(void) "\"filename\": \"/tmp/testfle\"" "}" "}", - "<source file='/tmp/testfle'/>\n", 0); + "<source file='/tmp/testfle'>\n" + " <slices>\n" + " <slice type='format' offset='10752' size='4063232'/>\n" + " </slices>\n" + "</source>\n", 0); #endif /* WITH_YAJL */ -- 2.24.1