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

Alessandro Carminati
2022-10-31T16:58:35Z

Hello Richard,

Il giorno lun 31 ott 2022 alle ore 13:54 Richard Palethorpe
<rpalethorpe@suse.de> ha scritto:
>
> 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.

Yes, use the ioctl could be a solution for btrfs.
I ran a little test to understand what filesystem I can expect.
ioctl_loop05 is supported on a few filesystems, I tried listing them,
and on a few, I run the test:

| Filesystem | tested | notes
+---------------+--------+----------------------------
|nfs | no | not tested, no idea.
|9p | no | not tested, no idea.
|ramfs | no | theoretically possible to have root on
ramfs, but I think it very unlikely
|btrfs | yes | both strategies have problems with it.
|xfs | yes | tested ok.
|ext2/ext3/ext4 | yes | tested ok.
|minix | yes | max size is 64MB, possible but not
probable. Stat on mounted fs shows minor/major corresponding to
backend device.
|udf | no | optical media filesystem. Very unlikely to
have it as root filesystem. Stat on mounted fs shows minor/major
corresponding to backend device.
|sysv | no | not tested, no idea.
|ufs | no | not tested, no idea.
|f2fs | yes | tested ok.
|nilfs | yes | tested ok.
|exofs | no | it has been removed from kernel supported
FS since 5.1
|fuse | no | Unlikely.
|vfat | no | I'm not aware you can mount vfat as rootfs.
Since it lacks on a lot of features, it is unlikely having it as
rootfs.
|exfat | no | not tested, no idea.

It seems to me likely that btrfs is the only filesystem, among the
supported, that can present virtual devices with different
minor/major.
I'm not 100% sure, but I would bet on the fact that we can identify
these btrfs scenarios just by looking at the major number. If it is 0
we are dealing with it.
That been said, if major == 0 we can extract the backing device by
using the ioct, as you suggested:
struct btrfs_ioctl_dev_info_args di_args = {0};
ioctl(fd, BTRFS_IOC_DEV_INFO, &di_args)
and use di_args.path

Do you see any downside on such approach?

>
> >
> > >
> > > 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...
>

I ran a multiple tests with btrfs, and the mountinfo strategy.
I didn't find a single scenario where it succeded.
In my situation I always had that the backend device presnted a
different couple minor/major with the one listed by mountinfo.
Could you suggest a btrfs scenario where the mountinfo strategy ends
with success?

> 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.

Cheers
Alessandro Carminati