Re: [LTP] [PATCH 1/2] tst_find_backing_dev: Get dev name from /sys/dev/block/*/uevent

[Richard Palethorpe <rpalethorpe@suse.de>]

Hello,

Alessandro Carminati <alessandro.carminati@gmail.com> writes:

> Hello Richard,
>
> Il giorno gio 27 ott 2022 alle ore 11:16 Richard Palethorpe <rpalethorpe@suse.de> ha scritto:
>
>  Hello,
>
>  Alessandro Carminati <alessandro.carminati@gmail.com> writes:
>
>  > In some minimal Linux the /dev/root can be missing. The consequence for this
>  > is that mountinfo doesn't contain the correct information.
>  >
>  > The unevent file in sysfs is another method to retrieve device info given 
>  > a major/minor.
>  >
>  > btrfs subvolumes can be an exception to this rule, but they are not expected
>  > to be used in the current usecase.
>
>  Unfortunately this is not true. If you mount /tmp on BTRFS (or set
>  TMPDIR to some BTRFS mount), then we hit this issue.
>
> This is also true if you use the mountinfo strategy.
> I ran a few tests on my test environment, and I could see that the ioctl_loop05 
> on btrfs filesystem doesn't work either with the mountinfo strategy.

OK, so to summarise niether strategy works when the FS is BTRFS and init
has not done something about /dev/root.

I suppose part of the problem is BTRFS can span multiple devices
(IIUC). So there is no definite single backing device.

The command "btrfs devices stat <path>", uses the BTRFS_IOC_DEV_INFO
ioctl to get backing device paths.

>
>  >
>  > This solution requires sysfs to be mounted in /sys, but considering many 
>  > tests depends on the same, including the ioctl_loop05 that triggered this 
>  > patch, this requirement is not too restricted, and the old mountinfo can be 
>  > dropped altogether.
>
>  The mountinfo method is not such a maintenance issue that it needs to be
>  removed IMO. Possibly it could be replaced by tst_stat_mount_dev
>  instead, but we're cautious about touching this function.
>
> tst_find_backing_dev(), the function that is the target of this patch, seems to 
> be referenced in only a couple of points in all the LTP test suite.
> One place is in the ioctl_loop05 test, the other reference I found is in the 
> lib/tst_device.c - tst_dev_block_size().  tst_dev_block_size() is a function 
> that seems not to be referenced by any test.
>  
>  
>  To be clear, I'm not sure anyone compiles Linux without /sys then tries
>  to run LTP on it. However the fact that it is possible to remove /sys is
>  another signal (in addition to BTRFS) that the situation is complicated.
>
> Indeed, if we want to have a test that works in all the possible scenarios, 
> it needs additional work. 
> But IMHO, keeping the mountinfo strategy gives no advantage over not 
> having it. 
>

Well it allows the test to work on BTRFS in most situations. Possibly if
we find that the FS is BTRFS, the device is /dev/root and it doesn't
exist. Then we should call tst_brk(TCONF...

AFAICT what the test actually wants is the block device sector size
(BLKSSZGET). Possibly this can be retrieved with the BTRFS_IOC_FS_INFO
ioctl.

The final option would be to try using some other BTRFS specific ioctl
to get the backing device(s). If there is more than one then just return
the first. That's probably not worth the effort for the current test,
but I think it is quite likely this issue will arise again. io_control01
has a similar requirement.

-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp