5:30 p.m.
Now that we can crawl the chain of backing files, we can do
argument validation and implement the 'shallow' flag. In
testing this, I discovered that it can be handy to pass the
shallow flag and an explicit base, as a means of validating
that the base is indeed the file we expected.
* src/qemu/qemu_driver.c (qemuDomainBlockCommit): Crawl through
chain to implement shallow flag.
* src/libvirt.c (virDomainBlockCommit): Relax API.
---
v3: rebase to changes earlier in series
src/libvirt.c | 2 --
src/qemu/qemu_driver.c | 63 ++++++++++++++++++++++++++++++++++++++++----------
2 files changed, 51 insertions(+), 14 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c
index e3ddf27..6d65965 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -19381,8 +19381,6 @@ int virDomainBlockCommit(virDomainPtr dom, const char *disk,
}
virCheckNonNullArgGoto(disk, error);
- if (flags & VIR_DOMAIN_BLOCK_COMMIT_SHALLOW)
- virCheckNullArgGoto(base, error);
if (conn->driver->domainBlockCommit) {
int ret;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c63d16b..73804fe 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12664,8 +12664,12 @@ qemuDomainBlockCommit(virDomainPtr dom, const char *path, const
char *base,
int ret = -1;
int idx;
virDomainDiskDefPtr disk;
+ const char *top_canon = NULL;
+ virStorageFileMetadataPtr top_meta = NULL;
+ const char *top_parent = NULL;
+ const char *base_canon = NULL;
- virCheckFlags(0, -1);
+ virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW, -1);
if (!(vm = qemuDomObjFromDomain(dom)))
goto cleanup;
@@ -12696,20 +12700,55 @@ qemuDomainBlockCommit(virDomainPtr dom, const char *path, const
char *base,
disk->dst);
goto endjob;
}
+ if (qemuDomainDetermineDiskChain(driver, disk, false) < 0)
+ goto endjob;
- if (!top)
- top = disk->src;
+ if (!top) {
+ top_canon = disk->src;
+ top_meta = disk->backingChain;
+ } else if (!(top_canon = virStorageFileChainLookup(disk->backingChain,
+ disk->src,
+ top, &top_meta,
+ &top_parent))) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("could not find top '%s' in chain for
'%s'"),
+ top, path);
+ goto endjob;
+ }
+ if (!top_meta || !top_meta->backingStore) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("top '%s' in chain for '%s' has no backing
file"),
+ top, path);
+ goto endjob;
+ }
+ if (!base && (flags & VIR_DOMAIN_BLOCK_COMMIT_SHALLOW)) {
+ base_canon = top_meta->backingStore;
+ } else if (!(base_canon = virStorageFileChainLookup(top_meta, top_canon,
+ base, NULL, NULL))) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("could not find base '%s' below '%s' in
chain "
+ "for '%s'"),
+ base ? base : "(default)", top_canon, path);
+ goto endjob;
+ }
+ /* Note that this code exploits the fact that
+ * virStorageFileChainLookup guarantees a simple pointer
+ * comparison will work, rather than needing full-blown STREQ. */
+ if ((flags & VIR_DOMAIN_BLOCK_COMMIT_SHALLOW) &&
+ base_canon != top_meta->backingStore) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("base '%s' is not immediately below '%s' in
chain "
+ "for '%s'"),
+ base, top_canon, path);
+ goto endjob;
+ }
- /* XXX For now, we are relying on qemu to check that 'top' and
- * 'base' resolve to members of the backing chain in correct
- * order; but if we ever get more paranoid and track the backing
- * chain ourself, we should be pre-validating the data rather than
- * relying on qemu. For that matter, we need to be changing the
- * SELinux label on both 'base' and the parent of 'top', so that
- * qemu can open(O_RDWR) those files for the duration of the
- * commit. */
+ /* XXX We need to be changing the SELinux label on both 'base' and
+ * the parent of 'top', so that qemu can open(O_RDWR) those files
+ * for the duration of the commit. */
qemuDomainObjEnterMonitor(driver, vm);
- ret = qemuMonitorBlockCommit(priv->mon, device, top, base, bandwidth);
+ ret = qemuMonitorBlockCommit(priv->mon, device, top_canon, base_canon,
+ bandwidth);
qemuDomainObjExitMonitor(driver, vm);
endjob:
--
1.7.11.7