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

In some minimal Linux, the /dev/root can be missing. The consequence of
this is that mountinfo doesn't contain the correct information. btrfs
file systems are yet another point of trouble for this function.

The unevent file in sysfs is another method to retrieve device info
using the sysfs.

btrfs file systems are special from the device name retrieval, and in
place of use of the minor/major they are approached by using the uuid.
In the end, btrfs strategy is a slightly modified version of the same
unevent strategy.

Non btrfs look in "/sys/dev/block/%d:%d/uevent" major, minor
btrfs look in /sys/fs/btrfs/%s/devices/%s/uevent, uuid, devname

The btrfs handling requires BTRFS specific ioctl for finding the
file system uuid, and for this reason, btrfs/ioctl.h is needed.

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
Signed-off-by: Alessandro Carminati <alessandro.carminati@gmail.com>
---
 lib/tst_device.c | 91 ++++++++++++++++++++++++++++++++----------------
 1 file changed, 61 insertions(+), 30 deletions(-)

Hello,

I'd like to merge, but discovered some more issues that requuire more
than a fixup before merge.

Also please use the -v flag in git format-patch to version the patches
after the first revision. I don't mind which version you start at now.

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

> In some minimal Linux, the /dev/root can be missing. The consequence of
> this is that mountinfo doesn't contain the correct information. btrfs
> file systems are yet another point of trouble for this function.
>
> The unevent file in sysfs is another method to retrieve device info
> using the sysfs.
>
> btrfs file systems are special from the device name retrieval, and in
> place of use of the minor/major they are approached by using the uuid.
> In the end, btrfs strategy is a slightly modified version of the same
> unevent strategy.
>
> Non btrfs look in "/sys/dev/block/%d:%d/uevent" major, minor
> btrfs look in /sys/fs/btrfs/%s/devices/%s/uevent, uuid, devname
>
> The btrfs handling requires BTRFS specific ioctl for finding the
> file system uuid, and for this reason, btrfs/ioctl.h is needed.
>
> Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
> Signed-off-by: Alessandro Carminati <alessandro.carminati@gmail.com>
> ---
>  lib/tst_device.c | 91 ++++++++++++++++++++++++++++++++----------------
>  1 file changed, 61 insertions(+), 30 deletions(-)
>
> diff --git a/lib/tst_device.c b/lib/tst_device.c
> index 8419b80c3..054e39bcd 100644
> --- a/lib/tst_device.c
> +++ b/lib/tst_device.c
> @@ -33,6 +33,9 @@
>  #include <stdint.h>
>  #include <inttypes.h>
>  #include <sys/sysmacros.h>
> +#include <linux/btrfs.h>
> +#include <linux/limits.h>
> +#include <dirent.h>
>  #include "lapi/syscalls.h"
>  #include "test.h"
>  #include "safe_macros.h"
> @@ -45,6 +48,8 @@
>  
>  #define DEV_FILE "test_dev.img"
>  #define DEV_SIZE_MB 300u
> +#define UUID_STR_SZ 37
> +#define UUID_FMT "%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x"
>  
>  static char dev_path[1024];
>  static int device_acquired;
> @@ -519,48 +524,74 @@ static int count_match_len(const char *first, const char *second)
>  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];
> +	struct btrfs_ioctl_fs_info_args args = {0};
> +	struct dirent *d;
> +	char uevent_path[PATH_MAX];
> +	char dev_name[NAME_MAX];
> +	char bdev_path[PATH_MAX];
> +	char btrfs_uuid_str[UUID_STR_SZ];
> +	DIR *dir;
> +	unsigned int dev_major, dev_minor;
> +	int fd;
>  
>  	if (stat(path, &buf) < 0)
>  		tst_brkm(TWARN | TERRNO, NULL, "stat() failed");
>  
>  	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);
> -
> -		if (line_mjr == dev_major && line_mnr == dev_minor) {
> -			strcpy(dev, pre);
> -			break;
> -		}
> -
> -		len = count_match_len(path, mnt_point);
> -		if (len > best_match_len) {
> -			strcpy(dev, pre);
> -			best_match_len = len;
> +	if (dev_major == 0) {
> +		tst_resm(TINFO, "Use BTRFS specific strategy");
> +
> +		fd = SAFE_OPEN(NULL, dirname(path), O_DIRECTORY);

There are two problems here. One is simple and that dirname can modify
path, but path is a const pointer (compiler should warn about dropping
const modifiers). The simple solution is just to copy path into a buffer.

Secondly ioctl_loop05 passes the path to an image, but the self test in
/lib/newlib_tests/tst_device.c passes the mount point. So unless I am
mistaken dirname will return the dir below the mount point which is
wrong.

One option is to try opening path as a dir first and if that fails, use
dirname to get the containing folder. Changeing ioctl_loop05 would also
be valid.

> +		if (!ioctl(fd, BTRFS_IOC_FS_INFO, &args)) {
> +			sprintf(btrfs_uuid_str,
> +				UUID_FMT,
> +				args.fsid[0], args.fsid[1],
> +				args.fsid[2], args.fsid[3],
> +				args.fsid[4], args.fsid[5],
> +				args.fsid[6], args.fsid[7],
> +				args.fsid[8], args.fsid[9],
> +				args.fsid[10], args.fsid[11],
> +				args.fsid[12], args.fsid[13],
> +				args.fsid[14], args.fsid[15]);
> +			sprintf(bdev_path,
> +				"/sys/fs/btrfs/%s/devices", btrfs_uuid_str);
> +		} else {
> +			tst_brkm(TBROK, NULL, "BTRFS ioctl failed. Is %s
> on a tmpfs?", path);
Need TERRNO here and/or check that the errorno is ENOTTY otherwise the
hint makes no sense.
> +			}
> +		SAFE_CLOSE(NULL, fd);
> +		dir = SAFE_OPENDIR(NULL, bdev_path);
> +		while (d = SAFE_READDIR(NULL, dir)) {
> +			if (d->d_name[0]!='.')

There are a few formatting errors like the missing spaces around !=.

Run make check-tst_device in the lib dir and see the kernel style
guidelines.

> +				break;
>  		}
> +		uevent_path[0] = '\0';
> +		if (d) {
> +			sprintf(uevent_path, "%s/%s/uevent",
> +				bdev_path, d->d_name);
> +		} else {
> +			tst_brkm(TBROK, NULL, "No backining device
> found");

Still need to print some information about where we are looking (bdev_path).

> +			}
> +		if (SAFE_READDIR(NULL, dir))
> +			tst_resm(TINFO, "Warning: used first of multiple backing device.");
> +		SAFE_CLOSEDIR(NULL, dir);
> +	} else {
> +
> +		tst_resm(TINFO, "Use uevent strategy");
> +		sprintf(uevent_path,
> +			"/sys/dev/block/%d:%d/uevent", dev_major, dev_minor);
>  	}
>  
> -	SAFE_FCLOSE(NULL, file);
> +	if (!access(uevent_path, R_OK)) {
> +		FILE_LINES_SCANF(NULL, uevent_path, "DEVNAME=%s", dev_name);
>  
> -	if (!*dev)
> -		tst_brkm(TBROK, NULL, "Cannot find block device for %s", path);
> +		if (dev_name[0])
> +			sprintf(dev, "/dev/%s", dev_name);
> +	} else {
> +		tst_brkm(TBROK, NULL, "uevent file (%s) access failed",
> uevent_path);

Also we can use (TBROK | TERRNO) here as access sets that.

> +		}

make check somehow missing this. The } is indented too far.

>  
>  	if (stat(dev, &buf) < 0)
>  		tst_brkm(TWARN | TERRNO, NULL, "stat(%s) failed", dev);