Skip to content

Commit

Permalink
fix: Remove partition table from disk removed from a VG
Browse files Browse the repository at this point in the history
When removing a PV from a VG we currently remove only the PV
partition and not the partition table on the disk. This fixes this
by removing the partition table too.
  • Loading branch information
vojtechtrefny committed Jul 31, 2024
1 parent 7fa70a5 commit da0f166
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 15 deletions.
37 changes: 22 additions & 15 deletions library/blivet.py
Original file line number Diff line number Diff line change
Expand Up @@ -1841,10 +1841,10 @@ def _manage_members(self):
return

add_disks = [d for d in self._disks if d not in self._device.ancestors]
remove_disks = [pv for pv in self._device.pvs if not any(d in pv.ancestors for d in self._disks)]
remove_pvs = [pv for pv in self._device.pvs if not any(d in pv.ancestors for d in self._disks)]

Check warning on line 1844 in library/blivet.py

View check run for this annotation

Codecov / codecov/patch

library/blivet.py#L1844

Added line #L1844 was not covered by tests

if self._pool['grow_to_fill']:
grow_pv_candidates = [pv for pv in self._device.pvs if pv not in remove_disks and pv not in add_disks]
grow_pv_candidates = [pv for pv in self._device.pvs if pv not in remove_pvs and pv not in add_disks]

Check warning on line 1847 in library/blivet.py

View check run for this annotation

Codecov / codecov/patch

library/blivet.py#L1847

Added line #L1847 was not covered by tests

for pv in grow_pv_candidates:
if abs(self._device.size - self._device.current_size) < 2 * self._device.pe_size:
Expand All @@ -1859,12 +1859,12 @@ def _manage_members(self):
else:
log.warning("cannot grow/resize PV '%s', format is not resizable", pv.name)

if not (add_disks or remove_disks):
if not (add_disks or remove_pvs):

Check warning on line 1862 in library/blivet.py

View check run for this annotation

Codecov / codecov/patch

library/blivet.py#L1862

Added line #L1862 was not covered by tests
return

if remove_disks and safe_mode:
if remove_pvs and safe_mode:

Check warning on line 1865 in library/blivet.py

View check run for this annotation

Codecov / codecov/patch

library/blivet.py#L1865

Added line #L1865 was not covered by tests
raise BlivetAnsibleError("cannot remove members '%s' from pool '%s' in safe mode" %
(", ".join(d.name for d in remove_disks),
(", ".join(d.name for d in remove_pvs),
self._device.name))

if self._is_raid:
Expand All @@ -1882,26 +1882,33 @@ def _manage_members(self):
self._pool['name'],
str(e)))

for disk in remove_disks:
if self._device.free_space < disk.size:
raise BlivetAnsibleError("disk '%s' cannot be removed from pool '%s'" % (disk.name,
self._pool['name']))
for pv in remove_pvs:
if self._device.free_space < pv.size:
raise BlivetAnsibleError("member '%s' cannot be removed from pool '%s'" % (pv.name,

Check warning on line 1887 in library/blivet.py

View check run for this annotation

Codecov / codecov/patch

library/blivet.py#L1885-L1887

Added lines #L1885 - L1887 were not covered by tests
self._pool['name']))

try:
ac = ActionRemoveMember(self._device, disk)
ac = ActionRemoveMember(self._device, pv)

Check warning on line 1891 in library/blivet.py

View check run for this annotation

Codecov / codecov/patch

library/blivet.py#L1891

Added line #L1891 was not covered by tests
# XXX: scheduling ActionRemoveMember is currently broken, we need to execute
# the action now manually, see https://bugzilla.redhat.com/show_bug.cgi?id=2076956
# self._blivet.devicetree.actions.add(ac)
ac.apply()
ac.execute()
except Exception as e:
raise BlivetAnsibleError("failed to remove disk '%s' from pool '%s': %s" % (disk.name,
self._pool['name'],
str(e)))
raise BlivetAnsibleError("failed to remove member '%s' from pool '%s': %s" % (pv.name,

Check warning on line 1898 in library/blivet.py

View check run for this annotation

Codecov / codecov/patch

library/blivet.py#L1898

Added line #L1898 was not covered by tests
self._pool['name'],
str(e)))
# XXX workaround for https://github.com/storaged-project/blivet/pull/1040
disk.format.vg_name = None
pv.format.vg_name = None

Check warning on line 1902 in library/blivet.py

View check run for this annotation

Codecov / codecov/patch

library/blivet.py#L1902

Added line #L1902 was not covered by tests

self._blivet.devicetree.recursive_remove(pv.raw_device)

Check warning on line 1904 in library/blivet.py

View check run for this annotation

Codecov / codecov/patch

library/blivet.py#L1904

Added line #L1904 was not covered by tests

self._blivet.devicetree.recursive_remove(disk.raw_device)
# if we are using partitions remove also the disklabel from the disk (but make sure
# there aren't other partitions first)
if use_partitions and not pv.is_disk:
for disk in pv.disks:
if disk.format.type == "disklabel" and not disk.format.partitions:
self._blivet.devicetree.recursive_remove(disk)

Check warning on line 1911 in library/blivet.py

View check run for this annotation

Codecov / codecov/patch

library/blivet.py#L1908-L1911

Added lines #L1908 - L1911 were not covered by tests

def _create(self):
if not self._device:
Expand Down
18 changes: 18 additions & 0 deletions tests/tests_lvm_pool_members.yml
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,15 @@
- name: foo
disks: "{{ [unused_disks[0]] }}"

- name: Get the partition table type on disk removed from the VG
command: lsblk -o PTTYPE --noheadings --nodeps /dev/{{ unused_disks[1] }}
register: storage_test_removed_pttype
changed_when: false

- name: Verify that removing the PV from the VG also removed the partition table
assert:
that: storage_test_removed_pttype.stdout == ''

- name: Verify role results
include_tasks: verify-role-results.yml

Expand Down Expand Up @@ -279,6 +288,15 @@
- name: test
size: "{{ volume_size }}"

- name: Get the partition table type on disk removed from the VG
command: lsblk -o PTTYPE --noheadings --nodeps /dev/{{ unused_disks[0] }}
register: storage_test_removed_pttype
changed_when: false

- name: Verify that removing the PV from the VG also removed the partition table
assert:
that: storage_test_removed_pttype.stdout == ''

- name: Get UUID of the 'foo' volume group
command: vgs --noheading -o vg_uuid foo
changed_when: false
Expand Down

0 comments on commit da0f166

Please sign in to comment.