[libvirt] [PATCH 0/9] virStorageSource and backing chain refactors and fixes (blockdev-add saga)

This series fixes and improves storage of backing store indexes and of the completness of the backing chain in the XML. Peter Krempa (9): conf: domain: Simplify return from backing store parser conf: Make backing store index optional util: storage: Store backing chain index in virStorageSource util: storage: use stored index to lookup disks util: storagefile: Tolerate NULL path when looking up volume in chain storage: Fill in 'type' field for virStorageSource in storage driver test: set 'type' field of virStorageSource util: storagefile: Add helpers to check presence of backing store Terminate backing chains explicitly docs/schemas/domaincommon.rng | 2 + src/conf/domain_conf.c | 57 ++++++++++---------- src/conf/storage_conf.c | 9 +++- src/libvirt_private.syms | 2 + src/qemu/qemu_block.c | 4 +- src/qemu/qemu_cgroup.c | 4 +- src/qemu/qemu_domain.c | 6 +-- src/qemu/qemu_driver.c | 21 ++++++-- src/security/security_dac.c | 2 +- src/security/security_selinux.c | 4 +- src/security/virt-aa-helper.c | 2 +- src/storage/storage_backend_gluster.c | 2 + src/storage/storage_backend_logical.c | 3 +- src/storage/storage_source.c | 57 ++++++++++---------- src/storage/storage_util.c | 14 ++--- src/util/virstoragefile.c | 62 ++++++++++++++-------- src/util/virstoragefile.h | 7 +++ .../qemuhotplug-base-ccw-live+ccw-virtio.xml | 1 - ...live-with-2-ccw-virtio+ccw-virtio-1-reverse.xml | 2 - ...qemuhotplug-base-ccw-live-with-2-ccw-virtio.xml | 1 - ...-live-with-ccw-virtio+ccw-virtio-2-explicit.xml | 2 - ...-base-ccw-live-with-ccw-virtio+ccw-virtio-2.xml | 2 - .../qemuhotplug-base-ccw-live-with-ccw-virtio.xml | 1 - ...-live+disk-scsi-wwn+disk-scsi-duplicate-wwn.xml | 2 - .../qemuhotplug-base-live+disk-scsi.xml | 1 - .../qemuhotplug-base-live+disk-usb.xml | 1 - .../qemuhotplug-base-live+disk-virtio.xml | 1 - ...se-without-scsi-controller-live+disk-scsi-2.xml | 1 - ...otplug-console-compat-2-live+console-virtio.xml | 2 - .../qemuhotplug-console-compat-2-live.xml | 2 - .../qemuxml2xmlout-channel-virtio-state-active.xml | 1 - .../qemuxml2xmlout-disk-active-commit.xml | 1 - .../qemuxml2xmlout-disk-backing-chains-active.xml | 5 -- .../qemuxml2xmlout-disk-mirror-active.xml | 4 -- .../qemuxml2xmlout-disk-mirror-old.xml | 4 -- .../qemuxml2xmlout-seclabel-static-labelskip.xml | 1 - tests/sexpr2xmldata/sexpr2xml-boot-grub.xml | 1 - tests/sexpr2xmldata/sexpr2xml-bridge-ipaddr.xml | 1 - tests/sexpr2xmldata/sexpr2xml-curmem.xml | 1 - .../sexpr2xml-disk-block-shareable.xml | 1 - tests/sexpr2xmldata/sexpr2xml-disk-block.xml | 1 - .../sexpr2xml-disk-drv-blktap-qcow.xml | 1 - .../sexpr2xml-disk-drv-blktap-raw.xml | 1 - .../sexpr2xml-disk-drv-blktap2-raw.xml | 1 - tests/sexpr2xmldata/sexpr2xml-disk-file.xml | 1 - tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-empty-kernel.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-force-hpet.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-force-nohpet.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-kernel.xml | 1 - tests/sexpr2xmldata/sexpr2xml-fv-localtime.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-net-netfront.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-parallel-tcp.xml | 2 - .../sexpr2xml-fv-serial-dev-2-ports.xml | 2 - .../sexpr2xml-fv-serial-dev-2nd-port.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-serial-file.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-serial-null.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-serial-pipe.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-serial-pty.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-serial-stdio.xml | 2 - .../sexpr2xml-fv-serial-tcp-telnet.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-serial-udp.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-serial-unix.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-sound-all.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-sound.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-usbmouse.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-usbtablet.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-utc.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-v2.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv.xml | 2 - tests/sexpr2xmldata/sexpr2xml-net-bridged.xml | 1 - tests/sexpr2xmldata/sexpr2xml-net-e1000.xml | 1 - tests/sexpr2xmldata/sexpr2xml-net-routed.xml | 1 - tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.xml | 2 - tests/sexpr2xmldata/sexpr2xml-pci-devs.xml | 1 - .../sexpr2xml-pv-bootloader-cmdline.xml | 1 - tests/sexpr2xmldata/sexpr2xml-pv-bootloader.xml | 1 - tests/sexpr2xmldata/sexpr2xml-pv-localtime.xml | 1 - tests/sexpr2xmldata/sexpr2xml-pv-vcpus.xml | 1 - .../sexpr2xml-pv-vfb-new-vncdisplay.xml | 1 - tests/sexpr2xmldata/sexpr2xml-pv-vfb-new.xml | 1 - .../sexpr2xmldata/sexpr2xml-pv-vfb-type-crash.xml | 1 - tests/sexpr2xmldata/sexpr2xml-pv.xml | 1 - tests/sexpr2xmldata/sexpr2xml-vif-rate.xml | 2 - tests/virstoragetest.c | 3 +- 86 files changed, 159 insertions(+), 213 deletions(-) -- 2.14.1

Use VIR_STEAL_PTR to remove conditional cleanup. --- src/conf/domain_conf.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 54be9028d..f7c9160b4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8315,12 +8315,11 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, virDomainDiskBackingStoreParse(ctxt, backingStore, flags) < 0) goto cleanup; - src->backingStore = backingStore; + VIR_STEAL_PTR(src->backingStore, backingStore); ret = 0; cleanup: - if (ret < 0) - virStorageSourceFree(backingStore); + virStorageSourceFree(backingStore); VIR_FREE(type); VIR_FREE(format); ctxt->node = save_ctxt; -- 2.14.1

On 10/12/2017 02:07 PM, Peter Krempa wrote:
Use VIR_STEAL_PTR to remove conditional cleanup. --- src/conf/domain_conf.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 54be9028d..f7c9160b4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8315,12 +8315,11 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, virDomainDiskBackingStoreParse(ctxt, backingStore, flags) < 0) goto cleanup;
- src->backingStore = backingStore; + VIR_STEAL_PTR(src->backingStore, backingStore); ret = 0;
cleanup: - if (ret < 0) - virStorageSourceFree(backingStore); + virStorageSourceFree(backingStore); VIR_FREE(type); VIR_FREE(format); ctxt->node = save_ctxt;
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org

Index will remain an internal property even if we allow backing store parsing from the XML, so we need to allow backing store without it in the schema. --- docs/schemas/domaincommon.rng | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 4dbda6932..a3aa6eba2 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1415,9 +1415,11 @@ <define name="diskBackingStore"> <element name="backingStore"> + <optional> <attribute name="index"> <ref name="positiveInteger"/> </attribute> + </optional> <interleave> <ref name="diskSource"/> <ref name="diskBackingChain"/> -- 2.14.1

On 10/12/2017 02:07 PM, Peter Krempa wrote:
Index will remain an internal property even if we allow backing store parsing from the XML, so we need to allow backing store without it in the schema. --- docs/schemas/domaincommon.rng | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 4dbda6932..a3aa6eba2 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1415,9 +1415,11 @@
<define name="diskBackingStore"> <element name="backingStore"> + <optional> <attribute name="index"> <ref name="positiveInteger"/> </attribute> + </optional> <interleave>
Spacing looks funky (not the usual 2-space indentation, including reindenting the <attribute> tag). The parser doesn't care, but it might be nice to touch up on commit. Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org

