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

Richard Palethorpe
2022-10-27T09:16:49Z

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

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.

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