[LTP] [PATCH v3 2/7] Refactor mqns_01 using new LTP API

[Richard Palethorpe]

Hello,

Andrea Cervesato <andrea.cervesato@suse.com> writes:

> Hi
>
> Are we sure that we don't need this modification before adding
> tst_clone? We can add the patch and then starting to think how to
> replace tst_clone_quick with tst_clone in all tests.

You're not the first person to use this argument. So it's actually
important for precisely the reason you don't want to do it. The next
person wont' want to do it either and we'll sleep walk into never
replacing it.

Meanwhile there are solid reasons why clone3 exists and why we should
test it. That's possibly more important than the API conversion.

BTW I could take over one of these patches and do the work on
tst_clone? I'm pretty familiar with it.

>
> Andrea
>
> On 10/11/22 11:17, Richard Palethorpe wrote:
>> Hello,
>>
>> Richard Palethorpe <rpalethorpe@suse.de> writes:
>>
>>> Hello,
>>>
>>> Andrea Cervesato <andrea.cervesato@suse.com> writes:
>>>
>>>> Hi!
>>>>
>>>> On 8/11/22 11:53, Richard Palethorpe wrote:
>>>>> Hello,
>>>>>
>>>>> Andrea Cervesato via ltp <ltp@lists.linux.it> writes:
>>>>>
>>>>>> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
>>>>>> ---
>>>>>>    runtest/containers                         |   3 +-
>>>>>>    testcases/kernel/containers/mqns/common.h  | 101 +++++++++++
>>>>>>    testcases/kernel/containers/mqns/mqns_01.c | 193 +++++++--------------
>>>>>>    3 files changed, 166 insertions(+), 131 deletions(-)
>>>>>>    create mode 100644 testcases/kernel/containers/mqns/common.h
>>>>>>
>>>>>> diff --git a/runtest/containers b/runtest/containers
>>>>>> index 2637b62fe..863a964ad 100644
>>>>>> --- a/runtest/containers
>>>>>> +++ b/runtest/containers
>>>>>> @@ -16,7 +16,8 @@ pidns31 pidns31
>>>>>>    pidns32 pidns32
>>>>>>      mqns_01 mqns_01
>>>>>> -mqns_01_clone mqns_01 -clone
>>>>>> +mqns_01_clone mqns_01 -m clone
>>>>>> +mqns_01_unshare mqns_01 -m unshare
>>>>>>    mqns_02 mqns_02
>>>>>>    mqns_02_clone mqns_02 -clone
>>>>>>    mqns_03 mqns_03
>>>>>> diff --git a/testcases/kernel/containers/mqns/common.h b/testcases/kernel/containers/mqns/common.h
>>>>>> new file mode 100644
>>>>>> index 000000000..92a77b566
>>>>>> --- /dev/null
>>>>>> +++ b/testcases/kernel/containers/mqns/common.h
>>>>>> @@ -0,0 +1,101 @@
>>>>>> +// SPDX-License-Identifier: GPL-2.0-or-later
>>>>>> +/*
>>>>>> + * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
>>>>>> + */
>>>>>> +
>>>>>> +#ifndef MQNS_H
>>>>>> +#define MQNS_H
>>>>>> +
>>>>>> +#include <stdlib.h>
>>>>>> +#include "lapi/namespaces_constants.h"
>>>>>> +#include "tst_test.h"
>>>>>> +#include "tst_safe_posix_ipc.h"
>>>>>> +
>>>>>> +enum {
>>>>>> +	T_CLONE,
>>>>>> +	T_UNSHARE,
>>>>>> +	T_NONE,
>>>>>> +};
>>>>>> +
>>>>>> +static int dummy_child1(void *v)
>>>>>> +{
>>>>>> +	(void)v;
>>>>>> +	return 0;
>>>>>> +}
>>>>>> +
>>>>>> +static inline void check_newipc(void)
>>>>>> +{
>>>>>> +	int pid, status;
>>>>>> +
>>>>>> +	pid = ltp_clone_quick(CLONE_NEWIPC | SIGCHLD, dummy_child1,
>>>>>>    NULL);
>>>>> ltp_clone_quick is still part of the old API and only uses clone2. I
>>>>> think it should be replaced with tst_clone. This may require extending
>>>>> tst_clone. In fact we probably need a test variant to switch between the
>>>>> clone2 and clone3 syscalls when using tst_clone.
>>>>>
>>>>> I'll leave it to you whether you want to try that and rebase this patch
>>>>> set on it.
>>>>>
>>>> I see ltp_clone_quick as wrapper of ltp_clone, since it's using
>>>> ltp_alloc_stack without calling it explicitly all the times before
>>>> ltp_clone.
>>> ltp_clone is also part of the old API. At some point we should remove
>>> that.
>> I'm marking this as changes requested. tst_clone should be made to
>> support this scenario.
>>


-- 
Thank you,
Richard.