[PATCH v3 00/15] qemu: Handle 'size' and 'offset' attributes of 'raw' format

This series fixes and improves the 'json:' pseudo-protocol parser and implements the 'offset' and 'size' attributes and exposes them as <slice> in the XML. The previous version attempted an easy route, but that didn't cover all cases. This version adds storage slice support for everything except image creation. https://bugzilla.redhat.com/show_bug.cgi?id=1791788 Peter Krempa (15): qemu: domain: Refactor formatting of node names into status XML docs: formatdomain: Close <source> on one of disk examples tests: virstorage: Add test data for json specified raw image with offset/size util: virstoragefile: Add data structure for storing storage source slices qemuBlockStorageSourceGetFormatRawProps: format 'offset' and 'size' for slice qemuDomainValidateStorageSource: Reject unsupported slices qemu: block: forbid creation of storage sources with <slice> docs: Document the new <slices> sub-element of disk's <source> conf: Implement support for <slices> of disk source qemu: domain: Store nodenames of slice in status XML qemu: block: Properly format storage slice into backing store strings tests: qemublock: Add cases for creating image overlays on top of disks with <slice> qemu: Add support for slices of type 'storage' tests: qemu: Add test data for the new <slice> element virStorageSourceParseBackingJSONRaw: Parse 'offset' and 'size' attributes docs/formatdomain.html.in | 14 ++ docs/schemas/domaincommon.rng | 19 ++ src/conf/domain_conf.c | 86 +++++++++ src/qemu/qemu_block.c | 169 ++++++++++++++---- src/qemu/qemu_block.h | 4 + src/qemu/qemu_blockjob.c | 1 + src/qemu/qemu_command.c | 8 + src/qemu/qemu_domain.c | 36 +++- src/util/virstoragefile.c | 49 +++++ src/util/virstoragefile.h | 12 ++ tests/qemublocktest.c | 2 + .../qcow2-backing-qcow2-slice.json | 15 ++ .../imagecreate/qcow2-backing-qcow2-slice.xml | 1 + .../imagecreate/qcow2-backing-raw-slice.json | 15 ++ .../imagecreate/qcow2-backing-raw-slice.xml | 1 + .../imagecreate/qcow2-slice.xml | 14 ++ .../imagecreate/raw-slice.xml | 14 ++ tests/qemustatusxml2xmldata/modern-in.xml | 4 + .../disk-slices.x86_64-latest.args | 53 ++++++ tests/qemuxml2argvdata/disk-slices.xml | 45 +++++ tests/qemuxml2argvtest.c | 2 + .../disk-slices.x86_64-latest.xml | 56 ++++++ tests/qemuxml2xmltest.c | 2 + tests/virstoragetest.c | 13 ++ 24 files changed, 590 insertions(+), 45 deletions(-) create mode 100644 tests/qemublocktestdata/imagecreate/qcow2-backing-qcow2-slice.json create mode 120000 tests/qemublocktestdata/imagecreate/qcow2-backing-qcow2-slice.xml create mode 100644 tests/qemublocktestdata/imagecreate/qcow2-backing-raw-slice.json create mode 120000 tests/qemublocktestdata/imagecreate/qcow2-backing-raw-slice.xml create mode 100644 tests/qemublocktestdata/imagecreate/qcow2-slice.xml create mode 100644 tests/qemublocktestdata/imagecreate/raw-slice.xml 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 -- 2.24.1

Use virXMLFormatElement to simplify the logic. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 6fc0bd4e68..139496307f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2370,15 +2370,12 @@ qemuStorageSourcePrivateDataFormat(virStorageSourcePtr src, { g_auto(virBuffer) tmp = VIR_BUFFER_INIT_CHILD(buf); qemuDomainStorageSourcePrivatePtr srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src); + g_auto(virBuffer) nodenamesChildBuf = VIR_BUFFER_INIT_CHILD(buf); - if (src->nodestorage || src->nodeformat) { - virBufferAddLit(buf, "<nodenames>\n"); - virBufferAdjustIndent(buf, 2); - virBufferEscapeString(buf, "<nodename type='storage' name='%s'/>\n", src->nodestorage); - virBufferEscapeString(buf, "<nodename type='format' name='%s'/>\n", src->nodeformat); - virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "</nodenames>\n"); - } + virBufferEscapeString(&nodenamesChildBuf, "<nodename type='storage' name='%s'/>\n", src->nodestorage); + virBufferEscapeString(&nodenamesChildBuf, "<nodename type='format' name='%s'/>\n", src->nodeformat); + + virXMLFormatElement(buf, "nodenames", NULL, &nodenamesChildBuf); if (src->pr) virBufferAsprintf(buf, "<reservations mgralias='%s'/>\n", src->pr->mgralias); -- 2.24.1

On Wed, Feb 12, 2020 at 07:03:12PM +0100, Peter Krempa wrote:
Use virXMLFormatElement to simplify the logic.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

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

