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:

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

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