[LTP] [PATCH v6 1/3] syscalls/quotactl09: Test error when quota info hidden in filesystem

[Cyril Hrubis]

Hi!
> +/*\
> + * [Description]
> + *
> + * Tests basic error handling of the quotactl syscall without visible quota files
> + * (use quotactl and quotactl_fd syscall):
> + *
> + * - EFAULT when addr or special is invalid
> + * - EINVAL when cmd or type is invalid
> + * - ENOTBLK when special is not a block device
> + * - ERANGE when cmd is Q_SETQUOTA, but the specified limits are out of the range
> + *   allowed by the quota format
> + * - EPERM when the caller lacked the required privilege (CAP_SYS_ADMIN) for the
> + *   specified operation
> + * - EINVAL when cmd is Q_QUOTAON, but the fd refers to a regular file(doesn't
> + *   under this moutpoint)
> + *
> + * Minimum e2fsprogs version required is 1.43.
> + */
> +
> +#include <errno.h>
> +#include <sys/quota.h>
> +#include "tst_test.h"
> +#include "tst_capability.h"
> +#include "quotactl_syscall_var.h"
> +
> +#define OPTION_INVALID 999
> +
> +static int32_t fmt_id = QFMT_VFS_V1;
> +static int test_id, mount_flag;
> +static int getnextquota_nsup, external_fd = -1;
> +
> +static struct if_nextdqblk res_ndq;
> +
> +static struct dqblk set_dqmax = {
> +	.dqb_bsoftlimit = 0x7fffffffffffffffLL,  /* 2^63-1 */
> +	.dqb_valid = QIF_BLIMITS
> +};
> +
> +static struct tst_cap dropadmin = {
> +	.action = TST_CAP_DROP,
> +	.id = CAP_SYS_ADMIN,
> +	.name = "CAP_SYS_ADMIN",
> +};
> +
> +static struct tst_cap needadmin = {
> +	.action = TST_CAP_REQ,
> +	.id = CAP_SYS_ADMIN,
> +	.name = "CAP_SYS_ADMIN",
> +};
> +
> +static struct tcase {
> +	int cmd;
> +	int *id;
> +	void *addr;
> +	int exp_err;
> +	int on_flag;
> +	int use_external_fd;
> +	char *des;
> +} tcases[] = {
> +	{QCMD(Q_SETQUOTA, USRQUOTA), &fmt_id, NULL, EFAULT, 1, 0,
> +	"EFAULT when addr or special is invalid"},
> +
> +	{QCMD(OPTION_INVALID, USRQUOTA), &fmt_id, NULL, EINVAL, 0, 0,
> +	"EINVAL when cmd or type is invalid"},
> +
> +	{QCMD(Q_QUOTAON, USRQUOTA), &fmt_id, NULL, ENOTBLK, 0, 0,
> +	"ENOTBLK when special is not a block device"},
> +
> +	{QCMD(Q_SETQUOTA, USRQUOTA), &test_id, &set_dqmax, ERANGE, 1, 0,
> +	"ERANGE when cmd is Q_SETQUOTA, but the specified limits are out of the range"},
> +
> +	{QCMD(Q_QUOTAON, USRQUOTA), &fmt_id, NULL, EPERM, 0, 0,
> +	"EPERM when the caller lacked the required privilege for specified operations"},
> +
> +	{QCMD(Q_QUOTAON, USRQUOTA), &fmt_id, NULL, EINVAL, 0, 1,
> +	"EINVAL when cmd is Q_QUOTAON, but the fd refers to a regular file(doesn't under "
> +	"this mountpoint)"}
> +};
> +
> +static void verify_quotactl(unsigned int n)
> +{
> +	struct tcase *tc = &tcases[n];
> +	int quota_on = 0;
> +	int drop_flag = 0;
> +
> +	tst_res(TINFO, "Testing %s", tc->des);
> +	if (tc->cmd == QCMD(Q_GETNEXTQUOTA, USRQUOTA) && getnextquota_nsup) {
> +		tst_res(TCONF, "current system doesn't support Q_GETNEXTQUOTA");
> +		return;
> +	}
> +
> +	if (tc->on_flag) {
> +		TST_EXP_PASS_SILENT(do_quotactl(fd, QCMD(Q_QUOTAON, USRQUOTA), tst_device->dev,
> +			fmt_id, NULL), "do_quotactl(QCMD(Q_QUOTAON, USRQUOTA))");
> +		if (!TST_PASS)
> +			return;
> +		quota_on = 1;
> +	}
> +
> +	if (tc->exp_err == EPERM) {
> +		tst_cap_action(&dropadmin);
> +		drop_flag = 1;
> +	}
> +
> +	if (tst_variant) {
> +		if (tc->exp_err == ENOTBLK) {
> +			tst_res(TCONF, "quotactl_fd() doesn't have this error, skip");
> +			return;
> +		}
> +	} else {
> +		if (tc->use_external_fd) {
> +			tst_res(TCONF, "quotactl() doesn't use fd, skip");
> +			return;
> +		}
> +	}
> +	if (tc->exp_err == ENOTBLK)
> +		TST_EXP_FAIL(do_quotactl(fd, tc->cmd, "/dev/null", *tc->id, tc->addr),
> +			ENOTBLK, "do_quotactl()");
> +	else
> +		TST_EXP_FAIL(do_quotactl(tc->use_external_fd ? external_fd : fd, tc->cmd,
> +			tst_device->dev, *tc->id, tc->addr), tc->exp_err, "do_quotactl()");
> +
> +	if (quota_on) {
> +		TST_EXP_PASS_SILENT(do_quotactl(fd, QCMD(Q_QUOTAOFF, USRQUOTA), tst_device->dev,
> +			fmt_id, NULL), "do_quotactl(QCMD(Q_QUOTAOFF, USRQUOTA)");
> +		if (!TST_PASS)
> +			return;
> +		quota_on = 0;

There is no need to clear this flag.

> +	}
> +
> +	if (drop_flag) {
> +		tst_cap_action(&needadmin);
> +		drop_flag = 0;

And this flag.

> +	}
> +}
> +
> +static void setup(void)
> +{
> +	unsigned int i;
> +	const char *const fs_opts[] = { "-O quota", NULL};
> +
> +	quotactl_info();
> +	external_fd = SAFE_CREAT("testfile", O_RDONLY);
> +
> +	SAFE_MKFS(tst_device->dev, tst_device->fs_type, fs_opts, NULL);
> +	SAFE_MOUNT(tst_device->dev, MNTPOINT, tst_device->fs_type, 0, NULL);

Why don't we pass the fs_opts in tst_test instead and use .mount_device = 1?

Should be as easy as:

static struct tst_test test = {
	...
	.dev_fs_opts = (const char *const []){"-O quota", NULL},
	.mount_device = 1,
	...
};

Or is there a reason why this wouldn't work?


With the changes outlined in the other email and the minor issues I've
pointed out here fixed:

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>


-- 
Cyril Hrubis
chrubis@suse.cz