All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Theodore Ts'o" <tytso@mit.edu>
To: Lukas Czerner <lczerner@redhat.com>
Cc: Ext4 Developers List <linux-ext4@vger.kernel.org>,
	darrick.wong@oracle.com
Subject: Re: [PATCH 8/9] e2scrub_all: refactor device probe loop
Date: Thu, 21 Mar 2019 10:31:41 -0400	[thread overview]
Message-ID: <20190321143141.GB9434@mit.edu> (raw)
In-Reply-To: <20190321102742.k2oos4epoj6fyjao@work>

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

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

:-)

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

Cheers,

					- Ted

  reply	other threads:[~2019-03-21 14:31 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-21  2:02 [PATCH 1/9] e2scrub: check to make sure lvm2 is installed Theodore Ts'o
2019-03-21  2:02 ` [PATCH 2/9] debian: drop lvm2 from the recommends line Theodore Ts'o
2019-03-21  3:57   ` Darrick J. Wong
2019-03-21  2:02 ` [PATCH 3/9] Fix "make install-strip" Theodore Ts'o
2019-03-21  2:02 ` [PATCH 4/9] e2scrub: fix up "make install-strip" support Theodore Ts'o
2019-03-21  2:02 ` [PATCH 5/9] e2fscrub: add the -n option which shows what commands e2scrub would execute Theodore Ts'o
2019-03-21  3:59   ` Darrick J. Wong
2019-03-21 10:57   ` Lukas Czerner
2019-03-21 14:32     ` Theodore Ts'o
2019-03-21  2:02 ` [PATCH 6/9] e2scrub_all: add the -n option which shows what e2scrub_all would do Theodore Ts'o
2019-03-21  4:01   ` Darrick J. Wong
2019-03-21  2:02 ` [PATCH 7/9] e2scrub_all: make sure there's enough free space for a snapshot Theodore Ts'o
2019-03-21  4:02   ` Darrick J. Wong
2019-03-21 11:18   ` Lukas Czerner
2019-03-21 14:26     ` Theodore Ts'o
2019-03-21  2:02 ` [PATCH 8/9] e2scrub_all: refactor device probe loop Theodore Ts'o
2019-03-21  4:05   ` Darrick J. Wong
2019-03-21 10:27   ` Lukas Czerner
2019-03-21 14:31     ` Theodore Ts'o [this message]
2019-03-21 15:57       ` Lukas Czerner
2019-03-21 18:24         ` Theodore Ts'o
2019-03-21 20:17           ` Lukas Czerner
2019-03-21 20:48             ` Theodore Ts'o
2019-03-21 21:14               ` Lukas Czerner
2019-03-21 22:04               ` Theodore Ts'o
2019-03-21 22:08                 ` Theodore Ts'o
2019-03-22  9:38                   ` Lukas Czerner
2019-03-21 20:09         ` Andreas Dilger
2019-03-21 17:48     ` Theodore Ts'o
2019-03-21 19:49       ` Lukas Czerner
2019-03-21 20:23         ` Theodore Ts'o
2019-03-21 16:10   ` Lukas Czerner
2019-03-21  2:02 ` [PATCH 9/9] e2scrub,e2scrub_all: print a (more understandable) error if not run as root Theodore Ts'o
2019-03-21  4:04   ` Darrick J. Wong
2019-03-21 11:36   ` Lukas Czerner
2019-03-21 14:40     ` Theodore Ts'o
2019-03-21  3:55 ` [PATCH 1/9] e2scrub: check to make sure lvm2 is installed Darrick J. Wong
2019-03-21 20:25 [PATCH -v2 0/9] e2fsprogs: e2scrub cleanups Theodore Ts'o
2019-03-21 20:25 ` [PATCH 8/9] e2scrub_all: refactor device probe loop Theodore Ts'o
2019-03-21 20:55   ` Lukas Czerner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190321143141.GB9434@mit.edu \
    --to=tytso@mit.edu \
    --cc=darrick.wong@oracle.com \
    --cc=lczerner@redhat.com \
    --cc=linux-ext4@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.