Re: [libvirt] [PATCHv3 12/19] storage: cache backing chain while qemu domain is live

Technically, we should not be re-probing any file that qemu might be currently writing to. As such, we should cache the backing file chain prior to starting qemu. This patch adds the cache, but does not use it until the next patch. Ultimately, we want to also store the chain in domain XML, so that it is remembered across libvirtd restarts, and so that the only kosher way to modify the backing chain of an offline domain will be through libvirt API calls, but we aren't there yet. So for now, we merely invalidate the cache any time we do a live operation that alters the chain (block-pull, block-commit, external disk snapshot), as well as tear down the cache when the domain is not running. * src/conf/domain_conf.h (_virDomainDiskDef): New field. * src/conf/domain_conf.c (virDomainDiskDefFree): Clean new field. * src/qemu/qemu_domain.h (qemuDomainDetermineDiskChain): New prototype. * src/qemu/qemu_domain.c (qemuDomainDetermineDiskChain): New function. * src/qemu/qemu_driver.c (qemuDomainAttachDeviceDiskLive) (qemuDomainChangeDiskMediaLive): Pre-populate chain. (qemuDomainSnapshotCreateSingleDiskActive): Uncache chain before snapshot. * src/qemu/qemu_process.c (qemuProcessHandleBlockJob): Update chain after block pull. --- v3: drop useless processStop cleanup, s/chain/backingChain/, don't make force discard errors src/conf/domain_conf.c | 1 + src/conf/domain_conf.h | 2 ++ src/qemu/qemu_domain.c | 26 ++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 3 +++ src/qemu/qemu_driver.c | 14 ++++++++++++++ src/qemu/qemu_process.c | 8 ++++++++ 6 files changed, 54 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a360c25..6487e6b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -971,6 +971,7 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def) VIR_FREE(def->src); VIR_FREE(def->dst); VIR_FREE(def->driverName); + virStorageFileFreeMetadata(def->backingChain); VIR_FREE(def->mirror); VIR_FREE(def->auth.username); VIR_FREE(def->wwn); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index eec1f25..e7f0e9c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -47,6 +47,7 @@ # include "virobject.h" # include "device_conf.h" # include "bitmap.h" +# include "storage_file.h" /* forward declarations of all device types, required by * virDomainDeviceDef @@ -568,6 +569,7 @@ struct _virDomainDiskDef { } auth; char *driverName; int format; /* enum virStorageFileFormat */ + virStorageFileMetadataPtr backingChain; char *mirror; int mirrorFormat; /* enum virStorageFileFormat */ diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index b51edc2..45f3a5e 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2010,3 +2010,29 @@ qemuDomainCleanupRun(struct qemud_driver *driver, priv->ncleanupCallbacks = 0; priv->ncleanupCallbacks_max = 0; } + +int +qemuDomainDetermineDiskChain(struct qemud_driver *driver, + virDomainDiskDefPtr disk, + bool force) +{ + bool probe = driver->allowDiskFormatProbing; + + if (!disk->src) + return 0; + + if (disk->backingChain) { + if (force) { + virStorageFileFreeMetadata(disk->backingChain); + disk->backingChain = NULL; + } else { + return 0; + } + } + disk->backingChain = virStorageFileGetMetadata(disk->src, disk->format, + driver->user, driver->group, + probe); + if (!disk->backingChain) + return -1; + return 0; +} diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 26b6c55..8a66f14 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -343,6 +343,9 @@ bool qemuDomainJobAllowed(qemuDomainObjPrivatePtr priv, int qemuDomainCheckDiskPresence(struct qemud_driver *driver, virDomainObjPtr vm, bool start_with_state); +int qemuDomainDetermineDiskChain(struct qemud_driver *driver, + virDomainDiskDefPtr disk, + bool force); int qemuDomainCleanupAdd(virDomainObjPtr vm, qemuDomainCleanupCallback cb); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7f240a0..dfbf04d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5816,6 +5816,9 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn, goto end; } + if (qemuDomainDetermineDiskChain(driver, disk, false) < 0) + goto end; + if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0)) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -6044,6 +6047,9 @@ qemuDomainChangeDiskMediaLive(virDomainObjPtr vm, virCgroupPtr cgroup = NULL; int ret = -1; + if (qemuDomainDetermineDiskChain(driver, disk, false) < 0) + goto end; + if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) != 0) { @@ -10788,6 +10794,14 @@ qemuDomainSnapshotCreateSingleDiskActive(struct qemud_driver *driver, disk->src = source; origdriver = disk->format; disk->format = VIR_STORAGE_FILE_RAW; /* Don't want to probe backing files */ + /* XXX Here, we know we are about to alter disk->backingChain if + * successful, so we nuke the existing chain so that future + * commands will recompute it. Better would be storing the chain + * ourselves rather than reprobing, but this requires modifying + * domain_conf and our XML to fully track the chain across + * libvirtd restarts. */ + virStorageFileFreeMetadata(disk->backingChain); + disk->backingChain = NULL; if (virDomainLockDiskAttach(driver->lockManager, driver->uri, vm, disk) < 0) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index e08ec67..0e98c8b 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -909,6 +909,14 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED, if (disk) { path = disk->src; event = virDomainEventBlockJobNewFromObj(vm, path, type, status); + /* XXX If we completed a block pull, then recompute the cached + * backing chain to match. Better would be storing the chain + * ourselves rather than reprobing, but this requires + * modifying domain_conf and our XML to fully track the chain + * across libvirtd restarts. */ + if (type == VIR_DOMAIN_BLOCK_JOB_TYPE_PULL && + status == VIR_DOMAIN_BLOCK_JOB_COMPLETED) + qemuDomainDetermineDiskChain(driver, disk, true); } virDomainObjUnlock(vm);