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

[Andrea Cervesato]

Hi

On 10/11/22 12:49, Richard Palethorpe wrote:
> 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.

I can take care to change all tst_quick_clone after pushing these 
patches, no problem.
If you have an example how to use tst_clone, please do so. I will repeat 
the same patter to all remaining tests. Thanks!

Andrea

>> 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.
>>>
>