public inbox for ltp@lists.linux.it
 help / color / mirror / Atom feed
From: Cyril Hrubis <chrubis@suse.cz>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH v3 2/2] lib/tst_test.c: Add .needs_devfs flag
Date: Thu, 30 Aug 2018 16:49:43 +0200	[thread overview]
Message-ID: <20180830144943.GA20702@rei.lan> (raw)
In-Reply-To: <1534480415-18932-1-git-send-email-yangx.jy@cn.fujitsu.com>

Hi!
> +static void prepare_and_mount_dev_fs(const char *mntpoint)
> +{
> +	const char *flags[] = {"nodev", NULL};
> +	char abs_path[PATH_MAX];
> +
> +	sprintf(abs_path, "%s/%s", tst_get_tmpdir(), mntpoint);

The tst_get_tmpdir() allocates memory and also we are sure that mntpoint
is on the same filesystem as the path returned by tst_get_tmpdir().

So I would suggest something like this:

	char *tmpdir;
	int mounted_nodev;

	tmpdir = tst_get_tmpdir();
	mounted_nodev = tst_path_has_mnt_flags(NULL, tmpdir, flags);
	free(tmpdir);

	if (mounted_nodev) {
		...
	}

> +	if (tst_path_has_mnt_flags(NULL, abs_path, flags)) {
> +		tst_res(TINFO, "%s isn't suitable for creating devices, "
> +			"so mount tmpfs without nodev over it", abs_path);
> +		SAFE_MOUNT(NULL, mntpoint, "tmpfs", 0, NULL);
> +		mntpoint_mounted = 1;
> +	}
> +}
> +
>  static void prepare_device(void)
>  {
>  	if (tst_test->format_device) {
> @@ -786,11 +801,15 @@ static void do_setup(int argc, char *argv[])
>  	if (tst_test->mntpoint)
>  		SAFE_MKDIR(tst_test->mntpoint, 0777);
>  
> -	if ((tst_test->needs_rofs || tst_test->mount_device ||
> -	     tst_test->all_filesystems) && !tst_test->mntpoint) {
> +	if ((tst_test->needs_devfs || tst_test->needs_rofs ||
> +	     tst_test->mount_device || tst_test->all_filesystems) &&
> +	     !tst_test->mntpoint) {
>  		tst_brk(TBROK, "tst_test->mntpoint must be set!");
>  	}

I guess that we should also make sure only one of needs_rofs,
needs_devfs or needs_device is set, because otherwise we would attempt
to mount multiple filesystems over the mntpoint.

something as:

	if (!!tst_test->needs_rofs +
	    !!tst_test->needs_devfs +
	    !!tst_test->needs_device > 1) {
		tst_brk(TBROK, "Two or more of needs_{rofs, devfs, device} are set");
	}

> +	if (tst_test->needs_devfs)
> +		prepare_and_mount_dev_fs(tst_test->mntpoint);
> +
>  	if (tst_test->needs_rofs) {
>  		/* If we failed to mount read-only tmpfs. Fallback to
>  		 * using a device with read-only filesystem.

Other than the minor nits this version looks fine.

-- 
Cyril Hrubis
chrubis@suse.cz

  reply	other threads:[~2018-08-30 14:49 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-04  8:00 [LTP] [PATCH] syscalls/open11.c: Fix the failure of opening device special file Xiao Yang
2018-07-23  1:42 ` Xiao Yang
2018-08-15 11:42 ` Cyril Hrubis
2018-08-16  8:45   ` [LTP] [PATCH v2 1/4] lib/tst_path_has_mnt_flags.c: Factor out tst_path_has_mnt_flags() for new API Xiao Yang
2018-08-16  8:45     ` [LTP] [PATCH v2 2/4] lib/tst_test.c: add .needs_devfs flag Xiao Yang
2018-08-16 13:28       ` Cyril Hrubis
2018-08-17  4:26         ` Xiao Yang
2018-08-17  4:33         ` [LTP] [PATCH v3 2/2] lib/tst_test.c: Add " Xiao Yang
2018-08-30 14:49           ` Cyril Hrubis [this message]
2018-08-31  2:58             ` Xiao Yang
2018-09-01  4:07             ` [LTP] [PATCH v4 " Xiao Yang
2018-08-31 12:31               ` Cyril Hrubis
2018-09-03  2:14                 ` Xiao Yang
2018-09-03 13:01                   ` Cyril Hrubis
2018-08-16  8:45     ` [LTP] [PATCH v2 3/4] syscalls/open11.c: Fix the failure of opening device special files Xiao Yang
2018-08-16  8:45     ` [LTP] [PATCH v2 4/4] syscalls/fsetxattr02.c: Fix the failure of opening device files Xiao Yang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180830144943.GA20702@rei.lan \
    --to=chrubis@suse.cz \
    --cc=ltp@lists.linux.it \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox