[LTP] [PATCH v3 2/3] Refactor pidns30 test using new LTP API
Andrea Cervesato
andrea.cervesato@suse.com
Wed Feb 8 10:42:38 CET 2023
Hi,
On 11/1/22 13:01, Richard Palethorpe wrote:
> Hello,
>
> Andrea Cervesato via ltp <ltp@lists.linux.it> writes:
>
>> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
>> ---
>> testcases/kernel/containers/pidns/pidns30.c | 312 +++++---------------
>> 1 file changed, 66 insertions(+), 246 deletions(-)
>>
>> diff --git a/testcases/kernel/containers/pidns/pidns30.c b/testcases/kernel/containers/pidns/pidns30.c
>> index c8b0806c0..b01759a87 100644
>> --- a/testcases/kernel/containers/pidns/pidns30.c
>> +++ b/testcases/kernel/containers/pidns/pidns30.c
>> @@ -1,296 +1,116 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> /*
>> -* Copyright (c) Bull S.A.S. 2008
>> -* This program is free software; you can redistribute it and/or modify
>> -* it under the terms of the GNU General Public License as published by
>> -* the Free Software Foundation; either version 2 of the License, or
>> -* (at your option) any later version.
>> -* This program is distributed in the hope that it will be useful,
>> -* but WITHOUT ANY WARRANTY; without even the implied warranty of
>> -* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
>> -* the GNU General Public License for more details.
>> -* You should have received a copy of the GNU General Public License
>> -* along with this program; if not, write to the Free Software
>> -* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>> -*
>> -***************************************************************************
>> -* File: pidns30.c
>> -*
>> -* Description:
>> -* This testcase checks if the si_pid is correctly set when a process
>> -* that has registered for notification on a posix mqueue is in a
>> -* descendant namespace wrt the process that sends a message to that posix
>> -* mqueue.
>> -*
>> -* Test Assertion & Strategy:
>> -* Parent Child
>> -* --------------------------------------------------------------------------
>> -* Create a POSIX mqueue.
>> -* Create a PID namespace container.
>> -* Open that mqueue for reading
>> -* Register for notification when a
>> -* message arrives in that mqueue
>> -* Install a handler for SIGUSR1.
>> -* Write something to the mqueue.
>> -* Inside the handler, check that
>> -* si_pid is set to 0
>> -*
>> -* Usage: <for command-line>
>> -* pidns30
>> -*
>> -* History:
>> -* DATE NAME DESCRIPTION
>> -* 01/12/08 Nadia Derbey Creation of this test.
>> -* <Nadia.Derbey@bull.net>
>> -*
>> -******************************************************************************/
>> + * Copyright (c) Bull S.A.S. 2008
>> + * 01/12/08 Nadia Derbey <Nadia.Derbey@bull.net>
>> + * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
>> + */
>> +
>> +/*\
>> + * [Description]
>> + *
>> + * Clone a process with CLONE_NEWPID flag, register notification on a posix
>> + * mqueue and send a mqueue message from the parent. Then check if signal
>> + * notification contains si_pid of the parent.
>> + */
>> +
>> #define _GNU_SOURCE 1
>> -#include <sys/wait.h>
>> -#include <sys/types.h>
>> #include <signal.h>
>> -#include <stdlib.h>
>> -#include <unistd.h>
>> -#include <stdio.h>
>> #include <mqueue.h>
>> -#include "lapi/syscalls.h"
>> -#include "pidns_helper.h"
>> -#include "test.h"
>> -
>> -char *TCID = "pidns30";
>> -int TST_TOTAL = 1;
>> -
>> -char *mqname = "mq1";
>> -int result = TFAIL;
>> +#include "tst_test.h"
>> +#include "tst_safe_posix_ipc.h"
>> +#include "lapi/namespaces_constants.h"
>>
>> -int father_to_child[2];
>> -int child_to_father[2];
>> +#define MQNAME "/LTP_PIDNS30_MQ"
>>
>> -#define CHILD_PID 1
>> -#define PARENT_PID 0
>> -
>> -#define MSG "HOW ARE YOU"
>> -#define MSG_PRIO 1
>> -
>> -#define NO_STEP -1
>> -#define F_STEP_0 0x00
>> -#define F_STEP_1 0x01
>> -#define F_STEP_2 0x02
>> -#define F_STEP_3 0x03
>> -#define C_STEP_0 0x10
>> -#define C_STEP_1 0x11
>> -#define C_STEP_2 0x12
>> -
>> -mqd_t rc = -1;
>> -mqd_t mqd = -1;
>> -
>> -static void remove_pipe(int *fd)
>> -{
>> - close(fd[0]);
>> - close(fd[1]);
>> -}
>> +static mqd_t mqd = -1;
>> +static int received;
> This should be volatile because it is used in the sighandler.
>
>>
>> static void remove_mqueue(mqd_t mqd)
>> {
>> - mq_close(mqd);
>> - tst_syscall(__NR_mq_unlink, mqname);
>> -}
>> -
>> -static void cleanup(void)
>> -{
>> - if (mqd != -1) {
>> - remove_mqueue(mqd);
>> - }
>> - if (rc != -1) {
>> - remove_mqueue(rc);
>> - }
>> - remove_pipe(father_to_child);
>> - remove_pipe(child_to_father);
>> -}
>> + if (mqd != -1)
>> + SAFE_MQ_CLOSE(mqd);
>>
>> -static void cleanup_child(void)
>> -{
>> - if (mqd != -1) {
>> - tst_syscall(__NR_mq_notify, mqd, NULL);
>> - }
>> - cleanup();
>> + mq_unlink(MQNAME);
> We don't know that mq_unlink ever succeeds as we never check the
> result. This seems like lazyness because it is used at the beginning of
> run where it is expected to fail.
>
>> }
>>
>> -/*
>> - * child_signal_handler() - to handle SIGUSR1
>> - *
>> - * XXX (garrcoop): add calls to cleanup_child() -- or should this be handled
>> - * from the libltp signal handler?
>> - */
>> -static void child_signal_handler(int sig, siginfo_t * si, void *unused)
>> +static void child_signal_handler(LTP_ATTRIBUTE_UNUSED int sig, siginfo_t *si, LTP_ATTRIBUTE_UNUSED void *unused)
>> {
>> - char buf[256];
>> - struct mq_attr attr;
>> + tst_res(TINFO, "Received signal %s from pid %d",
>> tst_strsig(si->si_signo), si->si_pid);
> Again we should avoid calling tst_res in the signal handler
>
>>
>> - if (si->si_signo != SIGUSR1) {
>> - printf("received signal = %d unexpectedly\n", si->si_signo);
>> + if (si->si_signo != SIGUSR1 || si->si_code != SI_MESGQ ||
>> si->si_pid != 0)
> If the wrong signal or details are sent then we won't get any debug
> information. Unlike with the old version.
>
>> return;
>> - }
>> -
>> - if (si->si_code != SI_MESGQ) {
>> - printf("expected signal code SI_MESGQ; got %d instead\n",
>> - si->si_code);
>> - return;
>> - }
>> -
>> - if (si->si_pid) {
>> - printf("expected signal originator PID = 0; got %d instead\n",
>> - si->si_pid);
>> - return;
>> - } else {
>> - printf("signal originator PID = 0\n");
>> - result = TPASS;
>> - }
>>
>> - /*
>> - * Now read the message - Be silent on errors since this is not the
>> - * test purpose.
>> - */
>> - rc = mq_getattr(si->si_int, &attr);
>> - if (rc != -1)
>> - mq_receive(si->si_int, buf, attr.mq_msgsize, NULL);
>> + received++;
>> }
>>
>> -/*
>> - * child_fn() - Inside container
>> - *
>> - * XXX (garrcoop): add more calls to cleanup_child()?
>> - */
>> -int child_fn(void *arg)
>> +static int child_func(LTP_ATTRIBUTE_UNUSED void *arg)
>> {
>> - pid_t pid, ppid;
>> + pid_t cpid, ppid;
>> struct sigaction sa;
>> struct sigevent notif;
>> - char buf[5];
>> + mqd_t mqd_child;
>>
>> - /* Set process id and parent pid */
>> - pid = getpid();
>> + cpid = getpid();
>> ppid = getppid();
>>
>> - if (pid != CHILD_PID || ppid != PARENT_PID) {
>> - printf("pidns was not created\n");
>> - return 1;
>> + if (cpid != 1 || ppid != 0) {
>> + tst_res(TFAIL, "got unexpected result of cpid=%d ppid=%d", cpid, ppid);
>> + return 0;
> Again TST_EXP_EQ*
>
>> }
>>
>> - /* Close the appropriate end of each pipe */
>> - close(child_to_father[0]);
>> - close(father_to_child[1]);
>> + TST_CHECKPOINT_WAIT(0);
>>
>> - while (read(father_to_child[0], buf, 1) != 1)
>> - sleep(1);
>> -
>> - mqd = tst_syscall(__NR_mq_open, mqname, O_RDONLY, 0, NULL);
>> - if (mqd == -1) {
>> - perror("mq_open failed");
>> - return 1;
>> - } else
>> - printf("mq_open succeeded\n");
>> -
>> - /* Register for notification on message arrival */
>> + mqd_child = SAFE_MQ_OPEN(MQNAME, O_RDONLY, 0, NULL);
>> notif.sigev_notify = SIGEV_SIGNAL;
>> notif.sigev_signo = SIGUSR1;
>> - notif.sigev_value.sival_int = mqd;
>> - if (tst_syscall(__NR_mq_notify, mqd, ¬if) == -1) {
>> - perror("mq_notify failed");
>> - return 1;
>> - } else
>> - printf("successfully registered for notification\n");
>> + notif.sigev_value.sival_int = mqd_child;
>> +
>> + SAFE_MQ_NOTIFY(mqd_child, ¬if);
>>
>> - /* Define handler for SIGUSR1 */
>> sa.sa_flags = SA_SIGINFO;
>> - sigemptyset(&sa.sa_mask);
>> + SAFE_SIGEMPTYSET(&sa.sa_mask);
>> sa.sa_sigaction = child_signal_handler;
>> - if (sigaction(SIGUSR1, &sa, NULL) == -1) {
>> - perror("sigaction failed");
>> - return 1;
>> - } else
>> - printf("successfully registered handler for SIGUSR1\n");
>> + SAFE_SIGACTION(SIGUSR1, &sa, NULL);
>>
>> - /* Ask parent to send a message to the mqueue */
>> - if (write(child_to_father[1], "c:ok", 5) != 5) {
>> - perror("write failed");
>> - return 1;
>> - }
>> -
>> - sleep(3);
>> + TST_CHECKPOINT_WAKE_AND_WAIT(0);
>>
>> - /* Has parent sent a message? */
>> - read(father_to_child[0], buf, 5);
>> - if (strcmp(buf, "f:ok") != 0) {
>> - printf("parent did not send the message!\n");
>> - return 1;
>> + if (received != 1) {
>> + tst_res(TFAIL, "Signal hasn't been received after mqueue
>> event");
> If we receive more than one signal then this will also be printed. In
> any case we can shorted this with TST_EXP_EQ_*.
I would keep this message because it's easier to read when test fails.
>> + return 0;
>> }
>> - printf("parent is done - cleaning up\n");
>>
>> - cleanup_child();
>> + tst_res(TPASS, "Signal has been received after mqueue event");
>>
>> - exit(0);
>> + return 0;
>> }
>>
>> -static void setup(void)
>> +static void cleanup(void)
>> {
>> - tst_require_root();
>> - check_newpid();
>> + remove_mqueue(mqd);
>> }
>>
>> -int main(void)
>> +static void run(void)
>> {
>> - int status;
>> - char buf[5];
>> - pid_t cpid;
>> -
>> - setup();
>> -
>> - if (pipe(child_to_father) == -1 || pipe(father_to_child) == -1) {
>> - tst_brkm(TBROK | TERRNO, cleanup, "pipe failed");
>> - }
>> + int ret;
>>
>> - tst_syscall(__NR_mq_unlink, mqname);
>> + remove_mqueue(mqd);
>>
>> - /* container creation on PID namespace */
>> - cpid = ltp_clone_quick(CLONE_NEWPID | SIGCHLD, child_fn, NULL);
>> - if (cpid == -1)
>> - tst_brkm(TBROK | TERRNO, cleanup, "clone failed");
>> + ret = ltp_clone_quick(CLONE_NEWPID | SIGCHLD, child_func, NULL);
>> + if (ret < 0)
>> + tst_brk(TBROK | TERRNO, "clone failed");
>>
>> - mqd =
>> - tst_syscall(__NR_mq_open, mqname, O_RDWR | O_CREAT | O_EXCL, 0777,
>> - NULL);
>> - if (mqd == -1)
>> - tst_brkm(TBROK | TERRNO, cleanup, "mq_open failed");
>> - else
>> - tst_resm(TINFO, "successfully created posix mqueue");
>> + mqd = SAFE_MQ_OPEN(MQNAME, O_RDWR | O_CREAT | O_EXCL, 0777, 0);
>>
>> - if (write(father_to_child[1], buf, 1) != 1)
>> - tst_brkm(TBROK | TERRNO, cleanup, "write failed");
>> + TST_CHECKPOINT_WAKE_AND_WAIT(0);
>>
>> - /* Close the appropriate end of each pipe */
>> - close(child_to_father[1]);
>> - close(father_to_child[0]);
>> + SAFE_MQ_SEND(mqd, "pippo", 5, 1);
>>
>> - /* Is container ready */
>> - read(child_to_father[0], buf, 5);
>> - if (strcmp(buf, "c:ok") != 0)
>> - tst_brkm(TBROK, cleanup,
>> - "container did not respond as expected!");
>> -
>> - rc = mq_send(mqd, MSG, strlen(MSG), MSG_PRIO);
>> - if (rc == -1)
>> - tst_brkm(TBROK | TERRNO, cleanup, "mq_send failed");
>> - else
>> - tst_resm(TINFO, "mq_send succeeded");
>> -
>> - /* Tell the child the message has been sent */
>> - if (write(father_to_child[1], "f:ok", 5) != 5)
>> - tst_brkm(TBROK | TERRNO, cleanup, "write failed");
>> -
>> - /* Wait for child to finish */
>> - if (wait(&status) == -1)
>> - tst_resm(TBROK | TERRNO, "wait failed");
>> -
>> - cleanup();
>> -
>> - tst_exit();
>> + TST_CHECKPOINT_WAKE(0);
>> }
>> +
>> +static struct tst_test test = {
>> + .test_all = run,
>> + .cleanup = cleanup,
>> + .needs_root = 1,
>> + .needs_checkpoints = 1,
>> +};
>> --
>> 2.35.3
>
Andrea
More information about the ltp
mailing list