Re: [LTP] [PATCH] mount/mount05.c: new case to test MS_BIND of mount

chrubis
2013-07-02T12:16:49Z

Hi!
> Create new test case for MS_BIND of mount.
>
> Checking if file is visible from target mountpoint after bind mount.
>
>
> Signed-off-by: DAN LI <li.dan@cn.fujitsu.com>
> ---
> runtest/syscalls | 1 +
> testcases/kernel/syscalls/.gitignore | 1 +
> testcases/kernel/syscalls/mount/mount05.c | 212 ++++++++++++++++++++++++++++++
> 3 files changed, 214 insertions(+)
> create mode 100644 testcases/kernel/syscalls/mount/mount05.c
>
> diff --git a/runtest/syscalls b/runtest/syscalls
> index 3ab19f5..1e36c92 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -578,6 +578,7 @@ mount01 mount01 -D DEVICE -T DEVICE_FS_TYPE
> mount02 mount02 -D DEVICE -T DEVICE_FS_TYPE
> mount03 mount03 -D DEVICE -T DEVICE_FS_TYPE
> mount04 mount04 -D DEVICE -T DEVICE_FS_TYPE
> +mount05 mount05 -D DEVICE -T DEVICE_FS_TYPE
>
> move_pages01 move_pages.sh 01
> move_pages02 move_pages.sh 02
> diff --git a/testcases/kernel/syscalls/.gitignore b/testcases/kernel/syscalls/.gitignore
> index fede145..6147085 100644
> --- a/testcases/kernel/syscalls/.gitignore
> +++ b/testcases/kernel/syscalls/.gitignore
> @@ -521,6 +521,7 @@
> /mount/mount02
> /mount/mount03
> /mount/mount04
> +/mount/mount05
> /mount/setuid_test
> /move_pages/move_pages01
> /move_pages/move_pages02
> diff --git a/testcases/kernel/syscalls/mount/mount05.c b/testcases/kernel/syscalls/mount/mount05.c
> new file mode 100644
> index 0000000..97fbb48
> --- /dev/null
> +++ b/testcases/kernel/syscalls/mount/mount05.c
> @@ -0,0 +1,212 @@
> +/*
> + * Copyright (c) 2013 FNST, DAN LI <li.dan@cn.fujitsu.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of version 2 of the GNU General Public License as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it would be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, write the Free Software Foundation, Inc.,
> + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> + *
> + */
> +/**************************************************************************
> + *
> + * EXECUTED BY : root / superuser
> + *
> + * DESCRIPTION
> + * Test for feature MS_BIND of mount.
> + * "Perform a bind mount, making a file or a directory subtree visible
> + * at another point within a file system."
> + *
> + * Setup:
> + * Setup signal handling.
> + * Create a mount point.
> + * Pause for SIGUSR1 if option specified.
> + *
> + * Test:
> + * Loop if the proper options are given.
> + * Execute system call
> + * Check return code, if system call failed (return=-1)
> + * Log the errno and Issue a FAIL message.
> + * Otherwise, Issue a PASS message.
> + *
> + * Cleanup:
> + * Delete the mount point.
> + *
> + * RESTRICTIONS
> + * test must run with the -D option
> + * test doesn't support -c option to run it in parallel, as mount
> + * syscall is not supposed to run in parallel.
> + *****************************************************************************/
> +
> +#include <errno.h>
> +#include <sys/mount.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <unistd.h>
> +#include <fcntl.h>
> +
> +#include "test.h"
> +#include "usctest.h"
> +
> +static void help(void);
> +static void setup(void);
> +static void cleanup(void);
> +
> +char *TCID = "mount05";
> +int TST_TOTAL = 1;
> +
> +#define DEFAULT_FSTYPE "ext2"
> +#define DIR_MODE (S_IRWXU | S_IRUSR | S_IXUSR | S_IRGRP | S_IXGRP)
> +
> +static char *fs_type;
> +
> +static int tflag;
> +static int dflag;
> +static char *fstype;
> +static char *device;
> +static char file_src[PATH_MAX];
> +static char file_des[PATH_MAX];
> +static char mntpoint_src[PATH_MAX];
> +static char mntpoint_des[PATH_MAX];
> +
> +static option_t options[] = {
> + {"T:", &tflag, &fstype},
> + {"D:", &dflag, &device},
> + {NULL, NULL, NULL},
> +};
> +
> +int main(int argc, char *argv[])
> +{
> + int lc;
> + char *msg;
> +
> + msg = parse_opts(argc, argv, options, &help);
> + if (msg != NULL)
> + tst_brkm(TBROK, NULL, "OPTION PARSING ERROR - %s", msg);
> +
> + /* Check for mandatory option of the testcase */
> + if (!dflag)
> + tst_brkm(TBROK, NULL,
> + "you must specify the device used for mounting with "
> + "the -D option");
> +
> + if (tflag) {
> + fs_type = malloc(strlen(fstype) + 1);
> + if (fs_type == NULL)
> + tst_brkm(TBROK | TERRNO, NULL,
> + "malloc - failed to alloc %zd",
> + strlen(fstype));
> +
> + strncpy(fs_type, fstype, strlen(fstype) + 1);
> +
> + } else {
> + fs_type = malloc(strlen(DEFAULT_FSTYPE) + 1);
> + if (fs_type == NULL)
> + tst_brkm(TBROK, NULL, "malloc - failed to alloc %zu",
> + strlen(DEFAULT_FSTYPE));
> +
> + strncpy(fs_type, DEFAULT_FSTYPE, strlen(DEFAULT_FSTYPE) + 1);
> + }
> +

Ah why are you copying the string, and if that is needed what about
using strdup()?

If you need to set the fs_type to either default one or passed one, it
could be done in single assigment:

static const char *fs_type = "ext2";

...

if (tflag)
fs_type = fstype;

> + if (STD_COPIES != 1) {
> + tst_resm(TINFO, "-c option has no effect for this testcase - "
> + "%s doesn't allow running more than one instance "
> + "at a time", TCID);
> + STD_COPIES = 1;
> + }
> +
> + setup();
> +
> + for (lc = 0; TEST_LOOPING(lc); lc++) {
> +
> + tst_count = 0;
> +
> + TEST(mount(mntpoint_src, mntpoint_des, fs_type, MS_BIND, NULL));
> +
> + if (TEST_RETURN != 0) {
> + tst_resm(TFAIL | TTERRNO, "mount(2) failed");
> + } else {
> +
> + if (open(file_des, O_CREAT | O_EXCL) == -1 &&
> + errno == EEXIST)
> + tst_resm(TPASS, "bind mount is ok");
> + else
> + tst_resm(TFAIL, "file %s is not available",
> + file_des);
> +
> + TEST(umount(mntpoint_des));
> + if (TEST_RETURN != 0)
> + tst_brkm(TBROK | TTERRNO, cleanup,
> + "umount(2) failed");
> + }
> + }
> +
> + cleanup();
> +
> + tst_exit();
> +}
> +
> +void setup(void)
> +{
> + int fd;
> +
> + tst_require_root(NULL);
> +
> + tst_sig(NOFORK, DEF_HANDLER, cleanup);
> +
> + tst_tmpdir();
> +
> + snprintf(mntpoint_src, PATH_MAX, "mnt_src_%d", getpid());
> + snprintf(mntpoint_des, PATH_MAX, "mnt_des_%d", getpid());

I think that all the work with getpid() to create unique dir names is
pointless since the tst_tmpdir() allready created unique directory for
the test. And if you omit that, all the paths will became constant
strings simplifying the test code.

> + if (mkdir(mntpoint_src, DIR_MODE) < 0)
> + tst_brkm(TBROK | TERRNO, cleanup, "mkdir(%s, %#o) failed",
> + mntpoint_src, DIR_MODE);
> +
> + if (mkdir(mntpoint_des, DIR_MODE) < 0)
> + tst_brkm(TBROK | TERRNO, cleanup, "mkdir(%s, %#o) failed",
> + mntpoint_des, DIR_MODE);

Use SAFE_MKDIR()?

> + if (mount(device, mntpoint_src, fs_type, 0, NULL) == -1)
> + tst_brkm(TBROK | TERRNO, cleanup, "mount failed");

I wonder if we really need to mount something at mntpoint_src, the bind
mount should work with any directory or file in fs hierarchy. Or we can
mount it only if -D option is specified and add two entires into the
runtest file, one with the -D option and one without...

> + snprintf(file_src, PATH_MAX, "%s/tmpfile", mntpoint_src);
> + snprintf(file_des, PATH_MAX, "%s/tmpfile", mntpoint_des);
> +
> + fd = open(file_src, O_CREAT | O_TRUNC, S_IRWXU);
> + if (fd == -1)
> + tst_brkm(TBROK, cleanup, "open file failed");
> + close(fd);

SAFE_FILE_PRINTF(cleanup, path, "") ?

> + TEST_PAUSE;
> +}
> +
> +void cleanup(void)
> +{
> + if (fs_type) {
> + free(fs_type);
> + fs_type = NULL;
> + }
> +
> + if (umount(mntpoint_src) != 0)
> + tst_brkm(TBROK | TTERRNO, NULL, "umount(2) failed");
> +
> + TEST_CLEANUP;
> +
> + tst_rmdir();
> +}
> +
> +/*
> + * issue a help message
> + */

I would say that this comment is pretty useless as it's saying something
that is pretty obvious.

> +void help(void)
> +{
> + printf("-T type : specifies the type of filesystem to be mounted. "
> + "Default ext2.\n");
> + printf("-D device : device used for mounting.\n");
> +}

--
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net