Update 110_include_lvm_code.sh to make sure vgremove is called before recreating the VG

[rmetrich]

Pull Request Details:

• Type: Bug Fix

• Impact: Normal

• How was this pull request tested? Test on RHEL7 with a LVM Raid

• Brief description of the changes in this pull request:

Make sure we delete the volume group before re-creating it.
The issue happens in Migration mode when ReaR is not trying to use vgcfgrestore.

Reproducer:

1. Install a system

2. Add 2 additional disks that will be used to host a LVM VG

   • /dev/vdb
   • /dev/vdc

3. Create a Raid volume

   # pvcreate /dev/vdb
   # pvcreate /dev/vdc
   # vgcreate data /dev/vdb /dev/vdc
   # lvcreate -n vol1 -L 1G -m 1 data
   # mkfs.xfs /dev/data/vol1
   # mount /dev/data/vol1 /mnt
   

4. Build a rescue image and perform a recovery

   Error is shown below:

   Start system layout restoration.
   Disk '/dev/vda': creating 'msdos' partition table
   Disk '/dev/vda': creating partition number 1 with name 'primary'
   Disk '/dev/vda': creating partition number 2 with name 'primary'
   Creating LVM PV /dev/vdb
   Creating LVM PV /dev/vdc
   Creating LVM PV /dev/vda2
   Creating LVM VG 'data'; Warning: some properties may not be preserved...
   The disk layout recreation script failed
   

   Log excerpt:

   +++ Print 'Creating LVM VG '\''data'\''; Warning: some properties may not be preserved...'
   +++ '[' -e /dev/data ']'
   +++ lvm vgcreate --physicalextentsize 4096k data /dev/vdb /dev/vdc
     A volume group called data already exists.
   

[gdha]

@rmetrich Yes I encountered the same issue also and fixed it the same way via Chef deployment on >20k systems ;-)

[jsmeix]

I am not a sufficient LVM expert to actually review it but from
#2514 (comment)
I guess that in practice lvm vgremove --force is OK in general.

But in
#2514 (comment)
I described my concerns with removing LVM stuff in general
from my non-LVM-expert point of view.
Therein I described my basic concern as

But deconstructing LVM stuff ... gets impossible
when there are LVs that belong to VGs that
contain disks that should not be wiped.

i.e. I wonder if it might happen that with an unusual LVM setup
lvm vgremove --force could possibly destroy things on a disk
that must not be touched by "rear recover", in
#2514 (comment)
see my offhanded example of such an unusual LVM setup where a VG
has a PV or LV on a disk that must not be touched by "rear recover"
and then I wonder if lvm vgremove --force of that VG could do
something bad on that disk that must not be touched by "rear recover" ?

[jsmeix]

@rmetrich
nice to hear from you again!

[jsmeix]

In #2514 (comment)
@gdha suggests lvm vgremove $vg --force --yes
i.e. additionally a --yes and in
https://github.com/rear/rear/blob/b98687ae9c61084b74d996179a0550abc887c005/usr/share/rear/layout/recreate/default/150_wipe_disks.sh
I had to even use a double force plus yes --force --force -y
(but in my case it was pvremove) see the comment in my code.

[rmetrich]

Hello,
Yes I agree --force twice and --yes would be better, just in case.

For the VG having a PV on a disk that should not be touched, then I would consider it's a bug/limitation: it's not possible to re-create a VG in that case at all.

[jsmeix]

But lvm vgremove --force --force --yes happens unconditioned for the VG
so I still fear this might do bad things when there is a VG that has PVs or LVs
also on another disk that must not be touched by "rear recover".

If I am right with my concerns then I am missing a test that a VG
that should be enforced 'vgremove'd does not have any PV or LV
on whatever other disk that must not be touched by "rear recover",
cf. my code about belongs_to_a_disk_to_be_wiped
and DISKS_TO_BE_WIPED in the files in
https://github.com/rear/rear/pull/2514/files

Or I am wrong with my concerns and it is safe to do an enforced 'vgremove'
regardless where that VG has its PVs and LVs because it will only remove
that specific VG without possible bad side effects on other storage objects.

I would be happy if I am wrong with my concerns because that would simplify
how to automatically get rid of old LVM stuff.

[jsmeix]

My concern is not when for a VG having a PV on a disk that should not be touched
it's not possible to re-create a VG in that case at all.

My concern is that an unconditioned enforced vgremove may somehow
damage things on a disk that should not be touched.

My concern is also not that such an enforced vgremove may leave
unused remainders on another disk (e.g. PVs there that are no longer
used in the recreated VG).

My concern is only possible damage / loss of data / whatever problem
on a disk that should not be touched.

[jsmeix]

I approve it "bona fide".

[gdha]

But lvm vgremove --force --force --yes happens unconditioned for the VG
so I still fear this might do bad things when there is a VG that has PVs or LVs
also on another disk that must not be touched by "rear recover".

If I am right with my concerns then I am missing a test that a VG
that should be enforced 'vgremove'd does not have any PV or LV
on whatever other disk that must not be touched by "rear recover",
cf. my code about belongs_to_a_disk_to_be_wiped
and DISKS_TO_BE_WIPED in the files in
https://github.com/rear/rear/pull/2514/files

@jsmeix I did test your code but that did not removed the VG in question. Only when I used a similar line as @rmetrich provides it worked perfectly and I have tested a real DR several times with real servers used by developers.

[jsmeix]

@gdha
my code in https://github.com/rear/rear/pull/2514/files
does not remove VGs or LVs.
It only removes PVs because for PVs it is easy to determine
to what disk a PV belongs.

Some code is there and it had worked for my tests but I commented it out

#for detected_LV in $detected_LVs ; do
...
#    if lvremove -f -y $detected_LV ; then

...

#for detected_VG in $detected_VGs ; do
#    if vgremove -ff -y $detected_VG ; then

because of my concern that enforced removal of VGs and/or LVs might result
bad things on disks that do not belong to DISKS_TO_BE_WIPED
and at that time I did not find a solution for the problem how to determine
to what disks a VG belongs. Probably this can be somehow deduced
from what disks the associated PVs or LVs belong.

What needs to be tested is a LVM setup with VGs that spread their PVs
over disks that should be wiped and other disks that should not be wiped
how then an enforced vgremove and/or lvremove behaves.

Did you test such a LVM setup?

[gdha]

@jsmeix

What needs to be tested is a LVM setup with VGs that spread their PVs
over disks that should be wiped and other disks that should not be wiped
how then an enforced vgremove and/or lvremove behaves.

Did you test such a LVM setup?

In short - yes ;-)