[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20190321155703.ili5ghofgm3hneq5@work>
Date: Thu, 21 Mar 2019 16:57:03 +0100
From: Lukas Czerner <lczerner@...hat.com>
To: Theodore Ts'o <tytso@....edu>
Cc: Ext4 Developers List <linux-ext4@...r.kernel.org>,
darrick.wong@...cle.com
Subject: Re: [PATCH 8/9] e2scrub_all: refactor device probe loop
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
Powered by blists - more mailing lists