* [LTP] [PATCH 1/1] tst_mkfs: Add -I option to mkfs.vfat
@ 2021-03-29 14:57 Petr Vorel
2021-03-30 10:00 ` Richard Palethorpe
0 siblings, 1 reply; 6+ messages in thread
From: Petr Vorel @ 2021-03-29 14:57 UTC (permalink / raw)
To: ltp
to workaround occasional error on tests with .all_filesystems flag:
tst_test.c:888: TINFO: Formatting /dev/loop0 with vfat opts='' extra opts=''
mkfs.vfat: Partitions or virtual mappings on device '/dev/loop0',
not making filesystem (use -I to override)
tst_test.c:888: TBROK: mkfs.vfat failed with exit code 1
Tested also on BusyBox v1.33.0, which accept but ignores -I since
beginning (1.14.0).
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Richard also suggested to wipe the device, but not sure how to do it
quickly and don't bring any more dependency.
Kind regards,
Petr
lib/tst_mkfs.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/lib/tst_mkfs.c b/lib/tst_mkfs.c
index 736324f04..2d335a3ad 100644
--- a/lib/tst_mkfs.c
+++ b/lib/tst_mkfs.c
@@ -52,6 +52,16 @@ void tst_mkfs_(const char *file, const int lineno, void (cleanup_fn)(void),
snprintf(mkfs, sizeof(mkfs), "mkfs.%s", fs_type);
+ /*
+ * Workaround a problem:
+ * mkfs.vfat: Partitions or virtual mappings on device '/dev/loop0', not
+ * making filesystem (use -I to override)
+ */
+ if (!strcmp(fs_type, "vfat")) {
+ argv[pos] = "-I";
+ strcat(fs_opts_str, argv[pos++]);
+ }
+
if (fs_opts) {
for (i = 0; fs_opts[i]; i++) {
argv[pos++] = fs_opts[i];
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [LTP] [PATCH 1/1] tst_mkfs: Add -I option to mkfs.vfat
2021-03-29 14:57 [LTP] [PATCH 1/1] tst_mkfs: Add -I option to mkfs.vfat Petr Vorel
@ 2021-03-30 10:00 ` Richard Palethorpe
2021-03-30 10:36 ` Martin Doucha
2021-03-30 12:25 ` Petr Vorel
0 siblings, 2 replies; 6+ messages in thread
From: Richard Palethorpe @ 2021-03-30 10:00 UTC (permalink / raw)
To: ltp
Hello,
Petr Vorel <pvorel@suse.cz> writes:
> to workaround occasional error on tests with .all_filesystems flag:
>
> tst_test.c:888: TINFO: Formatting /dev/loop0 with vfat opts='' extra opts=''
> mkfs.vfat: Partitions or virtual mappings on device '/dev/loop0',
> not making filesystem (use -I to override)
> tst_test.c:888: TBROK: mkfs.vfat failed with exit code 1
>
> Tested also on BusyBox v1.33.0, which accept but ignores -I since
> beginning (1.14.0).
>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> Richard also suggested to wipe the device, but not sure how to do it
> quickly and don't bring any more dependency.
I guess that you only need to wipe the headers by writing zeros to the
device. That is unless this is actually refering to a mapping in
/dev/mapper?
>
> Kind regards,
> Petr
>
> lib/tst_mkfs.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/lib/tst_mkfs.c b/lib/tst_mkfs.c
> index 736324f04..2d335a3ad 100644
> --- a/lib/tst_mkfs.c
> +++ b/lib/tst_mkfs.c
> @@ -52,6 +52,16 @@ void tst_mkfs_(const char *file, const int lineno, void (cleanup_fn)(void),
>
> snprintf(mkfs, sizeof(mkfs), "mkfs.%s", fs_type);
>
> + /*
> + * Workaround a problem:
> + * mkfs.vfat: Partitions or virtual mappings on device '/dev/loop0', not
> + * making filesystem (use -I to override)
> + */
> + if (!strcmp(fs_type, "vfat")) {
> + argv[pos] = "-I";
> + strcat(fs_opts_str, argv[pos++]);
Doesn't it need a space after -I? Might be better to put this after
fs_opts and then you can check i > 0.
> + }
> +
> if (fs_opts) {
> for (i = 0; fs_opts[i]; i++) {
> argv[pos++] = fs_opts[i];
--
Thank you,
Richard.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [LTP] [PATCH 1/1] tst_mkfs: Add -I option to mkfs.vfat
2021-03-30 10:00 ` Richard Palethorpe
@ 2021-03-30 10:36 ` Martin Doucha
2021-03-30 12:25 ` Petr Vorel
1 sibling, 0 replies; 6+ messages in thread
From: Martin Doucha @ 2021-03-30 10:36 UTC (permalink / raw)
To: ltp
On 30. 03. 21 12:00, Richard Palethorpe wrote:
> Hello,
>
> Petr Vorel <pvorel@suse.cz> writes:
>
>> to workaround occasional error on tests with .all_filesystems flag:
>>
>> tst_test.c:888: TINFO: Formatting /dev/loop0 with vfat opts='' extra opts=''
>> mkfs.vfat: Partitions or virtual mappings on device '/dev/loop0',
>> not making filesystem (use -I to override)
>> tst_test.c:888: TBROK: mkfs.vfat failed with exit code 1
>>
>> Tested also on BusyBox v1.33.0, which accept but ignores -I since
>> beginning (1.14.0).
>>
>> Signed-off-by: Petr Vorel <pvorel@suse.cz>
>> ---
>> Richard also suggested to wipe the device, but not sure how to do it
>> quickly and don't bring any more dependency.
>
> I guess that you only need to wipe the headers by writing zeros to the
> device. That is unless this is actually refering to a mapping in
> /dev/mapper?
We already call tst_clear_device(dev) in tst_mkfs_() but mkfs.vfat keeps
randomly failing with loop devices anyway.
--
Martin Doucha mdoucha@suse.cz
QA Engineer for Software Maintenance
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic
^ permalink raw reply [flat|nested] 6+ messages in thread
* [LTP] [PATCH 1/1] tst_mkfs: Add -I option to mkfs.vfat
2021-03-30 10:00 ` Richard Palethorpe
2021-03-30 10:36 ` Martin Doucha
@ 2021-03-30 12:25 ` Petr Vorel
2021-03-30 12:36 ` Richard Palethorpe
1 sibling, 1 reply; 6+ messages in thread
From: Petr Vorel @ 2021-03-30 12:25 UTC (permalink / raw)
To: ltp
Hi,
...
> > + /*
> > + * Workaround a problem:
> > + * mkfs.vfat: Partitions or virtual mappings on device '/dev/loop0', not
> > + * making filesystem (use -I to override)
> > + */
> > + if (!strcmp(fs_type, "vfat")) {
> > + argv[pos] = "-I";
> > + strcat(fs_opts_str, argv[pos++]);
> Doesn't it need a space after -I?
No, it's char *const argv[] passed to execvp() in tst_cmd_fds_(), which does the
separation. Adding spaces on the contrary causes failure (equivalent of
mkfs.vfat " -I " ...).
> Might be better to put this after fs_opts and then you can check i > 0.
Have I overlooked introduced regression? It works, only it needs to add " "
space for formatting output, but functionality works, adding tested file.
Kind regards,
Petr
Testing file:
#include "tst_test.h"
#define MOUNT_POINT "mount_ext"
static void do_test(void)
{
tst_res(TPASS, "here");
}
static struct tst_test test = {
.test_all = do_test,
.mount_device = 1,
.mntpoint = MOUNT_POINT,
.needs_root = 1,
.dev_fs_type = "vfat",
.dev_fs_opts = (const char *const []){"-v", NULL},
};
Needed fix:
--- lib/tst_mkfs.c
+++ lib/tst_mkfs.c
@@ -60,6 +60,7 @@ void tst_mkfs_(const char *file, const int lineno, void (cleanup_fn)(void),
if (!strcmp(fs_type, "vfat")) {
argv[pos] = "-I";
strcat(fs_opts_str, argv[pos++]);
+ strcat(fs_opts_str, " ");
}
if (fs_opts) {
^ permalink raw reply [flat|nested] 6+ messages in thread
* [LTP] [PATCH 1/1] tst_mkfs: Add -I option to mkfs.vfat
2021-03-30 12:25 ` Petr Vorel
@ 2021-03-30 12:36 ` Richard Palethorpe
2021-03-31 7:58 ` Petr Vorel
0 siblings, 1 reply; 6+ messages in thread
From: Richard Palethorpe @ 2021-03-30 12:36 UTC (permalink / raw)
To: ltp
Hello,
Petr Vorel <pvorel@suse.cz> writes:
> Hi,
>
> ...
>> > + /*
>> > + * Workaround a problem:
>> > + * mkfs.vfat: Partitions or virtual mappings on device '/dev/loop0', not
>> > + * making filesystem (use -I to override)
>> > + */
>> > + if (!strcmp(fs_type, "vfat")) {
>> > + argv[pos] = "-I";
>> > + strcat(fs_opts_str, argv[pos++]);
>
>> Doesn't it need a space after -I?
> No, it's char *const argv[] passed to execvp() in tst_cmd_fds_(), which does the
> separation. Adding spaces on the contrary causes failure (equivalent of
> mkfs.vfat " -I " ...).
>
>> Might be better to put this after fs_opts and then you can check i > 0.
> Have I overlooked introduced regression? It works, only it needs to add " "
> space for formatting output, but functionality works, adding tested
> file.
Ah, well given Martin's comment as well, this LGTM.
>
> Kind regards,
> Petr
>
> Testing file:
>
> #include "tst_test.h"
> #define MOUNT_POINT "mount_ext"
>
> static void do_test(void)
> {
> tst_res(TPASS, "here");
> }
>
> static struct tst_test test = {
> .test_all = do_test,
> .mount_device = 1,
> .mntpoint = MOUNT_POINT,
> .needs_root = 1,
> .dev_fs_type = "vfat",
> .dev_fs_opts = (const char *const []){"-v", NULL},
> };
>
> Needed fix:
>
> --- lib/tst_mkfs.c
> +++ lib/tst_mkfs.c
> @@ -60,6 +60,7 @@ void tst_mkfs_(const char *file, const int lineno, void (cleanup_fn)(void),
> if (!strcmp(fs_type, "vfat")) {
> argv[pos] = "-I";
> strcat(fs_opts_str, argv[pos++]);
> + strcat(fs_opts_str, " ");
> }
>
> if (fs_opts) {
--
Thank you,
Richard.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [LTP] [PATCH 1/1] tst_mkfs: Add -I option to mkfs.vfat
2021-03-30 12:36 ` Richard Palethorpe
@ 2021-03-31 7:58 ` Petr Vorel
0 siblings, 0 replies; 6+ messages in thread
From: Petr Vorel @ 2021-03-31 7:58 UTC (permalink / raw)
To: ltp
Hi,
instead of using -I I tried Martin Doucha's suggestion to rescan partition table
with ioctl BLKRRPART. But I get EINVAL. Not sure if O_RDONLY is a proper flag,
but I also tried to open with args used for manipulating loop (open(path,
O_CREAT|O_WRONLY|O_TRUNC, S_IRUSR|S_IWUSR)) and a same issue. Happens on recent
kernel and very old kernel 3.16.
I wonder if I have wrong flags or BLKRRPART is just not supported on loop
devices. Looking into kernel sources it's this part blkdev_reread_part():
if (!disk_part_scan_enabled(bdev->bd_disk) || bdev_is_partition(bdev))
return -EINVAL;
Kind regards,
Petr
diff --git lib/tst_mkfs.c lib/tst_mkfs.c
index 736324f04..06fafcd18 100644
--- lib/tst_mkfs.c
+++ lib/tst_mkfs.c
@@ -15,6 +15,10 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
+#include <sys/ioctl.h>
+#include <sys/mount.h>
+#include <fcntl.h>
+
#include "test.h"
#include "ltp_priv.h"
#include "tst_mkfs.h"
@@ -26,7 +30,7 @@ void tst_mkfs_(const char *file, const int lineno, void (cleanup_fn)(void),
const char *dev, const char *fs_type,
const char *const fs_opts[], const char *const extra_opts[])
{
- int i, pos = 1, ret;
+ int fd, i, pos = 1, ret;
char mkfs[64];
const char *argv[OPTS_MAX] = {mkfs};
char fs_opts_str[1024] = "";
@@ -93,6 +97,13 @@ void tst_mkfs_(const char *file, const int lineno, void (cleanup_fn)(void),
"tst_clear_device() failed");
}
+ /* force kernel to reread partition table */
+ fd = open(dev, O_RDONLY);
+ tst_resm_(file, lineno, TINFO, "dev: %s, fd: %d", dev, fd);
+ ret = ioctl(fd, BLKRRPART);
+ tst_resm_(file, lineno, TINFO, "rc: %d", ret);
+ close(fd);
+
tst_resm_(file, lineno, TINFO,
"Formatting %s with %s opts='%s' extra opts='%s'",
dev, fs_type, fs_opts_str, extra_opts_str);
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-03-31 7:58 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-29 14:57 [LTP] [PATCH 1/1] tst_mkfs: Add -I option to mkfs.vfat Petr Vorel
2021-03-30 10:00 ` Richard Palethorpe
2021-03-30 10:36 ` Martin Doucha
2021-03-30 12:25 ` Petr Vorel
2021-03-30 12:36 ` Richard Palethorpe
2021-03-31 7:58 ` Petr Vorel
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.