* [LTP] [PATCH] Fix tst_find_backing_dev when no initramfs @ 2022-10-17 11:05 Alessandro Carminati 2022-10-24 13:49 ` Richard Palethorpe 0 siblings, 1 reply; 7+ messages in thread From: Alessandro Carminati @ 2022-10-17 11:05 UTC (permalink / raw) To: ltp mount_root() is the kernel function responsible for mounting the primary rootfs. A dynamic there, prevents the /dev/root device node in the not yet mounted files system. For this reason, in the embedded system that starts without an initramfs, or however a proper initscript, the /dev/root device appears into the mount table in the / line. The test tries to open this /dev/root and fails with a warning. This patch aims to fix this situation. Signed-off-by: Alessandro Carminati <alessandro.carminati@gmail.com> typo fixes --- lib/tst_device.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/lib/tst_device.c b/lib/tst_device.c index 8419b80c3..c3427eb31 100644 --- a/lib/tst_device.c +++ b/lib/tst_device.c @@ -526,6 +526,8 @@ void tst_find_backing_dev(const char *path, char *dev) unsigned int dev_major, dev_minor, line_mjr, line_mnr; unsigned int len, best_match_len = 1; char mnt_point[PATH_MAX]; + char tmpbuf1[PATH_MAX]; + char tmpbuf2[PATH_MAX]; if (stat(path, &buf) < 0) tst_brkm(TWARN | TERRNO, NULL, "stat() failed"); @@ -562,6 +564,24 @@ void tst_find_backing_dev(const char *path, char *dev) if (!*dev) tst_brkm(TBROK, NULL, "Cannot find block device for %s", path); + if (stat(dev, &buf) < 0) { + if (strcmp("/dev/root", dev) != 0) { + tst_brkm(TWARN | TERRNO, NULL, "stat(%s) failed", dev); + } else { + sprintf(tmpbuf1, "/sys/dev/block/%d:%d/uevent", dev_major, dev_minor); + file = SAFE_FOPEN(NULL, tmpbuf1, "r"); + while (fgets(line, sizeof(line), file)) { + if (sscanf(line, "%[^=]=%s", tmpbuf1, tmpbuf2) != 2) + continue; + if (strcmp("DEVNAME", tmpbuf1) == 0) { + sprintf(dev, "/dev/%s", tmpbuf2); + break; + } + } + SAFE_FCLOSE(NULL, file); + } + } + if (stat(dev, &buf) < 0) tst_brkm(TWARN | TERRNO, NULL, "stat(%s) failed", dev); -- 2.34.1 -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [LTP] [PATCH] Fix tst_find_backing_dev when no initramfs 2022-10-17 11:05 [LTP] [PATCH] Fix tst_find_backing_dev when no initramfs Alessandro Carminati @ 2022-10-24 13:49 ` Richard Palethorpe 2022-10-25 9:11 ` Alessandro Carminati 0 siblings, 1 reply; 7+ messages in thread From: Richard Palethorpe @ 2022-10-24 13:49 UTC (permalink / raw) To: Alessandro Carminati; +Cc: ltp Hello, Alessandro Carminati <alessandro.carminati@gmail.com> writes: > mount_root() is the kernel function responsible for mounting the primary > rootfs. > A dynamic there, prevents the /dev/root device node in the not yet mounted > files system. For this reason, in the embedded system that starts without > an initramfs, or however a proper initscript, the /dev/root device appears > into the mount table in the / line. > The test tries to open this /dev/root and fails with a warning. > This patch aims to fix this situation. Thanks I probably would have hit this issue sooner or later. > > Signed-off-by: Alessandro Carminati <alessandro.carminati@gmail.com> > > typo fixes > --- > lib/tst_device.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/lib/tst_device.c b/lib/tst_device.c > index 8419b80c3..c3427eb31 100644 > --- a/lib/tst_device.c > +++ b/lib/tst_device.c > @@ -526,6 +526,8 @@ void tst_find_backing_dev(const char *path, char *dev) > unsigned int dev_major, dev_minor, line_mjr, line_mnr; > unsigned int len, best_match_len = 1; > char mnt_point[PATH_MAX]; > + char tmpbuf1[PATH_MAX]; > + char tmpbuf2[PATH_MAX]; It would be more readable to use three buffers and give them meaningful names. > > if (stat(path, &buf) < 0) > tst_brkm(TWARN | TERRNO, NULL, "stat() failed"); > @@ -562,6 +564,24 @@ void tst_find_backing_dev(const char *path, char *dev) > if (!*dev) > tst_brkm(TBROK, NULL, "Cannot find block device for %s", path); > > + if (stat(dev, &buf) < 0) { As there is no harm in calling stat twice; could just do if (stat(dev, &buf) < 0) && strcmp("/dev/root", dev) == 0) { Or even replace with strcmp with errno == ENOENT and use this as a general fallback. > + if (strcmp("/dev/root", dev) != 0) { > + tst_brkm(TWARN | TERRNO, NULL, "stat(%s) failed", dev); > + } else { Then remove this if statement > + sprintf(tmpbuf1, "/sys/dev/block/%d:%d/uevent", > dev_major, dev_minor); > + file = SAFE_FOPEN(NULL, tmpbuf1, "r"); > + while (fgets(line, sizeof(line), file)) { > + if (sscanf(line, "%[^=]=%s", tmpbuf1, tmpbuf2) != 2) > + continue; > + if (strcmp("DEVNAME", tmpbuf1) == 0) { > + sprintf(dev, "/dev/%s", tmpbuf2); > + break; > + } > + } > + SAFE_FCLOSE(NULL, file); > + } > + } > + > if (stat(dev, &buf) < 0) > tst_brkm(TWARN | TERRNO, NULL, "stat(%s) failed", dev); > > -- > 2.34.1 -- Thank you, Richard. -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [LTP] [PATCH] Fix tst_find_backing_dev when no initramfs 2022-10-24 13:49 ` Richard Palethorpe @ 2022-10-25 9:11 ` Alessandro Carminati 2022-10-25 14:59 ` [LTP] [PATCH] tst_find_backing_dev: Get dev name from /sys/dev/block/*/uevent Richard Palethorpe via ltp 0 siblings, 1 reply; 7+ messages in thread From: Alessandro Carminati @ 2022-10-25 9:11 UTC (permalink / raw) To: Richard Palethorpe; +Cc: ltp mount_root() is the kernel function responsible for mounting the primary rootfs. A dynamic there, prevents the /dev/root device node in the not yet mounted files system. For this reason, in the embedded system that starts without an initramfs, or however a proper initscript, the /dev/root device appears into the mount table in the / line. The test ioctl_loop05 tries to open this /dev/root and fails with a warning. This patch aims to fix this situation. Signed-off-by: Alessandro Carminati <alessandro.carminati@gmail.com> --- lib/tst_device.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/lib/tst_device.c b/lib/tst_device.c index 8419b80c3..c67328db3 100644 --- a/lib/tst_device.c +++ b/lib/tst_device.c @@ -526,6 +526,8 @@ void tst_find_backing_dev(const char *path, char *dev) unsigned int dev_major, dev_minor, line_mjr, line_mnr; unsigned int len, best_match_len = 1; char mnt_point[PATH_MAX]; + char devnum_info_fn[PATH_MAX]; + char new_dev_fn_buf[PATH_MAX]; if (stat(path, &buf) < 0) tst_brkm(TWARN | TERRNO, NULL, "stat() failed"); @@ -562,6 +564,20 @@ void tst_find_backing_dev(const char *path, char *dev) if (!*dev) tst_brkm(TBROK, NULL, "Cannot find block device for %s", path); + if (stat(dev, &buf) < 0) && (strcmp("/dev/root", dev)==0) { + sprintf(devnum_info_fn, "/sys/dev/block/%d:%d/uevent", dev_major, dev_minor); + file = SAFE_FOPEN(NULL, devnum_info_fn, "r"); + while (fgets(line, sizeof(line), file)) { + if (sscanf(line, "%[^=]=%s", devnum_info_fn, new_dev_fn_buf) != 2) + continue; + if (strcmp("DEVNAME", devnum_info_fn) == 0) { + sprintf(dev, "/dev/%s", new_dev_fn_buf); + break; + } + } + SAFE_FCLOSE(NULL, file); + } + if (stat(dev, &buf) < 0) tst_brkm(TWARN | TERRNO, NULL, "stat(%s) failed", dev); -- 2.34.1 On Mon, Oct 24, 2022 at 02:49:32PM +0100, Richard Palethorpe wrote: > Hello, > > Alessandro Carminati <alessandro.carminati@gmail.com> writes: > > > mount_root() is the kernel function responsible for mounting the primary > > rootfs. > > A dynamic there, prevents the /dev/root device node in the not yet mounted > > files system. For this reason, in the embedded system that starts without > > an initramfs, or however a proper initscript, the /dev/root device appears > > into the mount table in the / line. > > The test tries to open this /dev/root and fails with a warning. > > This patch aims to fix this situation. > > Thanks I probably would have hit this issue sooner or later. > > > > > Signed-off-by: Alessandro Carminati <alessandro.carminati@gmail.com> > > > > typo fixes > > --- > > lib/tst_device.c | 20 ++++++++++++++++++++ > > 1 file changed, 20 insertions(+) > > > > diff --git a/lib/tst_device.c b/lib/tst_device.c > > index 8419b80c3..c3427eb31 100644 > > --- a/lib/tst_device.c > > +++ b/lib/tst_device.c > > @@ -526,6 +526,8 @@ void tst_find_backing_dev(const char *path, char *dev) > > unsigned int dev_major, dev_minor, line_mjr, line_mnr; > > unsigned int len, best_match_len = 1; > > char mnt_point[PATH_MAX]; > > + char tmpbuf1[PATH_MAX]; > > + char tmpbuf2[PATH_MAX]; > > It would be more readable to use three buffers and give them meaningful names. > > > > > if (stat(path, &buf) < 0) > > tst_brkm(TWARN | TERRNO, NULL, "stat() failed"); > > @@ -562,6 +564,24 @@ void tst_find_backing_dev(const char *path, char *dev) > > if (!*dev) > > tst_brkm(TBROK, NULL, "Cannot find block device for %s", path); > > > > + if (stat(dev, &buf) < 0) { > > As there is no harm in calling stat twice; could just do > > if (stat(dev, &buf) < 0) && strcmp("/dev/root", dev) == 0) { > > Or even replace with strcmp with errno == ENOENT and use this as a > general fallback. > > > + if (strcmp("/dev/root", dev) != 0) { > > + tst_brkm(TWARN | TERRNO, NULL, "stat(%s) failed", dev); > > + } else { > > Then remove this if statement > > > + sprintf(tmpbuf1, "/sys/dev/block/%d:%d/uevent", > > dev_major, dev_minor); > > + file = SAFE_FOPEN(NULL, tmpbuf1, "r"); > > + while (fgets(line, sizeof(line), file)) { > > + if (sscanf(line, "%[^=]=%s", tmpbuf1, tmpbuf2) != 2) > > + continue; > > + if (strcmp("DEVNAME", tmpbuf1) == 0) { > > + sprintf(dev, "/dev/%s", tmpbuf2); > > + break; > > + } > > + } > > + SAFE_FCLOSE(NULL, file); > > + } > > + } > > + > > if (stat(dev, &buf) < 0) > > tst_brkm(TWARN | TERRNO, NULL, "stat(%s) failed", dev); > > > > -- > > 2.34.1 > > > -- > Thank you, > Richard. -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [LTP] [PATCH] tst_find_backing_dev: Get dev name from /sys/dev/block/*/uevent 2022-10-25 9:11 ` Alessandro Carminati @ 2022-10-25 14:59 ` Richard Palethorpe via ltp 2022-10-25 20:13 ` Alessandro Carminati 2022-10-26 10:56 ` Jan Stancek 0 siblings, 2 replies; 7+ messages in thread From: Richard Palethorpe via ltp @ 2022-10-25 14:59 UTC (permalink / raw) To: ltp; +Cc: Alessandro Carminati, Richard Palethorpe mountinfo doesn't always contain the correct device name. For example /dev/root may be displayed, but not exist in devtmpfs[1]. The unevent file in sysfs is another way of finding the device name from the major and minor numbers. Possibly it always displays the proper device name. One caveat is the sysfs can be disabled, so this commit does not remove the mountinfo method altogether, but leaves it as a fallback. Alessandro Carminati originally sent two patches[1] which added the uevent file method as a fallback. However it seems like the better method. [1]: https://lore.kernel.org/ltp/Y0023HcAOlhfAcJw@lab.hqhome163.com/ Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com> Suggested-by: Alessandro Carminati <alessandro.carminati@gmail.com> Reported-by: Alessandro Carminati <alessandro.carminati@gmail.com> --- Alessandro, it seems you tried to edit the last patch by hand? In any case it did not apply and I ended up making some other changes. So I took the liberty of submitting a new patch. Thanks, Richard. lib/tst_device.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/lib/tst_device.c b/lib/tst_device.c index 8419b80c3..676903fff 100644 --- a/lib/tst_device.c +++ b/lib/tst_device.c @@ -526,14 +526,30 @@ void tst_find_backing_dev(const char *path, char *dev) unsigned int dev_major, dev_minor, line_mjr, line_mnr; unsigned int len, best_match_len = 1; char mnt_point[PATH_MAX]; + 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); + + sprintf(uevent_path, + "/sys/dev/block/%d:%d/uevent", dev_major, dev_minor); + + if (!access(uevent_path, R_OK)) { + FILE_LINES_SCANF(NULL, uevent_path, "DEVNAME=%s", dev_name); + + if (dev_name[0]) + sprintf(dev, "/dev/%s", dev_name); + } + + if (!stat(dev, &buf)) + goto out; + 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", @@ -564,7 +580,7 @@ void tst_find_backing_dev(const char *path, char *dev) if (stat(dev, &buf) < 0) tst_brkm(TWARN | TERRNO, NULL, "stat(%s) failed", dev); - +out: if (S_ISBLK(buf.st_mode) != 1) tst_brkm(TCONF, NULL, "dev(%s) isn't a block dev", dev); } -- 2.36.1 -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [LTP] [PATCH] tst_find_backing_dev: Get dev name from /sys/dev/block/*/uevent 2022-10-25 14:59 ` [LTP] [PATCH] tst_find_backing_dev: Get dev name from /sys/dev/block/*/uevent Richard Palethorpe via ltp @ 2022-10-25 20:13 ` Alessandro Carminati 2022-10-26 10:56 ` Jan Stancek 1 sibling, 0 replies; 7+ messages in thread From: Alessandro Carminati @ 2022-10-25 20:13 UTC (permalink / raw) To: Richard Palethorpe; +Cc: ltp [-- Attachment #1.1: Type: text/plain, Size: 3021 bytes --] Hello, Thank you this new version of the patch. in this moment doc/c-test-api.txt is out of sync with the code, I can handle its fix. Regards Alessandro On Tue, Oct 25, 2022, 16:59 Richard Palethorpe <rpalethorpe@suse.com> wrote: > mountinfo doesn't always contain the correct device name. For example > /dev/root may be displayed, but not exist in devtmpfs[1]. > > The unevent file in sysfs is another way of finding the device name > from the major and minor numbers. Possibly it always displays the > proper device name. > > One caveat is the sysfs can be disabled, so this commit does not > remove the mountinfo method altogether, but leaves it as a fallback. > > Alessandro Carminati originally sent two patches[1] which added the > uevent file method as a fallback. However it seems like the better > method. > > [1]: https://lore.kernel.org/ltp/Y0023HcAOlhfAcJw@lab.hqhome163.com/ > > Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com> > Suggested-by: Alessandro Carminati <alessandro.carminati@gmail.com> > Reported-by: Alessandro Carminati <alessandro.carminati@gmail.com> > --- > > Alessandro, it seems you tried to edit the last patch by hand? In any > case it did not apply and I ended up making some other changes. So I > took the liberty of submitting a new patch. > > Thanks, > Richard. > > lib/tst_device.c | 20 ++++++++++++++++++-- > 1 file changed, 18 insertions(+), 2 deletions(-) > > diff --git a/lib/tst_device.c b/lib/tst_device.c > index 8419b80c3..676903fff 100644 > --- a/lib/tst_device.c > +++ b/lib/tst_device.c > @@ -526,14 +526,30 @@ void tst_find_backing_dev(const char *path, char > *dev) > unsigned int dev_major, dev_minor, line_mjr, line_mnr; > unsigned int len, best_match_len = 1; > char mnt_point[PATH_MAX]; > + 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); > + > + sprintf(uevent_path, > + "/sys/dev/block/%d:%d/uevent", dev_major, dev_minor); > + > + if (!access(uevent_path, R_OK)) { > + FILE_LINES_SCANF(NULL, uevent_path, "DEVNAME=%s", > dev_name); > + > + if (dev_name[0]) > + sprintf(dev, "/dev/%s", dev_name); > + } > + > + if (!stat(dev, &buf)) > + goto out; > + > 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", > @@ -564,7 +580,7 @@ void tst_find_backing_dev(const char *path, char *dev) > > if (stat(dev, &buf) < 0) > tst_brkm(TWARN | TERRNO, NULL, "stat(%s) failed", dev); > - > +out: > if (S_ISBLK(buf.st_mode) != 1) > tst_brkm(TCONF, NULL, "dev(%s) isn't a block dev", dev); > } > -- > 2.36.1 > > [-- Attachment #1.2: Type: text/html, Size: 4357 bytes --] [-- Attachment #2: Type: text/plain, Size: 60 bytes --] -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [LTP] [PATCH] tst_find_backing_dev: Get dev name from /sys/dev/block/*/uevent 2022-10-25 14:59 ` [LTP] [PATCH] tst_find_backing_dev: Get dev name from /sys/dev/block/*/uevent Richard Palethorpe via ltp 2022-10-25 20:13 ` Alessandro Carminati @ 2022-10-26 10:56 ` Jan Stancek 2022-10-27 6:50 ` Alessandro Carminati 1 sibling, 1 reply; 7+ messages in thread From: Jan Stancek @ 2022-10-26 10:56 UTC (permalink / raw) To: Richard Palethorpe; +Cc: Alessandro Carminati, ltp On Tue, Oct 25, 2022 at 4:59 PM Richard Palethorpe <rpalethorpe@suse.com> wrote: > > mountinfo doesn't always contain the correct device name. For example > /dev/root may be displayed, but not exist in devtmpfs[1]. > > The unevent file in sysfs is another way of finding the device name ^^ small typo here > from the major and minor numbers. Possibly it always displays the > proper device name. major/minor numbers may still fail for btrfs, but that issue exists outside of scope of this patch. > > One caveat is the sysfs can be disabled, so this commit does not > remove the mountinfo method altogether, but leaves it as a fallback. Could we add some TINFO message to output, so we know where dev_name came from? (uevent vs mountinfo) > > Alessandro Carminati originally sent two patches[1] which added the > uevent file method as a fallback. However it seems like the better > method. > > [1]: https://lore.kernel.org/ltp/Y0023HcAOlhfAcJw@lab.hqhome163.com/ > > Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com> > Suggested-by: Alessandro Carminati <alessandro.carminati@gmail.com> > Reported-by: Alessandro Carminati <alessandro.carminati@gmail.com> Acked-by: Jan Stancek <jstancek@redhat.com> -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [LTP] [PATCH] tst_find_backing_dev: Get dev name from /sys/dev/block/*/uevent 2022-10-26 10:56 ` Jan Stancek @ 2022-10-27 6:50 ` Alessandro Carminati 0 siblings, 0 replies; 7+ messages in thread From: Alessandro Carminati @ 2022-10-27 6:50 UTC (permalink / raw) To: ltp [-- Attachment #1.1: Type: text/plain, Size: 1551 bytes --] Hello, Just proposed a new patch version for the same topic https://lore.kernel.org/ltp/20221026140408.471609-1-alessandro.carminati@gmail.com/ cheers Alessandro Carminati Il giorno mer 26 ott 2022 alle ore 12:56 Jan Stancek <jstancek@redhat.com> ha scritto: > On Tue, Oct 25, 2022 at 4:59 PM Richard Palethorpe <rpalethorpe@suse.com> > wrote: > > > > mountinfo doesn't always contain the correct device name. For example > > /dev/root may be displayed, but not exist in devtmpfs[1]. > > > > The unevent file in sysfs is another way of finding the device name > ^^ small typo here > > > from the major and minor numbers. Possibly it always displays the > > proper device name. > > major/minor numbers may still fail for btrfs, but that issue exists > outside of scope of this patch. > > > > > One caveat is the sysfs can be disabled, so this commit does not > > remove the mountinfo method altogether, but leaves it as a fallback. > > Could we add some TINFO message to output, so we know where dev_name > came from? (uevent vs mountinfo) > > > > > Alessandro Carminati originally sent two patches[1] which added the > > uevent file method as a fallback. However it seems like the better > > method. > > > > [1]: https://lore.kernel.org/ltp/Y0023HcAOlhfAcJw@lab.hqhome163.com/ > > > > Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com> > > Suggested-by: Alessandro Carminati <alessandro.carminati@gmail.com> > > Reported-by: Alessandro Carminati <alessandro.carminati@gmail.com> > > Acked-by: Jan Stancek <jstancek@redhat.com> > > [-- Attachment #1.2: Type: text/html, Size: 2793 bytes --] [-- Attachment #2: Type: text/plain, Size: 60 bytes --] -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-10-27 6:50 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-10-17 11:05 [LTP] [PATCH] Fix tst_find_backing_dev when no initramfs Alessandro Carminati 2022-10-24 13:49 ` Richard Palethorpe 2022-10-25 9:11 ` Alessandro Carminati 2022-10-25 14:59 ` [LTP] [PATCH] tst_find_backing_dev: Get dev name from /sys/dev/block/*/uevent Richard Palethorpe via ltp 2022-10-25 20:13 ` Alessandro Carminati 2022-10-26 10:56 ` Jan Stancek 2022-10-27 6:50 ` Alessandro Carminati
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.