oVirt / vdsm

The Virtual Desktop Server Manager
GNU General Public License v2.0
160 stars 201 forks source link

Skip LVM filesystem check on LV reduction #417

Closed BrooklynDewolf closed 2 months ago

BrooklynDewolf commented 3 months ago

Since LVM v2.03.17 (https://github.com/lvmteam/lvm2/commit/f6f2737015746b1b6c7fbd0d297a4596c584749b), reducing a logical volume (LV) requires the LV to be active due to the default 'checksize' option, which requires an active LV. This results in the following error when attempting to reduce a non-active LV:

err=[\' The LV must be active to safely reduce (see --fs options.)\']' ----

To resolve this, we now check if the LVM version is newer than 2.03.17. If so, we bypass the 'checksize' by using the '--fs ignore' option. This approach is viable because oVirt already handles checksize, making lvreduce redundant.

dupondje commented 3 months ago

Reviewed-by: Jean-Louis Dupond jean-louis@dupond.be

Quite an important one. As if the lvreduce fails, the snapshot disk becomes ILLEGAL. This only affects el9 hosts btw, as el8 has older lvm version without this --fs option.

BrooklynDewolf commented 3 months ago

Thank you for you feedback. I have added a parameterized test that tests the reduce function with different LVM versions which runs without any problems!

aesteve-rh commented 3 months ago

Great, can you please check for lvm at osinfo_test.py too?

BrooklynDewolf commented 3 months ago

I am not quite sure what you mean. I've added an extra assert to the test_package_versions() function.

aesteve-rh commented 3 months ago

/ost

BrooklynDewolf commented 3 months ago

No changes, just squashed my commits together.

aesteve-rh commented 2 months ago

/ost

sandrobonazzola commented 2 months ago

Merging based on previous review and approval