On 2/4/19 12:16 PM, Darrick J. Wong wrote:
> 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?
bleah. Sounds fragile. :(
Let me check with kzak on this. If your patch improves it for now, super,
but I don't want to leave a little time bomb here.
>>> - 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.)
Yeah I'd expect that as well, just wondered how much of a landmine it
might be, I guess.
-Eric
> --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):
>>>
>
