Veerendra [EMAIL PROTECTED] wrote:
> Hi
>
>   Attaching the patch for the testcase on PIDNS.

I just have some coding style/nits and a couple of minor comments below.

>
> Assertion:
> 1. kill -9 1 from inside a container does not kill container
> Steps:
> a) create container
> b) kill -9 1
> c) Should not kill the containers
>
> Test Result: Currently this is failing, which eventually be passed.
>
> # ./pidns04
> pid_namespace4    0  INFO  :  PIDNS test is running inside container
> pid_namespace4    1  FAIL  :  Container init is killed by SIGKILL !!!
> pid_namespace4    2  FAIL  :  Container init pid got killed by signal 9
>
> Regards
> Veerendra C
>
>

| diff -uprN testcases/kernel/containers/pidns.old/pidns04.c 
testcases/kernel/containers/pidns/pidns04.c
| --- testcases/kernel/containers/pidns.old/pidns04.c   1970-01-01 
05:30:00.000000000 +0530
| +++ testcases/kernel/containers/pidns/pidns04.c       2008-10-14 
16:48:28.000000000 +0530
| @@ -0,0 +1,165 @@
| +/*
| +* Copyright (c) International Business Machines Corp., 2007
| +* 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., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
| +*
| +***************************************************************************
| +
| +* File: pidns04.c
| +*
| +* Description:
| +*  The pidns04.c testcase builds into the ltp framework to verify
| +*  the basic functionality of PID Namespace.
| +*
| +* Verify that:
| +* 1. When parent clone a process with flag CLONE_NEWPID, the process ID of
| +* child should be one.
| +*
| +* 2. When parent clone a process with flag CLONE_NEWPID, the parent process 
ID
| +* of should be zero.
| +*
| +* 3. The container init process (one), should not get killed by the SIGKILL 
in 
| +* the childNS
| +*
| +* Total Tests:
| +*
| +* Test Name: pidns04
| +*
| +* Test Assertion & Strategy:
| +*
| +* From main() clone a new child process with passing the clone_flag as 
| +* CLONE_NEWPID. 
| +* The container init, should not get killed by the SIGKILL inside the child 
NS.
| +* Usage: <for command-line>
| +* pidns04
| +*
| +* History:
| +*
| +* FLAG DATE          NAME                            DESCRIPTION
| +* 08/10/08      Veerendra C <[EMAIL PROTECTED]> Verifies killing of cont 
init.
| +*
| 
+*******************************************************************************/
| +#define _GNU_SOURCE 1
| +#include <sys/wait.h>
| +#include <assert.h>
| +#include <stdio.h>
| +#include <stdlib.h>
| +#include <unistd.h>
| +#include <string.h>
| +#include <errno.h>
| +#include <usctest.h>
| +#include <test.h>
| +#include <libclone.h>
| +
| +#define INIT_PID        1
| +#define CHILD_PID       1

Replace CHILD_PID with CINIT_PID ?

| +#define PARENT_PID      0
| +
| +char *TCID = "pid_namespace4";
| +int TST_TOTAL=1;
| +int     fd[2] ;

white-space

