* [LTP] [PATCH v1] ioctl_loop01.c: Use proper device for partitioning
@ 2025-09-01 7:47 Wei Gao via ltp
2025-09-01 10:38 ` Petr Vorel
2025-09-02 3:12 ` [LTP] [PATCH v2] " Wei Gao via ltp
0 siblings, 2 replies; 21+ messages in thread
From: Wei Gao via ltp @ 2025-09-01 7:47 UTC (permalink / raw)
To: ltp; +Cc: Jan Kara
This is same patch used on ioctl09,the page cache of loop0 can cache old
version of the partition table which is then used by the partitioning
code. Fix the problem by calling parted against the loop device directly.
Link: https://lore.kernel.org/ltp/20250829141932.31997-1-jack@suse.cz/
Signed-off-by: Wei Gao <wegao@suse.com>
---
testcases/kernel/syscalls/ioctl/ioctl_loop01.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop01.c b/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
index c9137bf1e..5ee7a474a 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
@@ -98,7 +98,7 @@ static void verify_ioctl_loop(void)
static void setup(void)
{
int ret;
- const char *const cmd_parted[] = {"parted", "-s", "test.img", "mklabel", "msdos", "mkpart",
+ const char *const cmd_parted[] = {"parted", "-s", dev_path, "mklabel", "msdos", "mkpart",
"primary", "ext4", "1M", "10M", NULL};
dev_num = tst_find_free_loopdev(dev_path, sizeof(dev_path));
--
2.51.0
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [LTP] [PATCH v1] ioctl_loop01.c: Use proper device for partitioning
2025-09-01 7:47 [LTP] [PATCH v1] ioctl_loop01.c: Use proper device for partitioning Wei Gao via ltp
@ 2025-09-01 10:38 ` Petr Vorel
2025-09-02 2:16 ` Wei Gao via ltp
2025-09-02 3:12 ` [LTP] [PATCH v2] " Wei Gao via ltp
1 sibling, 1 reply; 21+ messages in thread
From: Petr Vorel @ 2025-09-01 10:38 UTC (permalink / raw)
To: Wei Gao; +Cc: Jan Kara, ltp
Hi Wei,
> This is same patch used on ioctl09,the page cache of loop0 can cache old
> version of the partition table which is then used by the partitioning
> code. Fix the problem by calling parted against the loop device directly.
> Link: https://lore.kernel.org/ltp/20250829141932.31997-1-jack@suse.cz/
> Signed-off-by: Wei Gao <wegao@suse.com>
> ---
> testcases/kernel/syscalls/ioctl/ioctl_loop01.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop01.c b/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
> index c9137bf1e..5ee7a474a 100644
> --- a/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
> +++ b/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
> @@ -98,7 +98,7 @@ static void verify_ioctl_loop(void)
> static void setup(void)
> {
> int ret;
> - const char *const cmd_parted[] = {"parted", "-s", "test.img", "mklabel", "msdos", "mkpart",
> + const char *const cmd_parted[] = {"parted", "-s", dev_path, "mklabel", "msdos", "mkpart",
> "primary", "ext4", "1M", "10M", NULL};
Indeed I was able to trigger the same error:
[ 2642.979234] ioctl_loop01
[ 2642.997424] loop8: detected capacity change from 0 to 20480
[ 2643.015094] loop8: p1
[ 2643.040903] blk_update_request: I/O error, dev loop8, sector 20352 op 0x0:(READ) flags 0x80700 phys_seg 1 prio class 0
[ 2643.042105] blk_update_request: I/O error, dev loop8, sector 20352 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 0
[ 2643.043361] Buffer I/O error on dev loop8p1, logical block 2288, async page read
[ 2643.044539] __loop_clr_fd: partition scan of loop8 failed (rc=-16)
[ 2643.097517] blk_update_request: I/O error, dev loop8, sector 20352 op 0x0:(READ) flags 0x80700 phys_seg 1 prio class 0
[ 2643.098175] blk_update_request: I/O error, dev loop8, sector 20352 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 0
[ 2643.098767] Buffer I/O error on dev loop8p1, logical block 2288, async page read
[ 2643.109133] ioctl_ficlone02
[ 2643.109824] loop8: detected capacity change from 0 to 614400
[ 2643.175605] /dev/zero: Can't open blockdev
[ 2643.307531] EXT4-fs (loop8): mounting ext2 file system using the ext4 subsystem
[ 2643.312401] EXT4-fs (loop8): mounted filesystem without journal. Opts: (null). Quota mode: none.
[ 2643.471527] EXT4-fs (loop8): mounting ext3 file system using the ext4 subsystem
[ 2643.476826] EXT4-fs (loop8): mounted filesystem with ordered data mode. Opts: (null). Quota mode: none.
[ 2643.644734] EXT4-fs (loop8): mounted filesystem with ordered data mode. Opts: (null). Quota mode: none.
But you change introduces one TCONF:
--- old 2025-09-01 05:05:50.401105483 -0400
+++ new 2025-09-01 05:05:01.925105483 -0400
@@ -5,6 +5,7 @@
tst_kconfig.c:678: TINFO: CONFIG_FAULT_INJECTION kernel option detected which might slow the execution
tst_test.c:1827: TINFO: Overall timeout per run is 0h 02m 04s
tst_device.c:99: TINFO: Found free device 6 '/dev/loop6'
+ioctl_loop01.c:119: TCONF: parted exited with 1
tst_buffers.c:57: TINFO: Test is using guarded buffers
ioctl_loop01.c:84: TPASS: /sys/block/loop6/loop/partscan = 0
ioctl_loop01.c:85: TPASS: /sys/block/loop6/loop/autoclear = 0
@@ -12,18 +13,16 @@
ioctl_loop01.c:56: TPASS: get expected lo_flag 12
ioctl_loop01.c:58: TPASS: /sys/block/loop6/loop/partscan = 1
ioctl_loop01.c:59: TPASS: /sys/block/loop6/loop/autoclear = 1
-ioctl_loop01.c:68: TPASS: access /dev/loop6p1 succeeds
-ioctl_loop01.c:74: TPASS: access /sys/block/loop6/loop6p1 succeeds
+ioctl_loop01.c:62: TINFO: Current environment doesn't have parted disk, skip it
ioctl_loop01.c:90: TINFO: Test flag can be clear
ioctl_loop01.c:56: TPASS: get expected lo_flag 8
ioctl_loop01.c:58: TPASS: /sys/block/loop6/loop/partscan = 1
ioctl_loop01.c:59: TPASS: /sys/block/loop6/loop/autoclear = 0
-ioctl_loop01.c:68: TPASS: access /dev/loop6p1 succeeds
-ioctl_loop01.c:74: TPASS: access /sys/block/loop6/loop6p1 succeeds
+ioctl_loop01.c:62: TINFO: Current environment doesn't have parted disk, skip it
Summary:
-passed 13
+passed 9
failed 0
broken 0
-skipped 0
+skipped 1
warnings 0
That means your change effectively disable that code. => NACK
in ioctl09.c uses the first command with "test.img", then loop device on the
second run.
Surprisingly trying to attach helps 'parted' to use it, although it produces a
warning:
+++ testcases/kernel/syscalls/ioctl/ioctl_loop01.c
@@ -106,6 +106,7 @@ static void setup(void)
tst_brk(TBROK, "Failed to find free loop device");
tst_fill_file("test.img", 0, 1024 * 1024, 10);
+ tst_attach_device(dev_path, "test.img");
ret = tst_cmd(cmd_parted, NULL, NULL, TST_CMD_PASS_RETVAL);
switch (ret) {
tst_device.c:99: TINFO: Found free device 9 '/dev/loop9'
tst_buffers.c:57: TINFO: Test is using guarded buffers
tst_device.c:176: TWARN: ioctl(/dev/loop9, LOOP_SET_FD, test.img) failed: EBUSY (16)
ioctl_loop01.c:84: TPASS: /sys/block/loop9/loop/partscan = 0
ioctl_loop01.c:85: TPASS: /sys/block/loop9/loop/autoclear = 0
ioctl_loop01.c:86: TPASS: /sys/block/loop9/loop/backing_file = '/tmp/LTP_iocfMLSpX/test.img'
ioctl_loop01.c:56: TPASS: get expected lo_flag 12
ioctl_loop01.c:58: TPASS: /sys/block/loop9/loop/partscan = 1
ioctl_loop01.c:59: TPASS: /sys/block/loop9/loop/autoclear = 1
ioctl_loop01.c:68: TPASS: access /dev/loop9p1 succeeds
ioctl_loop01.c:74: TPASS: access /sys/block/loop9/loop9p1 succeeds
ioctl_loop01.c:90: TINFO: Test flag can be clear
ioctl_loop01.c:56: TPASS: get expected lo_flag 8
ioctl_loop01.c:58: TPASS: /sys/block/loop9/loop/partscan = 1
ioctl_loop01.c:59: TPASS: /sys/block/loop9/loop/autoclear = 0
ioctl_loop01.c:68: TPASS: access /dev/loop9p1 succeeds
ioctl_loop01.c:74: TPASS: access /sys/block/loop9/loop9p1 succeeds
Of course TWARN is also no-go.
@Jan @Cyril @Li Any hint what am I missing.
Besides missing there is missing TST_CMD_TCONF_ON_MISSING on the flags (see
today's fix [1]), but that's minor.
Kind regards,
Petr
[1] https://github.com/linux-test-project/ltp/commit/9691c4b2bea4f772d61ca9e9a93d2087c88f6040
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [LTP] [PATCH v1] ioctl_loop01.c: Use proper device for partitioning
2025-09-01 10:38 ` Petr Vorel
@ 2025-09-02 2:16 ` Wei Gao via ltp
0 siblings, 0 replies; 21+ messages in thread
From: Wei Gao via ltp @ 2025-09-02 2:16 UTC (permalink / raw)
To: Petr Vorel; +Cc: Jan Kara, ltp
On Mon, Sep 01, 2025 at 12:38:05PM +0200, Petr Vorel wrote:
> Hi Wei,
>
> > This is same patch used on ioctl09,the page cache of loop0 can cache old
> > version of the partition table which is then used by the partitioning
> > code. Fix the problem by calling parted against the loop device directly.
>
> > Link: https://lore.kernel.org/ltp/20250829141932.31997-1-jack@suse.cz/
> > Signed-off-by: Wei Gao <wegao@suse.com>
> > ---
> > testcases/kernel/syscalls/ioctl/ioctl_loop01.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
>
> > diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop01.c b/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
> > index c9137bf1e..5ee7a474a 100644
> > --- a/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
> > +++ b/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
> > @@ -98,7 +98,7 @@ static void verify_ioctl_loop(void)
> > static void setup(void)
> > {
> > int ret;
> > - const char *const cmd_parted[] = {"parted", "-s", "test.img", "mklabel", "msdos", "mkpart",
> > + const char *const cmd_parted[] = {"parted", "-s", dev_path, "mklabel", "msdos", "mkpart",
> > "primary", "ext4", "1M", "10M", NULL};
>
> Indeed I was able to trigger the same error:
>
> [ 2642.979234] ioctl_loop01
> [ 2642.997424] loop8: detected capacity change from 0 to 20480
> [ 2643.015094] loop8: p1
> [ 2643.040903] blk_update_request: I/O error, dev loop8, sector 20352 op 0x0:(READ) flags 0x80700 phys_seg 1 prio class 0
> [ 2643.042105] blk_update_request: I/O error, dev loop8, sector 20352 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 0
> [ 2643.043361] Buffer I/O error on dev loop8p1, logical block 2288, async page read
> [ 2643.044539] __loop_clr_fd: partition scan of loop8 failed (rc=-16)
> [ 2643.097517] blk_update_request: I/O error, dev loop8, sector 20352 op 0x0:(READ) flags 0x80700 phys_seg 1 prio class 0
> [ 2643.098175] blk_update_request: I/O error, dev loop8, sector 20352 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 0
> [ 2643.098767] Buffer I/O error on dev loop8p1, logical block 2288, async page read
> [ 2643.109133] ioctl_ficlone02
> [ 2643.109824] loop8: detected capacity change from 0 to 614400
> [ 2643.175605] /dev/zero: Can't open blockdev
> [ 2643.307531] EXT4-fs (loop8): mounting ext2 file system using the ext4 subsystem
> [ 2643.312401] EXT4-fs (loop8): mounted filesystem without journal. Opts: (null). Quota mode: none.
> [ 2643.471527] EXT4-fs (loop8): mounting ext3 file system using the ext4 subsystem
> [ 2643.476826] EXT4-fs (loop8): mounted filesystem with ordered data mode. Opts: (null). Quota mode: none.
> [ 2643.644734] EXT4-fs (loop8): mounted filesystem with ordered data mode. Opts: (null). Quota mode: none.
>
> But you change introduces one TCONF:
>
> --- old 2025-09-01 05:05:50.401105483 -0400
> +++ new 2025-09-01 05:05:01.925105483 -0400
> @@ -5,6 +5,7 @@
> tst_kconfig.c:678: TINFO: CONFIG_FAULT_INJECTION kernel option detected which might slow the execution
> tst_test.c:1827: TINFO: Overall timeout per run is 0h 02m 04s
> tst_device.c:99: TINFO: Found free device 6 '/dev/loop6'
> +ioctl_loop01.c:119: TCONF: parted exited with 1
> tst_buffers.c:57: TINFO: Test is using guarded buffers
> ioctl_loop01.c:84: TPASS: /sys/block/loop6/loop/partscan = 0
> ioctl_loop01.c:85: TPASS: /sys/block/loop6/loop/autoclear = 0
> @@ -12,18 +13,16 @@
> ioctl_loop01.c:56: TPASS: get expected lo_flag 12
> ioctl_loop01.c:58: TPASS: /sys/block/loop6/loop/partscan = 1
> ioctl_loop01.c:59: TPASS: /sys/block/loop6/loop/autoclear = 1
> -ioctl_loop01.c:68: TPASS: access /dev/loop6p1 succeeds
> -ioctl_loop01.c:74: TPASS: access /sys/block/loop6/loop6p1 succeeds
> +ioctl_loop01.c:62: TINFO: Current environment doesn't have parted disk, skip it
> ioctl_loop01.c:90: TINFO: Test flag can be clear
> ioctl_loop01.c:56: TPASS: get expected lo_flag 8
> ioctl_loop01.c:58: TPASS: /sys/block/loop6/loop/partscan = 1
> ioctl_loop01.c:59: TPASS: /sys/block/loop6/loop/autoclear = 0
> -ioctl_loop01.c:68: TPASS: access /dev/loop6p1 succeeds
> -ioctl_loop01.c:74: TPASS: access /sys/block/loop6/loop6p1 succeeds
> +ioctl_loop01.c:62: TINFO: Current environment doesn't have parted disk, skip it
>
> Summary:
> -passed 13
> +passed 9
> failed 0
> broken 0
> -skipped 0
> +skipped 1
> warnings 0
>
> That means your change effectively disable that code. => NACK
>
> in ioctl09.c uses the first command with "test.img", then loop device on the
> second run.
>
> Surprisingly trying to attach helps 'parted' to use it, although it produces a
> warning:
>
> +++ testcases/kernel/syscalls/ioctl/ioctl_loop01.c
> @@ -106,6 +106,7 @@ static void setup(void)
> tst_brk(TBROK, "Failed to find free loop device");
>
> tst_fill_file("test.img", 0, 1024 * 1024, 10);
> + tst_attach_device(dev_path, "test.img");
>
> ret = tst_cmd(cmd_parted, NULL, NULL, TST_CMD_PASS_RETVAL);
> switch (ret) {
>
> tst_device.c:99: TINFO: Found free device 9 '/dev/loop9'
> tst_buffers.c:57: TINFO: Test is using guarded buffers
> tst_device.c:176: TWARN: ioctl(/dev/loop9, LOOP_SET_FD, test.img) failed: EBUSY (16)
Thanks for your investigation.
You add tst_attach_device in setup, then second call tst_attach_device
in verify_ioctl_loop trigger this TWARN, I will send second patch.
> ioctl_loop01.c:84: TPASS: /sys/block/loop9/loop/partscan = 0
> ioctl_loop01.c:85: TPASS: /sys/block/loop9/loop/autoclear = 0
> ioctl_loop01.c:86: TPASS: /sys/block/loop9/loop/backing_file = '/tmp/LTP_iocfMLSpX/test.img'
> ioctl_loop01.c:56: TPASS: get expected lo_flag 12
> ioctl_loop01.c:58: TPASS: /sys/block/loop9/loop/partscan = 1
> ioctl_loop01.c:59: TPASS: /sys/block/loop9/loop/autoclear = 1
> ioctl_loop01.c:68: TPASS: access /dev/loop9p1 succeeds
> ioctl_loop01.c:74: TPASS: access /sys/block/loop9/loop9p1 succeeds
> ioctl_loop01.c:90: TINFO: Test flag can be clear
> ioctl_loop01.c:56: TPASS: get expected lo_flag 8
> ioctl_loop01.c:58: TPASS: /sys/block/loop9/loop/partscan = 1
> ioctl_loop01.c:59: TPASS: /sys/block/loop9/loop/autoclear = 0
> ioctl_loop01.c:68: TPASS: access /dev/loop9p1 succeeds
> ioctl_loop01.c:74: TPASS: access /sys/block/loop9/loop9p1 succeeds
>
> Of course TWARN is also no-go.
> @Jan @Cyril @Li Any hint what am I missing.
>
> Besides missing there is missing TST_CMD_TCONF_ON_MISSING on the flags (see
> today's fix [1]), but that's minor.
>
> Kind regards,
> Petr
>
> [1] https://github.com/linux-test-project/ltp/commit/9691c4b2bea4f772d61ca9e9a93d2087c88f6040
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 21+ messages in thread
* [LTP] [PATCH v2] ioctl_loop01.c: Use proper device for partitioning
2025-09-01 7:47 [LTP] [PATCH v1] ioctl_loop01.c: Use proper device for partitioning Wei Gao via ltp
2025-09-01 10:38 ` Petr Vorel
@ 2025-09-02 3:12 ` Wei Gao via ltp
2025-09-02 10:44 ` Petr Vorel
2025-09-02 11:18 ` [LTP] [PATCH v3] " Wei Gao via ltp
1 sibling, 2 replies; 21+ messages in thread
From: Wei Gao via ltp @ 2025-09-02 3:12 UTC (permalink / raw)
To: ltp; +Cc: Jan Kara
This is same patch used on ioctl09,the page cache of loop0 can cache old
version of the partition table which is then used by the partitioning
code. Fix the problem by calling parted against the loop device directly.
Link: https://lore.kernel.org/ltp/20250829141932.31997-1-jack@suse.cz/
Signed-off-by: Wei Gao <wegao@suse.com>
---
.../kernel/syscalls/ioctl/ioctl_loop01.c | 26 +++++--------------
1 file changed, 7 insertions(+), 19 deletions(-)
diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop01.c b/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
index c9137bf1e..695aaeb0b 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
@@ -78,7 +78,13 @@ static void check_loop_value(int set_flag, int get_flag, int autoclear_field)
static void verify_ioctl_loop(void)
{
+ const char *const cmd_parted[] = {"parted", "-s", dev_path, "mklabel", "msdos", "mkpart",
+ "primary", "ext4", "1M", "10M", NULL};
+
+ tst_fill_file("test.img", 0, 1024 * 1024, 10);
tst_attach_device(dev_path, "test.img");
+ SAFE_CMD(cmd_parted, NULL, NULL);
+
attach_flag = 1;
TST_ASSERT_INT(partscan_path, 0);
@@ -92,34 +98,16 @@ static void verify_ioctl_loop(void)
tst_detach_device_by_fd(dev_path, dev_fd);
dev_fd = SAFE_OPEN(dev_path, O_RDWR);
+
attach_flag = 0;
}
static void setup(void)
{
- int ret;
- const char *const cmd_parted[] = {"parted", "-s", "test.img", "mklabel", "msdos", "mkpart",
- "primary", "ext4", "1M", "10M", NULL};
-
dev_num = tst_find_free_loopdev(dev_path, sizeof(dev_path));
if (dev_num < 0)
tst_brk(TBROK, "Failed to find free loop device");
- tst_fill_file("test.img", 0, 1024 * 1024, 10);
-
- ret = tst_cmd(cmd_parted, NULL, NULL, TST_CMD_PASS_RETVAL);
- switch (ret) {
- case 0:
- parted_sup = 1;
- break;
- case 255:
- tst_res(TCONF, "parted binary not installed or failed");
- break;
- default:
- tst_res(TCONF, "parted exited with %i", ret);
- break;
- }
-
sprintf(partscan_path, "/sys/block/loop%d/loop/partscan", dev_num);
sprintf(autoclear_path, "/sys/block/loop%d/loop/autoclear", dev_num);
sprintf(backing_path, "/sys/block/loop%d/loop/backing_file", dev_num);
--
2.51.0
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [LTP] [PATCH v2] ioctl_loop01.c: Use proper device for partitioning
2025-09-02 3:12 ` [LTP] [PATCH v2] " Wei Gao via ltp
@ 2025-09-02 10:44 ` Petr Vorel
2025-09-02 11:18 ` [LTP] [PATCH v3] " Wei Gao via ltp
1 sibling, 0 replies; 21+ messages in thread
From: Petr Vorel @ 2025-09-02 10:44 UTC (permalink / raw)
To: Wei Gao; +Cc: Jan Kara, ltp
Hi Wei,
> This is same patch used on ioctl09,the page cache of loop0 can cache old
> version of the partition table which is then used by the partitioning
> code. Fix the problem by calling parted against the loop device directly.
LGTM, thanks!
Reviewed-by: Petr Vorel <pvorel@suse.cz>
> Link: https://lore.kernel.org/ltp/20250829141932.31997-1-jack@suse.cz/
very nit: Link in kernel commits (and I use this approach) is used for the link
to the patch which was merged (kernel maintainer or here LTP maintainer adds
it). Linking everything else I would use [1].
> Signed-off-by: Wei Gao <wegao@suse.com>
> ---
> .../kernel/syscalls/ioctl/ioctl_loop01.c | 26 +++++--------------
> 1 file changed, 7 insertions(+), 19 deletions(-)
> diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop01.c b/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
> index c9137bf1e..695aaeb0b 100644
> --- a/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
> +++ b/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
> @@ -78,7 +78,13 @@ static void check_loop_value(int set_flag, int get_flag, int autoclear_field)
> static void verify_ioctl_loop(void)
> {
> + const char *const cmd_parted[] = {"parted", "-s", dev_path, "mklabel", "msdos", "mkpart",
> + "primary", "ext4", "1M", "10M", NULL};
> +
> + tst_fill_file("test.img", 0, 1024 * 1024, 10);
> tst_attach_device(dev_path, "test.img");
> + SAFE_CMD(cmd_parted, NULL, NULL);
I'll would probably keep the previous approach where the rest of the testing is
now quited:
Now:
...
tst_cmd.c:65: TCONF: Couldn't find 'parted' in $PATH at tst_cmd.c:65
Summary:
passed 0
failed 0
broken 0
skipped 1
warnings 0
Previously:
ioctl_loop01.c:116: TCONF: parted binary not installed or failed
tst_buffers.c:57: TINFO: Test is using guarded buffers
ioctl_loop01.c:84: TPASS: /sys/block/loop2/loop/partscan = 0
ioctl_loop01.c:85: TPASS: /sys/block/loop2/loop/autoclear = 0
ioctl_loop01.c:86: TPASS: /sys/block/loop2/loop/backing_file = '/tmp/LTP_ioco0FWzP/test.img'
ioctl_loop01.c:56: TPASS: get expected lo_flag 12
ioctl_loop01.c:58: TPASS: /sys/block/loop2/loop/partscan = 1
ioctl_loop01.c:59: TPASS: /sys/block/loop2/loop/autoclear = 1
ioctl_loop01.c:62: TINFO: Current environment doesn't have parted disk, skip it
ioctl_loop01.c:90: TINFO: Test flag can be clear
ioctl_loop01.c:56: TPASS: get expected lo_flag 8
ioctl_loop01.c:58: TPASS: /sys/block/loop2/loop/partscan = 1
ioctl_loop01.c:59: TPASS: /sys/block/loop2/loop/autoclear = 0
ioctl_loop01.c:62: TINFO: Current environment doesn't have parted disk, skip it
Summary:
passed 9
failed 0
broken 0
skipped 1
warnings 0
My point was to add TST_CMD_TCONF_ON_MISSING to the previous call. But I was
wrong, that would work the same (skip whole testing on missing 'parted').
...
> static void setup(void)
> {
> - int ret;
> - const char *const cmd_parted[] = {"parted", "-s", "test.img", "mklabel", "msdos", "mkpart",
> - "primary", "ext4", "1M", "10M", NULL};
> -
> dev_num = tst_find_free_loopdev(dev_path, sizeof(dev_path));
> if (dev_num < 0)
> tst_brk(TBROK, "Failed to find free loop device");
> - tst_fill_file("test.img", 0, 1024 * 1024, 10);
> -
> - ret = tst_cmd(cmd_parted, NULL, NULL, TST_CMD_PASS_RETVAL);
> - switch (ret) {
> - case 0:
> - parted_sup = 1;
> - break;
> - case 255:
> - tst_res(TCONF, "parted binary not installed or failed");
> - break;
> - default:
> - tst_res(TCONF, "parted exited with %i", ret);
> - break;
Therefore either keeping this, or use if/else equivalent (IMHO more readable):
ret = tst_cmd(cmd_parted, NULL, NULL, TST_CMD_PASS_RETVAL);
if (!ret)
parted_sup = 1;
else if (ret == 255)
tst_res(TCONF, "parted binary not installed or failed");
else
tst_res(TCONF, "parted exited with %i", ret);
Kind regards,
Petr
> - }
> -
> sprintf(partscan_path, "/sys/block/loop%d/loop/partscan", dev_num);
> sprintf(autoclear_path, "/sys/block/loop%d/loop/autoclear", dev_num);
> sprintf(backing_path, "/sys/block/loop%d/loop/backing_file", dev_num);
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 21+ messages in thread
* [LTP] [PATCH v3] ioctl_loop01.c: Use proper device for partitioning
2025-09-02 3:12 ` [LTP] [PATCH v2] " Wei Gao via ltp
2025-09-02 10:44 ` Petr Vorel
@ 2025-09-02 11:18 ` Wei Gao via ltp
2025-09-03 12:48 ` Petr Vorel
2025-09-09 11:50 ` Cyril Hrubis
1 sibling, 2 replies; 21+ messages in thread
From: Wei Gao via ltp @ 2025-09-02 11:18 UTC (permalink / raw)
To: ltp; +Cc: Jan Kara
This is same patch used on ioctl09,the page cache of loop0 can cache old
version of the partition table which is then used by the partitioning
code. Fix the problem by calling parted against the loop device directly.
More detail see patch [1].
[1] https://lore.kernel.org/ltp/20250829141932.31997-1-jack@suse.cz/
Signed-off-by: Wei Gao <wegao@suse.com>
Reviewed-by: Petr Vorel <pvorel@suse.cz>
---
.../kernel/syscalls/ioctl/ioctl_loop01.c | 34 ++++++++-----------
1 file changed, 15 insertions(+), 19 deletions(-)
diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop01.c b/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
index c9137bf1e..b70e9fc22 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
@@ -78,7 +78,21 @@ static void check_loop_value(int set_flag, int get_flag, int autoclear_field)
static void verify_ioctl_loop(void)
{
+ int ret;
+ const char *const cmd_parted[] = {"parted", "-s", dev_path, "mklabel", "msdos", "mkpart",
+ "primary", "ext4", "1M", "10M", NULL};
+
+ tst_fill_file("test.img", 0, 1024 * 1024, 10);
tst_attach_device(dev_path, "test.img");
+
+ ret = tst_cmd(cmd_parted, NULL, NULL, TST_CMD_PASS_RETVAL);
+ if (!ret)
+ parted_sup = 1;
+ else if (ret == 255)
+ tst_res(TCONF, "parted binary not installed or failed");
+ else
+ tst_res(TCONF, "parted exited with %i", ret);
+
attach_flag = 1;
TST_ASSERT_INT(partscan_path, 0);
@@ -92,34 +106,16 @@ static void verify_ioctl_loop(void)
tst_detach_device_by_fd(dev_path, dev_fd);
dev_fd = SAFE_OPEN(dev_path, O_RDWR);
+
attach_flag = 0;
}
static void setup(void)
{
- int ret;
- const char *const cmd_parted[] = {"parted", "-s", "test.img", "mklabel", "msdos", "mkpart",
- "primary", "ext4", "1M", "10M", NULL};
-
dev_num = tst_find_free_loopdev(dev_path, sizeof(dev_path));
if (dev_num < 0)
tst_brk(TBROK, "Failed to find free loop device");
- tst_fill_file("test.img", 0, 1024 * 1024, 10);
-
- ret = tst_cmd(cmd_parted, NULL, NULL, TST_CMD_PASS_RETVAL);
- switch (ret) {
- case 0:
- parted_sup = 1;
- break;
- case 255:
- tst_res(TCONF, "parted binary not installed or failed");
- break;
- default:
- tst_res(TCONF, "parted exited with %i", ret);
- break;
- }
-
sprintf(partscan_path, "/sys/block/loop%d/loop/partscan", dev_num);
sprintf(autoclear_path, "/sys/block/loop%d/loop/autoclear", dev_num);
sprintf(backing_path, "/sys/block/loop%d/loop/backing_file", dev_num);
--
2.51.0
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [LTP] [PATCH v3] ioctl_loop01.c: Use proper device for partitioning
2025-09-02 11:18 ` [LTP] [PATCH v3] " Wei Gao via ltp
@ 2025-09-03 12:48 ` Petr Vorel
2025-09-09 11:50 ` Cyril Hrubis
1 sibling, 0 replies; 21+ messages in thread
From: Petr Vorel @ 2025-09-03 12:48 UTC (permalink / raw)
To: Wei Gao; +Cc: Jan Kara, ltp
Hi Wei,
> This is same patch used on ioctl09,the page cache of loop0 can cache old
> version of the partition table which is then used by the partitioning
> code. Fix the problem by calling parted against the loop device directly.
> More detail see patch [1].
> [1] https://lore.kernel.org/ltp/20250829141932.31997-1-jack@suse.cz/
Thanks! Indeed I'm not able to trigger the problem any more.
Tested-by: Petr Vorel <pvorel@suse.cz>
Kind regards,
Petr
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [LTP] [PATCH v3] ioctl_loop01.c: Use proper device for partitioning
2025-09-02 11:18 ` [LTP] [PATCH v3] " Wei Gao via ltp
2025-09-03 12:48 ` Petr Vorel
@ 2025-09-09 11:50 ` Cyril Hrubis
2025-09-10 1:35 ` Wei Gao via ltp
1 sibling, 1 reply; 21+ messages in thread
From: Cyril Hrubis @ 2025-09-09 11:50 UTC (permalink / raw)
To: Wei Gao; +Cc: Jan Kara, ltp
Hi!
> .../kernel/syscalls/ioctl/ioctl_loop01.c | 34 ++++++++-----------
> 1 file changed, 15 insertions(+), 19 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop01.c b/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
> index c9137bf1e..b70e9fc22 100644
> --- a/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
> +++ b/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
> @@ -78,7 +78,21 @@ static void check_loop_value(int set_flag, int get_flag, int autoclear_field)
>
> static void verify_ioctl_loop(void)
> {
> + int ret;
> + const char *const cmd_parted[] = {"parted", "-s", dev_path, "mklabel", "msdos", "mkpart",
> + "primary", "ext4", "1M", "10M", NULL};
> +
> + tst_fill_file("test.img", 0, 1024 * 1024, 10);
> tst_attach_device(dev_path, "test.img");
> +
> + ret = tst_cmd(cmd_parted, NULL, NULL, TST_CMD_PASS_RETVAL);
> + if (!ret)
> + parted_sup = 1;
> + else if (ret == 255)
> + tst_res(TCONF, "parted binary not installed or failed");
> + else
> + tst_res(TCONF, "parted exited with %i", ret);
The test should have needs_cmds set to parted (we do that properly in
ioctl09.c) then we do not have to handle the 255 exit code here since
the test would be skipped if it's missing.
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [LTP] [PATCH v3] ioctl_loop01.c: Use proper device for partitioning
2025-09-09 11:50 ` Cyril Hrubis
@ 2025-09-10 1:35 ` Wei Gao via ltp
2025-09-18 14:53 ` Petr Vorel
0 siblings, 1 reply; 21+ messages in thread
From: Wei Gao via ltp @ 2025-09-10 1:35 UTC (permalink / raw)
To: Cyril Hrubis; +Cc: Jan Kara, ltp
On Tue, Sep 09, 2025 at 01:50:30PM +0200, Cyril Hrubis wrote:
> Hi!
> > .../kernel/syscalls/ioctl/ioctl_loop01.c | 34 ++++++++-----------
> > 1 file changed, 15 insertions(+), 19 deletions(-)
> >
> > diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop01.c b/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
> > index c9137bf1e..b70e9fc22 100644
> > --- a/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
> > +++ b/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
> > @@ -78,7 +78,21 @@ static void check_loop_value(int set_flag, int get_flag, int autoclear_field)
> >
> > static void verify_ioctl_loop(void)
> > {
> > + int ret;
> > + const char *const cmd_parted[] = {"parted", "-s", dev_path, "mklabel", "msdos", "mkpart",
> > + "primary", "ext4", "1M", "10M", NULL};
> > +
> > + tst_fill_file("test.img", 0, 1024 * 1024, 10);
> > tst_attach_device(dev_path, "test.img");
> > +
> > + ret = tst_cmd(cmd_parted, NULL, NULL, TST_CMD_PASS_RETVAL);
> > + if (!ret)
> > + parted_sup = 1;
> > + else if (ret == 255)
> > + tst_res(TCONF, "parted binary not installed or failed");
> > + else
> > + tst_res(TCONF, "parted exited with %i", ret);
>
> The test should have needs_cmds set to parted (we do that properly in
> ioctl09.c) then we do not have to handle the 255 exit code here since
> the test would be skipped if it's missing.
If we use needs_cmds all the check will be skipped in this case.
Current test case will still continue do some check even parted is
missing. Such as following test log:
tst_tmpdir.c:316: TINFO: Using /tmp/LTP_iocxxPKhg as tmpdir (ext2/ext3/ext4 filesystem)
tst_test.c:2004: TINFO: LTP version: 20250130-399-g47167e082
tst_test.c:2007: TINFO: Tested kernel: 6.11.0-1027-oem #27-Ubuntu SMP PREEMPT_DYNAMIC Tue Jul 22 06:12:35 UTC 2025 x86_64
tst_kconfig.c:88: TINFO: Parsing kernel config '/lib/modules/6.11.0-1027-oem/build/.config'
tst_test.c:1825: TINFO: Overall timeout per run is 0h 00m 31s
tst_device.c:98: TINFO: Found free device 14 '/dev/loop14'
tst_buffers.c:57: TINFO: Test is using guarded buffers
ioctl_loop01.c:92: TCONF: parted binary not installed or failed
ioctl_loop01.c:98: TPASS: /sys/block/loop14/loop/partscan = 0
ioctl_loop01.c:99: TPASS: /sys/block/loop14/loop/autoclear = 0
ioctl_loop01.c:100: TPASS: /sys/block/loop14/loop/backing_file = '/tmp/LTP_iocxxPKhg/test.img'
ioctl_loop01.c:56: TPASS: get expected lo_flag 12
ioctl_loop01.c:58: TPASS: /sys/block/loop14/loop/partscan = 1
ioctl_loop01.c:59: TPASS: /sys/block/loop14/loop/autoclear = 1
ioctl_loop01.c:62: TINFO: Current environment doesn't have parted disk, skip it
ioctl_loop01.c:104: TINFO: Test flag can be clear
ioctl_loop01.c:56: TPASS: get expected lo_flag 8
ioctl_loop01.c:58: TPASS: /sys/block/loop14/loop/partscan = 1
ioctl_loop01.c:59: TPASS: /sys/block/loop14/loop/autoclear = 0
ioctl_loop01.c:62: TINFO: Current environment doesn't have parted disk, skip it
Summary:
passed 9
failed 0
broken 0
skipped 1
warnings 0
>
> --
> Cyril Hrubis
> chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [LTP] [PATCH v3] ioctl_loop01.c: Use proper device for partitioning
2025-09-10 1:35 ` Wei Gao via ltp
@ 2025-09-18 14:53 ` Petr Vorel
2025-09-18 15:35 ` Cyril Hrubis
0 siblings, 1 reply; 21+ messages in thread
From: Petr Vorel @ 2025-09-18 14:53 UTC (permalink / raw)
To: Wei Gao; +Cc: Jan Kara, ltp
> On Tue, Sep 09, 2025 at 01:50:30PM +0200, Cyril Hrubis wrote:
> > Hi!
> > > .../kernel/syscalls/ioctl/ioctl_loop01.c | 34 ++++++++-----------
> > > 1 file changed, 15 insertions(+), 19 deletions(-)
> > > diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop01.c b/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
> > > index c9137bf1e..b70e9fc22 100644
> > > --- a/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
> > > +++ b/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
> > > @@ -78,7 +78,21 @@ static void check_loop_value(int set_flag, int get_flag, int autoclear_field)
> > > static void verify_ioctl_loop(void)
> > > {
> > > + int ret;
> > > + const char *const cmd_parted[] = {"parted", "-s", dev_path, "mklabel", "msdos", "mkpart",
> > > + "primary", "ext4", "1M", "10M", NULL};
> > > +
> > > + tst_fill_file("test.img", 0, 1024 * 1024, 10);
> > > tst_attach_device(dev_path, "test.img");
> > > +
> > > + ret = tst_cmd(cmd_parted, NULL, NULL, TST_CMD_PASS_RETVAL);
> > > + if (!ret)
> > > + parted_sup = 1;
> > > + else if (ret == 255)
> > > + tst_res(TCONF, "parted binary not installed or failed");
> > > + else
> > > + tst_res(TCONF, "parted exited with %i", ret);
> > The test should have needs_cmds set to parted (we do that properly in
> > ioctl09.c) then we do not have to handle the 255 exit code here since
> > the test would be skipped if it's missing.
> If we use needs_cmds all the check will be skipped in this case.
@Cyril: only single test require 'parted' as I reported in v1 [1].
Yeah, code gets slightly more complicated just because single test requires
parted. Or you would not care? IMHO it does not make sense to split test into
two (too much duplicity).
But TINFO message should be turned in TCONF so that people notice.
tst_res(TINFO, "Current environment doesn't have parted disk, skip it");
Kind regards,
Petr
[1] https://lore.kernel.org/ltp/20250901103805.GA30224@pevik/
> Current test case will still continue do some check even parted is
> missing. Such as following test log:
> tst_tmpdir.c:316: TINFO: Using /tmp/LTP_iocxxPKhg as tmpdir (ext2/ext3/ext4 filesystem)
> tst_test.c:2004: TINFO: LTP version: 20250130-399-g47167e082
> tst_test.c:2007: TINFO: Tested kernel: 6.11.0-1027-oem #27-Ubuntu SMP PREEMPT_DYNAMIC Tue Jul 22 06:12:35 UTC 2025 x86_64
> tst_kconfig.c:88: TINFO: Parsing kernel config '/lib/modules/6.11.0-1027-oem/build/.config'
> tst_test.c:1825: TINFO: Overall timeout per run is 0h 00m 31s
> tst_device.c:98: TINFO: Found free device 14 '/dev/loop14'
> tst_buffers.c:57: TINFO: Test is using guarded buffers
> ioctl_loop01.c:92: TCONF: parted binary not installed or failed
> ioctl_loop01.c:98: TPASS: /sys/block/loop14/loop/partscan = 0
> ioctl_loop01.c:99: TPASS: /sys/block/loop14/loop/autoclear = 0
> ioctl_loop01.c:100: TPASS: /sys/block/loop14/loop/backing_file = '/tmp/LTP_iocxxPKhg/test.img'
> ioctl_loop01.c:56: TPASS: get expected lo_flag 12
> ioctl_loop01.c:58: TPASS: /sys/block/loop14/loop/partscan = 1
> ioctl_loop01.c:59: TPASS: /sys/block/loop14/loop/autoclear = 1
> ioctl_loop01.c:62: TINFO: Current environment doesn't have parted disk, skip it
> ioctl_loop01.c:104: TINFO: Test flag can be clear
> ioctl_loop01.c:56: TPASS: get expected lo_flag 8
> ioctl_loop01.c:58: TPASS: /sys/block/loop14/loop/partscan = 1
> ioctl_loop01.c:59: TPASS: /sys/block/loop14/loop/autoclear = 0
> ioctl_loop01.c:62: TINFO: Current environment doesn't have parted disk, skip it
> Summary:
> passed 9
> failed 0
> broken 0
> skipped 1
> warnings 0
> > --
> > Cyril Hrubis
> > chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [LTP] [PATCH v3] ioctl_loop01.c: Use proper device for partitioning
2025-09-18 14:53 ` Petr Vorel
@ 2025-09-18 15:35 ` Cyril Hrubis
2025-09-19 13:22 ` Petr Vorel
2025-09-24 2:26 ` Wei Gao via ltp
0 siblings, 2 replies; 21+ messages in thread
From: Cyril Hrubis @ 2025-09-18 15:35 UTC (permalink / raw)
To: Petr Vorel; +Cc: Jan Kara, ltp
Hi!
> > > The test should have needs_cmds set to parted (we do that properly in
> > > ioctl09.c) then we do not have to handle the 255 exit code here since
> > > the test would be skipped if it's missing.
>
> > If we use needs_cmds all the check will be skipped in this case.
>
> @Cyril: only single test require 'parted' as I reported in v1 [1].
> Yeah, code gets slightly more complicated just because single test requires
> parted. Or you would not care? IMHO it does not make sense to split test into
> two (too much duplicity).
The problem here is how to handle the metadata. One posible solution is
to add a notion of optional dependencies so that we would have
'needs_foo' and 'wants_foo'. Or turn the needs_foo into a structure with
an .optional boolean flag.
> But TINFO message should be turned in TCONF so that people notice.
> tst_res(TINFO, "Current environment doesn't have parted disk, skip it");
Yes please.
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [LTP] [PATCH v3] ioctl_loop01.c: Use proper device for partitioning
2025-09-18 15:35 ` Cyril Hrubis
@ 2025-09-19 13:22 ` Petr Vorel
2025-09-22 7:28 ` Cyril Hrubis
2025-09-24 2:26 ` Wei Gao via ltp
1 sibling, 1 reply; 21+ messages in thread
From: Petr Vorel @ 2025-09-19 13:22 UTC (permalink / raw)
To: Cyril Hrubis; +Cc: ltp
Hi Cyril,
> Hi!
> > > > The test should have needs_cmds set to parted (we do that properly in
> > > > ioctl09.c) then we do not have to handle the 255 exit code here since
> > > > the test would be skipped if it's missing.
> > > If we use needs_cmds all the check will be skipped in this case.
> > @Cyril: only single test require 'parted' as I reported in v1 [1].
> > Yeah, code gets slightly more complicated just because single test requires
> > parted. Or you would not care? IMHO it does not make sense to split test into
> > two (too much duplicity).
> The problem here is how to handle the metadata. One posible solution is
> to add a notion of optional dependencies so that we would have
> 'needs_foo' and 'wants_foo'. Or turn the needs_foo into a structure with
> an .optional boolean flag.
+1 but that should wait after the release.
Can I merge it with your RBT with the following diff below?
Kind regards,
Petr
> > But TINFO message should be turned in TCONF so that people notice.
> > tst_res(TINFO, "Current environment doesn't have parted disk, skip it");
> Yes please.
+++ testcases/kernel/syscalls/ioctl/ioctl_loop01.c
@@ -59,7 +59,7 @@ static void check_loop_value(int set_flag, int get_flag, int autoclear_field)
TST_ASSERT_INT(autoclear_path, autoclear_field);
if (!parted_sup) {
- tst_res(TINFO, "Current environment doesn't have parted disk, skip it");
+ tst_res(TCONF, "Current environment doesn't have parted disk, skip it");
return;
}
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [LTP] [PATCH v3] ioctl_loop01.c: Use proper device for partitioning
2025-09-19 13:22 ` Petr Vorel
@ 2025-09-22 7:28 ` Cyril Hrubis
2025-09-22 7:32 ` Petr Vorel
0 siblings, 1 reply; 21+ messages in thread
From: Cyril Hrubis @ 2025-09-22 7:28 UTC (permalink / raw)
To: Petr Vorel; +Cc: ltp
Hi!
> > The problem here is how to handle the metadata. One posible solution is
> > to add a notion of optional dependencies so that we would have
> > 'needs_foo' and 'wants_foo'. Or turn the needs_foo into a structure with
> > an .optional boolean flag.
>
> +1 but that should wait after the release.
Yes.
> Can I merge it with your RBT with the following diff below?
Yes please.
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [LTP] [PATCH v3] ioctl_loop01.c: Use proper device for partitioning
2025-09-22 7:28 ` Cyril Hrubis
@ 2025-09-22 7:32 ` Petr Vorel
0 siblings, 0 replies; 21+ messages in thread
From: Petr Vorel @ 2025-09-22 7:32 UTC (permalink / raw)
To: Cyril Hrubis; +Cc: ltp
Hi Wei, Cyril,
> Hi!
> > > The problem here is how to handle the metadata. One posible solution is
> > > to add a notion of optional dependencies so that we would have
> > > 'needs_foo' and 'wants_foo'. Or turn the needs_foo into a structure with
> > > an .optional boolean flag.
> > +1 but that should wait after the release.
> Yes.
> > Can I merge it with your RBT with the following diff below?
> Yes please.
Great, merged! Thanks to both.
Kind regards,
Petr
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [LTP] [PATCH v3] ioctl_loop01.c: Use proper device for partitioning
2025-09-18 15:35 ` Cyril Hrubis
2025-09-19 13:22 ` Petr Vorel
@ 2025-09-24 2:26 ` Wei Gao via ltp
2025-09-24 7:03 ` Petr Vorel
2025-09-24 9:54 ` Cyril Hrubis
1 sibling, 2 replies; 21+ messages in thread
From: Wei Gao via ltp @ 2025-09-24 2:26 UTC (permalink / raw)
To: Cyril Hrubis; +Cc: Jan Kara, ltp
On Thu, Sep 18, 2025 at 05:35:15PM +0200, Cyril Hrubis wrote:
> Hi!
> > > > The test should have needs_cmds set to parted (we do that properly in
> > > > ioctl09.c) then we do not have to handle the 255 exit code here since
> > > > the test would be skipped if it's missing.
> >
> > > If we use needs_cmds all the check will be skipped in this case.
> >
> > @Cyril: only single test require 'parted' as I reported in v1 [1].
> > Yeah, code gets slightly more complicated just because single test requires
> > parted. Or you would not care? IMHO it does not make sense to split test into
> > two (too much duplicity).
>
> The problem here is how to handle the metadata. One posible solution is
> to add a notion of optional dependencies so that we would have
> 'needs_foo' and 'wants_foo'. Or turn the needs_foo into a structure with
> an .optional boolean flag.
What's difference between needs_foo and wants_foo? wants_foo means we do
not do brk if not exist foo?
I guess we need wants_parted support for .needs_cmds like following
change? Could you give me more guidance
--- a/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
@@ -147,6 +147,10 @@ static struct tst_test test = {
"loop",
NULL
},
+ .needs_cmds= (const char *const []) {
+ "wants_parted",
+ NULL
+ },
>
> > But TINFO message should be turned in TCONF so that people notice.
> > tst_res(TINFO, "Current environment doesn't have parted disk, skip it");
>
> Yes please.
>
> --
> Cyril Hrubis
> chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [LTP] [PATCH v3] ioctl_loop01.c: Use proper device for partitioning
2025-09-24 2:26 ` Wei Gao via ltp
@ 2025-09-24 7:03 ` Petr Vorel
2025-09-24 9:54 ` Cyril Hrubis
1 sibling, 0 replies; 21+ messages in thread
From: Petr Vorel @ 2025-09-24 7:03 UTC (permalink / raw)
To: Wei Gao; +Cc: Jan Kara, ltp
Hi all,
> On Thu, Sep 18, 2025 at 05:35:15PM +0200, Cyril Hrubis wrote:
> > Hi!
> > > > > The test should have needs_cmds set to parted (we do that properly in
> > > > > ioctl09.c) then we do not have to handle the 255 exit code here since
> > > > > the test would be skipped if it's missing.
> > > > If we use needs_cmds all the check will be skipped in this case.
> > > @Cyril: only single test require 'parted' as I reported in v1 [1].
> > > Yeah, code gets slightly more complicated just because single test requires
> > > parted. Or you would not care? IMHO it does not make sense to split test into
> > > two (too much duplicity).
> > The problem here is how to handle the metadata. One posible solution is
> > to add a notion of optional dependencies so that we would have
> > 'needs_foo' and 'wants_foo'. Or turn the needs_foo into a structure with
> > an .optional boolean flag.
> What's difference between needs_foo and wants_foo? wants_foo means we do
> not do brk if not exist foo?
> I guess we need wants_parted support for .needs_cmds like following
> change? Could you give me more guidance
> --- a/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
> +++ b/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
> @@ -147,6 +147,10 @@ static struct tst_test test = {
> "loop",
> NULL
> },
> + .needs_cmds= (const char *const []) {
> + "wants_parted",
IMHO this is a wrong way. The command name ("value") should not need to be
parsed. Why?
1) not obvious
2) theoretically there can be a binary "wants_*")
Cyril's approach to change "key" (i.e. .needs_cmd => .wants_cmd) is better.
Alternatives to wants_* could be "needs_foo_subtest" or "uses_foo"
Kind regards,
Petr
> + NULL
> + },
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [LTP] [PATCH v3] ioctl_loop01.c: Use proper device for partitioning
2025-09-24 2:26 ` Wei Gao via ltp
2025-09-24 7:03 ` Petr Vorel
@ 2025-09-24 9:54 ` Cyril Hrubis
2025-09-24 10:40 ` Wei Gao via ltp
1 sibling, 1 reply; 21+ messages in thread
From: Cyril Hrubis @ 2025-09-24 9:54 UTC (permalink / raw)
To: Wei Gao; +Cc: Jan Kara, ltp
Hi!
> What's difference between needs_foo and wants_foo? wants_foo means we do
> not do brk if not exist foo?
> I guess we need wants_parted support for .needs_cmds like following
> change? Could you give me more guidance
>
> --- a/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
> +++ b/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
> @@ -147,6 +147,10 @@ static struct tst_test test = {
> "loop",
> NULL
> },
> + .needs_cmds= (const char *const []) {
> + "wants_parted",
> + NULL
> + },
As Peter explained support for this is not in the test library yet.
My proposal was either of:
1. Add .wants_cmd key to the tst_test structure
- pros: easy to add, easy to handle
- cons: the number of tst_test structure members will eventually
double
2. Add a flag to the needs_cmd, i.e. convert needs_cmd into an array of
structures, we do that for other cases like saving and restoring
proc/sys files
- pros: we will keep the number of members of tst_test
this is easily expandable, e.g. adding support for
minimal version
- cons: all tests that have needs cmds will need to be adjusted
This would look like:
struct tst_cmd {
const char *cmd;
unsigned int required:1;
};
After thinking about this I've started to lean towards option 2.
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [LTP] [PATCH v3] ioctl_loop01.c: Use proper device for partitioning
2025-09-24 9:54 ` Cyril Hrubis
@ 2025-09-24 10:40 ` Wei Gao via ltp
2025-09-24 10:54 ` Cyril Hrubis
0 siblings, 1 reply; 21+ messages in thread
From: Wei Gao via ltp @ 2025-09-24 10:40 UTC (permalink / raw)
To: Cyril Hrubis; +Cc: Jan Kara, ltp
On Wed, Sep 24, 2025 at 11:54:00AM +0200, Cyril Hrubis wrote:
> Hi!
> > What's difference between needs_foo and wants_foo? wants_foo means we do
> > not do brk if not exist foo?
> > I guess we need wants_parted support for .needs_cmds like following
> > change? Could you give me more guidance
> >
> > --- a/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
> > +++ b/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
> > @@ -147,6 +147,10 @@ static struct tst_test test = {
> > "loop",
> > NULL
> > },
> > + .needs_cmds= (const char *const []) {
> > + "wants_parted",
> > + NULL
> > + },
>
> As Peter explained support for this is not in the test library yet.
>
> My proposal was either of:
>
> 1. Add .wants_cmd key to the tst_test structure
> - pros: easy to add, easy to handle
> - cons: the number of tst_test structure members will eventually
> double
>
> 2. Add a flag to the needs_cmd, i.e. convert needs_cmd into an array of
> structures, we do that for other cases like saving and restoring
> proc/sys files
> - pros: we will keep the number of members of tst_test
> this is easily expandable, e.g. adding support for
> minimal version
> - cons: all tests that have needs cmds will need to be adjusted
>
> This would look like:
>
> struct tst_cmd {
> const char *cmd;
> unsigned int required:1;
> };
>
>
> After thinking about this I've started to lean towards option 2.
If we select option 2 then we have following setting for all related
test cases.
+ .needs_cmds = (const struct tst_cmd[]) {
+ {"parted", "0"},
+ {}
+ },
But how to get "parted" command support status? In ioctl_loop01.c there
is a local var "parted_sup" which flag parted command support or not, and
use this flag to decide which sub test needed.
If we do this logic in test lib then we need create another
flag in tst_cmd used for give status of command support or not.
During test lib logic will set tst_cmd.support flag.
struct tst_cmd {
const char *cmd;
unsigned int required:1;
unsigned int support:1;
};
Correct me if any misunderstanding, many thanks.
>
> --
> Cyril Hrubis
> chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [LTP] [PATCH v3] ioctl_loop01.c: Use proper device for partitioning
2025-09-24 10:40 ` Wei Gao via ltp
@ 2025-09-24 10:54 ` Cyril Hrubis
2025-09-24 12:55 ` Petr Vorel
0 siblings, 1 reply; 21+ messages in thread
From: Cyril Hrubis @ 2025-09-24 10:54 UTC (permalink / raw)
To: Wei Gao; +Cc: Jan Kara, ltp
Hi!
> If we select option 2 then we have following setting for all related
> test cases.
> + .needs_cmds = (const struct tst_cmd[]) {
> + {"parted", "0"},
> + {}
> + },
>
> But how to get "parted" command support status? In ioctl_loop01.c there
> is a local var "parted_sup" which flag parted command support or not, and
> use this flag to decide which sub test needed.
> If we do this logic in test lib then we need create another
> flag in tst_cmd used for give status of command support or not.
> During test lib logic will set tst_cmd.support flag.
>
> struct tst_cmd {
> const char *cmd;
> unsigned int required:1;
> unsigned int support:1;
> };
>
> Correct me if any misunderstanding, many thanks.
Sounds reasonable and we could also add a nice function to the test
library:
bool tst_cmd_present(const char *cmd);
that would loop over the tst_cmd array and return the supported flag
value.
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [LTP] [PATCH v3] ioctl_loop01.c: Use proper device for partitioning
2025-09-24 10:54 ` Cyril Hrubis
@ 2025-09-24 12:55 ` Petr Vorel
2025-09-24 13:17 ` Cyril Hrubis
0 siblings, 1 reply; 21+ messages in thread
From: Petr Vorel @ 2025-09-24 12:55 UTC (permalink / raw)
To: Cyril Hrubis; +Cc: Jan Kara, ltp
> Hi!
> > If we select option 2 then we have following setting for all related
> > test cases.
> > + .needs_cmds = (const struct tst_cmd[]) {
> > + {"parted", "0"},
> > + {}
> > + },
> > But how to get "parted" command support status? In ioctl_loop01.c there
> > is a local var "parted_sup" which flag parted command support or not, and
> > use this flag to decide which sub test needed.
> > If we do this logic in test lib then we need create another
> > flag in tst_cmd used for give status of command support or not.
> > During test lib logic will set tst_cmd.support flag.
> > struct tst_cmd {
> > const char *cmd;
> > unsigned int required:1;
> > unsigned int support:1;
> > };
> > Correct me if any misunderstanding, many thanks.
> Sounds reasonable and we could also add a nice function to the test
> library:
> bool tst_cmd_present(const char *cmd);
> that would loop over the tst_cmd array and return the supported flag
> value.
+1, although not sure if we want to finish this before the release.
Kind regards,
Petr
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [LTP] [PATCH v3] ioctl_loop01.c: Use proper device for partitioning
2025-09-24 12:55 ` Petr Vorel
@ 2025-09-24 13:17 ` Cyril Hrubis
0 siblings, 0 replies; 21+ messages in thread
From: Cyril Hrubis @ 2025-09-24 13:17 UTC (permalink / raw)
To: Petr Vorel; +Cc: Jan Kara, ltp
Hi!
> > > Correct me if any misunderstanding, many thanks.
>
> > Sounds reasonable and we could also add a nice function to the test
> > library:
>
> > bool tst_cmd_present(const char *cmd);
>
> > that would loop over the tst_cmd array and return the supported flag
> > value.
>
> +1, although not sure if we want to finish this before the release.
This is obviously something to be done later.
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2025-09-24 13:17 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-01 7:47 [LTP] [PATCH v1] ioctl_loop01.c: Use proper device for partitioning Wei Gao via ltp
2025-09-01 10:38 ` Petr Vorel
2025-09-02 2:16 ` Wei Gao via ltp
2025-09-02 3:12 ` [LTP] [PATCH v2] " Wei Gao via ltp
2025-09-02 10:44 ` Petr Vorel
2025-09-02 11:18 ` [LTP] [PATCH v3] " Wei Gao via ltp
2025-09-03 12:48 ` Petr Vorel
2025-09-09 11:50 ` Cyril Hrubis
2025-09-10 1:35 ` Wei Gao via ltp
2025-09-18 14:53 ` Petr Vorel
2025-09-18 15:35 ` Cyril Hrubis
2025-09-19 13:22 ` Petr Vorel
2025-09-22 7:28 ` Cyril Hrubis
2025-09-22 7:32 ` Petr Vorel
2025-09-24 2:26 ` Wei Gao via ltp
2025-09-24 7:03 ` Petr Vorel
2025-09-24 9:54 ` Cyril Hrubis
2025-09-24 10:40 ` Wei Gao via ltp
2025-09-24 10:54 ` Cyril Hrubis
2025-09-24 12:55 ` Petr Vorel
2025-09-24 13:17 ` Cyril Hrubis
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox