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

Alessandro Carminati
2022-10-27T18:59:26Z

[-- Attachment #1.1: Type: text/plain, Size: 11948 bytes --]

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.

Here is a sample running the original (with a few additional debug output)
test
using the mountinfo strategy on ext3:

VFS: Mounted root (ext2 filesystem) on device 8:0.
devtmpfs: mounted
Freeing unused kernel image (initmem) memory: 956K
Write protecting the kernel read-only data: 14336k
Freeing unused kernel image (text/rodata gap) memory: 2044K
Freeing unused kernel image (rodata/data gap) memory: 48K
Run /sbin/init as init process
random: fast init done
EXT4-fs (sda): re-mounted. Opts: (null). Quota mode: disabled.
Starting syslogd: OK
Starting klogd: OK
Running sysctl: OK
Saving random seed: random: dd: uninitialized urandom read (512 bytes read)
OK
Starting network: Waiting for interface eth0 to appear...............
timeout!
run-parts: /etc/network/if-pre-up.d/wait_iface: exit status 1
FAIL

Welcome to Buildroot
buildroot login: root
# /usr/lib/ltp-testsuite/testcases/bin/ioctl_loop05
tst_test.c:1363: TINFO: Timeout per run is 0h 05m 00s
tst_test.c:1115: TINFO: ext2/ext3/ext4 is supported by the test
tst_device.c:88: TINFO: Found free device 0 '/dev/loop0'
loop0: detected capacity change from 0 to 2048
tst_device.c:522: TINFO: Debug: Device numbers: 8/0
tst_device.c:527: TINFO: Debug: 14 1 8:0 / / rw,relatime - ext2 /dev/root rw
tst_device.c:543: TINFO: Debug: devneme=/dev/root
tst_device.c:548: TWARN: stat(/dev/root) failed: ENOENT (2)

Summary:
passed 0
failed 0
broken 0
skipped 0
warnings 1
# ln -s /dev/sda /dev/root
# /usr/lib/ltp-testsuite/testcases/bin/ioctl_loop05
tst_test.c:1363: TINFO: Timeout per run is 0h 05m 00s
tst_test.c:1115: TINFO: ext2/ext3/ext4 is supported by the test
tst_device.c:88: TINFO: Found free device 0 '/dev/loop0'
loop0: detected capacity change from 0 to 2048
tst_device.c:522: TINFO: Debug: Device numbers: 8/0
tst_device.c:527: TINFO: Debug: 14 1 8:0 / / rw,relatime - ext2 /dev/root rw
tst_device.c:543: TINFO: Debug: devneme=/dev/root
ioctl_loop05.c:126: TINFO: backing dev(/dev/root) logical_block_size is 512
ioctl_loop05.c:62: TINFO: Without setting lo_offset or sizelimit
ioctl_loop05.c:45: TINFO: In dio mode
ioctl_loop05.c:48: TPASS: lo_flags has LO_FLAGS_DIRECT_IO flag
ioctl_loop05.c:52: TPASS: /sys/block/loop0/loop/dio = 1
ioctl_loop05.c:45: TINFO: In non dio mode
ioctl_loop05.c:50: TPASS: lo_flags doesn't have LO_FLAGS_DIRECT_IO flag
ioctl_loop05.c:52: TPASS: /sys/block/loop0/loop/dio = 0
ioctl_loop05.c:69: TINFO: With offset equal to logical_block_size
loop0: detected capacity change from 2048 to 2047
ioctl_loop05.c:74: TPASS: LOOP_SET_DIRECT_IO succeeded
ioctl_loop05.c:45: TINFO: In dio mode
ioctl_loop05.c:48: TPASS: lo_flags has LO_FLAGS_DIRECT_IO flag
ioctl_loop05.c:52: TPASS: /sys/block/loop0/loop/dio = 1
ioctl_loop05.c:81: TINFO: With nonzero offset less than logical_block_size
ioctl_loop05.c:92: TPASS: LOOP_SET_DIRECT_IO failed as expected: EINVAL (22)

Summary:
passed 8
failed 0
broken 0
skipped 0
warnings 0

here the same system on a btrfs

VFS: Mounted root (btrfs filesystem) on device 0:13.
random: fast init done
devtmpfs: mounted
Freeing unused kernel image (initmem) memory: 956K
Write protecting the kernel read-only data: 14336k
Freeing unused kernel image (text/rodata gap) memory: 2044K
Freeing unused kernel image (rodata/data gap) memory: 48K
Run /sbin/init as init process
BTRFS info (device sda): disk space caching is enabled
Starting syslogd: OK
Starting klogd: OK
Running sysctl: OK
Saving random seed: random: dd: uninitialized urandom read (512 bytes read)
OK
Starting network: Waiting for interface eth0 to appear...............
timeout!
run-parts: /etc/network/if-pre-up.d/wait_iface: exit status 1
FAIL

Welcome to Buildroot
buildroot login: root
# kill 71
# umount /tmp
# umount /run
# /usr/lib/ltp-testsuite/testcases/bin/ioctl_loop05
tst_test.c:1363: TINFO: Timeout per run is 0h 05m 00s
tst_test.c:1115: TINFO: btrfs is supported by the test
tst_device.c:88: TINFO: Found free device 0 '/dev/loop0'
loop0: detected capacity change from 0 to 2048
tst_device.c:522: TINFO: Debug: Device numbers: 0/14
tst_device.c:527: TINFO: Debug: 15 1 0:13 / / rw,relatime - btrfs /dev/root
rw,space_cache,subvolid=5,subvol=/
tst_device.c:527: TINFO: Debug: 16 15 0:5 / /dev rw,relatime - devtmpfs
devtmpfs rw,size=505416k,nr_inodes=126354,mode=755
tst_device.c:527: TINFO: Debug: 14 15 0:15 / /proc rw,relatime - proc proc
rw
tst_device.c:527: TINFO: Debug: 17 16 0:16 / /dev/pts rw,relatime - devpts
devpts rw,gid=5,mode=620,ptmxmode=666
tst_device.c:527: TINFO: Debug: 18 16 0:17 / /dev/shm rw,relatime - tmpfs
tmpfs rw,mode=777
tst_device.c:527: TINFO: Debug: 21 15 0:20 / /sys rw,relatime - sysfs sysfs
rw
tst_device.c:543: TINFO: Debug: devneme=
tst_device.c:545: TBROK: Cannot find block device for
/tmp/iocz4RUVG/test.img

Summary:
passed 0
failed 0
broken 1
skipped 0
warnings 0
# ln -s /dev/sda /dev/root
# /usr/lib/ltp-testsuite/testcases/bin/ioctl_loop05
tst_test.c:1363: TINFO: Timeout per run is 0h 05m 00s
tst_test.c:1115: TINFO: btrfs is supported by the test
tst_device.c:88: TINFO: Found free device 0 '/dev/loop0'
loop0: detected capacity change from 0 to 2048
tst_device.c:522: TINFO: Debug: Device numbers: 0/14
tst_device.c:527: TINFO: Debug: 15 1 0:13 / / rw,relatime - btrfs /dev/root
rw,space_cache,subvolid=5,subvol=/
tst_device.c:527: TINFO: Debug: 16 15 0:5 / /dev rw,relatime - devtmpfs
devtmpfs rw,size=505416k,nr_inodes=126354,mode=755
tst_device.c:527: TINFO: Debug: 14 15 0:15 / /proc rw,relatime - proc proc
rw
tst_device.c:527: TINFO: Debug: 17 16 0:16 / /dev/pts rw,relatime - devpts
devpts rw,gid=5,mode=620,ptmxmode=666
tst_device.c:527: TINFO: Debug: 18 16 0:17 / /dev/shm rw,relatime - tmpfs
tmpfs rw,mode=777
tst_device.c:527: TINFO: Debug: 21 15 0:20 / /sys rw,relatime - sysfs sysfs
rw
tst_device.c:543: TINFO: Debug: devneme=
tst_device.c:545: TBROK: Cannot find block device for
/tmp/ioctVo52r/test.img

Summary:
passed 0
failed 0
broken 1
skipped 0
warnings 0
#

a stat on the /, results in a couple of minor/major that is different from
the
couple listed in the mountinfo. This leads, as a result, a test failure.

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

> >
> > $ sudo /usr/lib/ltp-testsuite/testcases/bin/ioctl_loop05
> > tst_test.c:1363: TINFO: Timeout per run is 0h 05m 00s
> > tst_test.c:1115: TINFO: ext2/ext3/ext4 is supported by the test
> > tst_device.c:91: TINFO: Found free device 0 '/dev/loop0'
> > loop0: detected capacity change from 0 to 2048
> > tst_device.c:570: TINFO: Device name retrived through mountinfo
> > ioctl_loop05.c:126: TINFO: backing dev(/dev/root) logical_block_size is
> 512
> > ioctl_loop05.c:62: TINFO: Without setting lo_offset or sizelimit
> > ioctl_loop05.c:45: TINFO: In dio mode
> > ioctl_loop05.c:48: TPASS: lo_flags has LO_FLAGS_DIRECT_IO flag
> > ioctl_loop05.c:52: TBROK: Failed to open FILE
> '/sys/block/loop0/loop/dio' for reading: ENOENT (2)
> >
> > Summary:
> > passed 1
> > failed 0
> > broken 1
> > skipped 0
> > warnings 0
> >
> > The example here shows what happen if sysfs is not mounted and the
> > mountinfo method is used: the tests that depends on sysfs fail with
> > broken and not just warn.
> >
> > Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
> > Signed-off-by: Alessandro Carminati <alessandro.carminati@gmail.com>
> > ---
> > lib/tst_device.c | 41 ++++++++++-------------------------------
> > 1 file changed, 10 insertions(+), 31 deletions(-)
> >
> > diff --git a/lib/tst_device.c b/lib/tst_device.c
> > index 8419b80c3..4ccb71ac8 100644
> > --- a/lib/tst_device.c
> > +++ b/lib/tst_device.c
> > @@ -520,48 +520,27 @@ void tst_find_backing_dev(const char *path, char
> *dev)
> > {
> > struct stat buf;
> > FILE *file;
> > - char line[PATH_MAX];
> > - char *pre = NULL;
> > - char *next = NULL;
> > - unsigned int dev_major, dev_minor, line_mjr, line_mnr;
> > - unsigned int len, best_match_len = 1;
> > - char mnt_point[PATH_MAX];
> > + unsigned int dev_major, dev_minor;
> > + char uevent_path[PATH_MAX];
> > + char dev_name[NAME_MAX];
> >
> > if (stat(path, &buf) < 0)
> > tst_brkm(TWARN | TERRNO, NULL, "stat() failed");
> >
> > + *dev = '\0';
> > dev_major = major(buf.st_dev);
> > dev_minor = minor(buf.st_dev);
> > - file = SAFE_FOPEN(NULL, "/proc/self/mountinfo", "r");
> > - *dev = '\0';
> > -
> > - while (fgets(line, sizeof(line), file)) {
> > - if (sscanf(line, "%*d %*d %d:%d %*s %s",
> > - &line_mjr, &line_mnr, mnt_point) != 3)
> > - continue;
> >
> > - pre = strstr(line, " - ");
> > - pre = strtok_r(pre, " ", &next);
> > - pre = strtok_r(NULL, " ", &next);
> > - pre = strtok_r(NULL, " ", &next);
> > + sprintf(uevent_path,
> > + "/sys/dev/block/%d:%d/uevent", dev_major, dev_minor);
> >
> > - if (line_mjr == dev_major && line_mnr == dev_minor) {
> > - strcpy(dev, pre);
> > - break;
> > - }
> > + if (!access(uevent_path, R_OK)) {
> > + FILE_LINES_SCANF(NULL, uevent_path, "DEVNAME=%s",
> dev_name);
> >
> > - len = count_match_len(path, mnt_point);
> > - if (len > best_match_len) {
> > - strcpy(dev, pre);
> > - best_match_len = len;
> > - }
> > + if (dev_name[0])
> > + sprintf(dev, "/dev/%s", dev_name);
> > }
> >
> > - SAFE_FCLOSE(NULL, file);
> > -
> > - if (!*dev)
> > - tst_brkm(TBROK, NULL, "Cannot find block device for %s",
> path);
> > -
> > if (stat(dev, &buf) < 0)
> > tst_brkm(TWARN | TERRNO, NULL, "stat(%s) failed", dev);
>
>
> --
> Thank you,
> Richard.
>

Thank you for your time.
Alessandro

[-- Attachment #1.2: Type: text/html, Size: 15292 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]