On Wed, Feb 12, 2020 at 07:03:13PM +0100, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Missing Reviewed-by tag from v2 of this series. Also, you could have pushed it already since it's independent. Jano
--- docs/formatdomain.html.in | 1 + 1 file changed, 1 insertion(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 5ccf39abd1..200a9cb79a 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2881,6 +2881,7 @@ <reservations managed='no'> <source type='unix' path='/path/to/qemu-pr-helper' mode='client'/> </reservations> + </source> <target dev='sda' bus='scsi'/> <address type='drive' controller='0' bus='0' target='3' unit='0'/> </disk> -- 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@redhat.com> --- 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

On Wed, Feb 12, 2020 at 07:03:14PM +0100, Peter Krempa wrote:
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@redhat.com> --- tests/virstoragetest.c | 9 +++++++++ 1 file changed, 9 insertions(+)
https://www.redhat.com/archives/libvir-list/2020-February/msg00288.html Jano

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@redhat.com> --- src/util/virstoragefile.c | 29 +++++++++++++++++++++++++++++ src/util/virstoragefile.h | 12 ++++++++++++ 2 files changed, 41 insertions(+) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 7ffb2cdcf4..890ec69929 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2248,6 +2248,30 @@ virStorageSourcePoolDefCopy(const virStorageSourcePoolDef *src) } +static virStorageSourceSlicePtr +virStorageSourceSliceCopy(const virStorageSourceSlice *src) +{ + virStorageSourceSlicePtr ret = g_new0(virStorageSourceSlice, 1); + + ret->offset = src->offset; + ret->size = src->size; + ret->nodename = g_strdup(src->nodename); + + return ret; +} + + +static void +virStorageSourceSliceFree(virStorageSourceSlicePtr slice) +{ + if (!slice) + return; + + g_free(slice->nodename); + g_free(slice); +} + + /** * virStorageSourcePtr: * @@ -2302,6 +2326,9 @@ virStorageSourceCopy(const virStorageSource *src, def->tlsAlias = g_strdup(src->tlsAlias); def->tlsCertdir = g_strdup(src->tlsCertdir); + if (src->sliceStorage) + def->sliceStorage = virStorageSourceSliceCopy(src->sliceStorage); + if (src->nhosts) { if (!(def->hosts = virStorageNetHostDefCopy(src->nhosts, src->hosts))) return NULL; @@ -2581,6 +2608,8 @@ virStorageSourceClear(virStorageSourcePtr def) VIR_FREE(def->timestamps); VIR_FREE(def->externalDataStoreRaw); + virStorageSourceSliceFree(def->sliceStorage); + virObjectUnref(def->externalDataStore); def->externalDataStore = NULL; diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 39e50a989d..1f41e6e357 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -242,6 +242,16 @@ 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; + char *nodename; +}; + + typedef struct _virStorageDriverData virStorageDriverData; typedef virStorageDriverData *virStorageDriverDataPtr; @@ -286,6 +296,8 @@ struct _virStorageSource { bool nocow; bool sparse; + virStorageSourceSlicePtr sliceStorage; + virStoragePermsPtr perms; virStorageTimestampsPtr timestamps; unsigned long long capacity; /* in bytes, 0 if unknown */ -- 2.24.1

On Wed, Feb 12, 2020 at 07:03:15PM +0100, Peter Krempa wrote:
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@redhat.com> --- src/util/virstoragefile.c | 29 +++++++++++++++++++++++++++++ src/util/virstoragefile.h | 12 ++++++++++++ 2 files changed, 41 insertions(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

If we have a 'format' type slice for a raw driver we can directly format the values. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 710ddfd2cf..6ac1d34281 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1194,16 +1194,21 @@ 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; + /* Currently only storage slices are supported. We'll have to calculate + * the union of the slices here if we don't want to be adding needless + * 'raw' nodes. */ + if (src->sliceStorage && + virJSONValueObjectAdd(props, + "p:offset", src->sliceStorage->offset, + "p:size", src->sliceStorage->size, + NULL) < 0) + return -1; + return 0; } -- 2.24.1

On Wed, Feb 12, 2020 at 07:03:16PM +0100, Peter Krempa wrote:
If we have a 'format' type slice for a raw driver we can directly format the values.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 710ddfd2cf..6ac1d34281 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1194,16 +1194,21 @@ 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;
+ /* Currently only storage slices are supported. We'll have to calculate + * the union of the slices here if we don't want to be adding needless + * 'raw' nodes. */ + if (src->sliceStorage && + virJSONValueObjectAdd(props, + "p:offset", src->sliceStorage->offset, + "p:size", src->sliceStorage->size,
https://www.redhat.com/archives/libvir-list/2020-February/msg00293.html Jano

We will currently support slice only for the 'raw' format slice reject any other option. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 139496307f..d24d1c81db 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6821,6 +6821,18 @@ qemuDomainValidateStorageSource(virStorageSourcePtr src, return -1; } + if (src->sliceStorage) { + /* In pre-blockdev era we can't configure the slice so we can allow them + * only for detected backing store entries as they are populated + * from a place that qemu would be able to read */ + if (!src->detected && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("storage slice is not supported by this QEMU binary")); + return -1; + } + } + return 0; } -- 2.24.1

On Wed, Feb 12, 2020 at 07:03:17PM +0100, Peter Krempa wrote:
We will currently support slice only for the 'raw' format slice reject
format slice. Reject any other option.
any other option.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

On Thu, Feb 13, 2020 at 14:51:20 +0100, Ján Tomko wrote:
On Wed, Feb 12, 2020 at 07:03:17PM +0100, Peter Krempa wrote:
We will currently support slice only for the 'raw' format slice reject
format slice. Reject any other option.
Actually I completely forgot to fix this commit message and would not be accurate. For the state this commit implemets the commit message should read: We support explicit storage slices only when using blockdev. Storage slices expressed via the backing store string are left to qemu to open correctly. Reject storage slices configured via the XML for non-blockdev usage.

Specifically creating such images via libvirt during blockjobs would be much more hassle than it's worth. Just forbid them for now. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 6ac1d34281..b23a4baae6 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -2529,6 +2529,12 @@ qemuBlockStorageSourceCreate(virDomainObjPtr vm, int ret = -1; int rc; + if (src->sliceStorage) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("creation of images with slice type='storage' is not supported")); + return -1; + } + if (qemuDomainObjEnterMonitorAsync(priv->driver, vm, asyncJob) < 0) goto cleanup; -- 2.24.1

On Wed, Feb 12, 2020 at 07:03:18PM +0100, Peter Krempa wrote:
Specifically creating such images via libvirt during blockjobs would be much more hassle than it's worth. Just forbid them for now.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 6 ++++++ 1 file changed, 6 insertions(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

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@redhat.com> --- docs/formatdomain.html.in | 13 +++++++++++++ docs/schemas/domaincommon.rng | 19 +++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 200a9cb79a..717cbd026f 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2878,6 +2878,9 @@ <disk type='block' device='lun'> <driver name='qemu' type='raw'/> <source dev='/dev/sda'> + <slices> + <slice type='format' offset='12345' size='123'/> + </slices> <reservations managed='no'> <source type='unix' path='/path/to/qemu-pr-helper' mode='client'/> </reservations> @@ -3360,6 +3363,16 @@ 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>) + inside the storage source or the guest data inside the image format + container (future expansion). + + The <code>offset</code> and <code>size</code> values are in bytes. + <span class="since">Since 6.1.0</span> + </dd> </dl> <p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 9577d26c2a..093e9f2aaf 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1595,12 +1595,31 @@ </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'> + <element name='slice'> + <attribute name='type'> + <value>storage</value> + </attribute> + <ref name="diskSourceSlice"/> + </element> + </element> + </optional> </define> <define name="diskSource"> -- 2.24.1