| +void cleanup(void);
| +
| +/*
| + * child_fn1() - Inside container
| +*/
| +static int child_fn1(void *ttype)
| +{
| +     pid_t cpid, ppid;
| +     cpid = getpid();
| +     ppid = getppid();
| +     char mesg[] = "I was not killed !";
| +             /* Child process closes up read side of pipe */
| +             close(fd[0]);

indentation

| +
| +     /* Comparing the values to make sure pidns is created correctly */
| +     if(( cpid == CHILD_PID) && ( ppid == PARENT_PID ) ) {
| +             tst_resm(TINFO, "PIDNS test is running inside container");
| +             kill(INIT_PID, SIGKILL);
| +             /* Verifying whether the container init is not killed, "
| +              If so writing into the pipe created in the parent NS" */

Coding style - space after '((' and before ')'

The comments don't seem to help explaining what is going on here unless
we go back and note that CHILD_PID == INIT_PID.

How about:

        if ((cpid == CINIT_PID && ((ppid == PARENT_PID)) {

                kill(CINIT_PID, SIGKILL);
                /* We did not kill ourselves. Let parent know */
        }

| +
| +             /* Send "mesg" through the write side of pipe */
| +             write(fd[1], mesg, (strlen(mesg)+1));
| +     }
| +     else {
| +             tst_resm(TFAIL, "FAIL: Got unexpected result of"
| +             " cpid=%d ppid=%d\n", cpid, ppid);
| +     }
| +     close(fd[1]);
| +     cleanup();
| +
| +     /* NOT REACHED */
| +     return 0;
| +}
| +
| +/***********************************************************************
| +*   M A I N
| +***********************************************************************/
| +
| +int main(int argc, char *argv[])
| +{
| +     int ret, status, nbytes;
| +        char    readbuffer[80];

indentation

| +
| +     pipe(fd);

Unlikely to fail, but would be good to check for error from pipe().

| +     ret = do_clone_unshare_test(T_CLONE, CLONE_NEWPID, child_fn1, NULL);
| +     if ((wait(&status)) < 0) {
| +             tst_resm(TWARN, "wait() failed, skipping this test case");
| +             /* Cleanup & continue with next test case */
| +             cleanup();
| +     }

If relying on the message in pipe, do we still need the wait() or
the if(WTERMSIG()) below

| +     if (ret == -1) {
| +             tst_resm(TFAIL, "clone() Failed, errno = %d :"
| +                     " %s", ret, strerror(ret));
| +             /* Cleanup & continue with next test case */
| +             cleanup();
| +     }
| +
| +     /* Parent process closes up write side of pipe */
| +     close(fd[1]);
| +     /* Read in a string from the pipe */
| +     nbytes = read(fd[0], readbuffer, sizeof(readbuffer));
| +
| +     if (nbytes !=0 ) {

nit-pick: (nbytes > 0) would be better (in case read() fails for some
unlikely reason - or check (nbytes < 0) separately.

| +             tst_resm(TPASS, "Container init : %s", readbuffer);
| +     }
| +     else {
| +             tst_resm(TFAIL, "Container init is killed by SIGKILL !!!");
| +     }
| +
| +     if (WTERMSIG(status)) {
| +             tst_resm(TFAIL, "Container init pid got killed by signal %d",
| +             WTERMSIG(status));
| +     }

Strictly speaking, WTERMSIG() is meaningful only if WIFSIGNALED() is true.

| +        /* cleanup and exit */
| +     cleanup();
| +     close(fd[0]);
| +
| +     /*NOTREACHED*/
| +     return 0;
| +
| +}    /* End main */
| +
| +/*
| + * cleanup() - performs all ONE TIME cleanup for this test at
| + *             completion or premature exit.
| + */
| +void
| +cleanup()
| +{
| +     /* Clean the test testcase as LTP wants*/
| +     TEST_CLEANUP;
| +
| +     /* exit with return code appropriate for results */
| +     tst_exit();
| +}
| diff -uprN testcases/kernel/containers/pidns.old/runpidnstest.sh 
testcases/kernel/containers/pidns/runpidnstest.sh
| --- testcases/kernel/containers/pidns.old/runpidnstest.sh     2008-09-30 
18:06:38.000000000 +0530
| +++ testcases/kernel/containers/pidns/runpidnstest.sh 2008-10-14 
11:25:40.000000000 +0530
| @@ -35,4 +35,9 @@ if [ $? -ne 0 ]; then
|       exit_code="$?"
|       exit $exit_code
|  fi
| +pidns04
| +if [ $? -ne 0 ]; then
| +     exit_code="$?"
| +     exit $exit_code
| +fi
|  exit $exit_code


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

Reply via email to