fix(instances): the Migrate tab keeps disks of one file name apart

Two disks of a VM with the same file name in different directories
were both offered the same destination pool by default. The form was
sent, and the migration job refused it only then: both copies would
be one file.

The plan now gives each such disk, by default, a pool no earlier disk
of that name is copied into, and says under its choice that another
disk has the file name. A choice that still puts them in one pool is
refused by the job, as before.

instances/templates/instances/migrate_plan.html

@@ -23,6 +23,7 @@
         {% endfor %}
         {% endif %}
     </select>
+    {% if disk.same_name %}<div class="form-text">{% trans "Another disk of the VM has this file name: copy them to different pools." %}</div>{% endif %}
     {% elif disk.refusal %}
     <p class="small text-danger mb-0" data-refused>{% blocktrans with reason=disk.refusal %}Cannot be copied: {{ reason }}{% endblocktrans %}</p>
     {% elif pools %}

instances/tests/test_migrate_job.py

@@ -104,7 +104,7 @@ class MigrateJobTestCase(TransactionTestCase):
                 {"target": "vda", "path": "/i/mig-vm.qcow2", "capacity": 1 << 30, "shared": False, "refusal": None,
                  "free_pools": ["default", "fast"], "default": "default"},
                 {"target": "vdb", "path": "/i/data.qcow2", "capacity": 1 << 30, "shared": False, "refusal": None,
-                 "free_pools": ["default"], "default": "default"},
+                 "free_pools": ["default"], "default": "default", "same_name": True},
                 {"target": "sda", "path": "/i/os.iso", "capacity": 1 << 20, "shared": False,
                  "refusal": "an image is not copied; eject it first", "free_pools": [], "default": None}],
                 # the next start's disks the migration does not carry
@@ -120,6 +120,7 @@ class MigrateJobTestCase(TransactionTestCase):
         self.assertIn('<option value="fast">Copy to fast (8.0\xa0GB free)</option>', page)
         self.assertIn('<option value="fast" disabled>fast has a file of this name</option>', page)
         self.assertIn("Cannot be copied: an image is not copied; eject it first", page)
+        self.assertEqual(page.count("Another disk of the VM has this file name: copy them to different pools."), 1)
         self.assertIn("A shut-off VM is started paused for a copy", page)
         self.assertIn("/nfs/new.qcow2</span>:\n            on storage both hosts share", page)
         # a disk that cannot go keeps the form from being sent, as does a destination that does not answer

vrtManager/instance.py

@@ -285,20 +285,26 @@ class wvmInstances(wvmConnect):
         live_xml), each with the pools here it can be copied into, as
         disk_copies takes them: {free_pools, default} added, and the file
         disks of next_xml (the next start's definition) that the migration
-        does not carry, which must be on shared storage."""
+        does not carry, which must be on shared storage. Disks of one file
+        name ({same_name} added) are copied into different pools by default."""
         here = self.file_pools()
         for pool, directory in here.values():
             pool.refresh(0)
         disks = self.disk_plan(conn, live_xml)
-        for disk in disks:
-            file = os.path.basename(disk["path"])
+        files = [os.path.basename(disk["path"]) for disk in disks]
+        taken = {}  # file name: the pools the disks before of that name are copied into by default
+        for disk, file in zip(disks, files):
             # a copy never replaces a file: a pool that has one of its name cannot take it
             disk["free_pools"] = [name for name, (pool, directory) in sorted(here.items())
                                   if self._find_volume(pool, file) is None]
-            # a copy goes by default to the pool on the same directory, else the first
-            names = [name for name in disk["free_pools"] if here[name][1] == os.path.dirname(disk["path"])]
-            names = names or disk["free_pools"]
+            # a copy goes by default to the pool on the same directory, else the first,
+            # but not where another disk of its name goes
+            free = [name for name in disk["free_pools"] if name not in taken.get(file, [])]
+            names = [name for name in free if here[name][1] == os.path.dirname(disk["path"])] or free
             disk["default"] = names[0] if names else None
+            disk["same_name"] = files.count(file) > 1
+            if not disk["shared"]:
+                taken.setdefault(file, []).append(disk["default"])
         carried = {disk["path"] for disk in disks}
         pending = [row for row in self.disk_plan(conn, next_xml) if row["path"] not in carried]
         return disks, pending

vrtManager/tests/test_migration.py

@@ -512,6 +512,23 @@ class MigrationPlanTestCase(unittest.TestCase):
         dest._find_volume = lambda pool, name: object() if pool is pools["default"][0] else None
         disks, pending = dest.migration_plan(None, "live", "next")
         self.assertEqual((disks[0]["free_pools"], disks[0]["default"]), (["fast", "slow"], "fast"))
+        self.assertFalse(disks[0]["same_name"])
         self.assertEqual(pending, [vdb])
         for pool, directory in pools.values():
             pool.refresh.assert_called_once_with(0)  # a file made since is seen
+
+    def test_disks_of_one_file_name_go_to_different_pools(self):
+        dest = wvmInstances.__new__(wvmInstances)
+        pools = {"fast": (mock.MagicMock(), "/srv/fast"), "slow": (mock.MagicMock(), "/srv/slow")}
+        dest.file_pools = lambda: pools
+        dest._find_volume = lambda pool, name: None
+        rows = [{"target": "vda", "path": "/x/data.qcow2", "shared": False, "refusal": None},
+                {"target": "vdb", "path": "/nfs/data.qcow2", "shared": True, "refusal": None},
+                {"target": "vdc", "path": "/y/data.qcow2", "shared": False, "refusal": None},
+                {"target": "vdd", "path": "/z/data.qcow2", "shared": False, "refusal": None},
+                {"target": "vde", "path": "/x/os.qcow2", "shared": False, "refusal": None}]
+        dest.disk_plan = lambda conn, xml: [dict(row) for row in rows]
+        disks, pending = dest.migration_plan(None, "live", "next")
+        # the shared disk stays where it is by default: it takes no pool; the fourth finds none left
+        self.assertEqual([disk["default"] for disk in disks if not disk["shared"]], ["fast", "slow", None, "fast"])
+        self.assertEqual([disk["same_name"] for disk in disks], [True, True, True, True, False])