On Wed, Feb 12, 2020 at 07:03:19PM +0100, Peter Krempa wrote:
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.
Type format mentioned here
Add the XML documentation and RNG schema.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/formatdomain.html.in | 13 +++++++++++++ docs/schemas/domaincommon.rng | 19 +++++++++++++++++++ 2 files changed, 32 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 200a9cb79a..717cbd026f 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2878,6 +2878,9 @@ <disk type='block' device='lun'> <driver name='qemu' type='raw'/> <source dev='/dev/sda'> + <slices> + <slice type='format' offset='12345' size='123'/>
and here
+ </slices> <reservations managed='no'> <source type='unix' path='/path/to/qemu-pr-helper' mode='client'/> </reservations> @@ -3360,6 +3363,16 @@ 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>) + inside the storage source or the guest data inside the image format + container (future expansion). + + The <code>offset</code> and <code>size</code> values are in bytes. + <span class="since">Since 6.1.0</span> + </dd> </dl>
<p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 9577d26c2a..093e9f2aaf 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1595,12 +1595,31 @@ </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'> + <element name='slice'> + <attribute name='type'> + <value>storage</value>
but not here.
+ </attribute> + <ref name="diskSourceSlice"/> + </element> + </element> + </optional>
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Implement parsing and formatting of the 'source' and 'format' slices. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 86 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index de74a2a2d5..c0f58f8f5e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9469,6 +9469,57 @@ 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; + } + + return 0; +} + + /** * virDomainStorageSourceParse: * @node: XML node pointing to the source element to parse @@ -9534,6 +9585,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; @@ -24375,6 +24429,36 @@ 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, "storage", src->sliceStorage); + + virXMLFormatElement(buf, "slices", NULL, &childBuf); +} + + /** * virDomainDiskSourceFormat: * @buf: output buffer @@ -24445,6 +24529,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

On Wed, Feb 12, 2020 at 07:03:20PM +0100, Peter Krempa wrote:
Implement parsing and formatting of the 'source' and 'format' slices.
s/'source' and 'format'/'storage'/
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 86 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

The storage slice will require a specific node name in cases when the image format is not raw. Store and format them in the status XML. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 7 +++++++ tests/qemustatusxml2xmldata/modern-in.xml | 4 ++++ 2 files changed, 11 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index d24d1c81db..534a91cf82 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2322,6 +2322,9 @@ qemuStorageSourcePrivateDataParse(xmlXPathContextPtr ctxt, src->nodeformat = virXPathString("string(./nodenames/nodename[@type='format']/@name)", ctxt); src->tlsAlias = virXPathString("string(./objects/TLSx509/@alias)", ctxt); + if (src->sliceStorage) + src->sliceStorage->nodename = virXPathString("string(./nodenames/nodename[@type='slice-storage']/@name)", ctxt); + if (src->pr) src->pr->mgralias = virXPathString("string(./reservations/@mgralias)", ctxt); @@ -2375,6 +2378,10 @@ qemuStorageSourcePrivateDataFormat(virStorageSourcePtr src, virBufferEscapeString(&nodenamesChildBuf, "<nodename type='storage' name='%s'/>\n", src->nodestorage); virBufferEscapeString(&nodenamesChildBuf, "<nodename type='format' name='%s'/>\n", src->nodeformat); + if (src->sliceStorage) + virBufferEscapeString(&nodenamesChildBuf, "<nodename type='slice-storage' name='%s'/>\n", + src->sliceStorage->nodename); + virXMLFormatElement(buf, "nodenames", NULL, &nodenamesChildBuf); if (src->pr) diff --git a/tests/qemustatusxml2xmldata/modern-in.xml b/tests/qemustatusxml2xmldata/modern-in.xml index 8a2718293f..c8d21ceada 100644 --- a/tests/qemustatusxml2xmldata/modern-in.xml +++ b/tests/qemustatusxml2xmldata/modern-in.xml @@ -312,6 +312,9 @@ <backingStore type='file' index='1'> <format type='qcow2'/> <source file='/var/lib/libvirt/images/base.qcow2'> + <slices> + <slice type='storage' offset='1234' size='3456'/> + </slices> <seclabel model='dac' relabel='yes'> <label>qemu:qemu</label> </seclabel> @@ -322,6 +325,7 @@ <nodenames> <nodename type='storage' name='test-storage'/> <nodename type='format' name='test-format'/> + <nodename type='slice-storage' name='test-slice-storage'/> </nodenames> <reservations mgralias='test-alias'/> <relPath>base.qcow2</relPath> -- 2.24.1

On Wed, Feb 12, 2020 at 07:03:21PM +0100, Peter Krempa wrote:
The storage slice will require a specific node name in cases when the image format is not raw. Store and format them in the status XML.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 7 +++++++ tests/qemustatusxml2xmldata/modern-in.xml | 4 ++++ 2 files changed, 11 insertions(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

When creating overlay images e.g. for snapshots or when merging snapshots we often specify the backing store string to use. Make the formatter aware of backing chain entries which have a <slice> configured so that we record it properly. Otherwise such images would not work without the XML (when detecting the backing chain). Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 80 ++++++++++++++++++++++++++----------------- 1 file changed, 49 insertions(+), 31 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index b23a4baae6..8b94365c68 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1930,44 +1930,48 @@ qemuBlockGetBackingStoreString(virStorageSourcePtr src) { int actualType = virStorageSourceGetActualType(src); g_autoptr(virJSONValue) backingProps = NULL; + g_autoptr(virJSONValue) sliceProps = NULL; + virJSONValuePtr props = NULL; g_autoptr(virURI) uri = NULL; g_autofree char *backingJSON = NULL; char *ret = NULL; - if (virStorageSourceIsLocalStorage(src)) { - ret = g_strdup(src->path); - return ret; - } + if (!src->sliceStorage) { + if (virStorageSourceIsLocalStorage(src)) { + ret = g_strdup(src->path); + return ret; + } - /* generate simplified URIs for the easy cases */ - if (actualType == VIR_STORAGE_TYPE_NETWORK && - src->nhosts == 1 && - src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_TCP) { + /* generate simplified URIs for the easy cases */ + if (actualType == VIR_STORAGE_TYPE_NETWORK && + src->nhosts == 1 && + src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_TCP) { + + switch ((virStorageNetProtocol) src->protocol) { + case VIR_STORAGE_NET_PROTOCOL_NBD: + case VIR_STORAGE_NET_PROTOCOL_HTTP: + case VIR_STORAGE_NET_PROTOCOL_HTTPS: + case VIR_STORAGE_NET_PROTOCOL_FTP: + case VIR_STORAGE_NET_PROTOCOL_FTPS: + case VIR_STORAGE_NET_PROTOCOL_TFTP: + case VIR_STORAGE_NET_PROTOCOL_ISCSI: + case VIR_STORAGE_NET_PROTOCOL_GLUSTER: + if (!(uri = qemuBlockStorageSourceGetURI(src))) + return NULL; - switch ((virStorageNetProtocol) src->protocol) { - case VIR_STORAGE_NET_PROTOCOL_NBD: - case VIR_STORAGE_NET_PROTOCOL_HTTP: - case VIR_STORAGE_NET_PROTOCOL_HTTPS: - case VIR_STORAGE_NET_PROTOCOL_FTP: - case VIR_STORAGE_NET_PROTOCOL_FTPS: - case VIR_STORAGE_NET_PROTOCOL_TFTP: - case VIR_STORAGE_NET_PROTOCOL_ISCSI: - case VIR_STORAGE_NET_PROTOCOL_GLUSTER: - if (!(uri = qemuBlockStorageSourceGetURI(src))) - return NULL; + if (!(ret = virURIFormat(uri))) + return NULL; - if (!(ret = virURIFormat(uri))) - return NULL; + return ret; - return ret; - - case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: - case VIR_STORAGE_NET_PROTOCOL_RBD: - case VIR_STORAGE_NET_PROTOCOL_VXHS: - case VIR_STORAGE_NET_PROTOCOL_SSH: - case VIR_STORAGE_NET_PROTOCOL_LAST: - case VIR_STORAGE_NET_PROTOCOL_NONE: - break; + case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: + case VIR_STORAGE_NET_PROTOCOL_RBD: + case VIR_STORAGE_NET_PROTOCOL_VXHS: + case VIR_STORAGE_NET_PROTOCOL_SSH: + case VIR_STORAGE_NET_PROTOCOL_LAST: + case VIR_STORAGE_NET_PROTOCOL_NONE: + break; + } } } @@ -1975,7 +1979,21 @@ qemuBlockGetBackingStoreString(virStorageSourcePtr src) if (!(backingProps = qemuBlockStorageSourceGetBackendProps(src, false, true, false))) return NULL; - if (!(backingJSON = virJSONValueToString(backingProps, false))) + props = backingProps; + + if (src->sliceStorage) { + if (virJSONValueObjectCreate(&sliceProps, + "s:driver", "raw", + "U:offset", src->sliceStorage->offset, + "U:size", src->sliceStorage->size, + "a:file", &backingProps, + NULL) < 0) + return NULL; + + props = sliceProps; + } + + if (!(backingJSON = virJSONValueToString(props, false))) return NULL; ret = g_strdup_printf("json:%s", backingJSON); -- 2.24.1

On Wed, Feb 12, 2020 at 07:03:22PM +0100, Peter Krempa wrote:
When creating overlay images e.g. for snapshots or when merging snapshots we often specify the backing store string to use. Make the formatter aware of backing chain entries which have a <slice> configured so that we record it properly. Otherwise such images would not work without the XML (when detecting the backing chain).
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 80 ++++++++++++++++++++++++++----------------- 1 file changed, 49 insertions(+), 31 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Add a set of test data to see whether the backing store strings are formatted reasonably. Note that we don't support direct creation of such images so those tests are not enabled. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 2 ++ .../imagecreate/qcow2-backing-qcow2-slice.json | 15 +++++++++++++++ .../imagecreate/qcow2-backing-qcow2-slice.xml | 1 + .../imagecreate/qcow2-backing-raw-slice.json | 15 +++++++++++++++ .../imagecreate/qcow2-backing-raw-slice.xml | 1 + .../qemublocktestdata/imagecreate/qcow2-slice.xml | 14 ++++++++++++++ tests/qemublocktestdata/imagecreate/raw-slice.xml | 14 ++++++++++++++ 7 files changed, 62 insertions(+) create mode 100644 tests/qemublocktestdata/imagecreate/qcow2-backing-qcow2-slice.json create mode 120000 tests/qemublocktestdata/imagecreate/qcow2-backing-qcow2-slice.xml create mode 100644 tests/qemublocktestdata/imagecreate/qcow2-backing-raw-slice.json create mode 120000 tests/qemublocktestdata/imagecreate/qcow2-backing-raw-slice.xml create mode 100644 tests/qemublocktestdata/imagecreate/qcow2-slice.xml create mode 100644 tests/qemublocktestdata/imagecreate/raw-slice.xml diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index ada83a0fde..5a564b71df 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -1064,6 +1064,8 @@ mymain(void) TEST_IMAGE_CREATE("qcow2-backing-raw-nbd", "raw-nbd"); TEST_IMAGE_CREATE("qcow2-backing-luks", "luks-noopts"); TEST_IMAGE_CREATE("qcow2-luks-encopts-backing", "qcow2"); + TEST_IMAGE_CREATE("qcow2-backing-raw-slice", "raw-slice"); + TEST_IMAGE_CREATE("qcow2-backing-qcow2-slice", "qcow2-slice"); TEST_IMAGE_CREATE("network-gluster-qcow2", NULL); TEST_IMAGE_CREATE("network-rbd-qcow2", NULL); diff --git a/tests/qemublocktestdata/imagecreate/qcow2-backing-qcow2-slice.json b/tests/qemublocktestdata/imagecreate/qcow2-backing-qcow2-slice.json new file mode 100644 index 0000000000..2fa27c1933 --- /dev/null +++ b/tests/qemublocktestdata/imagecreate/qcow2-backing-qcow2-slice.json @@ -0,0 +1,15 @@ +protocol: +{ + "driver": "file", + "filename": "/var/lib/libvirt/images/i.qcow2", + "size": 4294967296 +} + +format: +{ + "driver": "qcow2", + "file": "0123456789ABCDEF0123456789ABCDE", + "size": 8589934590, + "backing-file": "json:{\"driver\":\"raw\",\"offset\":1234,\"size\":5768,\"file\":{\"driver\":\"file\",\"filename\":\"/var/lib/libvirt/images/i.qcow2\"}}", + "backing-fmt": "qcow2" +} diff --git a/tests/qemublocktestdata/imagecreate/qcow2-backing-qcow2-slice.xml b/tests/qemublocktestdata/imagecreate/qcow2-backing-qcow2-slice.xml new file mode 120000 index 0000000000..5769c2c866 --- /dev/null +++ b/tests/qemublocktestdata/imagecreate/qcow2-backing-qcow2-slice.xml @@ -0,0 +1 @@ +qcow2.xml \ No newline at end of file diff --git a/tests/qemublocktestdata/imagecreate/qcow2-backing-raw-slice.json b/tests/qemublocktestdata/imagecreate/qcow2-backing-raw-slice.json new file mode 100644 index 0000000000..761002afd9 --- /dev/null +++ b/tests/qemublocktestdata/imagecreate/qcow2-backing-raw-slice.json @@ -0,0 +1,15 @@ +protocol: +{ + "driver": "file", + "filename": "/var/lib/libvirt/images/i.qcow2", + "size": 4294967296 +} + +format: +{ + "driver": "qcow2", + "file": "0123456789ABCDEF0123456789ABCDE", + "size": 8589934590, + "backing-file": "json:{\"driver\":\"raw\",\"offset\":9876,\"size\":54321,\"file\":{\"driver\":\"file\",\"filename\":\"/var/lib/libvirt/images/i.img\"}}", + "backing-fmt": "raw" +} diff --git a/tests/qemublocktestdata/imagecreate/qcow2-backing-raw-slice.xml b/tests/qemublocktestdata/imagecreate/qcow2-backing-raw-slice.xml new file mode 120000 index 0000000000..5769c2c866 --- /dev/null +++ b/tests/qemublocktestdata/imagecreate/qcow2-backing-raw-slice.xml @@ -0,0 +1 @@ +qcow2.xml \ No newline at end of file diff --git a/tests/qemublocktestdata/imagecreate/qcow2-slice.xml b/tests/qemublocktestdata/imagecreate/qcow2-slice.xml new file mode 100644 index 0000000000..6c5ae3107b --- /dev/null +++ b/tests/qemublocktestdata/imagecreate/qcow2-slice.xml @@ -0,0 +1,14 @@ +<disk device='disk' name='vda'> + <driver type='qcow2'/> + <source file='/var/lib/libvirt/images/i.qcow2'> + <slices> + <slice type='storage' offset='1234' size='5768'/> + </slices> + <privateData> + <nodenames> + <nodename type='storage' name='0123456789ABCDEF0123456789ABCDE'/> + <nodename type='format' name='0123456789ABCDEF0123456789ABCDE'/> + </nodenames> + </privateData> + </source> +</disk> diff --git a/tests/qemublocktestdata/imagecreate/raw-slice.xml b/tests/qemublocktestdata/imagecreate/raw-slice.xml new file mode 100644 index 0000000000..adc7a175ce --- /dev/null +++ b/tests/qemublocktestdata/imagecreate/raw-slice.xml @@ -0,0 +1,14 @@ +<disk device='disk' name='vda'> + <driver type='raw'/> + <source file='/var/lib/libvirt/images/i.img'> + <slices> + <slice type='storage' offset='9876' size='54321'/> + </slices> + <privateData> + <nodenames> + <nodename type='storage' name='0123456789ABCDEF0123456789ABCDE'/> + <nodename type='format' name='0123456789ABCDEF0123456789ABCDE'/> + </nodenames> + </privateData> + </source> +</disk> -- 2.24.1

On Wed, Feb 12, 2020 at 07:03:23PM +0100, Peter Krempa wrote:
Add a set of test data to see whether the backing store strings are formatted reasonably. Note that we don't support direct creation of such images so those tests are not enabled.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 2 ++ .../imagecreate/qcow2-backing-qcow2-slice.json | 15 +++++++++++++++ .../imagecreate/qcow2-backing-qcow2-slice.xml | 1 + .../imagecreate/qcow2-backing-raw-slice.json | 15 +++++++++++++++ .../imagecreate/qcow2-backing-raw-slice.xml | 1 + .../qemublocktestdata/imagecreate/qcow2-slice.xml | 14 ++++++++++++++ tests/qemublocktestdata/imagecreate/raw-slice.xml | 14 ++++++++++++++ 7 files changed, 62 insertions(+) create mode 100644 tests/qemublocktestdata/imagecreate/qcow2-backing-qcow2-slice.json create mode 120000 tests/qemublocktestdata/imagecreate/qcow2-backing-qcow2-slice.xml create mode 100644 tests/qemublocktestdata/imagecreate/qcow2-backing-raw-slice.json create mode 120000 tests/qemublocktestdata/imagecreate/qcow2-backing-raw-slice.xml create mode 100644 tests/qemublocktestdata/imagecreate/qcow2-slice.xml create mode 100644 tests/qemublocktestdata/imagecreate/raw-slice.xml
diff --git a/tests/qemublocktestdata/imagecreate/qcow2-slice.xml b/tests/qemublocktestdata/imagecreate/qcow2-slice.xml new file mode 100644 index 0000000000..6c5ae3107b --- /dev/null +++ b/tests/qemublocktestdata/imagecreate/qcow2-slice.xml @@ -0,0 +1,14 @@ +<disk device='disk' name='vda'> + <driver type='qcow2'/> + <source file='/var/lib/libvirt/images/i.qcow2'> + <slices> + <slice type='storage' offset='1234' size='5768'/> + </slices> + <privateData> + <nodenames> + <nodename type='storage' name='0123456789ABCDEF0123456789ABCDE'/>
+ <nodename type='format' name='0123456789ABCDEF0123456789ABCDE'/>
This is not parsed.
+ </nodenames> + </privateData> + </source> +</disk> diff --git a/tests/qemublocktestdata/imagecreate/raw-slice.xml b/tests/qemublocktestdata/imagecreate/raw-slice.xml new file mode 100644 index 0000000000..adc7a175ce --- /dev/null +++ b/tests/qemublocktestdata/imagecreate/raw-slice.xml @@ -0,0 +1,14 @@ +<disk device='disk' name='vda'> + <driver type='raw'/> + <source file='/var/lib/libvirt/images/i.img'> + <slices> + <slice type='storage' offset='9876' size='54321'/> + </slices> + <privateData> + <nodenames> + <nodename type='storage' name='0123456789ABCDEF0123456789ABCDE'/>
+ <nodename type='format' name='0123456789ABCDEF0123456789ABCDE'/>
This is not parsed either.
+ </nodenames>
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

On Thu, Feb 13, 2020 at 15:54:53 +0100, Ján Tomko wrote:
On Wed, Feb 12, 2020 at 07:03:23PM +0100, Peter Krempa wrote:
Add a set of test data to see whether the backing store strings are formatted reasonably. Note that we don't support direct creation of such images so those tests are not enabled.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 2 ++ .../imagecreate/qcow2-backing-qcow2-slice.json | 15 +++++++++++++++ .../imagecreate/qcow2-backing-qcow2-slice.xml | 1 + .../imagecreate/qcow2-backing-raw-slice.json | 15 +++++++++++++++ .../imagecreate/qcow2-backing-raw-slice.xml | 1 + .../qemublocktestdata/imagecreate/qcow2-slice.xml | 14 ++++++++++++++ tests/qemublocktestdata/imagecreate/raw-slice.xml | 14 ++++++++++++++ 7 files changed, 62 insertions(+) create mode 100644 tests/qemublocktestdata/imagecreate/qcow2-backing-qcow2-slice.json create mode 120000 tests/qemublocktestdata/imagecreate/qcow2-backing-qcow2-slice.xml create mode 100644 tests/qemublocktestdata/imagecreate/qcow2-backing-raw-slice.json create mode 120000 tests/qemublocktestdata/imagecreate/qcow2-backing-raw-slice.xml create mode 100644 tests/qemublocktestdata/imagecreate/qcow2-slice.xml create mode 100644 tests/qemublocktestdata/imagecreate/raw-slice.xml
diff --git a/tests/qemublocktestdata/imagecreate/qcow2-slice.xml b/tests/qemublocktestdata/imagecreate/qcow2-slice.xml new file mode 100644 index 0000000000..6c5ae3107b --- /dev/null +++ b/tests/qemublocktestdata/imagecreate/qcow2-slice.xml @@ -0,0 +1,14 @@ +<disk device='disk' name='vda'> + <driver type='qcow2'/> + <source file='/var/lib/libvirt/images/i.qcow2'> + <slices> + <slice type='storage' offset='1234' size='5768'/> + </slices> + <privateData> + <nodenames> + <nodename type='storage' name='0123456789ABCDEF0123456789ABCDE'/>
+ <nodename type='format' name='0123456789ABCDEF0123456789ABCDE'/>
This is not parsed.
+ </nodenames> + </privateData> + </source> +</disk> diff --git a/tests/qemublocktestdata/imagecreate/raw-slice.xml b/tests/qemublocktestdata/imagecreate/raw-slice.xml new file mode 100644 index 0000000000..adc7a175ce --- /dev/null +++ b/tests/qemublocktestdata/imagecreate/raw-slice.xml @@ -0,0 +1,14 @@ +<disk device='disk' name='vda'> + <driver type='raw'/> + <source file='/var/lib/libvirt/images/i.img'> + <slices> + <slice type='storage' offset='9876' size='54321'/> + </slices> + <privateData> + <nodenames> + <nodename type='storage' name='0123456789ABCDEF0123456789ABCDE'/>
+ <nodename type='format' name='0123456789ABCDEF0123456789ABCDE'/>
This is not parsed either.
Both are parsed. These are nodenames of the storage and format layer when instantiating storage via -blockdev.
+ </nodenames>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jano

On Fri, Feb 14, 2020 at 01:50:22PM +0100, Peter Krempa wrote:
On Thu, Feb 13, 2020 at 15:54:53 +0100, Ján Tomko wrote:
On Wed, Feb 12, 2020 at 07:03:23PM +0100, Peter Krempa wrote:
Add a set of test data to see whether the backing store strings are formatted reasonably. Note that we don't support direct creation of such images so those tests are not enabled.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 2 ++ .../imagecreate/qcow2-backing-qcow2-slice.json | 15 +++++++++++++++ .../imagecreate/qcow2-backing-qcow2-slice.xml | 1 + .../imagecreate/qcow2-backing-raw-slice.json | 15 +++++++++++++++ .../imagecreate/qcow2-backing-raw-slice.xml | 1 + .../qemublocktestdata/imagecreate/qcow2-slice.xml | 14 ++++++++++++++ tests/qemublocktestdata/imagecreate/raw-slice.xml | 14 ++++++++++++++ 7 files changed, 62 insertions(+) create mode 100644 tests/qemublocktestdata/imagecreate/qcow2-backing-qcow2-slice.json create mode 120000 tests/qemublocktestdata/imagecreate/qcow2-backing-qcow2-slice.xml create mode 100644 tests/qemublocktestdata/imagecreate/qcow2-backing-raw-slice.json create mode 120000 tests/qemublocktestdata/imagecreate/qcow2-backing-raw-slice.xml create mode 100644 tests/qemublocktestdata/imagecreate/qcow2-slice.xml create mode 100644 tests/qemublocktestdata/imagecreate/raw-slice.xml
diff --git a/tests/qemublocktestdata/imagecreate/qcow2-slice.xml b/tests/qemublocktestdata/imagecreate/qcow2-slice.xml new file mode 100644 index 0000000000..6c5ae3107b --- /dev/null +++ b/tests/qemublocktestdata/imagecreate/qcow2-slice.xml @@ -0,0 +1,14 @@ +<disk device='disk' name='vda'> + <driver type='qcow2'/> + <source file='/var/lib/libvirt/images/i.qcow2'> + <slices> + <slice type='storage' offset='1234' size='5768'/> + </slices> + <privateData> + <nodenames> + <nodename type='storage' name='0123456789ABCDEF0123456789ABCDE'/>
+ <nodename type='format' name='0123456789ABCDEF0123456789ABCDE'/>
This is not parsed.
+ </nodenames> + </privateData> + </source> +</disk> diff --git a/tests/qemublocktestdata/imagecreate/raw-slice.xml b/tests/qemublocktestdata/imagecreate/raw-slice.xml new file mode 100644 index 0000000000..adc7a175ce --- /dev/null +++ b/tests/qemublocktestdata/imagecreate/raw-slice.xml @@ -0,0 +1,14 @@ +<disk device='disk' name='vda'> + <driver type='raw'/> + <source file='/var/lib/libvirt/images/i.img'> + <slices> + <slice type='storage' offset='9876' size='54321'/> + </slices> + <privateData> + <nodenames> + <nodename type='storage' name='0123456789ABCDEF0123456789ABCDE'/>
+ <nodename type='format' name='0123456789ABCDEF0123456789ABCDE'/>
This is not parsed either.
Both are parsed. These are nodenames of the storage and format layer when instantiating storage via -blockdev.
I sit corrected. Re-reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
+ </nodenames>

Implement support for the slice of type 'storage' which allows to set the offset and size which modifies where qemu should look for the start of the format container inside the image. Since slicing is done using the 'raw' driver we need to add another layer into the blockdev tree if there's any non-raw image format driver used to access the data. This patch adds the blockdev integration and setup of the image data so that we can use the slices for any backing image. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 68 +++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_block.h | 4 +++ src/qemu/qemu_blockjob.c | 1 + src/qemu/qemu_command.c | 8 +++++ src/qemu/qemu_domain.c | 4 +++ 5 files changed, 84 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 8b94365c68..36b61e5355 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1423,11 +1423,16 @@ qemuBlockStorageSourceGetBlockdevProps(virStorageSourcePtr src, virStorageSourcePtr backingStore) { g_autoptr(virJSONValue) props = NULL; + const char *storagenode = src->nodestorage; + + if (src->sliceStorage && + src->format != VIR_STORAGE_FILE_RAW) + storagenode = src->sliceStorage->nodename; if (!(props = qemuBlockStorageSourceGetBlockdevFormatProps(src))) return NULL; - if (virJSONValueObjectAppendString(props, "file", src->nodestorage) < 0) + if (virJSONValueObjectAppendString(props, "file", storagenode) < 0) return NULL; if (backingStore) { @@ -1456,6 +1461,32 @@ qemuBlockStorageSourceGetBlockdevProps(virStorageSourcePtr src, } +static virJSONValuePtr +qemuBlockStorageSourceGetBlockdevStorageSliceProps(virStorageSourcePtr src) +{ + g_autoptr(virJSONValue) props = NULL; + + if (qemuBlockNodeNameValidate(src->sliceStorage->nodename) < 0) + return NULL; + + if (virJSONValueObjectCreate(&props, + "s:driver", "raw", + "s:node-name", src->sliceStorage->nodename, + "U:offset", src->sliceStorage->offset, + "U:size", src->sliceStorage->size, + "s:file", src->nodestorage, + "b:auto-read-only", true, + "s:discard", "unmap", + NULL) < 0) + return NULL; + + if (qemuBlockStorageSourceGetBlockdevGetCacheProps(src, props) < 0) + return NULL; + + return g_steal_pointer(&props); +} + + void qemuBlockStorageSourceAttachDataFree(qemuBlockStorageSourceAttachDataPtr data) { @@ -1463,6 +1494,7 @@ qemuBlockStorageSourceAttachDataFree(qemuBlockStorageSourceAttachDataPtr data) return; virJSONValueFree(data->storageProps); + virJSONValueFree(data->storageSliceProps); virJSONValueFree(data->formatProps); virJSONValueFree(data->prmgrProps); virJSONValueFree(data->authsecretProps); @@ -1513,6 +1545,13 @@ qemuBlockStorageSourceAttachPrepareBlockdev(virStorageSourcePtr src, data->storageNodeName = src->nodestorage; data->formatNodeName = src->nodeformat; + if (src->sliceStorage && src->format != VIR_STORAGE_FILE_RAW) { + if (!(data->storageSliceProps = qemuBlockStorageSourceGetBlockdevStorageSliceProps(src))) + return NULL; + + data->storageSliceNodeName = src->sliceStorage->nodename; + } + return g_steal_pointer(&data); } @@ -1581,6 +1620,21 @@ qemuBlockStorageSourceAttachApplyFormat(qemuMonitorPtr mon, } +static int +qemuBlockStorageSourceAttachApplyStorageSlice(qemuMonitorPtr mon, + qemuBlockStorageSourceAttachDataPtr data) +{ + if (data->storageSliceProps) { + if (qemuMonitorBlockdevAdd(mon, &data->storageSliceProps) < 0) + return -1; + + data->storageSliceAttached = true; + } + + return 0; +} + + /** * qemuBlockStorageSourceAttachApply: * @mon: monitor object @@ -1600,6 +1654,7 @@ qemuBlockStorageSourceAttachApply(qemuMonitorPtr mon, { if (qemuBlockStorageSourceAttachApplyStorageDeps(mon, data) < 0 || qemuBlockStorageSourceAttachApplyStorage(mon, data) < 0 || + qemuBlockStorageSourceAttachApplyStorageSlice(mon, data) < 0 || qemuBlockStorageSourceAttachApplyFormatDeps(mon, data) < 0 || qemuBlockStorageSourceAttachApplyFormat(mon, data) < 0) return -1; @@ -1642,6 +1697,9 @@ qemuBlockStorageSourceAttachRollback(qemuMonitorPtr mon, if (data->formatAttached) ignore_value(qemuMonitorBlockdevDel(mon, data->formatNodeName)); + if (data->storageSliceAttached) + ignore_value(qemuMonitorBlockdevDel(mon, data->storageSliceNodeName)); + if (data->storageAttached) ignore_value(qemuMonitorBlockdevDel(mon, data->storageNodeName)); @@ -1689,6 +1747,14 @@ qemuBlockStorageSourceDetachPrepare(virStorageSourcePtr src, data->formatAttached = true; data->storageNodeName = src->nodestorage; data->storageAttached = true; + + /* 'raw' format doesn't need the extra 'raw' layer when slicing, thus + * the nodename is NULL */ + if (src->sliceStorage && + src->sliceStorage->nodename) { + data->storageSliceNodeName = src->sliceStorage->nodename; + data->storageSliceAttached = true; + } } if (src->pr && diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h index a816190bb7..eab0128d5d 100644 --- a/src/qemu/qemu_block.h +++ b/src/qemu/qemu_block.h @@ -82,6 +82,10 @@ struct qemuBlockStorageSourceAttachData { const char *storageNodeName; bool storageAttached; + virJSONValuePtr storageSliceProps; + const char *storageSliceNodeName; + bool storageSliceAttached; + virJSONValuePtr formatProps; const char *formatNodeName; bool formatAttached; diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index 6b59bbeb2c..71df0d1ab2 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -1316,6 +1316,7 @@ qemuBlockJobProcessEventConcludedCreate(virQEMUDriverPtr driver, backend->formatAttached = false; if (job->data.create.storage) { backend->storageAttached = false; + backend->storageSliceAttached = false; VIR_FREE(backend->encryptsecretAlias); } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0dbd78124b..6c26e7bb7d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2411,6 +2411,14 @@ qemuBuildBlockStorageSourceAttachDataCommandline(virCommandPtr cmd, VIR_FREE(tmp); } + if (data->storageSliceProps) { + if (!(tmp = virJSONValueToString(data->storageSliceProps, false))) + return -1; + + virCommandAddArgList(cmd, "-blockdev", tmp, NULL); + VIR_FREE(tmp); + } + if (data->formatProps) { if (!(tmp = virJSONValueToString(data->formatProps, false))) return -1; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 534a91cf82..f8d034c382 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -16224,6 +16224,10 @@ qemuDomainPrepareStorageSourceBlockdev(virDomainDiskDefPtr disk, src->nodestorage = g_strdup_printf("libvirt-%u-storage", src->id); src->nodeformat = g_strdup_printf("libvirt-%u-format", src->id); + if (src->sliceStorage && + src->format != VIR_STORAGE_FILE_RAW) + src->sliceStorage->nodename = g_strdup_printf("libvirt-%u-slice-sto", src->id); + if (qemuDomainValidateStorageSource(src, priv->qemuCaps) < 0) return -1; -- 2.24.1

On Wed, Feb 12, 2020 at 07:03:24PM +0100, Peter Krempa wrote:
Implement support for the slice of type 'storage' which allows to set the offset and size which modifies where qemu should look for the start of the format container inside the image.
Since slicing is done using the 'raw' driver we need to add another layer into the blockdev tree if there's any non-raw image format driver used to access the data.
This patch adds the blockdev integration and setup of the image data so that we can use the slices for any backing image.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 68 +++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_block.h | 4 +++ src/qemu/qemu_blockjob.c | 1 + src/qemu/qemu_command.c | 8 +++++ src/qemu/qemu_domain.c | 4 +++ 5 files changed, 84 insertions(+), 1 deletion(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- .../disk-slices.x86_64-latest.args | 53 ++++++++++++++++++ tests/qemuxml2argvdata/disk-slices.xml | 45 +++++++++++++++ tests/qemuxml2argvtest.c | 2 + .../disk-slices.x86_64-latest.xml | 56 +++++++++++++++++++ tests/qemuxml2xmltest.c | 2 + 5 files changed, 158 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..49daa293a4 --- /dev/null +++ b/tests/qemuxml2argvdata/disk-slices.x86_64-latest.args @@ -0,0 +1,53 @@ +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 '{"driver":"raw","node-name":"libvirt-2-slice-sto","offset":9876,\ +"size":123456789,"file":"libvirt-2-storage","auto-read-only":true,\ +"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-2-format","read-only":true,"driver":"qcow2",\ +"file":"libvirt-2-slice-sto","backing":null}' \ +-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..cff7cc7ee4 --- /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='storage' 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='qcow2'/> + <source file='/var/lib/libvirt/images/raw.img'> + <slices> + <slice type='storage' 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 028364a06c..0dbaab4c32 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1165,6 +1165,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..65590df417 --- /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='storage' 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='qcow2'/> + <source file='/var/lib/libvirt/images/raw.img'> + <slices> + <slice type='storage' 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 ecd12c3d30..b6072075c7 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

On Wed, Feb 12, 2020 at 07:03:25PM +0100, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- .../disk-slices.x86_64-latest.args | 53 ++++++++++++++++++ tests/qemuxml2argvdata/disk-slices.xml | 45 +++++++++++++++ tests/qemuxml2argvtest.c | 2 + .../disk-slices.x86_64-latest.xml | 56 +++++++++++++++++++ tests/qemuxml2xmltest.c | 2 + 5 files changed, 158 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
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

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@redhat.com> --- 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 890ec69929..9347c7ab30 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -3551,8 +3551,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->sliceStorage = g_new0(virStorageSourceSlice, 1); + + if (has_offset && + virJSONValueObjectGetNumberUlong(json, "offset", &src->sliceStorage->offset) < 0) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("malformed 'offset' property of 'raw' driver")); + return -1; + } + + if (has_size && + virJSONValueObjectGetNumberUlong(json, "size", &src->sliceStorage->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..39040bf4cb 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='storage' offset='10752' size='4063232'/>\n" + " </slices>\n" + "</source>\n", 0); #endif /* WITH_YAJL */ -- 2.24.1

On Wed, Feb 12, 2020 at 07:03:26PM +0100, Peter Krempa wrote:
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@redhat.com> --- src/util/virstoragefile.c | 20 ++++++++++++++++++++ tests/virstoragetest.c | 6 +++++- 2 files changed, 25 insertions(+), 1 deletion(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

On Wed, Feb 12, 2020 at 07:03:11PM +0100, Peter Krempa wrote:
This series fixes and improves the 'json:' pseudo-protocol parser and implements the 'offset' and 'size' attributes and exposes them as <slice> in the XML.
The previous version attempted an easy route, but that didn't cover all cases. This version adds storage slice support for everything except image creation.
This one seems to handle the ‘virt-v2v -i ova’ case, so it's good from my point of view. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
participants (3)
-
Ján Tomko
-
Peter Krempa
-
Richard W.M. Jones