diff --git a/coriolis/osmorphing/osmount/base.py b/coriolis/osmorphing/osmount/base.py index 1bab4144..bebf46a1 100644 --- a/coriolis/osmorphing/osmount/base.py +++ b/coriolis/osmorphing/osmount/base.py @@ -133,33 +133,48 @@ def _get_pvs(self): return pvs def _get_vgs(self): + """ Returns a dict of the form: { + "VG UUID": { + "name": "", + "pvs": ["list", "of", "PV", "names", "in", "VG"], + } + } + """ vgs_cmd = ( "sudo vgs -o vg_name,pv_name,vg_uuid, --noheadings --separator :") out = self._exec_cmd(vgs_cmd).decode().splitlines() LOG.debug("Output of %s command: %s", vgs_cmd, out) - vgs = {} + vgs_uuid_map = {} for line in out: if line == "": continue line = line.strip().split(":") if len(line) >= 3: vg_name, pv_name, vg_uuid = line[0], line[1], line[2] - if vgs.get(vg_name) is None: - vgs[vg_name] = pv_name + + if vg_uuid in vgs_uuid_map: + vgs_uuid_map[vg_uuid]["pvs"].append(pv_name) else: - new_name = str(uuid.uuid4()) - LOG.debug( - "VG with name '%s' already detected. Renaming VG with " - "UUID '%s' to '%s' to avoid conflicts", - vg_name, vg_uuid, new_name) - self._exec_cmd("sudo vgrename %s %s" % - (vg_uuid, new_name)) - vgs[new_name] = pv_name + new_name = vg_name + # Ensure that there isn't an existing VG with a + # coinciding name, and rename it if so. + if vg_name in [vg['name'] for vg in vgs_uuid_map.values()]: + new_name = str(uuid.uuid4()) + LOG.debug( + "VG with name '%s' already detected. Renaming VG " + "with UUID '%s' to '%s' to avoid conflicts", + vg_name, vg_uuid, new_name) + self._exec_cmd("sudo vgrename %s %s" % + (vg_uuid, new_name)) + vgs_uuid_map[vg_uuid] = { + "name": new_name, + "pvs": [pv_name] + } else: LOG.warning("Ignoring improper `vgs` output entry: %s", line) - LOG.debug("Volume groups: %s", vgs) + LOG.debug("Volume groups: %s", vgs_uuid_map) - return vgs + return vgs_uuid_map def _check_vgs(self): try: @@ -532,13 +547,15 @@ def mount_os(self): lvm_dev_paths = [] self._check_vgs() vgs = self._get_vgs() - for vg_name, pv_name in vgs.items(): - if pv_name in dev_paths: - self._exec_cmd("sudo vgchange -ay %s" % vg_name) - self._exec_cmd("sudo vgchange --refresh") - dev_paths_for_group = self._exec_cmd( - "sudo ls -1 /dev/%s/*" % vg_name).decode().splitlines() - lvm_dev_paths.extend(dev_paths_for_group) + for vg_uuid, vg_props in vgs.items(): + if not any([pv in dev_paths for pv in vg_props['pvs']]): + continue + + self._exec_cmd("sudo vgchange -ay -S vg_uuid=%s" % vg_uuid) + self._exec_cmd("sudo vgchange --refresh") + dev_paths_for_group = self._exec_cmd( + f"sudo ls -1 /dev/{vg_props['name']}/*").decode().splitlines() + lvm_dev_paths.extend(dev_paths_for_group) LOG.debug("All LVM vols to scan: %s", lvm_dev_paths) valid_filesystems = ['ext2', 'ext3', 'ext4', 'xfs', 'btrfs'] diff --git a/coriolis/tests/osmorphing/osmount/test_base.py b/coriolis/tests/osmorphing/osmount/test_base.py index 50d101ae..e9c727d5 100644 --- a/coriolis/tests/osmorphing/osmount/test_base.py +++ b/coriolis/tests/osmorphing/osmount/test_base.py @@ -196,7 +196,14 @@ def test__get_vgs(self, mock_exec_cmd): mock_exec_cmd.assert_called_once_with( "sudo vgs -o vg_name,pv_name,vg_uuid, --noheadings --separator :") - expected_result = {"vg1": "pv1", "vg2": "pv3"} + expected_result = { + "uuid1": { + "name": "vg1", + "pvs": ["pv1"]}, + "uuid2": { + "name": "vg2", + "pvs": ["pv3"]} + } self.assertEqual(result, expected_result) @@ -217,7 +224,17 @@ def test__get_vgs_duplicate_vg_names(self, mock_exec_cmd, mock_uuid4): mock.call(vgs_cmd), mock.call("sudo vgrename uuid2 random_uuid") ]) - expected_result = {"vg1": "pv1", "random_uuid": "pv2"} + + expected_result = { + "uuid1": { + "name": "vg1", + "pvs": ["pv1"] + }, + "uuid2": { + "name": "random_uuid", + "pvs": ["pv2"] + } + } self.assertEqual(result, expected_result) @@ -803,7 +820,12 @@ def test_mount_os(self, mock_check_mount_fstab_partitions, mock_check_fs): mock_get_volume_block_devices.return_value = ["/dev/sda", "/dev/sdb"] mock_find_and_mount_root.return_value = ("/tmp/tmp_dir", "/dev/sdb1") - mock_get_vgs.return_value = {"vg1": "/dev/sda1"} + mock_get_vgs.return_value = { + "vgid1": { + "name": "vg1", + "pvs": ["/dev/sda1"] + } + } mock_get_mounted_devices.return_value = ["/dev/sda1"] mock_find_dev_with_contents.return_value = "/dev/sdb1" mock_exec_cmd.side_effect = [ @@ -832,7 +854,7 @@ def test_mount_os(self, mock_check_mount_fstab_partitions, mock.call('sudo partx -v -a /dev/sdb || true'), mock.call('sudo ls -1 /dev/sdb*'), mock.call('sudo vgck'), - mock.call('sudo vgchange -ay vg1'), + mock.call('sudo vgchange -ay -S vg_uuid=vgid1'), mock.call('sudo vgchange --refresh'), mock.call('sudo ls -1 /dev/vg1/*'), mock.call('readlink -en /dev/sda1'), @@ -875,7 +897,12 @@ def test_mount_os_run_xfs(self, mock_check_mount_fstab_partitions, mock_exec_cmd): mock_get_volume_block_devices.return_value = ["/dev/sda", "/dev/sdb"] mock_find_and_mount_root.return_value = ("/tmp/tmp_dir", "/dev/sdb1") - mock_get_vgs.return_value = {"vg1": "/dev/sda1"} + mock_get_vgs.return_value = { + "vgid1": { + "name": "vg1", + "pvs": ["/dev/sda1"] + } + } mock_get_mounted_devices.return_value = ["/dev/sda1"] mock_exec_cmd.side_effect = [ b"",