* [LTP] [PATCH] containers/utsname04: don't use TST_EXP_EQ_LI for tst_clone
@ 2023-02-28 8:44 Yang Xu
2023-02-28 9:06 ` Andrea Cervesato via ltp
0 siblings, 1 reply; 9+ messages in thread
From: Yang Xu @ 2023-02-28 8:44 UTC (permalink / raw)
To: ltp
For old kernel that doesn't support clone3, tst_clone returns
-2 instead of -1. So we can't use TST_EXP_EQ_LI api.
Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
testcases/kernel/containers/utsname/utsname04.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/testcases/kernel/containers/utsname/utsname04.c b/testcases/kernel/containers/utsname/utsname04.c
index e8d636d0d..1adc4542c 100644
--- a/testcases/kernel/containers/utsname/utsname04.c
+++ b/testcases/kernel/containers/utsname/utsname04.c
@@ -22,6 +22,7 @@ static void run(void)
{
const struct tst_clone_args cargs = { CLONE_NEWUTS, SIGCHLD };
struct passwd *pw;
+ pid_t pid;
tst_res(TINFO, "Dropping root privileges");
@@ -29,7 +30,11 @@ static void run(void)
SAFE_SETRESUID(pw->pw_uid, pw->pw_uid, pw->pw_uid);
if (!str_op || !strcmp(str_op, "clone")) {
- TST_EXP_EQ_LI(tst_clone(&cargs), -1);
+ pid = tst_clone(&cargs);
+ if (pid == -1 || pid == -2)
+ tst_res(TPASS, "clone() returns an expected value %d", pid);
+ else
+ tst_res(TFAIL, "clone() returns an unexpected value %d", pid);
TST_EXP_PASS(errno == EPERM);
} else {
if (!SAFE_FORK()) {
--
2.39.1
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [LTP] [PATCH] containers/utsname04: don't use TST_EXP_EQ_LI for tst_clone
2023-02-28 8:44 [LTP] [PATCH] containers/utsname04: don't use TST_EXP_EQ_LI for tst_clone Yang Xu
@ 2023-02-28 9:06 ` Andrea Cervesato via ltp
2023-02-28 9:18 ` xuyang2018.jy
0 siblings, 1 reply; 9+ messages in thread
From: Andrea Cervesato via ltp @ 2023-02-28 9:06 UTC (permalink / raw)
To: ltp
Hi!
On 2/28/23 09:44, Yang Xu wrote:
> For old kernel that doesn't support clone3, tst_clone returns
it would be nice to add unsupported kernel version inside patch comment
(in this case, anything below 5.3).
> -2 instead of -1. So we can't use TST_EXP_EQ_LI api.
>
> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
> ---
> testcases/kernel/containers/utsname/utsname04.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/testcases/kernel/containers/utsname/utsname04.c b/testcases/kernel/containers/utsname/utsname04.c
> index e8d636d0d..1adc4542c 100644
> --- a/testcases/kernel/containers/utsname/utsname04.c
> +++ b/testcases/kernel/containers/utsname/utsname04.c
> @@ -22,6 +22,7 @@ static void run(void)
> {
> const struct tst_clone_args cargs = { CLONE_NEWUTS, SIGCHLD };
> struct passwd *pw;
> + pid_t pid;
>
> tst_res(TINFO, "Dropping root privileges");
>
> @@ -29,7 +30,11 @@ static void run(void)
> SAFE_SETRESUID(pw->pw_uid, pw->pw_uid, pw->pw_uid);
>
> if (!str_op || !strcmp(str_op, "clone")) {
> - TST_EXP_EQ_LI(tst_clone(&cargs), -1);
Did you try this?
TST_EXP_FAIL(tst_clone(&cargs), EPERM);
> + pid = tst_clone(&cargs);
> + if (pid == -1 || pid == -2)
> + tst_res(TPASS, "clone() returns an expected value %d", pid);
> + else
> + tst_res(TFAIL, "clone() returns an unexpected value %d", pid);
> TST_EXP_PASS(errno == EPERM);
> } else {
> if (!SAFE_FORK()) {
Regards,
Andrea
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [LTP] [PATCH] containers/utsname04: don't use TST_EXP_EQ_LI for tst_clone
2023-02-28 9:06 ` Andrea Cervesato via ltp
@ 2023-02-28 9:18 ` xuyang2018.jy
2023-02-28 9:39 ` Cyril Hrubis
0 siblings, 1 reply; 9+ messages in thread
From: xuyang2018.jy @ 2023-02-28 9:18 UTC (permalink / raw)
To: Andrea Cervesato, ltp@lists.linux.it
Hi Andrea
> Hi!
>
> On 2/28/23 09:44, Yang Xu wrote:
>
>> For old kernel that doesn't support clone3, tst_clone returns
> it would be nice to add unsupported kernel version inside patch comment
> (in this case, anything below 5.3).
Yes.
>> -2 instead of -1. So we can't use TST_EXP_EQ_LI api.
>>
>> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
>> ---
>> testcases/kernel/containers/utsname/utsname04.c | 7 ++++++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/testcases/kernel/containers/utsname/utsname04.c
>> b/testcases/kernel/containers/utsname/utsname04.c
>> index e8d636d0d..1adc4542c 100644
>> --- a/testcases/kernel/containers/utsname/utsname04.c
>> +++ b/testcases/kernel/containers/utsname/utsname04.c
>> @@ -22,6 +22,7 @@ static void run(void)
>> {
>> const struct tst_clone_args cargs = { CLONE_NEWUTS, SIGCHLD };
>> struct passwd *pw;
>> + pid_t pid;
>> tst_res(TINFO, "Dropping root privileges");
>> @@ -29,7 +30,11 @@ static void run(void)
>> SAFE_SETRESUID(pw->pw_uid, pw->pw_uid, pw->pw_uid);
>> if (!str_op || !strcmp(str_op, "clone")) {
>> - TST_EXP_EQ_LI(tst_clone(&cargs), -1);
>
> Did you try this?
>
> TST_EXP_FAIL(tst_clone(&cargs), EPERM);
I don't try this because I remembered TST_EXP_FAIL only thinks the
correct return value is -1.
tst_test.c:1560: TINFO: Timeout per run is 0h 00m 30s
utsname04.c:27: TINFO: Dropping root privileges
utsname04.c:33: TFAIL: tst_clone(&cargs) invalid retval -2: EPERM (1)
Best Regards
Yang Xu
>
>> + pid = tst_clone(&cargs);
>> + if (pid == -1 || pid == -2)
>> + tst_res(TPASS, "clone() returns an expected value %d", pid);
>> + else
>> + tst_res(TFAIL, "clone() returns an unexpected value %d",
>> pid);
>> TST_EXP_PASS(errno == EPERM);
>> } else {
>> if (!SAFE_FORK()) {
>
> Regards,
> Andrea
>
>
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [LTP] [PATCH] containers/utsname04: don't use TST_EXP_EQ_LI for tst_clone
2023-02-28 9:18 ` xuyang2018.jy
@ 2023-02-28 9:39 ` Cyril Hrubis
2023-03-03 6:11 ` xuyang2018.jy
0 siblings, 1 reply; 9+ messages in thread
From: Cyril Hrubis @ 2023-02-28 9:39 UTC (permalink / raw)
To: xuyang2018.jy@fujitsu.com; +Cc: Richard Palethorpe, ltp@lists.linux.it
Hi!
> > Did you try this?
> >
> > TST_EXP_FAIL(tst_clone(&cargs), EPERM);
>
> I don't try this because I remembered TST_EXP_FAIL only thinks the
> correct return value is -1.
>
> tst_test.c:1560: TINFO: Timeout per run is 0h 00m 30s
> utsname04.c:27: TINFO: Dropping root privileges
> utsname04.c:33: TFAIL: tst_clone(&cargs) invalid retval -2: EPERM (1)
It's actually the tst_clone() that returns -2 if fallback to __NR_clone
failed.
@Ritchie Is there actually a good reason why tst_clone() returns -2 on a
fialure? Can we fix the code by:
diff --git a/lib/tst_clone.c b/lib/tst_clone.c
index ecc84408c..bacd269d9 100644
--- a/lib/tst_clone.c
+++ b/lib/tst_clone.c
@@ -39,8 +39,5 @@ pid_t tst_clone(const struct tst_clone_args *tst_args)
pid = syscall(__NR_clone, flags, NULL);
#endif
- if (pid == -1)
- return -2;
-
return pid;
}
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [LTP] [PATCH] containers/utsname04: don't use TST_EXP_EQ_LI for tst_clone
2023-02-28 9:39 ` Cyril Hrubis
@ 2023-03-03 6:11 ` xuyang2018.jy
2023-03-06 14:10 ` Richard Palethorpe
0 siblings, 1 reply; 9+ messages in thread
From: xuyang2018.jy @ 2023-03-03 6:11 UTC (permalink / raw)
To: Cyril Hrubis; +Cc: Richard Palethorpe, ltp@lists.linux.it
Hi Cyril
> Hi!
>>> Did you try this?
>>>
>>> TST_EXP_FAIL(tst_clone(&cargs), EPERM);
>>
>> I don't try this because I remembered TST_EXP_FAIL only thinks the
>> correct return value is -1.
>>
>> tst_test.c:1560: TINFO: Timeout per run is 0h 00m 30s
>> utsname04.c:27: TINFO: Dropping root privileges
>> utsname04.c:33: TFAIL: tst_clone(&cargs) invalid retval -2: EPERM (1)
>
> It's actually the tst_clone() that returns -2 if fallback to __NR_clone
> failed.
>
> @Ritchie Is there actually a good reason why tst_clone() returns -2 on a
> fialure? Can we fix the code by:
I guess it is used to distinguish clone3(-1) and clone failure(-2).
@Ritchie Is this right? Or you have other meaning.
Maybe we can use clone instead of tst_clone in this case?
Best Regards
Yang Xu
>
> diff --git a/lib/tst_clone.c b/lib/tst_clone.c
> index ecc84408c..bacd269d9 100644
> --- a/lib/tst_clone.c
> +++ b/lib/tst_clone.c
> @@ -39,8 +39,5 @@ pid_t tst_clone(const struct tst_clone_args *tst_args)
> pid = syscall(__NR_clone, flags, NULL);
> #endif
>
> - if (pid == -1)
> - return -2;
> -
> return pid;
> }
>
>
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [LTP] [PATCH] containers/utsname04: don't use TST_EXP_EQ_LI for tst_clone
2023-03-03 6:11 ` xuyang2018.jy
@ 2023-03-06 14:10 ` Richard Palethorpe
2023-03-07 6:09 ` [LTP] [PATCH v2] " Yang Xu
0 siblings, 1 reply; 9+ messages in thread
From: Richard Palethorpe @ 2023-03-06 14:10 UTC (permalink / raw)
To: xuyang2018.jy@fujitsu.com; +Cc: ltp@lists.linux.it
Hello,
"xuyang2018.jy@fujitsu.com" <xuyang2018.jy@fujitsu.com> writes:
> Hi Cyril
>
>> Hi!
>>>> Did you try this?
>>>>
>>>> TST_EXP_FAIL(tst_clone(&cargs), EPERM);
>>>
>>> I don't try this because I remembered TST_EXP_FAIL only thinks the
>>> correct return value is -1.
>>>
>>> tst_test.c:1560: TINFO: Timeout per run is 0h 00m 30s
>>> utsname04.c:27: TINFO: Dropping root privileges
>>> utsname04.c:33: TFAIL: tst_clone(&cargs) invalid retval -2: EPERM (1)
>>
>> It's actually the tst_clone() that returns -2 if fallback to __NR_clone
>> failed.
>>
>> @Ritchie Is there actually a good reason why tst_clone() returns -2 on a
>> fialure? Can we fix the code by:
>
> I guess it is used to distinguish clone3(-1) and clone failure(-2).
> @Ritchie Is this right? Or you have other meaning.
Yes, this is how it is used in SAFE_CLONE to provide debug information
when tst_clone fails.
It's important to know which clone failed as obviously they do not
support all the same flags or features.
>
> Maybe we can use clone instead of tst_clone in this case?
I think just use the return value as it was intended and print which
version of clone failed.
>
> Best Regards
> Yang Xu
> >
>> diff --git a/lib/tst_clone.c b/lib/tst_clone.c
>> index ecc84408c..bacd269d9 100644
>> --- a/lib/tst_clone.c
>> +++ b/lib/tst_clone.c
>> @@ -39,8 +39,5 @@ pid_t tst_clone(const struct tst_clone_args *tst_args)
>> pid = syscall(__NR_clone, flags, NULL);
>> #endif
>>
>> - if (pid == -1)
>> - return -2;
>> -
>> return pid;
>> }
>>
>>
--
Thank you,
Richard.
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 9+ messages in thread
* [LTP] [PATCH v2] containers/utsname04: don't use TST_EXP_EQ_LI for tst_clone
2023-03-06 14:10 ` Richard Palethorpe
@ 2023-03-07 6:09 ` Yang Xu
2023-03-07 8:35 ` Richard Palethorpe
0 siblings, 1 reply; 9+ messages in thread
From: Yang Xu @ 2023-03-07 6:09 UTC (permalink / raw)
To: ltp
For old kernel(below 5.3) that doesn't support clone3, tst_clone returns
-2 instead of -1. So we can't use TST_EXP_EQ_LI api.
Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
testcases/kernel/containers/utsname/utsname04.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/testcases/kernel/containers/utsname/utsname04.c b/testcases/kernel/containers/utsname/utsname04.c
index e8d636d0d..61cc066d8 100644
--- a/testcases/kernel/containers/utsname/utsname04.c
+++ b/testcases/kernel/containers/utsname/utsname04.c
@@ -29,7 +29,13 @@ static void run(void)
SAFE_SETRESUID(pw->pw_uid, pw->pw_uid, pw->pw_uid);
if (!str_op || !strcmp(str_op, "clone")) {
- TST_EXP_EQ_LI(tst_clone(&cargs), -1);
+ TEST(tst_clone(&cargs));
+ if (TST_RET == -1)
+ tst_res(TPASS, "clone3() fails as expected");
+ else if (TST_RET == -2)
+ tst_res(TPASS, "clone() fails as expected");
+ else
+ tst_res(TFAIL, "tst_clone returns %ld", TST_RET);
TST_EXP_PASS(errno == EPERM);
} else {
if (!SAFE_FORK()) {
--
2.39.1
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [LTP] [PATCH v2] containers/utsname04: don't use TST_EXP_EQ_LI for tst_clone
2023-03-07 6:09 ` [LTP] [PATCH v2] " Yang Xu
@ 2023-03-07 8:35 ` Richard Palethorpe
2023-03-07 10:16 ` Richard Palethorpe
0 siblings, 1 reply; 9+ messages in thread
From: Richard Palethorpe @ 2023-03-07 8:35 UTC (permalink / raw)
To: Yang Xu; +Cc: ltp
Hello,
Yang Xu <xuyang2018.jy@fujitsu.com> writes:
> For old kernel(below 5.3) that doesn't support clone3, tst_clone returns
> -2 instead of -1. So we can't use TST_EXP_EQ_LI api.
>
> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
Reviewed-by: Richard Palethorpe <rpalethorpe@suse.com>
Thanks, I suppose this could be avoided in the future by hinting that
the return type is more complex than the usual for clone
typedef tst_clone_return_t pid_t;
enum tst_clone_error {
TST_CLONE_ERROR = -2,
TST_CLONE3_ERROR = -1
};
Possibly we could force a cast somehow? Otherwise it's just a hint.
> ---
> testcases/kernel/containers/utsname/utsname04.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/testcases/kernel/containers/utsname/utsname04.c b/testcases/kernel/containers/utsname/utsname04.c
> index e8d636d0d..61cc066d8 100644
> --- a/testcases/kernel/containers/utsname/utsname04.c
> +++ b/testcases/kernel/containers/utsname/utsname04.c
> @@ -29,7 +29,13 @@ static void run(void)
> SAFE_SETRESUID(pw->pw_uid, pw->pw_uid, pw->pw_uid);
>
> if (!str_op || !strcmp(str_op, "clone")) {
> - TST_EXP_EQ_LI(tst_clone(&cargs), -1);
> + TEST(tst_clone(&cargs));
> + if (TST_RET == -1)
> + tst_res(TPASS, "clone3() fails as expected");
> + else if (TST_RET == -2)
> + tst_res(TPASS, "clone() fails as expected");
> + else
> + tst_res(TFAIL, "tst_clone returns %ld", TST_RET);
> TST_EXP_PASS(errno == EPERM);
> } else {
> if (!SAFE_FORK()) {
> --
> 2.39.1
--
Thank you,
Richard.
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [LTP] [PATCH v2] containers/utsname04: don't use TST_EXP_EQ_LI for tst_clone
2023-03-07 8:35 ` Richard Palethorpe
@ 2023-03-07 10:16 ` Richard Palethorpe
0 siblings, 0 replies; 9+ messages in thread
From: Richard Palethorpe @ 2023-03-07 10:16 UTC (permalink / raw)
To: rpalethorpe; +Cc: ltp
Hello,
Merged, thanks!
Richard Palethorpe <rpalethorpe@suse.de> writes:
> Hello,
>
> Yang Xu <xuyang2018.jy@fujitsu.com> writes:
>
>> For old kernel(below 5.3) that doesn't support clone3, tst_clone returns
>> -2 instead of -1. So we can't use TST_EXP_EQ_LI api.
>>
>> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
>
> Reviewed-by: Richard Palethorpe <rpalethorpe@suse.com>
>
> Thanks, I suppose this could be avoided in the future by hinting that
> the return type is more complex than the usual for clone
>
> typedef tst_clone_return_t pid_t;
>
> enum tst_clone_error {
> TST_CLONE_ERROR = -2,
> TST_CLONE3_ERROR = -1
> };
>
> Possibly we could force a cast somehow? Otherwise it's just a hint.
>
>> ---
>> testcases/kernel/containers/utsname/utsname04.c | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/testcases/kernel/containers/utsname/utsname04.c b/testcases/kernel/containers/utsname/utsname04.c
>> index e8d636d0d..61cc066d8 100644
>> --- a/testcases/kernel/containers/utsname/utsname04.c
>> +++ b/testcases/kernel/containers/utsname/utsname04.c
>> @@ -29,7 +29,13 @@ static void run(void)
>> SAFE_SETRESUID(pw->pw_uid, pw->pw_uid, pw->pw_uid);
>>
>> if (!str_op || !strcmp(str_op, "clone")) {
>> - TST_EXP_EQ_LI(tst_clone(&cargs), -1);
>> + TEST(tst_clone(&cargs));
>> + if (TST_RET == -1)
>> + tst_res(TPASS, "clone3() fails as expected");
>> + else if (TST_RET == -2)
>> + tst_res(TPASS, "clone() fails as expected");
>> + else
>> + tst_res(TFAIL, "tst_clone returns %ld", TST_RET);
>> TST_EXP_PASS(errno == EPERM);
>> } else {
>> if (!SAFE_FORK()) {
>> --
>> 2.39.1
>
>
> --
> Thank you,
> Richard.
--
Thank you,
Richard.
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-03-07 10:17 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-28 8:44 [LTP] [PATCH] containers/utsname04: don't use TST_EXP_EQ_LI for tst_clone Yang Xu
2023-02-28 9:06 ` Andrea Cervesato via ltp
2023-02-28 9:18 ` xuyang2018.jy
2023-02-28 9:39 ` Cyril Hrubis
2023-03-03 6:11 ` xuyang2018.jy
2023-03-06 14:10 ` Richard Palethorpe
2023-03-07 6:09 ` [LTP] [PATCH v2] " Yang Xu
2023-03-07 8:35 ` Richard Palethorpe
2023-03-07 10:16 ` Richard Palethorpe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox