| Message ID | 20221026140408.471609-2-alessandro.carminati@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tst_find_backing_dev: fix stat fails /dev/root | expand |
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);
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
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. > > > > > 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... 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.
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
Hello, Alessandro Carminati <alessandro.carminati@gmail.com> writes: > 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: Interesting, thanks. > > | Filesystem | tested | notes > +---------------+--------+---------------------------- > |nfs | no | not tested, no idea. > |9p | no | not tested, no idea. Indeed I have no idea what running LTP on networked FS (including Ceph) produces. > |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 suppose CephFS would also, but we make no attempt to support that in LTP. > 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? Only the complication of adding a special approach for BTRFS. If we find more FS with this issue, then we may have to add code for them also. I would accept this approach though, because it is more direct and it's not clear that scanning mountinfo will work either. > >> >> > >> > > >> > > 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? The major and minor are never correct in mountinfo, but the device path is correct most of the time. This can be used to get the real major and minor if necessary or get the sector size as in this case. To verify that it works, you can run the test with TMPDIR set to a path on BTRFS. For e.g. using a loop device with BTRFS. $ cd /tmp $ fallocate -l 300MiB btrfs.img $ losetup -f btrfs.img $ mkfs.btrfs /dev/loop0 $ mkdir btrfs $ mount -t btrfs /dev/loop0 btrfs $ export TMPDIR=/tmp/btrfs $ ioctl_loop05 > >> 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
This tst_find_backing_dev patch version adds support for btrfs filesystems. Strategy here is to understand if the subject is btrfs or not. btrfs uses virtual device, and the major is 0. In all other test supported file systems the major is always non zero. If non btrfs it proceed as the previous version using sysfs minor and major. If btrfs it uses the BTRFS_IOC_FS_INFO ioctl to get it FS uuid. non btrfs: ---------------------------------------------------------------- EXT4-fs (sda): re-mounted. Opts: (null). Quota mode: none. 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:90: TINFO: Found free device 0 '/dev/loop0' loop0: detected capacity change from 0 to 2048 tst_device.c:579: TINFO: Device numbers: 8/0 tst_device.c:611: TINFO: Use uevent strategy ioctl_loop05.c:126: TINFO: backing dev(/dev/sda) 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 # btrfs ---------------------------------------------------------------- BTRFS info (device sda): disk space caching is enabled 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: btrfs is supported by the test tst_device.c:90: TINFO: Found free device 0 '/dev/loop0' loop0: detected capacity change from 0 to 2048 tst_device.c:579: TINFO: Device numbers: 0/15 tst_device.c:582: TINFO: Use BTRFS specific strategy ioctl_loop05.c:126: TINFO: backing dev(/dev/sda) 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:87: TPASS: LOOP_SET_DIRECT_IO succeeded, offset is ignored Summary: passed 8 failed 0 broken 0 skipped 0 warnings 0 # Alessandro Carminati (2): tst_find_backing_dev: Get dev name from /sys/dev/block/*/uevent c-test-api: Documentation updated doc/c-test-api.txt | 7 ++-- lib/tst_device.c | 86 ++++++++++++++++++++++++++++++---------------- 2 files changed, 62 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);