On Thu, Mar 21, 2019 at 10:31:41AM -0400, Theodore Ts'o wrote: > On Thu, Mar 21, 2019 at 11:27:42AM +0100, Lukas Czerner wrote: > > > > lv_role=public does not exclude snapshots and so it will fail later in > > e2scrub when you try to create a snapshot of a thicksnapshot which is > > not supported. > > Ah, good point. Thanks for pointing that out. > > > Also I am not sure what's the rush, but it seems you've ignored my other > > suggestions. > > I did consider them all, but there were reasons why I didn't use them. > One of them wasn't practical because I needed LVM2_VG_NAME; when you > made that suggestion, I hadn't published the patch needed to fix > Debian Bug #924301. Of course, if we use your suggestion of using "-S > vg_free > ${snap_size}" it will obviate that need; so thanks for that > suggestion. > > The reason why I didn't take one of your other suggestions is that we > need to check whether or not the file system is mounted, and so we > needed the mountpoint in lsblk, and once you ask for the mountpoint, > we can no longer use awk to select a field, since an unmounted file > system shows up as a an empty column. > > % sudo lsblk -o MOUNTPOINT,FSTYPE,NAME -l > LVM2_member nvme0n1p3_crypt > ext4 lambda-uroot > [SWAP] swap lambda-swap_1 > / ext4 lambda-root > ext4 lambda-old--files > ext4 lambda-library > ext4 lambda-test--4k > ext4 lambda-scratch > ext4 lambda-test--1k > lambda-scratch2 > lambda-scratch3 > ext4 lambda-results > lambda-thinpool_tmeta > lambda-thinpool_tdata So the good news is that it's not really a problem. What you really want is the mountpoint if it exists and the device otherwise right ? Then my suggestion will do just that becuase awk will separate the fieds with FS (field separator) which by default is a single space. But space is special in awk becuase the fields will be separated by a runs of space, so if you pick the fileds just right you'll get what you want. So if I run this on my system # lsblk -o MOUNTPOINT,NAME,FSTYPE -p -n -l `lvs -o lv_path -S lv_active=active,lv_role=public,lv_role!=snapshot,vg_free\>256 --noheadings` I'll get /dev/mapper/vg_perpetuum-ext4--suck /dev/mapper/vg_perpetuum-fedora29 /dev/mapper/vg_perpetuum-fedora29--2 /dev/mapper/vg_perpetuum-freebsd /mnt/kernel /dev/mapper/vg_perpetuum-kernel ext4 /home /dev/mapper/vg_perpetuum-lv_home xfs / /dev/mapper/vg_perpetuum-lv_root xfs [SWAP] /dev/mapper/vg_perpetuum-lv_swap swap /var /dev/mapper/vg_perpetuum-lv_var xfs /dev/mapper/vg_perpetuum-lvol001 ext4 /dev/mapper/vg_perpetuum-overlay ext4 /dev/mapper/vg_perpetuum-rhel7 /dev/mapper/vg_perpetuum-rhel8 /dev/mapper/vg_perpetuum-test0 ext3 /dev/mapper/vg_perpetuum-test1 xfs /dev/mapper/vg_perpetuum-test10 ext4 /dev/mapper/vg_perpetuum-test11 ext4 /dev/mapper/vg_perpetuum-test12 /dev/mapper/vg_perpetuum-test2 /dev/mapper/vg_perpetuum-test3 ext4 /dev/mapper/vg_perpetuum-test4 ext4 /dev/mapper/vg_perpetuum-test5 /dev/mapper/vg_perpetuum-test6 LVM2_member /dev/mapper/vg_perpetuum-test7 /dev/mapper/vg_perpetuum-test8 /dev/mapper/vg_perpetuum-test9 ext4 /dev/mapper/vg_perpetuum-testing ext4 /dev/mapper/vg_perpetuum-testing1 xfs /dev/mapper/vg_perpetuum-thinvolume now if I add | awk '{print $1}' I'll get /dev/mapper/vg_perpetuum-ext4--suck /dev/mapper/vg_perpetuum-fedora29 /dev/mapper/vg_perpetuum-fedora29--2 /dev/mapper/vg_perpetuum-freebsd /mnt/kernel /home / [SWAP] /var /dev/mapper/vg_perpetuum-lvol001 /dev/mapper/vg_perpetuum-overlay /dev/mapper/vg_perpetuum-rhel7 /dev/mapper/vg_perpetuum-rhel8 /dev/mapper/vg_perpetuum-test0 /dev/mapper/vg_perpetuum-test1 /dev/mapper/vg_perpetuum-test10 /dev/mapper/vg_perpetuum-test11 /dev/mapper/vg_perpetuum-test12 /dev/mapper/vg_perpetuum-test2 /dev/mapper/vg_perpetuum-test3 /dev/mapper/vg_perpetuum-test4 /dev/mapper/vg_perpetuum-test5 /dev/mapper/vg_perpetuum-test6 /dev/mapper/vg_perpetuum-test7 /dev/mapper/vg_perpetuum-test8 /dev/mapper/vg_perpetuum-test9 /dev/mapper/vg_perpetuum-testing /dev/mapper/vg_perpetuum-testing1 /dev/mapper/vg_perpetuum-thinvolume hence I get mountpoin where the volume is mounted and the device where it is not. That's what we need right ? What I did not consider was [SWAP] but we can get rid of that easily. grep -v '[SWAP]' > > I also really didn't like using grep to select the file system type > ext[234], since if it would falsely select a LV name that contained > "ext4", e.g.: > > /home/dave xfs rhel-ext4--sucks That can be dealt with as well as well just by grepping for ' ext[234]$' So in the end we'll have something like # lsblk -o MOUNTPOINT,NAME,FSTYPE -p -n -l `lvs -o lv_path -S lv_active=active,lv_role=public,lv_role!=snapshot,vg_free\>256 --noheadings` | grep -v '[SWAP]' | grep ' ext[234]$' | awk '{print $1}' and so on my system this gives me /mnt/kernel /dev/mapper/vg_perpetuum-lvol001 /dev/mapper/vg_perpetuum-overlay /dev/mapper/vg_perpetuum-test0 /dev/mapper/vg_perpetuum-test10 /dev/mapper/vg_perpetuum-test11 /dev/mapper/vg_perpetuum-test3 /dev/mapper/vg_perpetuum-test4 /dev/mapper/vg_perpetuum-test9 /dev/mapper/vg_perpetuum-testing And leaving out mounted file systems is again as simple as grep -v '^/dev/' Have I missed something ? Now for the performance. I drop caches, then run each twice the ls_scan_targets function runs real 0m0.575s user 0m0.086s sys 0m0.179s real 0m0.241s user 0m0.088s sys 0m0.169s My one-line suggestion runs real 0m0.275s user 0m0.027s sys 0m0.047s real 0m0.069s user 0m0.015s sys 0m0.052s So the difference is significant, although I would not consider it meaningful since it's really low anyway, but some people complained so I guess someone cares and people do have different systems so... > > :-) > > We could have used awk to select the field, but that still doesn't > deal with the mountpoint column being empty when it is unmounted. I > did briefly consider using lsblk -J, but I didn't want to add a > dependency on the jq[1] package (and I didn't even know if RHEL/Fedora > packages jq). > > [1] https://packages.debian.org/stretch/jq Yes Fedora does have it, but I agree that adding this dependency is not ideal. Thanks! -Lukas > > Cheers, > > - Ted