On Mon, Feb 04, 2019 at 12:08:49PM -0600, Eric Sandeen wrote:
> On 12/19/18 1:29 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> >
> > Back when I was designing xfs_scrub_all, I naïvely assumed that the
> > emitted output would always list physical storage before the virtual
> > devices stacked atop it. However, this is not actually true when one
> > omits the "NAME" column, which is crucial to forcing the output (json or
> > otherwise) to capture the block device hierarchy. If the assumption is
> > violated, the program crashes with a python exception.
>
> Is this a quirk or a documented feature of lsblk?
Not a documented feature, but seems to be a fairly common behavioral
quirk?
> > To fix this, force the hierarchal json output and restructure the
> > discovery routines to walk the json object that we receive, from the top
> > (physical devices) downwards to wherever there are live xfs filesystems.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > ---
> > scrub/xfs_scrub_all.in | 28 +++++++++++++++++-----------
> > 1 file changed, 17 insertions(+), 11 deletions(-)
> >
> >
> > diff --git a/scrub/xfs_scrub_all.in b/scrub/xfs_scrub_all.in
> > index c4e9899d..5b76b49a 100644
> > --- a/scrub/xfs_scrub_all.in
> > +++ b/scrub/xfs_scrub_all.in
> > @@ -28,9 +28,21 @@ def DEVNULL():
> >
> > def find_mounts():
> > '''Map mountpoints to physical disks.'''
> > + def find_xfs_mounts(bdev, fs, lastdisk):
> > + '''Attach lastdisk to each fs found under bdev.'''
> > + if bdev['fstype'] == 'xfs' and bdev['mountpoint'] is not None:
> > + mnt = bdev['mountpoint']
> > + if mnt in fs:
> > + fs[mnt].add(lastdisk)
> > + else:
> > + fs[mnt] = set([lastdisk])
> > + if 'children' not in bdev:
> > + return
> > + for child in bdev['children']:
> > + find_xfs_mounts(child, fs, lastdisk)
> >
> > fs = {}
> > - cmd=['lsblk', '-o', 'KNAME,TYPE,FSTYPE,MOUNTPOINT', '-J']
> > + cmd=['lsblk', '-o', 'NAME,KNAME,TYPE,FSTYPE,MOUNTPOINT', '-J']
>
> sorry for the ridonculously late review, and although '-J" isn't added
> new in this patch, FYI at least RHEL7 does not allow it:
>
> # lsblk -o KNAME -J
> lsblk: invalid option -- 'J'
>
> ... thoughts? Probably should be handled gracefully at least?
lsblk returns 1 for unrecognized arguments, so xfs_scrub_all will bail
if lsblk barfs. Not sure if we want to divert lsblk's stderr to
/dev/null or just let it spray out sloppily like we do now?
(Practically speaking I suspect that distros will pick up the util-linux
release that has json support before they pick up XFS kernel scrub...
but I should at least make sure that's really true.)
--D
> -Eric
>
> > result = subprocess.Popen(cmd, stdout=subprocess.PIPE)
> > result.wait()
> > if result.returncode != 0:
> > @@ -38,18 +50,12 @@ def find_mounts():
> > sarray = [x.decode(sys.stdout.encoding) for x in result.stdout.readlines()]
> > output = ' '.join(sarray)
> > bdevdata = json.loads(output)
> > +
> > # The lsblk output had better be in disks-then-partitions order
> > for bdev in bdevdata['blockdevices']:
> > - if bdev['type'] in ('disk', 'loop'):
> > - lastdisk = bdev['kname']
> > - if bdev['fstype'] == 'xfs':
> > - mnt = bdev['mountpoint']
> > - if mnt is None:
> > - continue
> > - if mnt in fs:
> > - fs[mnt].add(lastdisk)
> > - else:
> > - fs[mnt] = set([lastdisk])
> > + lastdisk = bdev['kname']
> > + find_xfs_mounts(bdev, fs, lastdisk)
> > +
> > return fs
> >
> > def kill_systemd(unit, proc):
> >
