* [LTP] [PATCH 0/3] Cleanup tst_ioctl.c, libswap.c
@ 2023-10-11 16:08 Petr Vorel
2023-10-11 16:08 ` [LTP] [PATCH 1/3] tst_ioctl: Cleanup the code Petr Vorel
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Petr Vorel @ 2023-10-11 16:08 UTC (permalink / raw)
To: ltp; +Cc: Richard Palethorpe
Hi,
this is somehow related to #1091 [1], but independent on it.
I also wonder if we should move tst_fibmap() out of lib/tst_ioctl.c
(it's the only function there) somewhere. include/tst_fs.h is used
several files, I wonder if there should be lib/tst_fs.c.
Maybe not all C files which use include/tst_fs.h should be merged in it,
but some of them contain only single function (e.g. lib/tst_fs_has_free.c).
Is there any benefit to have that separate? Size? In the end we link libltp.a
to every test binary (unfortunately). I also don't think that for
readability purposes of the sources we need 5 C files in lib/ which use
tst_test.h.
Kind regards,
Petr
[1] https://github.com/linux-test-project/ltp/issues/1091
Petr Vorel (3):
tst_ioctl: Cleanup the code
tst_fs.h: Improve tst_fibmap() doc
libltpswap: TCONF on EPERM
include/tst_fs.h | 5 ++++-
lib/tst_ioctl.c | 18 ++++--------------
libs/libltpswap/libswap.c | 10 +++++++---
3 files changed, 15 insertions(+), 18 deletions(-)
--
2.42.0
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 12+ messages in thread
* [LTP] [PATCH 1/3] tst_ioctl: Cleanup the code
2023-10-11 16:08 [LTP] [PATCH 0/3] Cleanup tst_ioctl.c, libswap.c Petr Vorel
@ 2023-10-11 16:08 ` Petr Vorel
2023-12-08 15:48 ` Cyril Hrubis
2023-10-11 16:08 ` [LTP] [PATCH 2/3] tst_fs.h: Improve tst_fibmap() doc Petr Vorel
` (2 subsequent siblings)
3 siblings, 1 reply; 12+ messages in thread
From: Petr Vorel @ 2023-10-11 16:08 UTC (permalink / raw)
To: ltp; +Cc: Richard Palethorpe
From: Petr Vorel <petr.vorel@gmail.com>
* Use SAFE_{OPEN,CLOSE}() (simplification), -1 return code is removed.
* Cleanup unused headers.
* Include tst_fs.h, to keep signature checked during compilation.
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
lib/tst_ioctl.c | 18 ++++--------------
1 file changed, 4 insertions(+), 14 deletions(-)
diff --git a/lib/tst_ioctl.c b/lib/tst_ioctl.c
index 364220bcd..34a056cf4 100644
--- a/lib/tst_ioctl.c
+++ b/lib/tst_ioctl.c
@@ -1,37 +1,27 @@
// SPDX-License-Identifier: GPL-2.0-or-later
-#include <errno.h>
-#include <stdio.h>
-#include <stdlib.h>
#include <sys/ioctl.h>
#include <linux/fs.h>
#define TST_NO_DEFAULT_MAIN
#include "tst_test.h"
+#include "tst_fs.h"
int tst_fibmap(const char *filename)
{
- /* test if FIBMAP ioctl is supported */
int fd, block = 0;
- fd = open(filename, O_RDWR | O_CREAT, 0666);
- if (fd < 0) {
- tst_res(TWARN | TERRNO,
- "open(%s, O_RDWR | O_CREAT, 0666) failed", filename);
- return -1;
- }
+ fd = SAFE_OPEN(filename, O_RDWR | O_CREAT, 0666);
if (ioctl(fd, FIBMAP, &block)) {
tst_res(TINFO | TERRNO, "FIBMAP ioctl is NOT supported");
close(fd);
return 1;
}
+
tst_res(TINFO, "FIBMAP ioctl is supported");
+ SAFE_CLOSE(fd);
- if (close(fd)) {
- tst_res(TWARN | TERRNO, "close(fd) failed");
- return -1;
- }
return 0;
}
--
2.42.0
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [LTP] [PATCH 2/3] tst_fs.h: Improve tst_fibmap() doc
2023-10-11 16:08 [LTP] [PATCH 0/3] Cleanup tst_ioctl.c, libswap.c Petr Vorel
2023-10-11 16:08 ` [LTP] [PATCH 1/3] tst_ioctl: Cleanup the code Petr Vorel
@ 2023-10-11 16:08 ` Petr Vorel
2024-01-19 11:20 ` Cyril Hrubis
2023-10-11 16:08 ` [LTP] [PATCH 3/3] libltpswap: TCONF on EPERM Petr Vorel
2023-10-11 16:18 ` [LTP] [PATCH 0/3] Cleanup tst_ioctl.c, libswap.c Marius Kittler
3 siblings, 1 reply; 12+ messages in thread
From: Petr Vorel @ 2023-10-11 16:08 UTC (permalink / raw)
To: ltp; +Cc: Richard Palethorpe
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
include/tst_fs.h | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/include/tst_fs.h b/include/tst_fs.h
index 769fac1e5..3dd0d7524 100644
--- a/include/tst_fs.h
+++ b/include/tst_fs.h
@@ -209,7 +209,10 @@ int tst_fs_in_skiplist(const char *fs_type, const char *const *skiplist);
void tst_fill_fs(const char *path, int verbose, enum tst_fill_access_pattern pattern);
/*
- * test if FIBMAP ioctl is supported
+ * Check if FIBMAP ioctl is supported.
+ * Tests which use tst_fibmap() should use .needs_root = 1 to avoid EPERM.
+ *
+ * @return 0: FIBMAP is supported, 1: FIBMAP is *not* supported.
*/
int tst_fibmap(const char *filename);
--
2.42.0
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [LTP] [PATCH 3/3] libltpswap: TCONF on EPERM
2023-10-11 16:08 [LTP] [PATCH 0/3] Cleanup tst_ioctl.c, libswap.c Petr Vorel
2023-10-11 16:08 ` [LTP] [PATCH 1/3] tst_ioctl: Cleanup the code Petr Vorel
2023-10-11 16:08 ` [LTP] [PATCH 2/3] tst_fs.h: Improve tst_fibmap() doc Petr Vorel
@ 2023-10-11 16:08 ` Petr Vorel
2023-12-08 16:02 ` Cyril Hrubis
2023-10-11 16:18 ` [LTP] [PATCH 0/3] Cleanup tst_ioctl.c, libswap.c Marius Kittler
3 siblings, 1 reply; 12+ messages in thread
From: Petr Vorel @ 2023-10-11 16:08 UTC (permalink / raw)
To: ltp; +Cc: Richard Palethorpe
Although tests, which use tst_fibmap() should use .needs_root = 1
to avoid EPERM, let's add a sanity check in case it's not used.
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
libs/libltpswap/libswap.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/libs/libltpswap/libswap.c b/libs/libltpswap/libswap.c
index a4427736f..4ef24ef5c 100644
--- a/libs/libltpswap/libswap.c
+++ b/libs/libltpswap/libswap.c
@@ -53,10 +53,14 @@ void is_swap_supported(const char *filename)
TEST(tst_syscall(__NR_swapon, filename, 0));
if (TST_RET == -1) {
- if (fibmap == 1 && errno == EINVAL)
- tst_brk(TCONF, "Swapfile on %s not implemented", fstype);
- else
+ if (fibmap == 1) {
+ if (errno == EINVAL)
+ tst_brk(TCONF, "Swapfile on %s not implemented", fstype);
+ if (errno == EPERM)
+ tst_brk(TCONF, "No permission for swapon() on %s", fstype);
+ } else {
tst_brk(TFAIL | TTERRNO, "swapon on %s failed", fstype);
+ }
}
TEST(tst_syscall(__NR_swapoff, filename, 0));
--
2.42.0
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [LTP] [PATCH 0/3] Cleanup tst_ioctl.c, libswap.c
2023-10-11 16:08 [LTP] [PATCH 0/3] Cleanup tst_ioctl.c, libswap.c Petr Vorel
` (2 preceding siblings ...)
2023-10-11 16:08 ` [LTP] [PATCH 3/3] libltpswap: TCONF on EPERM Petr Vorel
@ 2023-10-11 16:18 ` Marius Kittler
3 siblings, 0 replies; 12+ messages in thread
From: Marius Kittler @ 2023-10-11 16:18 UTC (permalink / raw)
To: ltp
LGTM
Reviewed-by: Marius Kittler <mkittler@suse.de>
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [LTP] [PATCH 1/3] tst_ioctl: Cleanup the code
2023-10-11 16:08 ` [LTP] [PATCH 1/3] tst_ioctl: Cleanup the code Petr Vorel
@ 2023-12-08 15:48 ` Cyril Hrubis
2023-12-10 16:22 ` Petr Vorel
0 siblings, 1 reply; 12+ messages in thread
From: Cyril Hrubis @ 2023-12-08 15:48 UTC (permalink / raw)
To: Petr Vorel; +Cc: Richard Palethorpe, ltp
Hi!
>
> if (ioctl(fd, FIBMAP, &block)) {
> tst_res(TINFO | TERRNO, "FIBMAP ioctl is NOT supported");
> close(fd);
This should be SAFE_CLOSE() as well, since we are doing the conversion.
Other than that:
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [LTP] [PATCH 3/3] libltpswap: TCONF on EPERM
2023-10-11 16:08 ` [LTP] [PATCH 3/3] libltpswap: TCONF on EPERM Petr Vorel
@ 2023-12-08 16:02 ` Cyril Hrubis
2023-12-10 16:24 ` Petr Vorel
2023-12-11 5:28 ` Petr Vorel
0 siblings, 2 replies; 12+ messages in thread
From: Cyril Hrubis @ 2023-12-08 16:02 UTC (permalink / raw)
To: Petr Vorel; +Cc: Richard Palethorpe, ltp
Hi!
> + if (fibmap == 1) {
> + if (errno == EINVAL)
> + tst_brk(TCONF, "Swapfile on %s not implemented", fstype);
> + if (errno == EPERM)
> + tst_brk(TCONF, "No permission for swapon() on %s", fstype);
I'm not sure if we actually need to print the filesystem type in the
case that we do not have a permission to do swapon.
Also this kind of works because the fibmap ioctl() fails as well,
however I guess that this should really be:
if (errno == EPERM)
tst_brk(TCONF, "Permission denied for swapon()");
else if (fibmap == 1 && errno == EINVAL)
tst_brk(TCONF, "Swapfile on %s not implemented", fstype);
else
tst_brk(TFAIL | TTERNO, ...);
> + } else {
> tst_brk(TFAIL | TTERRNO, "swapon on %s failed", fstype);
> + }
> }
>
> TEST(tst_syscall(__NR_swapoff, filename, 0));
> --
> 2.42.0
>
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [LTP] [PATCH 1/3] tst_ioctl: Cleanup the code
2023-12-08 15:48 ` Cyril Hrubis
@ 2023-12-10 16:22 ` Petr Vorel
0 siblings, 0 replies; 12+ messages in thread
From: Petr Vorel @ 2023-12-10 16:22 UTC (permalink / raw)
To: Cyril Hrubis; +Cc: Richard Palethorpe, ltp
Hi Cyril,
> Hi!
> > if (ioctl(fd, FIBMAP, &block)) {
> > tst_res(TINFO | TERRNO, "FIBMAP ioctl is NOT supported");
> > close(fd);
> This should be SAFE_CLOSE() as well, since we are doing the conversion.
Good point, thanks. I added missing license and merged.
Kind regards,
Petr
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [LTP] [PATCH 3/3] libltpswap: TCONF on EPERM
2023-12-08 16:02 ` Cyril Hrubis
@ 2023-12-10 16:24 ` Petr Vorel
2023-12-11 5:28 ` Petr Vorel
1 sibling, 0 replies; 12+ messages in thread
From: Petr Vorel @ 2023-12-10 16:24 UTC (permalink / raw)
To: Cyril Hrubis; +Cc: Richard Palethorpe, ltp
Hi Cyril,
> Hi!
> > + if (fibmap == 1) {
> > + if (errno == EINVAL)
> > + tst_brk(TCONF, "Swapfile on %s not implemented", fstype);
> > + if (errno == EPERM)
> > + tst_brk(TCONF, "No permission for swapon() on %s", fstype);
> I'm not sure if we actually need to print the filesystem type in the
> case that we do not have a permission to do swapon.
> Also this kind of works because the fibmap ioctl() fails as well,
> however I guess that this should really be:
> if (errno == EPERM)
> tst_brk(TCONF, "Permission denied for swapon()");
> else if (fibmap == 1 && errno == EINVAL)
> tst_brk(TCONF, "Swapfile on %s not implemented", fstype);
> else
> tst_brk(TFAIL | TTERNO, ...);
Ah, that's actually better, thanks!
Tomorrow, I'll modify it and merge with your RBT.
Kind regards,
Petr
> > + } else {
> > tst_brk(TFAIL | TTERRNO, "swapon on %s failed", fstype);
> > + }
> > }
> > TEST(tst_syscall(__NR_swapoff, filename, 0));
> > --
> > 2.42.0
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [LTP] [PATCH 3/3] libltpswap: TCONF on EPERM
2023-12-08 16:02 ` Cyril Hrubis
2023-12-10 16:24 ` Petr Vorel
@ 2023-12-11 5:28 ` Petr Vorel
1 sibling, 0 replies; 12+ messages in thread
From: Petr Vorel @ 2023-12-11 5:28 UTC (permalink / raw)
To: Cyril Hrubis; +Cc: Richard Palethorpe, ltp
Hi Cyril,
commit merged, thanks!
Kind regards,
Petr
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [LTP] [PATCH 2/3] tst_fs.h: Improve tst_fibmap() doc
2023-10-11 16:08 ` [LTP] [PATCH 2/3] tst_fs.h: Improve tst_fibmap() doc Petr Vorel
@ 2024-01-19 11:20 ` Cyril Hrubis
2024-01-19 13:15 ` Petr Vorel
0 siblings, 1 reply; 12+ messages in thread
From: Cyril Hrubis @ 2024-01-19 11:20 UTC (permalink / raw)
To: Petr Vorel; +Cc: Richard Palethorpe, ltp
Hi!
> /*
> - * test if FIBMAP ioctl is supported
> + * Check if FIBMAP ioctl is supported.
> + * Tests which use tst_fibmap() should use .needs_root = 1 to avoid EPERM.
^
Maybe just:
Tests nees to set .needs_root = 1 in order to avoid EPERM.
> + * @return 0: FIBMAP is supported, 1: FIBMAP is *not* supported.
Otherwise it looks good.
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [LTP] [PATCH 2/3] tst_fs.h: Improve tst_fibmap() doc
2024-01-19 11:20 ` Cyril Hrubis
@ 2024-01-19 13:15 ` Petr Vorel
0 siblings, 0 replies; 12+ messages in thread
From: Petr Vorel @ 2024-01-19 13:15 UTC (permalink / raw)
To: Cyril Hrubis; +Cc: ltp
> Hi!
> > /*
> > - * test if FIBMAP ioctl is supported
> > + * Check if FIBMAP ioctl is supported.
> > + * Tests which use tst_fibmap() should use .needs_root = 1 to avoid EPERM.
> ^
> Maybe just:
> Tests nees to set .needs_root = 1 in order to avoid EPERM.
+1 (with "needs")
Thanks, merged!
Kind regards,
Petr
> > + * @return 0: FIBMAP is supported, 1: FIBMAP is *not* supported.
> Otherwise it looks good.
> Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2024-01-19 13:15 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-11 16:08 [LTP] [PATCH 0/3] Cleanup tst_ioctl.c, libswap.c Petr Vorel
2023-10-11 16:08 ` [LTP] [PATCH 1/3] tst_ioctl: Cleanup the code Petr Vorel
2023-12-08 15:48 ` Cyril Hrubis
2023-12-10 16:22 ` Petr Vorel
2023-10-11 16:08 ` [LTP] [PATCH 2/3] tst_fs.h: Improve tst_fibmap() doc Petr Vorel
2024-01-19 11:20 ` Cyril Hrubis
2024-01-19 13:15 ` Petr Vorel
2023-10-11 16:08 ` [LTP] [PATCH 3/3] libltpswap: TCONF on EPERM Petr Vorel
2023-12-08 16:02 ` Cyril Hrubis
2023-12-10 16:24 ` Petr Vorel
2023-12-11 5:28 ` Petr Vorel
2023-10-11 16:18 ` [LTP] [PATCH 0/3] Cleanup tst_ioctl.c, libswap.c Marius Kittler
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.