[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On 10/17/2012 06:30 PM, Eric Blake wrote:
> 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);

ACK.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]