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

Re: [libvirt] [PATCHv4 08/18] blockjob: react to active block copy



On Mon, Apr 09, 2012 at 21:52:17 -0600, Eric Blake wrote:
> For now, disk migration via block copy job is not implemented.  But
> when we do implement it, we have to deal with the fact that qemu does
> not provide an easy way to re-start a qemu process with mirroring
> still intact (it _might_ be possible by using qemu -S then an
> initial 'drive-mirror' with disk reuse before starting the domain,
> but that gets hairy).  Even something like 'virDomainSave' becomes
> hairy, if you realize the implications that 'virDomainRestore' would
> be stuck with recreating the same mirror layout.
> 
> But if we step back and look at the bigger picture, we realize that
> the initial client of live storage migration via disk mirroring is
> oVirt, which always uses transient domains, and that if a transient
> domain is destroyed while a mirror exists, oVirt can easily restart
> the storage migration by creating a new domain that visits just the
> source storage, with no loss in data.
> 
> We can make life a lot easier by being cowards, and forbidding
> certain operations on a domain.  This patch guarantees that we
> never get in a state where we would have to restart a domain with
> a mirroring block copy, by preventing saves, snapshots, hot
> unplug of a disk in use, and conversion to a persistent domain.
> 
> * src/conf/domain_conf.h (virDomainHasDiskMirror): New prototype.
> * src/conf/domain_conf.c (virDomainHasDiskMirror): New function.
> * src/libvirt_private.syms (domain_conf.h): Export it.
> * src/qemu/qemu_driver.c (qemuDomainSaveInternal)
> (qemuDomainSnapshotCreateXML, qemuDomainRevertToSnapshot)
> (qemuDomainBlockJobImpl, qemudDomainDefine): Prevent dangerous
> actions while block copy is already in action.
> * src/qemu/qemu_hotplug.c (qemuDomainDetachDiskDevice): Likewise.
> ---
>  src/conf/domain_conf.c   |   12 ++++++++++++
>  src/conf/domain_conf.h   |    1 +
>  src/libvirt_private.syms |    1 +
>  src/qemu/qemu_driver.c   |   31 ++++++++++++++++++++++++++++++-
>  src/qemu/qemu_hotplug.c  |    7 +++++++
>  5 files changed, 51 insertions(+), 1 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 8899653..3aa6861 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -7183,6 +7183,18 @@ virDomainDiskRemoveByName(virDomainDefPtr def, const char *name)
>      return virDomainDiskRemove(def, i);
>  }
> 
> +/* Return true if VM has at least one disk involved in a current block
> + * copy job (that is, with a <mirror> element in the disk xml).  */
> +bool
> +virDomainHasDiskMirror(virDomainObjPtr vm)
> +{
> +    int i;
> +    for (i = 0; i < vm->def->ndisks; i++)
> +        if (vm->def->disks[i]->mirror)
> +            return true;
> +    return false;
> +}
> +
>  int virDomainNetInsert(virDomainDefPtr def, virDomainNetDefPtr net)
>  {
>      if (VIR_REALLOC_N(def->nets, def->nnets + 1) < 0)
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index abc953d..77c501c 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1954,6 +1954,7 @@ virDomainDiskDefPtr
>  virDomainDiskRemove(virDomainDefPtr def, size_t i);
>  virDomainDiskDefPtr
>  virDomainDiskRemoveByName(virDomainDefPtr def, const char *name);
> +bool virDomainHasDiskMirror(virDomainObjPtr vm);
> 
>  int virDomainNetIndexByMac(virDomainDefPtr def, const unsigned char *mac);
>  int virDomainNetInsert(virDomainDefPtr def, virDomainNetDefPtr net);
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index a90f8a0..570940d 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -354,6 +354,7 @@ virDomainGraphicsSpiceZlibCompressionTypeFromString;
>  virDomainGraphicsSpiceZlibCompressionTypeToString;
>  virDomainGraphicsTypeFromString;
>  virDomainGraphicsTypeToString;
> +virDomainHasDiskMirror;
>  virDomainHostdevDefAlloc;
>  virDomainHostdevDefClear;
>  virDomainHostdevDefFree;
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 425d340..b0937eb 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -2558,6 +2558,11 @@ qemuDomainSaveInternal(struct qemud_driver *driver, virDomainPtr dom,
>                          "%s", _("domain is marked for auto destroy"));
>          goto cleanup;
>      }
> +    if (virDomainHasDiskMirror(vm)) {
> +        qemuReportError(VIR_ERR_BLOCK_COPY_ACTIVE, "%s",
> +                        _("domain has active block copy job"));
> +        goto cleanup;
> +    }
> 
>      memset(&header, 0, sizeof(header));
>      memcpy(header.magic, QEMUD_SAVE_PARTIAL, sizeof(header.magic));
> @@ -4947,6 +4952,12 @@ static virDomainPtr qemudDomainDefine(virConnectPtr conn, const char *xml) {
>          goto cleanup;
>      }
>      def = NULL;
> +    if (virDomainHasDiskMirror(vm)) {
> +        qemuReportError(VIR_ERR_BLOCK_COPY_ACTIVE, "%s",
> +                        _("domain has active block copy job"));
> +        virDomainObjAssignDef(vm, NULL, false);
> +        goto cleanup;
> +    }
>      vm->persistent = 1;

