fix(instances): a migrated VM's source definition is always removed

Migration left the VM defined on the source host unless "Delete
original" was ticked (VIR_MIGRATE_UNDEFINE_SOURCE). With the shared
storage migration needs, that definition points at the disks the
destination VM is running on: the VM showed on both hosts and could be
started (or autostarted) a second time against the same disks; only a
configured lock manager stops that.

moveto now always passes VIR_MIGRATE_UNDEFINE_SOURCE with
VIR_MIGRATE_PERSIST_DEST. The option is gone from the form, the view,
the API (MigrateSerializer.xml_del) and the clone auto-migration. It
removes the definition, not the disk files.

instances/api/serializers.py

@@ -91,7 +91,6 @@ class CreateInstanceSerializer(serializers.ModelSerializer):
 class MigrateSerializer(serializers.ModelSerializer):
     instance = Instance.objects.all().prefetch_related("userinstance_set")
     live = serializers.BooleanField(initial=True)
-    xml_del = serializers.BooleanField(initial=True)
 
     class Meta:
         model = MigrateInstance
@@ -99,7 +98,6 @@ class MigrateSerializer(serializers.ModelSerializer):
             "instance",
             "target_compute",
             "live",
-            "xml_del",
             "offline",
             "autoconverge",
             "compress",

instances/api/viewsets.py

@@ -193,7 +193,6 @@ class MigrateViewSet(viewsets.ViewSet):
             target_host = serializer.validated_data["target_compute"]
             live = serializer.validated_data["live"]
             unsafe = serializer.validated_data["unsafe"]
-            xml_del = serializer.validated_data["xml_del"]
             offline = serializer.validated_data["offline"]
             autoconverge = serializer.validated_data["autoconverge"]
             postcopy = serializer.validated_data["postcopy"]
@@ -205,7 +204,6 @@ class MigrateViewSet(viewsets.ViewSet):
                 request.user,
                 live,
                 unsafe,
-                xml_del,
                 offline,
                 autoconverge,
                 compress,

instances/models.py

@@ -249,7 +249,6 @@ class MigrateInstance(models.Model):
     )
 
     live = models.BooleanField(_("Live"))
-    xml_del = models.BooleanField(_("Undefine XML"), default=True)
     offline = models.BooleanField(_("Offline"))
     autoconverge = models.BooleanField(_("Auto Converge"), default=True)
     compress = models.BooleanField(_("Compress"), default=False)

instances/templates/instances/settings_tab.html

@@ -615,14 +615,6 @@
                         </div>
                     </div>
                 </div>
-                <div class="row">
-                    <div class="col-sm-6 offset-3">
-                        <div class="form-check form-switch">
-                            <input class="form-check-input" type="checkbox" name="xml_delete" value="true" id="xml_delete" checked>
-                            <label class="form-check-label" for="xml_delete">{% trans "Delete original" %}</label>
-                        </div>
-                    </div>
-                </div>
                 <div class="row">
                     <div class="col-sm-6 offset-3">
                         <div class="form-check form-switch">

instances/utils.py

@@ -156,7 +156,6 @@ def migrate_instance(
     user,
     live=False,
     unsafe=False,
-    xml_del=False,
     offline=False,
     autoconverge=False,
     compress=False,
@@ -188,7 +187,6 @@ def migrate_instance(
                     instance.name,
                     live,
                     unsafe,
-                    xml_del,
                     offline,
                     autoconverge,
                     compress,

instances/views.py

@@ -657,7 +657,6 @@ def migrate(request, pk):
     compute_id = request.POST.get("compute_id", "")
     live = request.POST.get("live_migrate", False)
     unsafe = request.POST.get("unsafe_migrate", False)
-    xml_del = request.POST.get("xml_delete", False)
     offline = request.POST.get("offline_migrate", False)
     autoconverge = request.POST.get("autoconverge", False)
     compress = request.POST.get("compress", False)
@@ -675,7 +674,6 @@ def migrate(request, pk):
             request.user,
             live,
             unsafe,
-            xml_del,
             offline,
             autoconverge,
             compress,
@@ -1878,7 +1876,7 @@ def clone(request, pk):
                 new_compute = Compute.objects.order_by("?").first()
                 if new_compute != instance.compute:
                     utils.migrate_instance(
-                        new_compute, new_instance, request.user, xml_del=True, offline=True
+                        new_compute, new_instance, request.user, offline=True
                     )
 
             return redirect(reverse("instances:instance", args=[new_instance.id]))

vrtManager/instance.py

@@ -113,13 +113,14 @@ class wvmInstances(wvmConnect):
         name,
         live,
         unsafe,
-        undefine,
         offline,
         autoconverge=False,
         compress=False,
         postcopy=False,
     ):
-        flags = VIR_MIGRATE_PERSIST_DEST
+        # The source definition always goes: one left behind points at the
+        # same (shared) disks as the migrated VM and could be started again.
+        flags = VIR_MIGRATE_PERSIST_DEST | VIR_MIGRATE_UNDEFINE_SOURCE
         if live and conn.get_status() != 5:
             flags |= VIR_MIGRATE_LIVE
         if unsafe and conn.get_status() == 1:
@@ -132,8 +133,6 @@ class wvmInstances(wvmConnect):
             flags |= VIR_MIGRATE_COMPRESSED
         if not offline and postcopy and conn.get_status() == 1:
             flags |= VIR_MIGRATE_POSTCOPY
-        if undefine:
-            flags |= VIR_MIGRATE_UNDEFINE_SOURCE
 
         dom = conn.instance
 

vrtManager/test_migration.py

@@ -0,0 +1,52 @@
+"""Migration flags: the source definition never stays behind."""
+
+import unittest
+
+import libvirt
+from django.conf import settings
+
+if not settings.configured:
+    settings.configure(MAC_OUI="52:54:10")
+
+from vrtManager.instance import wvmInstances
+
+
+class FakeDomain:
+    def __init__(self):
+        self.calls = []
+
+    def migrate(self, dconn, flags, dname, uri, bandwidth):
+        self.calls.append((flags, uri))
+
+
+class FakeSource:
+    def __init__(self, state):
+        self.state = state
+        self.instance = FakeDomain()
+
+    def get_status(self):
+        return self.state
+
+    def get_arch(self):
+        return "x86_64"
+
+    def get_dom_emulator(self):
+        return "/usr/bin/qemu-system-x86_64"
+
+
+def destination():
+    dest = wvmInstances.__new__(wvmInstances)
+    dest.wvm = object()
+    dest.get_emulator = lambda arch: "/usr/bin/qemu-system-x86_64"
+    return dest
+
+
+class MigrationFlagsTestCase(unittest.TestCase):
+    def test_the_source_definition_is_always_removed(self):
+        for state, live, offline in ((1, True, False), (5, False, True)):
+            with self.subTest(state=state):
+                source = FakeSource(state)
+                destination().moveto(source, "vm", live=live, unsafe=False, offline=offline)
+                flags = source.instance.calls[0][0]
+                self.assertTrue(flags & libvirt.VIR_MIGRATE_UNDEFINE_SOURCE)
+                self.assertTrue(flags & libvirt.VIR_MIGRATE_PERSIST_DEST)