From e2fb96d92b4b986a2b5732416f7bfd302a848970 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Fri, 12 Aug 2011 13:23:09 -0600 Subject: [PATCH] snapshot: prevent migration from stranding snapshot data Migration is another case of stranding metadata. And since snapshot metadata is arbitrarily large, there's no way to shoehorn it into the migration cookie of migration v3. This patch consolidates two existing locations for migration validation into one helper function, then enhances that function to also do the new checks. If we could always trust the source to validate migration, then the destination would not have to do anything; but since older servers that did not do checking can migrate to newer destinations, we have to repeat some of the same checks on the destination; meanwhile, we want to detect failures as soon as possible. With migration v2, this means that validation will reject things at Prepare on the destination if the XML exposes the problem, otherwise at Perform on the source; with migration v3, this means that validation will reject things at Begin on the source, or if the source is old and the XML exposes the problem, then at Prepare on the destination. This patch is necessarily over-strict. Once a later patch properly handles auto-cleanup of snapshot metadata on the death of a transient domain, then the only time we actually need snapshots to prevent migration is when using the --undefinesource flag on a persistent source domain. It is possible to recreate snapshot metadata on the destination with VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE and VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT. But for now, that is limited, since if we delete the snapshot metadata prior to migration, then we won't know the name of the current snapshot to pass along; and if we delete the snapshot metadata after migration and use the v3 migration cookie to pass along the name of the current snapshot, then we need a way to bypass the fact that this patch refuses migration with snapshot metadata present. So eventually, we may have to introduce migration protocol v4 that allows feature negotiation and an arbitrary number of handshake exchanges, so as to pass as many rpc calls as needed to transfer all the snapshot xml hierarchy. But all of that is thoughts for the future; for now, the best course of action is to quit early, rather than get into a funky state of stale metadata; then relax restrictions later. * src/qemu/qemu_migration.h (qemuMigrationIsAllowed): Make static. * src/qemu/qemu_migration.c (qemuMigrationIsAllowed): Alter signature, and allow checks for both outgoing and incoming. (qemuMigrationBegin, qemuMigrationPrepareAny) (qemuMigrationPerformJob): Update callers. --- src/qemu/qemu_migration.c | 48 +++++++++++++++++++++++++++------------ src/qemu/qemu_migration.h | 2 -- 2 files changed, 33 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index d239cc8a1b..f849d05319 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -701,9 +701,36 @@ error: return NULL; } -bool -qemuMigrationIsAllowed(virDomainDefPtr def) +/* Validate whether the domain is safe to migrate. If vm is NULL, + * then this is being run in the v2 Prepare stage on the destination + * (where we only have the target xml); if vm is provided, then this + * is being run in either v2 Perform or v3 Begin (where we also have + * access to all of the domain's metadata, such as whether it is + * marked autodestroy or has snapshots). While it would be nice to + * assume that checking on source is sufficient to prevent ever + * talking to the destination in the first place, we are stuck with + * the fact that older servers did not do checks on the source. */ +static bool +qemuMigrationIsAllowed(struct qemud_driver *driver, virDomainObjPtr vm, + virDomainDefPtr def) { + int nsnapshots; + + if (vm) { + if (qemuProcessAutoDestroyActive(driver, vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is marked for auto destroy")); + return false; + } + if ((nsnapshots = virDomainSnapshotObjListNum(&vm->snapshots, 0))) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + _("cannot migrate domain with %d snapshots"), + nsnapshots); + return false; + } + + def = vm->def; + } if (def->nhostdevs > 0) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain with assigned host devices cannot be migrated")); @@ -915,13 +942,7 @@ char *qemuMigrationBegin(struct qemud_driver *driver, if (priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_OUT) qemuMigrationJobSetPhase(driver, vm, QEMU_MIGRATION_PHASE_BEGIN3); - if (qemuProcessAutoDestroyActive(driver, vm)) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain is marked for auto destroy")); - goto cleanup; - } - - if (!qemuMigrationIsAllowed(vm->def)) + if (!qemuMigrationIsAllowed(driver, vm, NULL)) goto cleanup; if (!(mig = qemuMigrationEatCookie(driver, vm, NULL, 0, 0))) @@ -990,7 +1011,7 @@ qemuMigrationPrepareAny(struct qemud_driver *driver, VIR_DOMAIN_XML_INACTIVE))) goto cleanup; - if (!qemuMigrationIsAllowed(def)) + if (!qemuMigrationIsAllowed(driver, NULL, def)) goto cleanup; /* Target domain name, maybe renamed. */ @@ -2194,11 +2215,8 @@ qemuMigrationPerformJob(struct qemud_driver *driver, goto endjob; } - if (qemuProcessAutoDestroyActive(driver, vm)) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain is marked for auto destroy")); - goto endjob; - } + if (!qemuMigrationIsAllowed(driver, vm, NULL)) + goto cleanup; resume = virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING; diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index ace411dfcf..ec70422080 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -73,8 +73,6 @@ bool qemuMigrationJobIsActive(virDomainObjPtr vm, int qemuMigrationJobFinish(struct qemud_driver *driver, virDomainObjPtr obj) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; -bool qemuMigrationIsAllowed(virDomainDefPtr def) - ATTRIBUTE_NONNULL(1); int qemuMigrationSetOffline(struct qemud_driver *driver, virDomainObjPtr vm); -- GitLab