#2291 PR merged: Error out in case of insufficient LVM tools (issue 2259)

Labels: enhancement, bug, cleanup, fixed / solved / done

jsmeix opened issue at 2019-12-02 15:24:

  • Type: Minor Bug Fix / Enhancement

  • Impact: Low / High
    Normally (i.e. on SLES and RHEL) nothing should be noticed
    but when there are insufficient LVM tools used
    (as in https://github.com/rear/rear/issues/2259)
    the impact is high because without the check the error is unnoticed
    so that things fail when it is too late during "rear recover".

  • Reference to related issue (URL):
    https://github.com/rear/rear/issues/2259

  • How was this pull request tested?
    Works for me on SLES12-SP4 with its default LVM(+btrfs) setup.
    My disklayout.conf is identical compared to before the changes here.

  • Brief description of the changes in this pull request:

According to
https://github.com/rear/rear/issues/2259#issuecomment-557019729
there is now a check before the longer lvs calls
to ensure all needed lvs fields are actually supported
and if not it errors out now during "rear mkrescue/mkbackup".

By the way I also removed the strange looking 8>&- 7>&- cf.
https://github.com/rear/rear/issues/2259#issuecomment-544999137

jsmeix commented at 2019-12-02 16:50:

My initial attempt to verify LVM entries in disklayout.conf via
https://github.com/rear/rear/pull/2291/commits/85e89691050e2a884f9b9d5f13ec79ea9adb0e9c
is not yet tested by me - I will do this tomorrow...

pcahyna commented at 2019-12-02 18:06:

Hi, why to parse the output of lvs -o help to get the list of supported fields? Wouldn't it be simpler and less error-prone to just check the error status from the lvm lvs commands?

jsmeix commented at 2019-12-03 09:35:

My current "solution" is plain add-on code
so that I do not need to reorganize existing commands.

To "just" check the error status from the lvm lvs commands
I "just" need to "just" reorganize existing code.

I do fully agree that it is in general always plain wrong
to ignore the error status from mandatory commands
cf. "Try hard to care about possible errors" in
https://github.com/rear/rear/wiki/Coding-Style

So I will now mercilessly clean up that mess...

jsmeix commented at 2019-12-03 15:41:

I completely overhauled layout/save/GNU/Linux/220_lvm_layout.sh
and for me on SLES12-SP4 with its default LVM(+btrfs) setup
my disklayout.conf is still identical compared to before all of
the changes in this pull request.

@pcahyna @rmetrich
I would very much appreciate it if you could test
if things also still work o.k. in your case.
I assume you have much more complicated LVM setups than I have.
In particular I did not test when the 'lv_layout' field is not supported.

My LVM entries in my disklayout.conf are only

# grep lvm disklayout.conf

part /dev/sda 21473787904 1048576 primary boot,lvm /dev/sda1
# lvmdev <volume_group> <device> [<uuid>] [<size(bytes)>]
lvmdev /dev/system /dev/sda1 7wwpcO-KmNN-qsTE-7sp7-JBJS-vBdC-Zyt1W7 41940992
# lvmgrp <volume_group> <extentsize> [<size(extents)>] [<size(bytes)>]
lvmgrp /dev/system 4096 5119 20967424
# lvmvol <volume_group> <name> <size(bytes)> <layout> [key:value ...]
lvmvol /dev/system root 19927138304b linear 
lvmvol /dev/system swap 1535115264b linear

and this is 'lsblk' on my system:

# lsblk -ipo NAME,KNAME,PKNAME,TRAN,TYPE,FSTYPE,SIZE,MOUNTPOINT

NAME                        KNAME     PKNAME    TRAN TYPE FSTYPE       SIZE MOUNTPOINT
/dev/sda                    /dev/sda            ata  disk               20G 
`-/dev/sda1                 /dev/sda1 /dev/sda       part LVM2_member   20G 
  |-/dev/mapper/system-swap /dev/dm-0 /dev/sda1      lvm  swap         1.4G [SWAP]
  `-/dev/mapper/system-root /dev/dm-1 /dev/sda1      lvm  btrfs       18.6G /

pcahyna commented at 2019-12-03 19:36:

@jsmeix wow, that's an impressive cleanup. I started reviewing it (not finished yet). I will also test it on my cases, although they not much more complicated than yours. Still they will have multiple disks and file systems.

jsmeix commented at 2019-12-13 10:47:

@rmetrich
could you have a look here (provided you find a bit of time)?
Perhaps you see something that is obviously wrong.

I would like to merge it next week if things look OK.

jsmeix commented at 2019-12-20 13:59:

A side note FYI:

When testing LVM recovery on my SLES12-SP4 KVM virtual test machine
using the SLES12-SP4 default LVM plus btrfs structure on a single disk
I noticed that the kernel device nodes of the logical volumes
on the recreated system are interchanged compared to what
they have been on the original system.

On the original system I have:

# lsblk -ipo NAME,KNAME,PKNAME,TRAN,TYPE,FSTYPE,SIZE,MOUNTPOINT

NAME                        KNAME     PKNAME    TRAN TYPE FSTYPE       SIZE MOUNTPOINT
/dev/sda                    /dev/sda            ata  disk               20G 
`-/dev/sda1                 /dev/sda1 /dev/sda       part LVM2_member   20G 
  |-/dev/mapper/system-swap /dev/dm-0 /dev/sda1      lvm  swap         1.4G [SWAP]
  `-/dev/mapper/system-root /dev/dm-1 /dev/sda1      lvm  btrfs       18.6G /

On the recreated system I have:

# lsblk -ipo NAME,KNAME,PKNAME,TRAN,TYPE,FSTYPE,SIZE,MOUNTPOINT

NAME                        KNAME     PKNAME    TRAN TYPE FSTYPE        SIZE MOUNTPOINT
/dev/sda                    /dev/sda            ata  disk                20G 
`-/dev/sda1                 /dev/sda1 /dev/sda       part LVM2_member    20G 
  |-/dev/mapper/system-root /dev/dm-0 /dev/sda1      lvm  btrfs        18.6G /
  `-/dev/mapper/system-swap /dev/dm-1 /dev/sda1      lvm  swap          1.4G [SWAP]

I assume it does not matter what kernel device node
/dev/dm-0 versus /dev/dm-1 belongs to the logical volumes
/dev/mapper/system-root and /dev/mapper/system-swap because
I assume that logical volumes are always accessed via their symlinks

/dev/mapper/system-root -> /dev/dm-0
/dev/mapper/system-swap -> /dev/dm-1

/dev/system/root -> /dev/dm-0
/dev/system/swap -> /dev/dm-1

but never directly via the kernel device nodes.

This is the disklayout.conf file:

# Disk layout dated 20191220121325 (YYYYmmddHHMMSS)
# NAME                        KNAME     PKNAME    TRAN TYPE FSTYPE       SIZE MOUNTPOINT
# /dev/sda                    /dev/sda            ata  disk               20G 
# `-/dev/sda1                 /dev/sda1 /dev/sda       part LVM2_member   20G 
#   |-/dev/mapper/system-swap /dev/dm-0 /dev/sda1      lvm  swap         1.4G [SWAP]
#   `-/dev/mapper/system-root /dev/dm-1 /dev/sda1      lvm  btrfs       18.6G /
# /dev/sr0                    /dev/sr0            ata  rom  iso9660      3.8G 
# Disk /dev/sda
# Format: disk <devname> <size(bytes)> <partition label type>
disk /dev/sda 21474836480 msdos
# Partitions on /dev/sda
# Format: part <device> <partition size(bytes)> <partition start(bytes)> <partition type|name> <flags> /dev/<partition>
part /dev/sda 21473787904 1048576 primary boot,lvm /dev/sda1
# Format for LVM PVs
# lvmdev <volume_group> <device> [<uuid>] [<size(bytes)>]
lvmdev /dev/system /dev/sda1 7wwpcO-KmNN-qsTE-7sp7-JBJS-vBdC-Zyt1W7 41940992
# Format for LVM VGs
# lvmgrp <volume_group> <extentsize> [<size(extents)>] [<size(bytes)>]
lvmgrp /dev/system 4096 5119 20967424
# Format for LVM LVs
# lvmvol <volume_group> <name> <size(bytes)> <layout> [key:value ...]
lvmvol /dev/system root 19927138304b linear 
lvmvol /dev/system swap 1535115264b linear 
# Filesystems (only ext2,ext3,ext4,vfat,xfs,reiserfs,btrfs are supported).
# Format: fs <device> <mountpoint> <fstype> [uuid=<uuid>] [label=<label>] [<attributes>]
fs /dev/mapper/system-root / btrfs uuid=624c4653-1e78-4159-8cdf-2998d2f3c670 label= options=rw,relatime,space_cache,subvolid=259,subvol=/@/.snapshots/1/snapshot
# Btrfs default subvolume for /dev/mapper/system-root at /
# Format: btrfsdefaultsubvol <device> <mountpoint> <btrfs_subvolume_ID> <btrfs_subvolume_path>
btrfsdefaultsubvol /dev/mapper/system-root / 259 @/.snapshots/1/snapshot
# Btrfs snapshot subvolumes for /dev/mapper/system-root at /
# Btrfs snapshot subvolumes are listed here only as documentation.
# There is no recovery of btrfs snapshot subvolumes.
# Format: btrfssnapshotsubvol <device> <mountpoint> <btrfs_subvolume_ID> <btrfs_subvolume_path>
#btrfssnapshotsubvol /dev/mapper/system-root / 259 @/.snapshots/1/snapshot
#btrfssnapshotsubvol /dev/mapper/system-root / 283 @/.snapshots/2/snapshot
#btrfssnapshotsubvol /dev/mapper/system-root / 284 @/.snapshots/3/snapshot
#btrfssnapshotsubvol /dev/mapper/system-root / 285 @/.snapshots/4/snapshot
# Btrfs normal subvolumes for /dev/mapper/system-root at /
# Format: btrfsnormalsubvol <device> <mountpoint> <btrfs_subvolume_ID> <btrfs_subvolume_path>
# SLES12-SP1 (and later) btrfs subvolumes setup needed for /dev/mapper/system-root (default subvolume path contains '@/.snapshots/')
# Btrfs subvolumes that belong to snapper are listed here only as documentation.
# Snapper's base subvolume '/@/.snapshots' is deactivated here because during 'rear recover'
# it is created by 'snapper/installation-helper --step 1' (which fails if it already exists).
# Furthermore any normal btrfs subvolume under snapper's base subvolume would be wrong.
# See https://github.com/rear/rear/issues/944#issuecomment-238239926
# and https://github.com/rear/rear/issues/963#issuecomment-240061392
# how to create a btrfs subvolume in compliance with the SLES12 default brtfs structure.
# In short: Normal btrfs subvolumes on SLES12 must be created directly below '/@/'
# e.g. '/@/var/lib/mystuff' (which requires that the btrfs root subvolume is mounted)
# and then the subvolume is mounted at '/var/lib/mystuff' to be accessible from '/'
# plus usually an entry in /etc/fstab to get it mounted automatically when booting.
# Because any '@/.snapshots' subvolume would let 'snapper/installation-helper --step 1' fail
# such subvolumes are deactivated here to not let 'rear recover' fail:
#btrfsnormalsubvol /dev/mapper/system-root / 258 @/.snapshots
btrfsnormalsubvol /dev/mapper/system-root / 257 @
btrfsnormalsubvol /dev/mapper/system-root / 260 @/boot/grub2/i386-pc
btrfsnormalsubvol /dev/mapper/system-root / 261 @/boot/grub2/x86_64-efi
btrfsnormalsubvol /dev/mapper/system-root / 262 @/home
btrfsnormalsubvol /dev/mapper/system-root / 263 @/opt
btrfsnormalsubvol /dev/mapper/system-root / 264 @/srv
btrfsnormalsubvol /dev/mapper/system-root / 265 @/tmp
btrfsnormalsubvol /dev/mapper/system-root / 266 @/usr/local
btrfsnormalsubvol /dev/mapper/system-root / 267 @/var/cache
btrfsnormalsubvol /dev/mapper/system-root / 268 @/var/crash
btrfsnormalsubvol /dev/mapper/system-root / 269 @/var/lib/libvirt/images
btrfsnormalsubvol /dev/mapper/system-root / 270 @/var/lib/machines
btrfsnormalsubvol /dev/mapper/system-root / 271 @/var/lib/mailman
btrfsnormalsubvol /dev/mapper/system-root / 272 @/var/lib/mariadb
btrfsnormalsubvol /dev/mapper/system-root / 273 @/var/lib/mysql
btrfsnormalsubvol /dev/mapper/system-root / 274 @/var/lib/named
btrfsnormalsubvol /dev/mapper/system-root / 275 @/var/lib/pgsql
btrfsnormalsubvol /dev/mapper/system-root / 276 @/var/log
btrfsnormalsubvol /dev/mapper/system-root / 277 @/var/opt
btrfsnormalsubvol /dev/mapper/system-root / 278 @/var/spool
btrfsnormalsubvol /dev/mapper/system-root / 279 @/var/tmp
# All mounted btrfs subvolumes (including mounted btrfs default subvolumes and mounted btrfs snapshot subvolumes).
# Determined by the findmnt command that shows the mounted btrfs_subvolume_path.
# Format: btrfsmountedsubvol <device> <subvolume_mountpoint> <mount_options> <btrfs_subvolume_path>
btrfsmountedsubvol /dev/mapper/system-root / rw,relatime,space_cache,subvolid=259,subvol=/@/.snapshots/1/snapshot @/.snapshots/1/snapshot
btrfsmountedsubvol /dev/mapper/system-root /.snapshots rw,relatime,space_cache,subvolid=258,subvol=/@/.snapshots @/.snapshots
btrfsmountedsubvol /dev/mapper/system-root /usr/local rw,relatime,space_cache,subvolid=266,subvol=/@/usr/local @/usr/local
btrfsmountedsubvol /dev/mapper/system-root /var/opt rw,relatime,space_cache,subvolid=277,subvol=/@/var/opt @/var/opt
btrfsmountedsubvol /dev/mapper/system-root /var/lib/mysql rw,relatime,space_cache,subvolid=273,subvol=/@/var/lib/mysql @/var/lib/mysql
btrfsmountedsubvol /dev/mapper/system-root /boot/grub2/i386-pc rw,relatime,space_cache,subvolid=260,subvol=/@/boot/grub2/i386-pc @/boot/grub2/i386-pc
btrfsmountedsubvol /dev/mapper/system-root /var/crash rw,relatime,space_cache,subvolid=268,subvol=/@/var/crash @/var/crash
btrfsmountedsubvol /dev/mapper/system-root /var/lib/pgsql rw,relatime,space_cache,subvolid=275,subvol=/@/var/lib/pgsql @/var/lib/pgsql
btrfsmountedsubvol /dev/mapper/system-root /var/lib/mailman rw,relatime,space_cache,subvolid=271,subvol=/@/var/lib/mailman @/var/lib/mailman
btrfsmountedsubvol /dev/mapper/system-root /var/spool rw,relatime,space_cache,subvolid=278,subvol=/@/var/spool @/var/spool
btrfsmountedsubvol /dev/mapper/system-root /tmp rw,relatime,space_cache,subvolid=265,subvol=/@/tmp @/tmp
btrfsmountedsubvol /dev/mapper/system-root /opt rw,relatime,space_cache,subvolid=263,subvol=/@/opt @/opt
btrfsmountedsubvol /dev/mapper/system-root /home rw,relatime,space_cache,subvolid=262,subvol=/@/home @/home
btrfsmountedsubvol /dev/mapper/system-root /var/cache rw,relatime,space_cache,subvolid=267,subvol=/@/var/cache @/var/cache
btrfsmountedsubvol /dev/mapper/system-root /var/lib/mariadb rw,relatime,space_cache,subvolid=272,subvol=/@/var/lib/mariadb @/var/lib/mariadb
btrfsmountedsubvol /dev/mapper/system-root /boot/grub2/x86_64-efi rw,relatime,space_cache,subvolid=261,subvol=/@/boot/grub2/x86_64-efi @/boot/grub2/x86_64-efi
btrfsmountedsubvol /dev/mapper/system-root /var/lib/named rw,relatime,space_cache,subvolid=274,subvol=/@/var/lib/named @/var/lib/named
btrfsmountedsubvol /dev/mapper/system-root /var/lib/libvirt/images rw,relatime,space_cache,subvolid=269,subvol=/@/var/lib/libvirt/images @/var/lib/libvirt/images
btrfsmountedsubvol /dev/mapper/system-root /srv rw,relatime,space_cache,subvolid=264,subvol=/@/srv @/srv
btrfsmountedsubvol /dev/mapper/system-root /var/lib/machines rw,relatime,space_cache,subvolid=270,subvol=/@/var/lib/machines @/var/lib/machines
btrfsmountedsubvol /dev/mapper/system-root /var/tmp rw,relatime,space_cache,subvolid=279,subvol=/@/var/tmp @/var/tmp
btrfsmountedsubvol /dev/mapper/system-root /var/log rw,relatime,space_cache,subvolid=276,subvol=/@/var/log @/var/log
# Mounted btrfs subvolumes that have the 'no copy on write' attribute set.
# Format: btrfsnocopyonwrite <btrfs_subvolume_path>
btrfsnocopyonwrite @/var/lib/mysql
btrfsnocopyonwrite @/var/lib/pgsql
btrfsnocopyonwrite @/var/lib/mariadb
btrfsnocopyonwrite @/var/lib/libvirt/images
# Swap partitions or swap files
# Format: swap <filename> uuid=<uuid> label=<label>
swap /dev/mapper/system-swap uuid=ab95b1b8-8747-48ef-89fa-e24b08fd1b8c label=

[Export of Github issue for rear/rear.]