[LTP] [PATCH 1/2] lib: tst_test: Add per filesystem mkfs and mount opts

[Martin Doucha]

Hi,
a design note below.

On 03. 06. 24 14:34, Cyril Hrubis wrote:
> This commit does:
> 
> * Group the filesystem type, mkfs and mount options into a separate
>    structure
> 
> * Add an array of these structures to be able to define per filesystem
>    mkfs and mount options
> 
> The details on the usage should be hopefully clear from the
> documentation comments for the struct tst_test.
> 
> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> ---
>   include/tst_test.h                            | 64 ++++++++++------
>   lib/tst_test.c                                | 73 +++++++++++++------
>   .../kernel/syscalls/fanotify/fanotify22.c     |  4 +-
>   .../kernel/syscalls/fanotify/fanotify23.c     |  4 +-
>   .../kernel/syscalls/getxattr/getxattr04.c     |  4 +-
>   testcases/kernel/syscalls/ioctl/ioctl08.c     |  4 +-
>   testcases/kernel/syscalls/mmap/mmap16.c       | 19 ++---
>   .../kernel/syscalls/quotactl/quotactl01.c     |  6 +-
>   .../kernel/syscalls/quotactl/quotactl02.c     |  6 +-
>   .../kernel/syscalls/quotactl/quotactl03.c     |  6 +-
>   .../kernel/syscalls/quotactl/quotactl04.c     |  4 +-
>   .../kernel/syscalls/quotactl/quotactl05.c     |  6 +-
>   .../kernel/syscalls/quotactl/quotactl06.c     |  6 +-
>   .../kernel/syscalls/quotactl/quotactl07.c     |  4 +-
>   .../kernel/syscalls/quotactl/quotactl08.c     |  4 +-
>   .../kernel/syscalls/quotactl/quotactl09.c     |  6 +-
>   testcases/kernel/syscalls/statx/statx05.c     |  4 +-
>   testcases/kernel/syscalls/statx/statx06.c     |  8 +-
>   testcases/kernel/syscalls/statx/statx07.c     |  4 +-
>   testcases/kernel/syscalls/statx/statx09.c     |  4 +-
>   20 files changed, 157 insertions(+), 83 deletions(-)
> 
> diff --git a/include/tst_test.h b/include/tst_test.h
> index 8dc20d110..6ac11f7d8 100644
> --- a/include/tst_test.h
> +++ b/include/tst_test.h
> @@ -247,6 +247,36 @@ struct tst_ulimit_val {
>   	rlim_t rlim_cur;
>   };
>   
> +/**
> + * struct tst_fs - A file system type, mkfs and mount options
> + *
> + * @fs_type A filesystem type to use.
> + *
> + * @mkfs_opts: A NULL terminated array of options passed to mkfs in the case
> + *             of 'tst_test.format_device'. These options are passed to mkfs
> + *             before the device path.
> + *
> + * @mkfs_size_opt: An option passed to mkfs in the case of
> + *                 'tst_test.format_device'. The device size in blocks is
> + *                 passed to mkfs after the device path and can be used to
> + *                 limit the file system not to use the whole block device.
> + *
> + * @mnt_flags: MS_* flags passed to mount(2) when the test library mounts a
> + *             device in the case of 'tst_test.mount_device'.
> + *
> + * @mnt_data: The data passed to mount(2) when the test library mounts a device
> + *            in the case of 'tst_test.mount_device'.
> + */
> +struct tst_fs {
> +	const char *type;
> +
> +	const char *const *mkfs_opts;
> +	const char *mkfs_size_opt;
> +
> +	const unsigned int mnt_flags;
> +	const void *mnt_data;
> +};
> +
>   /**
>    * struct tst_test - A test description.
>    *
> @@ -377,29 +407,21 @@ struct tst_ulimit_val {
>    *
>    * @dev_min_size: A minimal device size in megabytes.
>    *
> - * @dev_fs_type: If set overrides the default file system type for the device and
> - *               sets the tst_device.fs_type.
> + * @fs: If fs.type is set it overrides the default file system type for the
> + *      device. The rest of the parameters describe default parameters for
> + *      mkfs and mount.
>    *
> - * @dev_fs_opts: A NULL terminated array of options passed to mkfs in the case
> - *               of 'tst_test.format_device'. These options are passed to mkfs
> - *               before the device path.
> - *
> - * @dev_extra_opts: A NULL terminated array of extra options passed to mkfs in
> - *                  the case of 'tst_test.format_device'. Extra options are
> - *                  passed to mkfs after the device path. Commonly the option
> - *                  after mkfs is the number of blocks and can be used to limit
> - *                  the file system not to use the whole block device.
> + * @fss: A NULL type terminated array of per file system type options. If
> + *       tst_test.all_filesystems is not set the array describes a list of
> + *       file systems to test along with parameters to pass to mkfs and mount.
> + *       If tst_test.all_filesystems is set the mkfs and mount options are
> + *       taken from tst_test.fs unless there is an override for a given
> + *       file system type defined in this array.

I like the general idea but I don't see the point of having separate .fs 
and .fss. You could simply use .fss[0] for the same purpose as .fs.

-- 
Martin Doucha   mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic