All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alessandro Carminati <alessandro.carminati@gmail.com>
To: rpalethorpe@suse.de
Cc: acarmina@redhat.com, ltp@lists.linux.it
Subject: Re: [LTP] [PATCH 1/2] tst_find_backing_dev: Get dev name from /sys/dev/block/*/uevent
Date: Thu, 27 Oct 2022 20:58:31 +0200	[thread overview]
Message-ID: <CAPp5cGQ++39EMSV3MrMTTKrTyn5WWRou=yP7f4jbBQ7p-F+cyA@mail.gmail.com> (raw)
In-Reply-To: <87fsf9sk2e.fsf@suse.de>


[-- 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 --]


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

  reply	other threads:[~2022-10-27 18:59 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <[LTP] [PATCH] Fix tst_find_backing_dev when no initramfs>
2022-10-26 14:04 ` [LTP] [PATCH 0/2] tst_find_backing_dev: fix stat fails /dev/root Alessandro Carminati
2022-10-26 14:04   ` [LTP] [PATCH 1/2] tst_find_backing_dev: Get dev name from /sys/dev/block/*/uevent Alessandro Carminati
2022-10-27  8:08     ` Richard Palethorpe
2022-10-27 18:58       ` Alessandro Carminati [this message]
2022-10-31 12:08         ` Richard Palethorpe
2022-10-31 16:57           ` Alessandro Carminati
2022-11-01  9:09             ` Richard Palethorpe
2022-11-02 20:34               ` [LTP] [PATCH 0/2] tst_find_backing_dev: fix stat fails /dev/root Alessandro Carminati
2022-11-02 20:34                 ` [LTP] [PATCH 1/2] tst_find_backing_dev: Get dev name from /sys/dev/block/*/uevent Alessandro Carminati
2022-11-03  8:04                   ` Richard Palethorpe
2022-11-04  4:41                     ` [LTP] [PATCH 0/2] Fix tst_find_backing_dev adding BTRFS support and /dev/root missing case Alessandro Carminati
2022-11-04  4:41                       ` [LTP] [PATCH 1/2] tst_find_backing_dev: Get dev name from /sys/*/uevent Alessandro Carminati
2022-11-07  8:46                         ` Richard Palethorpe
2022-11-07 16:39                           ` [LTP] [PATCH 0/2] Fix tst_find_backing_dev adding BTRFS support and /dev/root missing case Alessandro Carminati
2022-11-07 16:39                             ` [LTP] [PATCH 1/2] tst_find_backing_dev: Get dev name from /sys/dev/block/*/uevent Alessandro Carminati
2022-11-08  9:39                               ` Richard Palethorpe
2022-11-09 19:38                                 ` [LTP] [PATCH v6 0/2] tst_find_backing_dev: fix stat fails /dev/root Alessandro Carminati
2022-11-09 19:38                                   ` [LTP] [PATCH v6 1/2] tst_find_backing_dev: Get dev name from /sys/dev/block/*/uevent Alessandro Carminati
2022-11-10 11:16                                     ` Richard Palethorpe
2022-11-09 19:38                                   ` [LTP] [PATCH v6 2/2] c-test-api: Documentation updated Alessandro Carminati
2022-11-10 11:29                                     ` Richard Palethorpe
2022-11-07 16:39                             ` [LTP] [PATCH " Alessandro Carminati
2022-11-04  4:41                       ` Alessandro Carminati
2022-11-07  8:12                       ` [LTP] [PATCH 0/2] Fix tst_find_backing_dev adding BTRFS support and /dev/root missing case Richard Palethorpe
2022-11-02 20:34                 ` [LTP] [PATCH 2/2] c-test-api: Documentation updated Alessandro Carminati
2022-10-26 14:04   ` Alessandro Carminati

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAPp5cGQ++39EMSV3MrMTTKrTyn5WWRou=yP7f4jbBQ7p-F+cyA@mail.gmail.com' \
    --to=alessandro.carminati@gmail.com \
    --cc=acarmina@redhat.com \
    --cc=ltp@lists.linux.it \
    --cc=rpalethorpe@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.