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.

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index d239cc8a1b3e9cd61df7ab653afe7ce7b1cfb564..f849d05319d4bda4dc8b0af12d4cf0953652982e 100644 (file)
--- 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 ace411dfcff8c2e62822e4f9a5f15843840a9afe..ec704220807b619abc93f56ddadcd6adac0f0566 100644 (file)
--- 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);