I think it would be better to do this check a bit earlier in the process to
avoid the need to undo virDomainObjAssignDef().

>      if (virDomainSaveConfig(driver->configDir,
> @@ -10262,6 +10273,12 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
>                          "%s", _("domain is marked for auto destroy"));
>          goto cleanup;
>      }
> +    if (virDomainHasDiskMirror(vm)) {
> +        qemuReportError(VIR_ERR_BLOCK_COPY_ACTIVE, "%s",
> +                        _("domain has active block copy job"));
> +        goto cleanup;
> +    }
> +
>      if (!vm->persistent && (flags & VIR_DOMAIN_SNAPSHOT_CREATE_HALT)) {
>          qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
>                          _("cannot halt after transient domain snapshot"));
> @@ -10869,6 +10886,11 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
>                          _("no domain with matching uuid '%s'"), uuidstr);
>          goto cleanup;
>      }
> +    if (virDomainHasDiskMirror(vm)) {
> +        qemuReportError(VIR_ERR_BLOCK_COPY_ACTIVE, "%s",
> +                        _("domain has active block copy job"));
> +        goto cleanup;
> +    }
> 
>      snap = virDomainSnapshotFindByName(&vm->snapshots, snapshot->name);
>      if (!snap) {
> @@ -11607,6 +11629,7 @@ qemuDomainBlockJobImpl(virDomainPtr dom, const char *path, const char *base,
>      char uuidstr[VIR_UUID_STRING_BUFLEN];
>      char *device = NULL;
>      int ret = -1;
> +    int idx;
> 
>      qemuDriverLock(driver);
>      virUUIDFormat(dom->uuid, uuidstr);
> @@ -11617,7 +11640,7 @@ qemuDomainBlockJobImpl(virDomainPtr dom, const char *path, const char *base,
>          goto cleanup;
>      }
> 
> -    device = qemuDiskPathToAlias(vm, path, NULL);
> +    device = qemuDiskPathToAlias(vm, path, &idx);
>      if (!device) {
>          goto cleanup;
>      }
> @@ -11633,6 +11656,12 @@ qemuDomainBlockJobImpl(virDomainPtr dom, const char *path, const char *base,
>                            "this QEMU binary"));
>          goto cleanup;
>      }
> +    if (mode == BLOCK_JOB_PULL && vm->def->disks[idx]->mirror) {
> +        qemuReportError(VIR_ERR_BLOCK_COPY_ACTIVE,
> +                        _("disk '%s' already in active block copy job"),
> +                        vm->def->disks[idx]->dst);
> +        goto cleanup;
> +    }
> 
>      if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0)
>          goto cleanup;
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 857b980..98fa8f8 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1721,6 +1721,13 @@ int qemuDomainDetachDiskDevice(struct qemud_driver *driver,
> 
>      detach = vm->def->disks[i];
> 
> +    if (detach->mirror) {
> +        qemuReportError(VIR_ERR_BLOCK_COPY_ACTIVE,
> +                        _("disk '%s' is in an active block copy job"),
> +                        detach->dst);
> +        goto cleanup;
> +    }
> +
>      if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) {
>          if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) != 0) {
>              qemuReportError(VIR_ERR_INTERNAL_ERROR,

OK

Jirka


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