diff --git a/instances/templates/instances/migrate_plan.html b/instances/templates/instances/migrate_plan.html index bcf39c4d..454b9ce9 100644 --- a/instances/templates/instances/migrate_plan.html +++ b/instances/templates/instances/migrate_plan.html @@ -23,6 +23,7 @@ {% endfor %} {% endif %} + {% if disk.same_name %}
{% trans "Another disk of the VM has this file name: copy them to different pools." %}
{% endif %} {% elif disk.refusal %}

{% blocktrans with reason=disk.refusal %}Cannot be copied: {{ reason }}{% endblocktrans %}

{% elif pools %} diff --git a/instances/tests/test_migrate_job.py b/instances/tests/test_migrate_job.py index f538c6f4..cca07075 100644 --- a/instances/tests/test_migrate_job.py +++ b/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('', page) self.assertIn('', 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:\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 diff --git a/vrtManager/instance.py b/vrtManager/instance.py index 5f24c71f..0b43e673 100644 --- a/vrtManager/instance.py +++ b/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 diff --git a/vrtManager/tests/test_migration.py b/vrtManager/tests/test_migration.py index 78cbbd7c..13f2999a 100644 --- a/vrtManager/tests/test_migration.py +++ b/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])