The backing store indexes were not bound to the storage sources in any way. To allow us to bind a given alias to a given storage source we need to save the index in virStorageSource. The backing store ids are now generated when detecting the backing chain. Since we don't re-detect the backing chain after snapshots, the numbering needs to be fixed there. --- src/conf/domain_conf.c | 22 +++++++++++++++------- src/qemu/qemu_driver.c | 14 ++++++++++++++ src/storage/storage_source.c | 9 ++++++--- src/util/virstoragefile.c | 1 + src/util/virstoragefile.h | 1 + 5 files changed, 37 insertions(+), 10 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f7c9160b4..45fa57a14 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8269,6 +8269,7 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, xmlNodePtr source; char *type = NULL; char *format = NULL; + char *idx = NULL; int ret = -1; if (!(ctxt->node = virXPathNode("./backingStore[*]", ctxt))) { @@ -8279,6 +8280,13 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, if (VIR_ALLOC(backingStore) < 0) goto cleanup; + if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) && + (idx = virXMLPropString(ctxt->node, "index")) && + virStrToLong_uip(idx, NULL, 10, &backingStore->id) < 0) { + virReportError(VIR_ERR_XML_ERROR, _("invalid disk index '%s'"), idx); + goto cleanup; + } + if (!(type = virXMLPropString(ctxt->node, "type"))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing disk backing store type")); @@ -21898,8 +21906,7 @@ virDomainDiskSourceFormat(virBufferPtr buf, static int virDomainDiskBackingStoreFormat(virBufferPtr buf, virStorageSourcePtr backingStore, - const char *backingStoreRaw, - unsigned int idx) + const char *backingStoreRaw) { const char *type; const char *format; @@ -21926,8 +21933,10 @@ virDomainDiskBackingStoreFormat(virBufferPtr buf, return -1; } - virBufferAsprintf(buf, "<backingStore type='%s' index='%u'>\n", - type, idx); + virBufferAsprintf(buf, "<backingStore type='%s'", type); + if (backingStore->id != 0) + virBufferAsprintf(buf, " index='%u'", backingStore->id); + virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 2); virBufferAsprintf(buf, "<format type='%s'/>\n", format); @@ -21935,8 +21944,7 @@ virDomainDiskBackingStoreFormat(virBufferPtr buf, if (virDomainDiskSourceFormatInternal(buf, backingStore, 0, 0, true) < 0 || virDomainDiskBackingStoreFormat(buf, backingStore->backingStore, - backingStore->backingStoreRaw, - idx + 1) < 0) + backingStore->backingStoreRaw) < 0) return -1; virBufferAdjustIndent(buf, -2); @@ -22072,7 +22080,7 @@ virDomainDiskDefFormat(virBufferPtr buf, * persistent storage of backing chains is ready. */ if (!(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE) && virDomainDiskBackingStoreFormat(buf, def->src->backingStore, - def->src->backingStoreRaw, 1) < 0) + def->src->backingStoreRaw) < 0) return -1; virBufferEscapeString(buf, "<backenddomain name='%s'/>\n", def->domain_name); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7c6f1674a..0260c40ef 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14442,6 +14442,17 @@ qemuDomainSnapshotDiskDataCollect(virQEMUDriverPtr driver, } +static void +qemuDomainSnapshotUpdateDiskSourcesRenumber(virStorageSourcePtr src) +{ + virStorageSourcePtr next; + unsigned int idx = 1; + + for (next = src->backingStore; next; next = next->backingStore) + next->id = idx++; +} + + /** * qemuDomainSnapshotUpdateDiskSources: * @dd: snapshot disk data object @@ -14464,6 +14475,9 @@ qemuDomainSnapshotUpdateDiskSources(qemuDomainSnapshotDiskDataPtr dd, VIR_STEAL_PTR(dd->src->backingStore, dd->disk->src); VIR_STEAL_PTR(dd->disk->src, dd->src); + /* fix numbering of disks */ + qemuDomainSnapshotUpdateDiskSourcesRenumber(dd->disk->src); + if (dd->persistdisk) { VIR_STEAL_PTR(dd->persistsrc->backingStore, dd->persistdisk->src); VIR_STEAL_PTR(dd->persistdisk->src, dd->persistsrc); diff --git a/src/storage/storage_source.c b/src/storage/storage_source.c index bf4762237..864c69928 100644 --- a/src/storage/storage_source.c +++ b/src/storage/storage_source.c @@ -396,7 +396,8 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src, uid_t uid, gid_t gid, bool allow_probe, bool report_broken, - virHashTablePtr cycle) + virHashTablePtr cycle, + unsigned int depth) { int ret = -1; const char *uniqueName; @@ -474,7 +475,7 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src, if ((ret = virStorageFileGetMetadataRecurse(backingStore, parent, uid, gid, allow_probe, report_broken, - cycle)) < 0) { + cycle, depth + 1)) < 0) { if (report_broken) goto cleanup; @@ -489,6 +490,8 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src, ret = 0; cleanup: + if (src->backingStore) + src->backingStore->id = depth; VIR_FREE(buf); virStorageFileDeinit(src); virStorageSourceFree(backingStore); @@ -543,7 +546,7 @@ virStorageFileGetMetadata(virStorageSourcePtr src, } ret = virStorageFileGetMetadataRecurse(src, src, uid, gid, - allow_probe, report_broken, cycle); + allow_probe, report_broken, cycle, 1); virHashFree(cycle); return ret; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index dd4494940..2b9f4c892 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2030,6 +2030,7 @@ virStorageSourceCopy(const virStorageSource *src, if (VIR_ALLOC(ret) < 0) return NULL; + ret->id = src->id; ret->type = src->type; ret->protocol = src->protocol; ret->format = src->format; diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 74dee10f2..d3bafefc3 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -227,6 +227,7 @@ typedef virStorageSource *virStorageSourcePtr; * IMPORTANT: When adding fields to this struct it's also necessary to add * appropriate code to the virStorageSourceCopy deep copy function */ struct _virStorageSource { + unsigned int id; /* backing chain identifier, 0 is unset */ int type; /* virStorageType */ char *path; int protocol; /* virStorageNetProtocol */ -- 2.14.1

On 10/12/2017 02:07 PM, Peter Krempa wrote:
The backing store indexes were not bound to the storage sources in any way. To allow us to bind a given alias to a given storage source we need to save the index in virStorageSource. The backing store ids are now generated when detecting the backing chain.
Since we don't re-detect the backing chain after snapshots, the numbering needs to be fixed there. --- src/conf/domain_conf.c | 22 +++++++++++++++------- src/qemu/qemu_driver.c | 14 ++++++++++++++ src/storage/storage_source.c | 9 ++++++--- src/util/virstoragefile.c | 1 + src/util/virstoragefile.h | 1 + 5 files changed, 37 insertions(+), 10 deletions(-)
+++ b/src/qemu/qemu_driver.c @@ -14442,6 +14442,17 @@ qemuDomainSnapshotDiskDataCollect(virQEMUDriverPtr driver, }
+static void +qemuDomainSnapshotUpdateDiskSourcesRenumber(virStorageSourcePtr src) +{ + virStorageSourcePtr next; + unsigned int idx = 1; + + for (next = src->backingStore; next; next = next->backingStore) + next->id = idx++; +} +
Is renumbering going to affect API? It's the difference between starting with: back <- mid <- active having numbers 3, 2, 1, vs. starting with: back <- mid then creating active via snapshot, and now having numbers 2, 1, 3. In other words, do we try to preserve that index 1 is tied to the file we opened first, no matter what order it is currently in the chain, or do we state that our use of "vda[1]" to denote a member of the chain may mean different files at different points in time based on what other chain manipulations have caused renumbering along the way? When we were always regenerating the chain on the fly, there wasn't much stability to worry about. But now that we are going to try to preserve the index across domain reboots, we need to make sure we know which way we want things to work. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org

On Thu, Oct 12, 2017 at 14:57:36 -0500, Eric Blake wrote:
On 10/12/2017 02:07 PM, Peter Krempa wrote:
The backing store indexes were not bound to the storage sources in any way. To allow us to bind a given alias to a given storage source we need to save the index in virStorageSource. The backing store ids are now generated when detecting the backing chain.
Since we don't re-detect the backing chain after snapshots, the numbering needs to be fixed there. --- src/conf/domain_conf.c | 22 +++++++++++++++------- src/qemu/qemu_driver.c | 14 ++++++++++++++ src/storage/storage_source.c | 9 ++++++--- src/util/virstoragefile.c | 1 + src/util/virstoragefile.h | 1 + 5 files changed, 37 insertions(+), 10 deletions(-)
+++ b/src/qemu/qemu_driver.c @@ -14442,6 +14442,17 @@ qemuDomainSnapshotDiskDataCollect(virQEMUDriverPtr driver, }
+static void +qemuDomainSnapshotUpdateDiskSourcesRenumber(virStorageSourcePtr src) +{ + virStorageSourcePtr next; + unsigned int idx = 1; + + for (next = src->backingStore; next; next = next->backingStore) + next->id = idx++; +} +
Is renumbering going to affect API?
It's the difference between starting with:
back <- mid <- active
having numbers 3, 2, 1, vs. starting with:
back <- mid
then creating active via snapshot, and now having numbers 2, 1, 3.
In case when we will not use blockdev-add, the order will be 1,2,3 always, since they will be in order. If blockdev add will be used it should be 4,1,3, so if you pop out 2 of the chain, it will never appear back.
In other words, do we try to preserve that index 1 is tied to the file we opened first, no matter what order it is currently in the chain, or do we state that our use of "vda[1]" to denote a member of the chain may mean different files at different points in time based on what other chain manipulations have caused renumbering along the way?
Exactly, but possible only when tracking the full chain.
When we were always regenerating the chain on the fly, there wasn't much stability to worry about. But now that we are going to try to preserve the index across domain reboots, we need to make sure we know which way we want things to work.
I don't really want to go as far as guaranteeing the numbers across reboots. I think keeping them across one lifetime should be good enough.
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org

On 10/13/2017 12:43 AM, Peter Krempa wrote:
On Thu, Oct 12, 2017 at 14:57:36 -0500, Eric Blake wrote:
On 10/12/2017 02:07 PM, Peter Krempa wrote:
The backing store indexes were not bound to the storage sources in any way. To allow us to bind a given alias to a given storage source we need to save the index in virStorageSource. The backing store ids are now generated when detecting the backing chain.
Since we don't re-detect the backing chain after snapshots, the numbering needs to be fixed there. ---
Is renumbering going to affect API?
It's the difference between starting with:
back <- mid <- active
having numbers 3, 2, 1, vs. starting with:
back <- mid
then creating active via snapshot, and now having numbers 2, 1, 3.
In case when we will not use blockdev-add, the order will be 1,2,3 always, since they will be in order. If blockdev add will be used it should be 4,1,3, so if you pop out 2 of the chain, it will never appear back.
In other words, do we try to preserve that index 1 is tied to the file we opened first, no matter what order it is currently in the chain, or do we state that our use of "vda[1]" to denote a member of the chain may mean different files at different points in time based on what other chain manipulations have caused renumbering along the way?
Exactly, but possible only when tracking the full chain.
When we were always regenerating the chain on the fly, there wasn't much stability to worry about. But now that we are going to try to preserve the index across domain reboots, we need to make sure we know which way we want things to work.
I don't really want to go as far as guaranteeing the numbers across reboots. I think keeping them across one lifetime should be good enough.
Unless anyone else has strong opinions about disk indices not always being stable, I can live with this as your design goal. As I mentioned in 4/9, I didn't see anything wrong with the code itself, so as long as we don't have a problem with the design implications: Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org

On Fri, Oct 13, 2017 at 08:36:54 -0500, Eric Blake wrote:
On 10/13/2017 12:43 AM, Peter Krempa wrote:
On Thu, Oct 12, 2017 at 14:57:36 -0500, Eric Blake wrote:
On 10/12/2017 02:07 PM, Peter Krempa wrote:
[..]
When we were always regenerating the chain on the fly, there wasn't much stability to worry about. But now that we are going to try to preserve the index across domain reboots, we need to make sure we know which way we want things to work.
I don't really want to go as far as guaranteeing the numbers across reboots. I think keeping them across one lifetime should be good enough.
Unless anyone else has strong opinions about disk indices not always being stable, I can live with this as your design goal. As I mentioned in 4/9, I didn't see anything wrong with the code itself, so as long as we don't have a problem with the design implications:
The idea for the indexes to always refer to the same image was there from the beginning, we only cut corners when implementing it the first time. I'm now paying back the technical debt with a lot of interest.
Reviewed-by: Eric Blake <eblake@redhat.com>
Thanks for the review. I've pushed these.

We can now use the backing store ID directly rather than counting the number of images seen while looking up images. --- src/util/virstoragefile.c | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 2b9f4c892..9568a5068 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1588,25 +1588,14 @@ virStorageFileChainLookup(virStorageSourcePtr chain, const char *start = chain->path; char *parentDir = NULL; bool nameIsFile = virStorageIsFile(name); - size_t i = 0; if (!parent) parent = &prev; *parent = NULL; if (startFrom) { - while (chain && chain != startFrom->backingStore) { + while (chain && chain != startFrom->backingStore) chain = chain->backingStore; - i++; - } - - if (idx && idx < i) { - virReportError(VIR_ERR_INVALID_ARG, - _("requested backing store index %u is above '%s' " - "in chain for '%s'"), - idx, NULLSTR(startFrom->path), NULLSTR(start)); - return NULL; - } *parent = startFrom; } @@ -1616,8 +1605,8 @@ virStorageFileChainLookup(virStorageSourcePtr chain, if (!chain->backingStore) break; } else if (idx) { - VIR_DEBUG("%zu: %s", i, chain->path); - if (idx == i) + VIR_DEBUG("%u: %s", chain->id, chain->path); + if (idx == chain->id) break; } else { if (STREQ_NULLABLE(name, chain->relPath) || @@ -1649,7 +1638,6 @@ virStorageFileChainLookup(virStorageSourcePtr chain, } *parent = chain; chain = chain->backingStore; - i++; } if (!chain) -- 2.14.1

On 10/12/2017 02:07 PM, Peter Krempa wrote:
We can now use the backing store ID directly rather than counting the number of images seen while looking up images. --- src/util/virstoragefile.c | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-)
I guess if we've always been counting the chain depth, the id renumbering question I had on patch 3 is somewhat answered: 'vda[1]' does not always refer to the same file in the chain. As long as we're okay with that, I didn't see any other problems in patch 3 or in this one. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org

chain->path may be NULL e.g. for NDB drives, so the check needs to avoid dereferencing the path in such case --- src/util/virstoragefile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 9568a5068..818a679de 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1610,7 +1610,7 @@ virStorageFileChainLookup(virStorageSourcePtr chain, break; } else { if (STREQ_NULLABLE(name, chain->relPath) || - STREQ(name, chain->path)) + STREQ_NULLABLE(name, chain->path)) break; if (nameIsFile && virStorageSourceIsLocalStorage(chain)) { -- 2.14.1

On 10/12/2017 02:07 PM, Peter Krempa wrote:
chain->path may be NULL e.g. for NDB drives, so the check needs to avoid
s/NDB/NBD/ ?
dereferencing the path in such case --- src/util/virstoragefile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 9568a5068..818a679de 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1610,7 +1610,7 @@ virStorageFileChainLookup(virStorageSourcePtr chain, break; } else { if (STREQ_NULLABLE(name, chain->relPath) || - STREQ(name, chain->path)) + STREQ_NULLABLE(name, chain->path)) break;
if (nameIsFile && virStorageSourceIsLocalStorage(chain)) {
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org

Storage driver uses virStorageSource only partially to store it's configuration but fully when parsing backing files of storage volumes. This patch sets the 'type' field to a value other than VIR_STORAGE_TYPE_NONE so that further patches can add a terminator element to backing chains without breaking iteration. --- src/conf/storage_conf.c | 4 ++++ src/storage/storage_backend_gluster.c | 2 ++ src/storage/storage_backend_logical.c | 1 + 3 files changed, 7 insertions(+) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index c35fa0e15..7c373e781 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1109,6 +1109,8 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, if (VIR_ALLOC(ret) < 0) return NULL; + ret->target.type = VIR_STORAGE_TYPE_FILE; + ret->name = virXPathString("string(./name)", ctxt); if (ret->name == NULL) { virReportError(VIR_ERR_XML_ERROR, "%s", @@ -1133,6 +1135,8 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, if (VIR_ALLOC(ret->target.backingStore) < 0) goto error; + ret->target.backingStore->type = VIR_STORAGE_TYPE_FILE; + ret->target.backingStore->path = backingStore; backingStore = NULL; diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index eac771b42..5eea84f16 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -306,6 +306,8 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state, if (VIR_ALLOC(vol->target.backingStore) < 0) goto cleanup; + vol->target.backingStore->type = VIR_STORAGE_TYPE_NETWORK; + vol->target.backingStore->path = meta->backingStoreRaw; if (backingFormat < 0) diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 0ad357729..1e0f04e4e 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -340,6 +340,7 @@ virStorageBackendLogicalMakeVol(char **const groups, goto cleanup; vol->target.backingStore->format = VIR_STORAGE_POOL_LOGICAL_LVM2; + vol->target.backingStore->type = VIR_STORAGE_TYPE_BLOCK; } if (!vol->key && VIR_STRDUP(vol->key, groups[2]) < 0) -- 2.14.1

On 10/12/2017 02:07 PM, Peter Krempa wrote:
Storage driver uses virStorageSource only partially to store it's configuration but fully when parsing backing files of storage volumes. This patch sets the 'type' field to a value other than VIR_STORAGE_TYPE_NONE so that further patches can add a terminator element to backing chains without breaking iteration. --- src/conf/storage_conf.c | 4 ++++ src/storage/storage_backend_gluster.c | 2 ++ src/storage/storage_backend_logical.c | 1 + 3 files changed, 7 insertions(+)
Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org

Set the type so that the iterators will work after upcoming modification. --- tests/virstoragetest.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index ffebd4dc1..ad4514871 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -580,6 +580,7 @@ testPathRelativePrepare(void) size_t i; for (i = 0; i < ARRAY_CARDINALITY(backingchain); i++) { + backingchain[i].type = VIR_STORAGE_TYPE_FILE; if (i < ARRAY_CARDINALITY(backingchain) - 1) backingchain[i].backingStore = &backingchain[i + 1]; else -- 2.14.1

On 10/12/2017 02:07 PM, Peter Krempa wrote:
Set the type so that the iterators will work after upcoming modification. --- tests/virstoragetest.c | 1 + 1 file changed, 1 insertion(+)
Could almost be squashed to the previous patch; but I'm also fine with it separate. Reviewed-by: Eric Blake <eblake@redhat.com>
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index ffebd4dc1..ad4514871 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -580,6 +580,7 @@ testPathRelativePrepare(void) size_t i;
for (i = 0; i < ARRAY_CARDINALITY(backingchain); i++) { + backingchain[i].type = VIR_STORAGE_TYPE_FILE; if (i < ARRAY_CARDINALITY(backingchain) - 1) backingchain[i].backingStore = &backingchain[i + 1]; else
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org

Add a helper that will retrun true if a virStorageSource has backing store. This will also aid in refactoring of the code further down. --- src/conf/domain_conf.c | 2 +- src/conf/storage_conf.c | 5 +++-- src/libvirt_private.syms | 2 ++ src/qemu/qemu_block.c | 4 ++-- src/qemu/qemu_cgroup.c | 4 ++-- src/qemu/qemu_domain.c | 6 ++--- src/qemu/qemu_driver.c | 9 ++++---- src/security/security_dac.c | 2 +- src/security/security_selinux.c | 4 ++-- src/security/virt-aa-helper.c | 2 +- src/storage/storage_backend_logical.c | 2 +- src/storage/storage_source.c | 2 +- src/storage/storage_util.c | 14 ++++++------ src/util/virstoragefile.c | 42 +++++++++++++++++++++++++++++------ src/util/virstoragefile.h | 6 +++++ tests/virstoragetest.c | 2 +- 16 files changed, 73 insertions(+), 35 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 45fa57a14..3eb6c7f6f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -26607,7 +26607,7 @@ virDomainDiskDefForeachPath(virDomainDiskDefPtr disk, } } - for (tmp = disk->src; tmp; tmp = tmp->backingStore) { + for (tmp = disk->src; virStorageSourceIsBacking(tmp); tmp = tmp->backingStore) { /* execute the callback only for local storage */ if (virStorageSourceIsLocalStorage(tmp) && tmp->path) { diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 7c373e781..f808cd291 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1169,7 +1169,8 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, if (virStorageSize(unit, capacity, &ret->target.capacity) < 0) goto error; } else if (!(flags & VIR_VOL_XML_PARSE_NO_CAPACITY) && - !((flags & VIR_VOL_XML_PARSE_OPT_CAPACITY) && ret->target.backingStore)) { + !((flags & VIR_VOL_XML_PARSE_OPT_CAPACITY) && + virStorageSourceHasBacking(&ret->target))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing capacity element")); goto error; } @@ -1497,7 +1498,7 @@ virStorageVolDefFormat(virStoragePoolDefPtr pool, &def->target, "target") < 0) goto cleanup; - if (def->target.backingStore && + if (virStorageSourceHasBacking(&def->target) && virStorageVolTargetDefFormat(options, &buf, def->target.backingStore, "backingStore") < 0) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 26c5ddb40..4d44c401b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2692,7 +2692,9 @@ virStorageSourceFindByNodeName; virStorageSourceFree; virStorageSourceGetActualType; virStorageSourceGetSecurityLabelDef; +virStorageSourceHasBacking; virStorageSourceInitChainElement; +virStorageSourceIsBacking; virStorageSourceIsBlockLocal; virStorageSourceIsEmpty; virStorageSourceIsLocalStorage; diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 8d232de3e..544b4893b 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -257,7 +257,7 @@ qemuBlockDiskClearDetectedNodes(virDomainDiskDefPtr disk) { virStorageSourcePtr next = disk->src; - while (next) { + while (virStorageSourceIsBacking(next)) { VIR_FREE(next->nodeformat); VIR_FREE(next->nodestorage); @@ -287,7 +287,7 @@ qemuBlockDiskDetectNodes(virDomainDiskDefPtr disk, goto cleanup; } - while (src && entry) { + while (virStorageSourceIsBacking(src) && entry) { if (src->nodeformat || src->nodestorage) { if (STRNEQ_NULLABLE(src->nodeformat, entry->nodeformat) || STRNEQ_NULLABLE(src->nodestorage, entry->nodestorage)) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 6fc413098..0f75e22f9 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -142,7 +142,7 @@ qemuSetupDiskCgroup(virDomainObjPtr vm, virStorageSourcePtr next; bool forceReadonly = false; - for (next = disk->src; next; next = next->backingStore) { + for (next = disk->src; virStorageSourceIsBacking(next); next = next->backingStore) { if (qemuSetupImageCgroupInternal(vm, next, forceReadonly) < 0) return -1; @@ -160,7 +160,7 @@ qemuTeardownDiskCgroup(virDomainObjPtr vm, { virStorageSourcePtr next; - for (next = disk->src; next; next = next->backingStore) { + for (next = disk->src; virStorageSourceIsBacking(next); next = next->backingStore) { if (qemuTeardownImageCgroup(vm, next) < 0) return -1; } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ed27a91fa..173fbca6e 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5929,7 +5929,7 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, if (virStorageSourceIsEmpty(disk->src)) goto cleanup; - if (disk->src->backingStore) { + if (virStorageSourceHasBacking(disk->src)) { if (force_probe) virStorageSourceBackingStoreClear(disk->src); else @@ -8548,7 +8548,7 @@ qemuDomainSetupDisk(virQEMUDriverConfigPtr cfg ATTRIBUTE_UNUSED, char *dst = NULL; int ret = -1; - for (next = disk->src; next; next = next->backingStore) { + for (next = disk->src; virStorageSourceIsBacking(next); next = next->backingStore) { if (!next->path || !virStorageSourceIsLocalStorage(next)) { /* Not creating device. Just continue. */ continue; @@ -9449,7 +9449,7 @@ qemuDomainNamespaceSetupDisk(virQEMUDriverPtr driver, &ndevMountsPath) < 0) goto cleanup; - for (next = src; next; next = next->backingStore) { + for (next = src; virStorageSourceIsBacking(next); next = next->backingStore) { if (virStorageSourceIsEmpty(next) || !virStorageSourceIsLocalStorage(next)) { /* Not creating device. Just continue. */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0260c40ef..803df9f06 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14448,7 +14448,7 @@ qemuDomainSnapshotUpdateDiskSourcesRenumber(virStorageSourcePtr src) virStorageSourcePtr next; unsigned int idx = 1; - for (next = src->backingStore; next; next = next->backingStore) + for (next = src->backingStore; virStorageSourceIsBacking(next); next = next->backingStore) next->id = idx++; } @@ -17048,7 +17048,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, } /* clear the _SHALLOW flag if there is only one layer */ - if (!disk->src->backingStore) + if (!virStorageSourceHasBacking(disk->src)) flags &= ~VIR_DOMAIN_BLOCK_COPY_SHALLOW; /* unless the user provides a pre-created file, shallow copy into a raw @@ -17439,7 +17439,7 @@ qemuDomainBlockCommit(virDomainPtr dom, goto endjob; } - if (!topSource->backingStore) { + if (!virStorageSourceHasBacking(topSource)) { virReportError(VIR_ERR_INVALID_ARG, _("top '%s' in chain for '%s' has no backing file"), topSource->path, path); @@ -19887,7 +19887,8 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver, virStorageSourcePtr src = disk->src; unsigned int backing_idx = 0; - while (src && (backing_idx == 0 || visitBacking)) { + while (virStorageSourceIsBacking(src) && + (backing_idx == 0 || visitBacking)) { if (qemuDomainGetStatsOneBlock(driver, cfg, dom, record, maxparams, disk, src, visited, backing_idx, stats, nodestats) < 0) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 349dbe81d..244b300a9 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -730,7 +730,7 @@ virSecurityDACSetDiskLabel(virSecurityManagerPtr mgr, { virStorageSourcePtr next; - for (next = disk->src; next; next = next->backingStore) { + for (next = disk->src; virStorageSourceIsBacking(next); next = next->backingStore) { if (virSecurityDACSetImageLabel(mgr, def, next) < 0) return -1; } diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 2e3082b7a..cd3e41193 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1539,7 +1539,7 @@ virSecuritySELinuxRestoreImageLabelInt(virSecurityManagerPtr mgr, * be tracked in domain XML, at which point labelskip should be a * per-file attribute instead of a disk attribute. */ if (disk_seclabel && disk_seclabel->labelskip && - !src->backingStore) + !virStorageSourceHasBacking(src)) return 0; /* Don't restore labels on readonly/shared disks, because other VMs may @@ -1673,7 +1673,7 @@ virSecuritySELinuxSetDiskLabel(virSecurityManagerPtr mgr, bool first = true; virStorageSourcePtr next; - for (next = disk->src; next; next = next->backingStore) { + for (next = disk->src; virStorageSourceIsBacking(next); next = next->backingStore) { if (virSecuritySELinuxSetImageLabelInternal(mgr, def, next, first) < 0) return -1; diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 95906e68a..ef1bf0136 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -942,7 +942,7 @@ get_files(vahControl * ctl) /* XXX - if we knew the qemu user:group here we could send it in * so that the open could be re-tried as that user:group. */ - if (!disk->src->backingStore) { + if (!virStorageSourceHasBacking(disk->src)) { bool probe = ctl->allowDiskFormatProbing; virStorageFileGetMetadata(disk->src, -1, -1, probe, false); } diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 1e0f04e4e..a872a2f88 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -977,7 +977,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn, } virCommandAddArgFormat(cmd, "%lluK", VIR_DIV_UP(vol->target.capacity, 1024)); - if (vol->target.backingStore) + if (virStorageSourceHasBacking(&vol->target)) virCommandAddArgList(cmd, "-s", vol->target.backingStore->path, NULL); else virCommandAddArg(cmd, def->source.name); diff --git a/src/storage/storage_source.c b/src/storage/storage_source.c index 864c69928..47b08f416 100644 --- a/src/storage/storage_source.c +++ b/src/storage/storage_source.c @@ -490,7 +490,7 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src, ret = 0; cleanup: - if (src->backingStore) + if (virStorageSourceHasBacking(src)) src->backingStore->id = depth; VIR_FREE(buf); virStorageFileDeinit(src); diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index a10e4590f..5252e429f 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -415,7 +415,7 @@ storageBackendCreateRaw(virConnectPtr conn ATTRIBUTE_UNUSED, goto cleanup; } - if (vol->target.backingStore) { + if (virStorageSourceHasBacking(&vol->target)) { virReportError(VIR_ERR_NO_SUPPORT, "%s", _("backing storage not supported for raw volumes")); goto cleanup; @@ -722,7 +722,7 @@ storageBackendCreatePloop(virConnectPtr conn ATTRIBUTE_UNUSED, return -1; } - if (vol->target.backingStore != NULL) { + if (virStorageSourceHasBacking(&vol->target)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("copy-on-write ploop volumes are not yet supported")); return -1; @@ -1055,7 +1055,7 @@ storageBackendCreateQemuImgSetBacking(virStoragePoolObjPtr pool, * backing store, not really sure what use it serves though, and it * may cause issues with lvm. Untested essentially. */ - if (inputvol && inputvol->target.backingStore && + if (inputvol && virStorageSourceHasBacking(&inputvol->target) && STRNEQ_NULLABLE(inputvol->target.backingStore->path, info->backingPath)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -1230,7 +1230,7 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn, storageBackendCreateQemuImgSetInput(inputvol, &info) < 0) return NULL; - if (vol->target.backingStore && + if (virStorageSourceHasBacking(&vol->target) && storageBackendCreateQemuImgSetBacking(pool, vol, inputvol, &info) < 0) return NULL; @@ -1840,7 +1840,7 @@ virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol, openflags, readflags)) < 0) return ret; - if (vol->target.backingStore && + if (virStorageSourceHasBacking(&vol->target) && (ret = storageBackendUpdateVolTargetInfo(VIR_STORAGE_VOL_FILE, vol->target.backingStore, withBlockVolFormat, @@ -2035,7 +2035,7 @@ createFileDir(virConnectPtr conn ATTRIBUTE_UNUSED, return -1; } - if (vol->target.backingStore) { + if (virStorageSourceHasBacking(&vol->target)) { virReportError(VIR_ERR_NO_SUPPORT, "%s", _("backing storage not supported for directories volumes")); return -1; @@ -3561,7 +3561,7 @@ virStorageBackendRefreshVolTargetUpdate(virStorageVolDefPtr vol) if (vol->target.format == VIR_STORAGE_FILE_PLOOP) vol->type = VIR_STORAGE_VOL_PLOOP; - if (vol->target.backingStore) { + if (virStorageSourceHasBacking(&vol->target)) { ignore_value(storageBackendUpdateVolTargetInfo(VIR_STORAGE_VOL_FILE, vol->target.backingStore, false, diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 818a679de..93995a331 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1292,7 +1292,7 @@ virStorageFileChainGetBroken(virStorageSourcePtr chain, if (!chain) return 0; - for (tmp = chain; tmp; tmp = tmp->backingStore) { + for (tmp = chain; virStorageSourceIsBacking(tmp); tmp = tmp->backingStore) { /* Break when we hit end of chain; report error if we detected * a missing backing file, infinite loop, or other error */ if (!tmp->backingStore && tmp->backingStoreRaw) { @@ -1566,6 +1566,33 @@ virStorageFileParseChainIndex(const char *diskTarget, return ret; } + +/** + * virStorageSourceIsBacking: + * @src: storage source + * + * Returns true if @src is a eligible backing store structure. Useful + * for iterators. + */ +bool +virStorageSourceIsBacking(const virStorageSource *src) +{ + return !!src; +} + +/** + * virStorageSourceHasBacking: + * @src: storage source + * + * Returns true if @src has backing store/chain. + */ +bool +virStorageSourceHasBacking(const virStorageSource *src) +{ + return virStorageSourceIsBacking(src) && src->backingStore; +} + + /* Given a @chain, look for the backing store @name that is a backing file * of @startFrom (or any member of @chain if @startFrom is NULL) and return * that location within the chain. @chain must always point to the top of @@ -1594,15 +1621,16 @@ virStorageFileChainLookup(virStorageSourcePtr chain, *parent = NULL; if (startFrom) { - while (chain && chain != startFrom->backingStore) + while (virStorageSourceIsBacking(chain) && + chain != startFrom->backingStore) chain = chain->backingStore; *parent = startFrom; } - while (chain) { + while (virStorageSourceIsBacking(chain)) { if (!name && !idx) { - if (!chain->backingStore) + if (!virStorageSourceHasBacking(chain)) break; } else if (idx) { VIR_DEBUG("%u: %s", chain->id, chain->path); @@ -1640,7 +1668,7 @@ virStorageFileChainLookup(virStorageSourcePtr chain, chain = chain->backingStore; } - if (!chain) + if (!virStorageSourceIsBacking(chain)) goto error; return chain; @@ -3854,7 +3882,7 @@ virStorageFileGetRelativeBackingPath(virStorageSourcePtr top, *relpath = NULL; - for (next = top; next; next = next->backingStore) { + for (next = top; virStorageSourceIsBacking(next); next = next->backingStore) { if (!next->relPath) { ret = 1; goto cleanup; @@ -3973,7 +4001,7 @@ virStorageSourceFindByNodeName(virStorageSourcePtr top, if (idx) *idx = 0; - for (tmp = top; tmp; tmp = tmp->backingStore) { + for (tmp = top; virStorageSourceIsBacking(tmp); tmp = tmp->backingStore) { if ((tmp->nodeformat && STREQ(tmp->nodeformat, nodeName)) || (tmp->nodestorage && STREQ(tmp->nodestorage, nodeName))) return tmp; diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index d3bafefc3..86e60de2a 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -425,4 +425,10 @@ void virStorageSourceNetworkAssignDefaultPorts(virStorageSourcePtr src) ATTRIBUTE_NONNULL(1); +bool +virStorageSourceIsBacking(const virStorageSource *src); +bool +virStorageSourceHasBacking(const virStorageSource *src); + + #endif /* __VIR_STORAGE_FILE_H__ */ diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index ad4514871..35e97ff26 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -356,7 +356,7 @@ testStorageChain(const void *args) } elt = meta; - while (elt) { + while (virStorageSourceIsBacking(elt)) { char *expect = NULL; char *actual = NULL; -- 2.14.1

On 10/12/2017 02:07 PM, Peter Krempa wrote:
Add a helper that will retrun true if a virStorageSource has backing
s/retrun/return/
store. This will also aid in refactoring of the code further down. ---
The commit message mentions adding only one helper, but doesn't name it. I like to mention new interfaces in commit messages, if only to make grepping 'git log' for a particular interface more likely to hit the relevant commits.
+++ b/src/conf/domain_conf.c @@ -26607,7 +26607,7 @@ virDomainDiskDefForeachPath(virDomainDiskDefPtr disk, } }
- for (tmp = disk->src; tmp; tmp = tmp->backingStore) { + for (tmp = disk->src; virStorageSourceIsBacking(tmp); tmp = tmp->backingStore) {
Here, you are adding a use of virStorageSourceIsBacking,
/* execute the callback only for local storage */ if (virStorageSourceIsLocalStorage(tmp) && tmp->path) { diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 7c373e781..f808cd291 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1169,7 +1169,8 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, if (virStorageSize(unit, capacity, &ret->target.capacity) < 0) goto error; } else if (!(flags & VIR_VOL_XML_PARSE_NO_CAPACITY) && - !((flags & VIR_VOL_XML_PARSE_OPT_CAPACITY) && ret->target.backingStore)) { + !((flags & VIR_VOL_XML_PARSE_OPT_CAPACITY) && + virStorageSourceHasBacking(&ret->target))) {
and here, virStorageSourceHasBacking. So my immediate thought was whether the 'Is' vs. 'Has' naming intentional? If so, you added two interfaces, so the commit message needs a tweak.
+++ b/src/libvirt_private.syms @@ -2692,7 +2692,9 @@ virStorageSourceFindByNodeName; virStorageSourceFree; virStorageSourceGetActualType; virStorageSourceGetSecurityLabelDef; +virStorageSourceHasBacking; virStorageSourceInitChainElement; +virStorageSourceIsBacking;
So you indeed are adding two interfaces, but it took me this far into the review to know it. One trick with git is that you can tell it to produce patches in a particular order (see qemu's scripts/git.orderfile, for example); this can be useful to hoist headers and build instructions first, showing the new interfaces, prior to the .c files that change to use the new interfaces, for a slightly faster review. But even with a preference of .syms before .h before .c, I still find it helpful to do one-off orderfile overrides; in this case, listing virstoragefile.c before other .c files would also aid review.
+++ b/src/qemu/qemu_cgroup.c @@ -160,7 +160,7 @@ qemuTeardownDiskCgroup(virDomainObjPtr vm, { virStorageSourcePtr next;
- for (next = disk->src; next; next = next->backingStore) { + for (next = disk->src; virStorageSourceIsBacking(next); next = next->backingStore) {
Line longer than 80 columns; do we care?
+++ b/src/util/virstoragefile.c @@ -1292,7 +1292,7 @@ virStorageFileChainGetBroken(virStorageSourcePtr chain, if (!chain) return 0;
- for (tmp = chain; tmp; tmp = tmp->backingStore) { + for (tmp = chain; virStorageSourceIsBacking(tmp); tmp = tmp->backingStore) { /* Break when we hit end of chain; report error if we detected * a missing backing file, infinite loop, or other error */ if (!tmp->backingStore && tmp->backingStoreRaw) { @@ -1566,6 +1566,33 @@ virStorageFileParseChainIndex(const char *diskTarget, return ret; }
+ +/** + * virStorageSourceIsBacking: + * @src: storage source + * + * Returns true if @src is a eligible backing store structure. Useful + * for iterators. + */ +bool +virStorageSourceIsBacking(const virStorageSource *src) +{ + return !!src; +} + +/** + * virStorageSourceHasBacking: + * @src: storage source + * + * Returns true if @src has backing store/chain. + */ +bool +virStorageSourceHasBacking(const virStorageSource *src) +{ + return virStorageSourceIsBacking(src) && src->backingStore; +}
The conversions look sane, and now you have an entry point for future patches to tweak what constitutes a valid reference, without having to hunt down all the iterations changed in this patch. Looks like a reasonable refactoring, so in spite of my comments, I'm fine with the code, and it is just the commit message that can be improved, and/or the use of an order file to present things in an altered order for ease of review. Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org

Express a properly terminated backing chain by putting a virStorageSource of type VIR_STORAGE_TYPE_NONE in the chain. The newly used helpers simplify this greatly. The change fixes a bug as formatting an incomplete backing chain and parsing it back would end up in expressing a terminated chain since src->backingStoreRaw was not populated. By relying on the terminator object this can be now processed appropriately. --- src/conf/domain_conf.c | 42 ++++++++---------- src/storage/storage_source.c | 50 +++++++++++----------- src/util/virstoragefile.c | 5 ++- .../qemuhotplug-base-ccw-live+ccw-virtio.xml | 1 - ...live-with-2-ccw-virtio+ccw-virtio-1-reverse.xml | 2 - ...qemuhotplug-base-ccw-live-with-2-ccw-virtio.xml | 1 - ...-live-with-ccw-virtio+ccw-virtio-2-explicit.xml | 2 - ...-base-ccw-live-with-ccw-virtio+ccw-virtio-2.xml | 2 - .../qemuhotplug-base-ccw-live-with-ccw-virtio.xml | 1 - ...-live+disk-scsi-wwn+disk-scsi-duplicate-wwn.xml | 2 - .../qemuhotplug-base-live+disk-scsi.xml | 1 - .../qemuhotplug-base-live+disk-usb.xml | 1 - .../qemuhotplug-base-live+disk-virtio.xml | 1 - ...se-without-scsi-controller-live+disk-scsi-2.xml | 1 - ...otplug-console-compat-2-live+console-virtio.xml | 2 - .../qemuhotplug-console-compat-2-live.xml | 2 - .../qemuxml2xmlout-channel-virtio-state-active.xml | 1 - .../qemuxml2xmlout-disk-active-commit.xml | 1 - .../qemuxml2xmlout-disk-backing-chains-active.xml | 5 --- .../qemuxml2xmlout-disk-mirror-active.xml | 4 -- .../qemuxml2xmlout-disk-mirror-old.xml | 4 -- .../qemuxml2xmlout-seclabel-static-labelskip.xml | 1 - tests/sexpr2xmldata/sexpr2xml-boot-grub.xml | 1 - tests/sexpr2xmldata/sexpr2xml-bridge-ipaddr.xml | 1 - tests/sexpr2xmldata/sexpr2xml-curmem.xml | 1 - .../sexpr2xml-disk-block-shareable.xml | 1 - tests/sexpr2xmldata/sexpr2xml-disk-block.xml | 1 - .../sexpr2xml-disk-drv-blktap-qcow.xml | 1 - .../sexpr2xml-disk-drv-blktap-raw.xml | 1 - .../sexpr2xml-disk-drv-blktap2-raw.xml | 1 - tests/sexpr2xmldata/sexpr2xml-disk-file.xml | 1 - tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-empty-kernel.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-force-hpet.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-force-nohpet.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-kernel.xml | 1 - tests/sexpr2xmldata/sexpr2xml-fv-localtime.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-net-netfront.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-parallel-tcp.xml | 2 - .../sexpr2xml-fv-serial-dev-2-ports.xml | 2 - .../sexpr2xml-fv-serial-dev-2nd-port.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-serial-file.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-serial-null.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-serial-pipe.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-serial-pty.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-serial-stdio.xml | 2 - .../sexpr2xml-fv-serial-tcp-telnet.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-serial-udp.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-serial-unix.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-sound-all.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-sound.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-usbmouse.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-usbtablet.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-utc.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv-v2.xml | 2 - tests/sexpr2xmldata/sexpr2xml-fv.xml | 2 - tests/sexpr2xmldata/sexpr2xml-net-bridged.xml | 1 - tests/sexpr2xmldata/sexpr2xml-net-e1000.xml | 1 - tests/sexpr2xmldata/sexpr2xml-net-routed.xml | 1 - tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.xml | 2 - tests/sexpr2xmldata/sexpr2xml-pci-devs.xml | 1 - .../sexpr2xml-pv-bootloader-cmdline.xml | 1 - tests/sexpr2xmldata/sexpr2xml-pv-bootloader.xml | 1 - tests/sexpr2xmldata/sexpr2xml-pv-localtime.xml | 1 - tests/sexpr2xmldata/sexpr2xml-pv-vcpus.xml | 1 - .../sexpr2xml-pv-vfb-new-vncdisplay.xml | 1 - tests/sexpr2xmldata/sexpr2xml-pv-vfb-new.xml | 1 - .../sexpr2xmldata/sexpr2xml-pv-vfb-type-crash.xml | 1 - tests/sexpr2xmldata/sexpr2xml-pv.xml | 1 - tests/sexpr2xmldata/sexpr2xml-vif-rate.xml | 2 - 71 files changed, 46 insertions(+), 162 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3eb6c7f6f..22f65b666 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8277,6 +8277,15 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, goto cleanup; } + if (!(type = virXMLPropString(ctxt->node, "type"))) { + /* terminator does not have a type */ + if (VIR_ALLOC(backingStore) < 0) + goto cleanup; + + ret = 0; + goto cleanup; + } + if (VIR_ALLOC(backingStore) < 0) goto cleanup; @@ -8287,12 +8296,6 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, goto cleanup; } - if (!(type = virXMLPropString(ctxt->node, "type"))) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing disk backing store type")); - goto cleanup; - } - backingStore->type = virStorageTypeFromString(type); if (backingStore->type <= 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -21905,24 +21908,16 @@ virDomainDiskSourceFormat(virBufferPtr buf, static int virDomainDiskBackingStoreFormat(virBufferPtr buf, - virStorageSourcePtr backingStore, - const char *backingStoreRaw) + virStorageSourcePtr backingStore) { - const char *type; const char *format; - if (!backingStore) { - if (!backingStoreRaw) - virBufferAddLit(buf, "<backingStore/>\n"); + if (!backingStore) return 0; - } - if (!backingStore->type || - !(type = virStorageTypeToString(backingStore->type))) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unexpected disk backing store type %d"), - backingStore->type); - return -1; + if (backingStore->type == VIR_STORAGE_TYPE_NONE) { + virBufferAddLit(buf, "<backingStore/>\n"); + return 0; } if (backingStore->format <= 0 || @@ -21933,7 +21928,8 @@ virDomainDiskBackingStoreFormat(virBufferPtr buf, return -1; } - virBufferAsprintf(buf, "<backingStore type='%s'", type); + virBufferAsprintf(buf, "<backingStore type='%s'", + virStorageTypeToString(backingStore->type)); if (backingStore->id != 0) virBufferAsprintf(buf, " index='%u'", backingStore->id); virBufferAddLit(buf, ">\n"); @@ -21943,8 +21939,7 @@ virDomainDiskBackingStoreFormat(virBufferPtr buf, /* We currently don't output seclabels for backing chain element */ if (virDomainDiskSourceFormatInternal(buf, backingStore, 0, 0, true) < 0 || virDomainDiskBackingStoreFormat(buf, - backingStore->backingStore, - backingStore->backingStoreRaw) < 0) + backingStore->backingStore) < 0) return -1; virBufferAdjustIndent(buf, -2); @@ -22079,8 +22074,7 @@ virDomainDiskDefFormat(virBufferPtr buf, /* Don't format backingStore to inactive XMLs until the code for * persistent storage of backing chains is ready. */ if (!(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE) && - virDomainDiskBackingStoreFormat(buf, def->src->backingStore, - def->src->backingStoreRaw) < 0) + virDomainDiskBackingStoreFormat(buf, def->src->backingStore) < 0) return -1; virBufferEscapeString(buf, "<backenddomain name='%s'/>\n", def->domain_name); diff --git a/src/storage/storage_source.c b/src/storage/storage_source.c index 47b08f416..419fa3d43 100644 --- a/src/storage/storage_source.c +++ b/src/storage/storage_source.c @@ -456,33 +456,33 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src, &backingFormat) < 0) goto cleanup; - /* check whether we need to go deeper */ - if (!src->backingStoreRaw) { - ret = 0; - goto cleanup; - } - - if (!(backingStore = virStorageSourceNewFromBacking(src))) - goto cleanup; - - if (backingFormat == VIR_STORAGE_FILE_AUTO && !allow_probe) - backingStore->format = VIR_STORAGE_FILE_RAW; - else if (backingFormat == VIR_STORAGE_FILE_AUTO_SAFE) - backingStore->format = VIR_STORAGE_FILE_AUTO; - else - backingStore->format = backingFormat; - - if ((ret = virStorageFileGetMetadataRecurse(backingStore, parent, - uid, gid, - allow_probe, report_broken, - cycle, depth + 1)) < 0) { - if (report_broken) + if (src->backingStoreRaw) { + if (!(backingStore = virStorageSourceNewFromBacking(src))) goto cleanup; - /* if we fail somewhere midway, just accept and return a - * broken chain */ - ret = 0; - goto cleanup; + if (backingFormat == VIR_STORAGE_FILE_AUTO && !allow_probe) + backingStore->format = VIR_STORAGE_FILE_RAW; + else if (backingFormat == VIR_STORAGE_FILE_AUTO_SAFE) + backingStore->format = VIR_STORAGE_FILE_AUTO; + else + backingStore->format = backingFormat; + + if ((ret = virStorageFileGetMetadataRecurse(backingStore, parent, + uid, gid, + allow_probe, report_broken, + cycle, depth + 1)) < 0) { + if (report_broken) + goto cleanup; + + /* if we fail somewhere midway, just accept and return a + * broken chain */ + ret = 0; + goto cleanup; + } + } else { + /* add terminator */ + if (VIR_ALLOC(backingStore) < 0) + goto cleanup; } src->backingStore = backingStore; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 93995a331..59229631b 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1577,7 +1577,7 @@ virStorageFileParseChainIndex(const char *diskTarget, bool virStorageSourceIsBacking(const virStorageSource *src) { - return !!src; + return src && src->type != VIR_STORAGE_TYPE_NONE; } /** @@ -1589,7 +1589,8 @@ virStorageSourceIsBacking(const virStorageSource *src) bool virStorageSourceHasBacking(const virStorageSource *src) { - return virStorageSourceIsBacking(src) && src->backingStore; + return virStorageSourceIsBacking(src) && src->backingStore && + src->backingStore->type != VIR_STORAGE_TYPE_NONE; } diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live+ccw-virtio.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live+ccw-virtio.xml index cd03d0e09..0fa8d036b 100644 --- a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live+ccw-virtio.xml +++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live+ccw-virtio.xml @@ -22,7 +22,6 @@ <disk type='file' device='disk'> <driver name='qemu' type='raw' cache='none'/> <source file='/dev/null'/> - <backingStore/> <target dev='vde' bus='virtio'/> <readonly/> <shareable/> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio+ccw-virtio-1-reverse.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio+ccw-virtio-1-reverse.xml index 7be75f977..135427fff 100644 --- a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio+ccw-virtio-1-reverse.xml +++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio+ccw-virtio-1-reverse.xml @@ -22,7 +22,6 @@ <disk type='file' device='disk'> <driver name='qemu' type='raw' cache='none'/> <source file='/dev/null'/> - <backingStore/> <target dev='hda' bus='virtio'/> <readonly/> <shareable/> @@ -32,7 +31,6 @@ <disk type='file' device='disk'> <driver name='qemu' type='raw' cache='none'/> <source file='/dev/null'/> - <backingStore/> <target dev='hdb' bus='virtio'/> <readonly/> <shareable/> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio.xml index a83f1b5d7..e17c4e43b 100644 --- a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio.xml +++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio.xml @@ -22,7 +22,6 @@ <disk type='file' device='disk'> <driver name='qemu' type='raw' cache='none'/> <source file='/dev/null'/> - <backingStore/> <target dev='hda' bus='virtio'/> <readonly/> <shareable/> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2-explicit.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2-explicit.xml index 0a51993cc..326d312fa 100644 --- a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2-explicit.xml +++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2-explicit.xml @@ -22,7 +22,6 @@ <disk type='file' device='disk'> <driver name='qemu' type='raw' cache='none'/> <source file='/dev/null'/> - <backingStore/> <target dev='hda' bus='virtio'/> <readonly/> <shareable/> @@ -32,7 +31,6 @@ <disk type='file' device='disk'> <driver name='qemu' type='raw' cache='none'/> <source file='/dev/null'/> - <backingStore/> <target dev='vde' bus='virtio'/> <readonly/> <shareable/> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2.xml index 0a51993cc..326d312fa 100644 --- a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2.xml +++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2.xml @@ -22,7 +22,6 @@ <disk type='file' device='disk'> <driver name='qemu' type='raw' cache='none'/> <source file='/dev/null'/> - <backingStore/> <target dev='hda' bus='virtio'/> <readonly/> <shareable/> @@ -32,7 +31,6 @@ <disk type='file' device='disk'> <driver name='qemu' type='raw' cache='none'/> <source file='/dev/null'/> - <backingStore/> <target dev='vde' bus='virtio'/> <readonly/> <shareable/> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio.xml index cd03d0e09..0fa8d036b 100644 --- a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio.xml +++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio.xml @@ -22,7 +22,6 @@ <disk type='file' device='disk'> <driver name='qemu' type='raw' cache='none'/> <source file='/dev/null'/> - <backingStore/> <target dev='vde' bus='virtio'/> <readonly/> <shareable/> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-live+disk-scsi-wwn+disk-scsi-duplicate-wwn.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-live+disk-scsi-wwn+disk-scsi-duplicate-wwn.xml index 4c3ea3202..9482b6794 100644 --- a/tests/qemuhotplugtestdomains/qemuhotplug-base-live+disk-scsi-wwn+disk-scsi-duplicate-wwn.xml +++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-live+disk-scsi-wwn+disk-scsi-duplicate-wwn.xml @@ -22,7 +22,6 @@ <disk type='file' device='disk'> <driver name='qemu' type='raw' cache='none'/> <source file='/dev/null'/> - <backingStore/> <target dev='sdf' bus='scsi'/> <readonly/> <shareable/> @@ -33,7 +32,6 @@ <disk type='file' device='disk'> <driver name='qemu' type='raw' cache='none'/> <source file='/dev/null'/> - <backingStore/> <target dev='sdg' bus='scsi'/> <readonly/> <shareable/> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-live+disk-scsi.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-live+disk-scsi.xml index 493a615fd..a6dbf0b1b 100644 --- a/tests/qemuhotplugtestdomains/qemuhotplug-base-live+disk-scsi.xml +++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-live+disk-scsi.xml @@ -22,7 +22,6 @@ <disk type='file' device='disk'> <driver name='qemu' type='raw' cache='none'/> <source file='/dev/null'/> - <backingStore/> <target dev='sdf' bus='scsi'/> <readonly/> <shareable/> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-live+disk-usb.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-live+disk-usb.xml index 3609819ea..6ccb88f14 100644 --- a/tests/qemuhotplugtestdomains/qemuhotplug-base-live+disk-usb.xml +++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-live+disk-usb.xml @@ -22,7 +22,6 @@ <disk type='file' device='disk'> <driver name='qemu' type='raw' cache='none'/> <source file='/dev/null'/> - <backingStore/> <target dev='sdq' bus='usb'/> <readonly/> <shareable/> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-live+disk-virtio.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-live+disk-virtio.xml index b88b220e3..b97c0b41e 100644 --- a/tests/qemuhotplugtestdomains/qemuhotplug-base-live+disk-virtio.xml +++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-live+disk-virtio.xml @@ -22,7 +22,6 @@ <disk type='file' device='disk'> <driver name='qemu' type='raw' cache='none'/> <source file='/dev/null'/> - <backingStore/> <target dev='vde' bus='virtio'/> <readonly/> <shareable/> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-without-scsi-controller-live+disk-scsi-2.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-without-scsi-controller-live+disk-scsi-2.xml index c12d18f71..6422e1640 100644 --- a/tests/qemuhotplugtestdomains/qemuhotplug-base-without-scsi-controller-live+disk-scsi-2.xml +++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-without-scsi-controller-live+disk-scsi-2.xml @@ -22,7 +22,6 @@ <disk type='file' device='disk'> <driver name='qemu' type='raw' cache='none'/> <source file='/dev/null'/> - <backingStore/> <target dev='sdf' bus='scsi'/> <readonly/> <shareable/> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-console-compat-2-live+console-virtio.xml b/tests/qemuhotplugtestdomains/qemuhotplug-console-compat-2-live+console-virtio.xml index 7ca36d57b..4e1dd49c2 100644 --- a/tests/qemuhotplugtestdomains/qemuhotplug-console-compat-2-live+console-virtio.xml +++ b/tests/qemuhotplugtestdomains/qemuhotplug-console-compat-2-live+console-virtio.xml @@ -29,7 +29,6 @@ <disk type='file' device='disk'> <driver name='qemu' type='qcow2' cache='none'/> <source file='/var/lib/libvirt/images/f17.qcow2'/> - <backingStore/> <target dev='vda' bus='virtio'/> <alias name='virtio-disk0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> @@ -37,7 +36,6 @@ <disk type='file' device='cdrom'> <driver name='qemu' type='raw' cache='none'/> <source file='/home/user/tmp/Fedora-17-x86_64-Live-KDE.iso'/> - <backingStore/> <target dev='hdc' bus='ide'/> <readonly/> <alias name='ide0-1-0'/> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-console-compat-2-live.xml b/tests/qemuhotplugtestdomains/qemuhotplug-console-compat-2-live.xml index f300940a3..c56d13ef4 100644 --- a/tests/qemuhotplugtestdomains/qemuhotplug-console-compat-2-live.xml +++ b/tests/qemuhotplugtestdomains/qemuhotplug-console-compat-2-live.xml @@ -29,7 +29,6 @@ <disk type='file' device='disk'> <driver name='qemu' type='qcow2' cache='none'/> <source file='/var/lib/libvirt/images/f17.qcow2'/> - <backingStore/> <target dev='vda' bus='virtio'/> <alias name='virtio-disk0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> @@ -37,7 +36,6 @@ <disk type='file' device='cdrom'> <driver name='qemu' type='raw' cache='none'/> <source file='/home/user/tmp/Fedora-17-x86_64-Live-KDE.iso'/> - <backingStore/> <target dev='hdc' bus='ide'/> <readonly/> <alias name='ide0-1-0'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-virtio-state-active.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-virtio-state-active.xml index 8cddbeff7..7d93fc0c0 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-virtio-state-active.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-virtio-state-active.xml @@ -16,7 +16,6 @@ <emulator>/usr/bin/qemu-system-i686</emulator> <disk type='block' device='disk'> <source dev='/dev/HostVG/QEMUGuest1'/> - <backingStore/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-active-commit.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-active-commit.xml index cc26af109..5766e4aea 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-active-commit.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-active-commit.xml @@ -20,7 +20,6 @@ <backingStore type='block' index='1'> <format type='raw'/> <source dev='/dev/HostVG/QEMUGuest1'/> - <backingStore/> </backingStore> <mirror type='block' job='active-commit'> <format type='raw'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-backing-chains-active.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-backing-chains-active.xml index 83d47df56..828defcc2 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-backing-chains-active.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-backing-chains-active.xml @@ -22,7 +22,6 @@ <backingStore type='file' index='1'> <format type='qcow2'/> <source file='/tmp/missing-backing-store.qcow'/> - <backingStore/> </backingStore> <target dev='vda' bus='virtio'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> @@ -50,7 +49,6 @@ <backingStore type='file' index='6'> <format type='raw'/> <source file='/tmp/Fedora-17-x86_64-Live-KDE.iso'/> - <backingStore/> </backingStore> </backingStore> </backingStore> @@ -65,7 +63,6 @@ <source protocol='gluster' name='Volume1/Image'> <host name='example.org' port='6000'/> </source> - <backingStore/> <target dev='vdc' bus='virtio'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> </disk> @@ -82,7 +79,6 @@ <backingStore type='file' index='1'> <format type='qcow2'/> <source file='/tmp/image.qcow'/> - <backingStore/> </backingStore> <target dev='vdd' bus='virtio'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> @@ -90,7 +86,6 @@ <disk type='block' device='disk'> <driver name='qemu' type='qcow2'/> <source dev='/dev/HostVG/QEMUGuest11'/> - <backingStore/> <target dev='vde' bus='virtio'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> </disk> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-active.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-active.xml index c1e8a33ec..252bde338 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-active.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-active.xml @@ -16,7 +16,6 @@ <emulator>/usr/bin/qemu-system-i686</emulator> <disk type='block' device='disk'> <source dev='/dev/HostVG/QEMUGuest1'/> - <backingStore/> <mirror type='block' job='copy' ready='yes'> <source dev='/dev/HostVG/QEMUGuest1Copy'/> </mirror> @@ -25,14 +24,12 @@ </disk> <disk type='block' device='cdrom'> <source dev='/dev/HostVG/QEMUGuest2'/> - <backingStore/> <target dev='hdc' bus='ide'/> <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> <disk type='file' device='disk'> <source file='/tmp/data.img'/> - <backingStore/> <mirror type='file' file='/tmp/copy.img' format='qcow2' job='copy'> <format type='qcow2'/> <source file='/tmp/copy.img'/> @@ -42,7 +39,6 @@ </disk> <disk type='file' device='disk'> <source file='/tmp/logs.img'/> - <backingStore/> <mirror type='file' file='/tmp/logcopy.img' format='qcow2' job='copy' ready='abort'> <format type='qcow2'/> <source file='/tmp/logcopy.img'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml index e390bc02f..f4bd39a58 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml @@ -16,7 +16,6 @@ <emulator>/usr/bin/qemu-system-i686</emulator> <disk type='block' device='disk'> <source dev='/dev/HostVG/QEMUGuest1'/> - <backingStore/> <mirror type='file' file='/dev/HostVG/QEMUGuest1Copy' job='copy' ready='yes'> <source file='/dev/HostVG/QEMUGuest1Copy'/> </mirror> @@ -25,14 +24,12 @@ </disk> <disk type='block' device='cdrom'> <source dev='/dev/HostVG/QEMUGuest2'/> - <backingStore/> <target dev='hdc' bus='ide'/> <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> <disk type='file' device='disk'> <source file='/tmp/data.img'/> - <backingStore/> <mirror type='file' file='/tmp/copy.img' format='qcow2' job='copy'> <format type='qcow2'/> <source file='/tmp/copy.img'/> @@ -42,7 +39,6 @@ </disk> <disk type='file' device='disk'> <source file='/tmp/logs.img'/> - <backingStore/> <target dev='vdb' bus='virtio'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </disk> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-seclabel-static-labelskip.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-seclabel-static-labelskip.xml index d37b950cb..91f573db7 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-seclabel-static-labelskip.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-seclabel-static-labelskip.xml @@ -18,7 +18,6 @@ <source dev='/dev/HostVG/QEMUGuest1'> <seclabel model='selinux' labelskip='yes'/> </source> - <backingStore/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> diff --git a/tests/sexpr2xmldata/sexpr2xml-boot-grub.xml b/tests/sexpr2xmldata/sexpr2xml-boot-grub.xml index 85cff4e8b..b9a8716b2 100644 --- a/tests/sexpr2xmldata/sexpr2xml-boot-grub.xml +++ b/tests/sexpr2xmldata/sexpr2xml-boot-grub.xml @@ -17,7 +17,6 @@ <disk type='block' device='disk'> <driver name='phy'/> <source dev='/dev/MainVG/GuestVG'/> - <backingStore/> <target dev='xvda' bus='xen'/> </disk> <console type='pty'> diff --git a/tests/sexpr2xmldata/sexpr2xml-bridge-ipaddr.xml b/tests/sexpr2xmldata/sexpr2xml-bridge-ipaddr.xml index a04496f11..a8f804423 100644 --- a/tests/sexpr2xmldata/sexpr2xml-bridge-ipaddr.xml +++ b/tests/sexpr2xmldata/sexpr2xml-bridge-ipaddr.xml @@ -18,7 +18,6 @@ <disk type='file' device='disk'> <driver name='file'/> <source file='/root/some.img'/> - <backingStore/> <target dev='xvda' bus='xen'/> </disk> <interface type='bridge'> diff --git a/tests/sexpr2xmldata/sexpr2xml-curmem.xml b/tests/sexpr2xmldata/sexpr2xml-curmem.xml index 601749e2b..a976986a1 100644 --- a/tests/sexpr2xmldata/sexpr2xml-curmem.xml +++ b/tests/sexpr2xmldata/sexpr2xml-curmem.xml @@ -19,7 +19,6 @@ <disk type='file' device='disk'> <driver name='tap' type='raw'/> <source file='/xen/rhel5.img'/> - <backingStore/> <target dev='xvda' bus='xen'/> </disk> <interface type='bridge'> diff --git a/tests/sexpr2xmldata/sexpr2xml-disk-block-shareable.xml b/tests/sexpr2xmldata/sexpr2xml-disk-block-shareable.xml index a3295a056..9f757efa2 100644 --- a/tests/sexpr2xmldata/sexpr2xml-disk-block-shareable.xml +++ b/tests/sexpr2xmldata/sexpr2xml-disk-block-shareable.xml @@ -16,7 +16,6 @@ <disk type='file' device='disk'> <driver name='tap' type='raw'/> <source file='/var/lib/xen/images/rhel5pv.img'/> - <backingStore/> <target dev='xvda' bus='xen'/> <shareable/> </disk> diff --git a/tests/sexpr2xmldata/sexpr2xml-disk-block.xml b/tests/sexpr2xmldata/sexpr2xml-disk-block.xml index 30dee466e..56d6db5a5 100644 --- a/tests/sexpr2xmldata/sexpr2xml-disk-block.xml +++ b/tests/sexpr2xmldata/sexpr2xml-disk-block.xml @@ -18,7 +18,6 @@ <disk type='block' device='disk'> <driver name='phy'/> <source dev='/dev/MainVG/GuestVG'/> - <backingStore/> <target dev='xvda' bus='xen'/> </disk> <console type='pty'> diff --git a/tests/sexpr2xmldata/sexpr2xml-disk-drv-blktap-qcow.xml b/tests/sexpr2xmldata/sexpr2xml-disk-drv-blktap-qcow.xml index 12b65d6f5..80f6dd205 100644 --- a/tests/sexpr2xmldata/sexpr2xml-disk-drv-blktap-qcow.xml +++ b/tests/sexpr2xmldata/sexpr2xml-disk-drv-blktap-qcow.xml @@ -18,7 +18,6 @@ <disk type='file' device='disk'> <driver name='tap' type='qcow'/> <source file='/root/some.img'/> - <backingStore/> <target dev='xvda' bus='xen'/> </disk> <console type='pty'> diff --git a/tests/sexpr2xmldata/sexpr2xml-disk-drv-blktap-raw.xml b/tests/sexpr2xmldata/sexpr2xml-disk-drv-blktap-raw.xml index f7d48f9e5..71e7c40b6 100644 --- a/tests/sexpr2xmldata/sexpr2xml-disk-drv-blktap-raw.xml +++ b/tests/sexpr2xmldata/sexpr2xml-disk-drv-blktap-raw.xml @@ -18,7 +18,6 @@ <disk type='file' device='disk'> <driver name='tap' type='raw'/> <source file='/root/some.img'/> - <backingStore/> <target dev='xvda' bus='xen'/> </disk> <console type='pty'> diff --git a/tests/sexpr2xmldata/sexpr2xml-disk-drv-blktap2-raw.xml b/tests/sexpr2xmldata/sexpr2xml-disk-drv-blktap2-raw.xml index 497413c6e..bd244bbc3 100644 --- a/tests/sexpr2xmldata/sexpr2xml-disk-drv-blktap2-raw.xml +++ b/tests/sexpr2xmldata/sexpr2xml-disk-drv-blktap2-raw.xml @@ -18,7 +18,6 @@ <disk type='file' device='disk'> <driver name='tap2' type='raw'/> <source file='/root/some.img'/> - <backingStore/> <target dev='xvda' bus='xen'/> </disk> <console type='pty'> diff --git a/tests/sexpr2xmldata/sexpr2xml-disk-file.xml b/tests/sexpr2xmldata/sexpr2xml-disk-file.xml index 18b3dbde2..c624fe31c 100644 --- a/tests/sexpr2xmldata/sexpr2xml-disk-file.xml +++ b/tests/sexpr2xmldata/sexpr2xml-disk-file.xml @@ -18,7 +18,6 @@ <disk type='file' device='disk'> <driver name='file'/> <source file='/root/some.img'/> - <backingStore/> <target dev='xvda' bus='xen'/> </disk> <console type='pty'> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml b/tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml index e8ae46c8a..5d4976fb3 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml @@ -21,14 +21,12 @@ <disk type='block' device='disk'> <driver name='phy'/> <source dev='/iscsi/winxp'/> - <backingStore/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='cdrom'> <driver name='file'/> <source file='/net/heaped/export/netimage/windows/xp-sp2-vol.iso'/> - <backingStore/> <target dev='hdc' bus='ide'/> <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-empty-kernel.xml b/tests/sexpr2xmldata/sexpr2xml-fv-empty-kernel.xml index 0adac3420..57b60ec78 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-empty-kernel.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-empty-kernel.xml @@ -21,14 +21,12 @@ <disk type='file' device='disk'> <driver name='file'/> <source file='/root/foo.img'/> - <backingStore/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='cdrom'> <driver name='file'/> <source file='/root/boot.iso'/> - <backingStore/> <target dev='hdc' bus='ide'/> <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-force-hpet.xml b/tests/sexpr2xmldata/sexpr2xml-fv-force-hpet.xml index 4f02e80c3..8266d10e0 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-force-hpet.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-force-hpet.xml @@ -23,14 +23,12 @@ <disk type='file' device='disk'> <driver name='file'/> <source file='/root/foo.img'/> - <backingStore/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='cdrom'> <driver name='file'/> <source file='/root/boot.iso'/> - <backingStore/> <target dev='hdc' bus='ide'/> <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-force-nohpet.xml b/tests/sexpr2xmldata/sexpr2xml-fv-force-nohpet.xml index caa10d9ff..0b360446f 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-force-nohpet.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-force-nohpet.xml @@ -23,14 +23,12 @@ <disk type='file' device='disk'> <driver name='file'/> <source file='/root/foo.img'/> - <backingStore/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='cdrom'> <driver name='file'/> <source file='/root/boot.iso'/> - <backingStore/> <target dev='hdc' bus='ide'/> <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-kernel.xml b/tests/sexpr2xmldata/sexpr2xml-fv-kernel.xml index ce6e97a25..c638d8c72 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-kernel.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-kernel.xml @@ -19,7 +19,6 @@ <disk type='file' device='disk'> <driver name='file'/> <source file='/root/some.img'/> - <backingStore/> <target dev='xvda' bus='xen'/> </disk> <serial type='pty'> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-localtime.xml b/tests/sexpr2xmldata/sexpr2xml-fv-localtime.xml index 189215ab4..40866d392 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-localtime.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-localtime.xml @@ -21,14 +21,12 @@ <disk type='file' device='disk'> <driver name='file'/> <source file='/root/foo.img'/> - <backingStore/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='cdrom'> <driver name='file'/> <source file='/root/boot.iso'/> - <backingStore/> <target dev='hdc' bus='ide'/> <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-net-netfront.xml b/tests/sexpr2xmldata/sexpr2xml-fv-net-netfront.xml index 386726aca..ed53462ab 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-net-netfront.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-net-netfront.xml @@ -21,14 +21,12 @@ <disk type='file' device='disk'> <driver name='file'/> <source file='/root/foo.img'/> - <backingStore/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='cdrom'> <driver name='file'/> <source file='/root/boot.iso'/> - <backingStore/> <target dev='hdc' bus='ide'/> <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-parallel-tcp.xml b/tests/sexpr2xmldata/sexpr2xml-fv-parallel-tcp.xml index 9fdc16f17..0fbce74c3 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-parallel-tcp.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-parallel-tcp.xml @@ -21,14 +21,12 @@ <disk type='file' device='disk'> <driver name='file'/> <source file='/root/foo.img'/> - <backingStore/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='cdrom'> <driver name='file'/> <source file='/root/boot.iso'/> - <backingStore/> <target dev='hdc' bus='ide'/> <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-serial-dev-2-ports.xml b/tests/sexpr2xmldata/sexpr2xml-fv-serial-dev-2-ports.xml index 50dac3bd1..bf2518465 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-serial-dev-2-ports.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-serial-dev-2-ports.xml @@ -21,14 +21,12 @@ <disk type='file' device='disk'> <driver name='file'/> <source file='/root/foo.img'/> - <backingStore/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='cdrom'> <driver name='file'/> <source file='/root/boot.iso'/> - <backingStore/> <target dev='hdc' bus='ide'/> <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-serial-dev-2nd-port.xml b/tests/sexpr2xmldata/sexpr2xml-fv-serial-dev-2nd-port.xml index f0c9c4f7b..60e9dd819 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-serial-dev-2nd-port.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-serial-dev-2nd-port.xml @@ -21,14 +21,12 @@ <disk type='file' device='disk'> <driver name='file'/> <source file='/root/foo.img'/> - <backingStore/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='cdrom'> <driver name='file'/> <source file='/root/boot.iso'/> - <backingStore/> <target dev='hdc' bus='ide'/> <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-serial-file.xml b/tests/sexpr2xmldata/sexpr2xml-fv-serial-file.xml index 127de7d4f..6dfc32f01 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-serial-file.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-serial-file.xml @@ -21,14 +21,12 @@ <disk type='file' device='disk'> <driver name='file'/> <source file='/root/foo.img'/> - <backingStore/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='cdrom'> <driver name='file'/> <source file='/root/boot.iso'/> - <backingStore/> <target dev='hdc' bus='ide'/> <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-serial-null.xml b/tests/sexpr2xmldata/sexpr2xml-fv-serial-null.xml index 2b4c0c954..274e3c36c 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-serial-null.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-serial-null.xml @@ -21,14 +21,12 @@ <disk type='file' device='disk'> <driver name='file'/> <source file='/root/foo.img'/> - <backingStore/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='cdrom'> <driver name='file'/> <source file='/root/boot.iso'/> - <backingStore/> <target dev='hdc' bus='ide'/> <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-serial-pipe.xml b/tests/sexpr2xmldata/sexpr2xml-fv-serial-pipe.xml index 2080a4dfc..fe251cb5f 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-serial-pipe.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-serial-pipe.xml @@ -21,14 +21,12 @@ <disk type='file' device='disk'> <driver name='file'/> <source file='/root/foo.img'/> - <backingStore/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='cdrom'> <driver name='file'/> <source file='/root/boot.iso'/> - <backingStore/> <target dev='hdc' bus='ide'/> <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-serial-pty.xml b/tests/sexpr2xmldata/sexpr2xml-fv-serial-pty.xml index c51a79af6..258d76c45 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-serial-pty.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-serial-pty.xml @@ -21,14 +21,12 @@ <disk type='file' device='disk'> <driver name='file'/> <source file='/root/foo.img'/> - <backingStore/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='cdrom'> <driver name='file'/> <source file='/root/boot.iso'/> - <backingStore/> <target dev='hdc' bus='ide'/> <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-serial-stdio.xml b/tests/sexpr2xmldata/sexpr2xml-fv-serial-stdio.xml index 6226a2844..768a4253b 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-serial-stdio.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-serial-stdio.xml @@ -21,14 +21,12 @@ <disk type='file' device='disk'> <driver name='file'/> <source file='/root/foo.img'/> - <backingStore/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='cdrom'> <driver name='file'/> <source file='/root/boot.iso'/> - <backingStore/> <target dev='hdc' bus='ide'/> <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp-telnet.xml b/tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp-telnet.xml index 071645f66..6d76e5752 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp-telnet.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp-telnet.xml @@ -21,14 +21,12 @@ <disk type='file' device='disk'> <driver name='file'/> <source file='/root/foo.img'/> - <backingStore/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='cdrom'> <driver name='file'/> <source file='/root/boot.iso'/> - <backingStore/> <target dev='hdc' bus='ide'/> <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp.xml b/tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp.xml index 3eda5137d..9b7edd110 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp.xml @@ -21,14 +21,12 @@ <disk type='file' device='disk'> <driver name='file'/> <source file='/root/foo.img'/> - <backingStore/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='cdrom'> <driver name='file'/> <source file='/root/boot.iso'/> - <backingStore/> <target dev='hdc' bus='ide'/> <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-serial-udp.xml b/tests/sexpr2xmldata/sexpr2xml-fv-serial-udp.xml index 35b6f8462..65c4ffcbb 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-serial-udp.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-serial-udp.xml @@ -21,14 +21,12 @@ <disk type='file' device='disk'> <driver name='file'/> <source file='/root/foo.img'/> - <backingStore/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='cdrom'> <driver name='file'/> <source file='/root/boot.iso'/> - <backingStore/> <target dev='hdc' bus='ide'/> <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-serial-unix.xml b/tests/sexpr2xmldata/sexpr2xml-fv-serial-unix.xml index fe5bf1108..2c75df14d 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-serial-unix.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-serial-unix.xml @@ -21,14 +21,12 @@ <disk type='file' device='disk'> <driver name='file'/> <source file='/root/foo.img'/> - <backingStore/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='cdrom'> <driver name='file'/> <source file='/root/boot.iso'/> - <backingStore/> <target dev='hdc' bus='ide'/> <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-sound-all.xml b/tests/sexpr2xmldata/sexpr2xml-fv-sound-all.xml index 65ba50603..cb5e9443c 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-sound-all.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-sound-all.xml @@ -21,14 +21,12 @@ <disk type='file' device='disk'> <driver name='file'/> <source file='/root/foo.img'/> - <backingStore/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='cdrom'> <driver name='file'/> <source file='/root/boot.iso'/> - <backingStore/> <target dev='hdc' bus='ide'/> <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-sound.xml b/tests/sexpr2xmldata/sexpr2xml-fv-sound.xml index 65ba50603..cb5e9443c 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-sound.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-sound.xml @@ -21,14 +21,12 @@ <disk type='file' device='disk'> <driver name='file'/> <source file='/root/foo.img'/> - <backingStore/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='cdrom'> <driver name='file'/> <source file='/root/boot.iso'/> - <backingStore/> <target dev='hdc' bus='ide'/> <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-usbmouse.xml b/tests/sexpr2xmldata/sexpr2xml-fv-usbmouse.xml index f034bc19a..6de380ba0 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-usbmouse.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-usbmouse.xml @@ -21,14 +21,12 @@ <disk type='file' device='disk'> <driver name='file'/> <source file='/root/foo.img'/> - <backingStore/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='cdrom'> <driver name='file'/> <source file='/root/boot.iso'/> - <backingStore/> <target dev='hdc' bus='ide'/> <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-usbtablet.xml b/tests/sexpr2xmldata/sexpr2xml-fv-usbtablet.xml index ab350c0c0..46628dff4 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-usbtablet.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-usbtablet.xml @@ -21,14 +21,12 @@ <disk type='file' device='disk'> <driver name='file'/> <source file='/root/foo.img'/> - <backingStore/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='cdrom'> <driver name='file'/> <source file='/root/boot.iso'/> - <backingStore/> <target dev='hdc' bus='ide'/> <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-utc.xml b/tests/sexpr2xmldata/sexpr2xml-fv-utc.xml index e49854f08..f75eac3a7 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-utc.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-utc.xml @@ -21,14 +21,12 @@ <disk type='file' device='disk'> <driver name='file'/> <source file='/root/foo.img'/> - <backingStore/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='cdrom'> <driver name='file'/> <source file='/root/boot.iso'/> - <backingStore/> <target dev='hdc' bus='ide'/> <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-v2.xml b/tests/sexpr2xmldata/sexpr2xml-fv-v2.xml index e49854f08..f75eac3a7 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-v2.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-v2.xml @@ -21,14 +21,12 @@ <disk type='file' device='disk'> <driver name='file'/> <source file='/root/foo.img'/> - <backingStore/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='cdrom'> <driver name='file'/> <source file='/root/boot.iso'/> - <backingStore/> <target dev='hdc' bus='ide'/> <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv.xml b/tests/sexpr2xmldata/sexpr2xml-fv.xml index e49854f08..f75eac3a7 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv.xml @@ -21,14 +21,12 @@ <disk type='file' device='disk'> <driver name='file'/> <source file='/root/foo.img'/> - <backingStore/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='cdrom'> <driver name='file'/> <source file='/root/boot.iso'/> - <backingStore/> <target dev='hdc' bus='ide'/> <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-net-bridged.xml b/tests/sexpr2xmldata/sexpr2xml-net-bridged.xml index 33961fc95..b680fd8aa 100644 --- a/tests/sexpr2xmldata/sexpr2xml-net-bridged.xml +++ b/tests/sexpr2xmldata/sexpr2xml-net-bridged.xml @@ -18,7 +18,6 @@ <disk type='file' device='disk'> <driver name='file'/> <source file='/root/some.img'/> - <backingStore/> <target dev='xvda' bus='xen'/> </disk> <interface type='bridge'> diff --git a/tests/sexpr2xmldata/sexpr2xml-net-e1000.xml b/tests/sexpr2xmldata/sexpr2xml-net-e1000.xml index 66afbfbfd..bd9f68048 100644 --- a/tests/sexpr2xmldata/sexpr2xml-net-e1000.xml +++ b/tests/sexpr2xmldata/sexpr2xml-net-e1000.xml @@ -18,7 +18,6 @@ <disk type='file' device='disk'> <driver name='file'/> <source file='/root/some.img'/> - <backingStore/> <target dev='xvda' bus='xen'/> </disk> <interface type='bridge'> diff --git a/tests/sexpr2xmldata/sexpr2xml-net-routed.xml b/tests/sexpr2xmldata/sexpr2xml-net-routed.xml index 9544954c4..f3cd1a7e7 100644 --- a/tests/sexpr2xmldata/sexpr2xml-net-routed.xml +++ b/tests/sexpr2xmldata/sexpr2xml-net-routed.xml @@ -18,7 +18,6 @@ <disk type='file' device='disk'> <driver name='file'/> <source file='/root/some.img'/> - <backingStore/> <target dev='xvda' bus='xen'/> </disk> <interface type='ethernet'> diff --git a/tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.xml b/tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.xml index 17018952e..8a0c28b49 100644 --- a/tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.xml +++ b/tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.xml @@ -23,12 +23,10 @@ <disk type='block' device='disk'> <driver name='phy'/> <source dev='/dev/sda8'/> - <backingStore/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='cdrom'> - <backingStore/> <target dev='hdc' bus='ide'/> <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-pci-devs.xml b/tests/sexpr2xmldata/sexpr2xml-pci-devs.xml index 11bd4c60b..ea8e5a5fa 100644 --- a/tests/sexpr2xmldata/sexpr2xml-pci-devs.xml +++ b/tests/sexpr2xmldata/sexpr2xml-pci-devs.xml @@ -18,7 +18,6 @@ <disk type='block' device='disk'> <driver name='phy'/> <source dev='/dev/MainVG/GuestVG'/> - <backingStore/> <target dev='xvda' bus='xen'/> </disk> <console type='pty'> diff --git a/tests/sexpr2xmldata/sexpr2xml-pv-bootloader-cmdline.xml b/tests/sexpr2xmldata/sexpr2xml-pv-bootloader-cmdline.xml index fafffffb0..f830742fc 100644 --- a/tests/sexpr2xmldata/sexpr2xml-pv-bootloader-cmdline.xml +++ b/tests/sexpr2xmldata/sexpr2xml-pv-bootloader-cmdline.xml @@ -18,7 +18,6 @@ <disk type='file' device='disk'> <driver name='file'/> <source file='/root/some.img'/> - <backingStore/> <target dev='xvda' bus='xen'/> </disk> <console type='pty'> diff --git a/tests/sexpr2xmldata/sexpr2xml-pv-bootloader.xml b/tests/sexpr2xmldata/sexpr2xml-pv-bootloader.xml index 96a50da09..131e04d50 100644 --- a/tests/sexpr2xmldata/sexpr2xml-pv-bootloader.xml +++ b/tests/sexpr2xmldata/sexpr2xml-pv-bootloader.xml @@ -17,7 +17,6 @@ <disk type='file' device='disk'> <driver name='file'/> <source file='/root/some.img'/> - <backingStore/> <target dev='xvda' bus='xen'/> </disk> <console type='pty'> diff --git a/tests/sexpr2xmldata/sexpr2xml-pv-localtime.xml b/tests/sexpr2xmldata/sexpr2xml-pv-localtime.xml index 6dba41613..c1d702d9a 100644 --- a/tests/sexpr2xmldata/sexpr2xml-pv-localtime.xml +++ b/tests/sexpr2xmldata/sexpr2xml-pv-localtime.xml @@ -18,7 +18,6 @@ <disk type='file' device='disk'> <driver name='file'/> <source file='/root/some.img'/> - <backingStore/> <target dev='xvda' bus='xen'/> </disk> <console type='pty'> diff --git a/tests/sexpr2xmldata/sexpr2xml-pv-vcpus.xml b/tests/sexpr2xmldata/sexpr2xml-pv-vcpus.xml index 971323493..968697802 100644 --- a/tests/sexpr2xmldata/sexpr2xml-pv-vcpus.xml +++ b/tests/sexpr2xmldata/sexpr2xml-pv-vcpus.xml @@ -18,7 +18,6 @@ <disk type='file' device='disk'> <driver name='file'/> <source file='/root/some.img'/> - <backingStore/> <target dev='xvda' bus='xen'/> </disk> <console type='pty'> diff --git a/tests/sexpr2xmldata/sexpr2xml-pv-vfb-new-vncdisplay.xml b/tests/sexpr2xmldata/sexpr2xml-pv-vfb-new-vncdisplay.xml index 72fec2559..a7553a520 100644 --- a/tests/sexpr2xmldata/sexpr2xml-pv-vfb-new-vncdisplay.xml +++ b/tests/sexpr2xmldata/sexpr2xml-pv-vfb-new-vncdisplay.xml @@ -18,7 +18,6 @@ <disk type='file' device='disk'> <driver name='file'/> <source file='/root/some.img'/> - <backingStore/> <target dev='xvda' bus='xen'/> </disk> <console type='pty'> diff --git a/tests/sexpr2xmldata/sexpr2xml-pv-vfb-new.xml b/tests/sexpr2xmldata/sexpr2xml-pv-vfb-new.xml index 0fd7a889d..6172dc336 100644 --- a/tests/sexpr2xmldata/sexpr2xml-pv-vfb-new.xml +++ b/tests/sexpr2xmldata/sexpr2xml-pv-vfb-new.xml @@ -18,7 +18,6 @@ <disk type='file' device='disk'> <driver name='file'/> <source file='/root/some.img'/> - <backingStore/> <target dev='xvda' bus='xen'/> </disk> <console type='pty'> diff --git a/tests/sexpr2xmldata/sexpr2xml-pv-vfb-type-crash.xml b/tests/sexpr2xmldata/sexpr2xml-pv-vfb-type-crash.xml index 06e7280bc..8062082c1 100644 --- a/tests/sexpr2xmldata/sexpr2xml-pv-vfb-type-crash.xml +++ b/tests/sexpr2xmldata/sexpr2xml-pv-vfb-type-crash.xml @@ -17,7 +17,6 @@ <disk type='block' device='disk'> <driver name='phy'/> <source dev='/dev/vg_dom0test/test2vm'/> - <backingStore/> <target dev='xvda' bus='xen'/> </disk> <interface type='bridge'> diff --git a/tests/sexpr2xmldata/sexpr2xml-pv.xml b/tests/sexpr2xmldata/sexpr2xml-pv.xml index 18b3dbde2..c624fe31c 100644 --- a/tests/sexpr2xmldata/sexpr2xml-pv.xml +++ b/tests/sexpr2xmldata/sexpr2xml-pv.xml @@ -18,7 +18,6 @@ <disk type='file' device='disk'> <driver name='file'/> <source file='/root/some.img'/> - <backingStore/> <target dev='xvda' bus='xen'/> </disk> <console type='pty'> diff --git a/tests/sexpr2xmldata/sexpr2xml-vif-rate.xml b/tests/sexpr2xmldata/sexpr2xml-vif-rate.xml index bfcd7c367..3b04ae23c 100644 --- a/tests/sexpr2xmldata/sexpr2xml-vif-rate.xml +++ b/tests/sexpr2xmldata/sexpr2xml-vif-rate.xml @@ -21,14 +21,12 @@ <disk type='file' device='disk'> <driver name='file'/> <source file='/root/foo.img'/> - <backingStore/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='cdrom'> <driver name='file'/> <source file='/root/boot.iso'/> - <backingStore/> <target dev='hdc' bus='ide'/> <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> -- 2.14.1

On 10/12/2017 02:07 PM, Peter Krempa wrote:
Express a properly terminated backing chain by putting a virStorageSource of type VIR_STORAGE_TYPE_NONE in the chain. The newly used helpers simplify this greatly.
The change fixes a bug as formatting an incomplete backing chain and parsing it back would end up in expressing a terminated chain since src->backingStoreRaw was not populated. By relying on the terminator object this can be now processed appropriately.
+++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live+ccw-virtio.xml @@ -22,7 +22,6 @@ <disk type='file' device='disk'> <driver name='qemu' type='raw' cache='none'/> <source file='/dev/null'/> - <backingStore/>
This gets rid of a lot of terminator markers in the test XML; the code looks like we will still generate a terminator marker when we know the full chain, and that lack of a terminator marker is not fatal, but merely means that we haven't specified a full chain. It feels like a lot of fallout, but the code with the special terminator does look nicer, so I can live with the testsuite churn that results. Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org

On Thu, Oct 12, 2017 at 15:34:28 -0500, Eric Blake wrote:
On 10/12/2017 02:07 PM, Peter Krempa wrote:
Express a properly terminated backing chain by putting a virStorageSource of type VIR_STORAGE_TYPE_NONE in the chain. The newly used helpers simplify this greatly.
The change fixes a bug as formatting an incomplete backing chain and parsing it back would end up in expressing a terminated chain since src->backingStoreRaw was not populated. By relying on the terminator object this can be now processed appropriately.
+++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live+ccw-virtio.xml @@ -22,7 +22,6 @@ <disk type='file' device='disk'> <driver name='qemu' type='raw' cache='none'/> <source file='/dev/null'/> - <backingStore/>
This gets rid of a lot of terminator markers in the test XML; the code looks like we will still generate a terminator marker when we know the full chain, and that lack of a terminator marker is not fatal, but merely means that we haven't specified a full chain. It feels like a lot of fallout, but the code with the special terminator does look nicer, so I can live with the testsuite churn that results.
Actually this was only a quirk of the tests since they parsed backing chains from XML rather than detecting them from the disk. When detecting them from disk storage we fill src->backingStoreRaw which was used to determine when to fill the terminator. Unfortunately the terminator is filled in when the variable is NULL, which is always in tests but not always in real use cases.
participants (2)
-
Eric Blake
-
Peter Krempa