Re: [LTP] [PATCH] Fix unlink09 test

[Petr Vorel <pvorel@suse.cz>]

Hi Martin,

> Hi Martin,

> > On 05. 06. 24 9:38, Petr Vorel wrote:
> > > BTW shouldn't this test use .all_filesystems = 1 ? Or is it unlink() really VFS
> > > only code? I see some specific functions in fs/*/, e.g. btrfs_unlink() or
> > > ext4_unlink(), which are used for struct inode_operations unlink member.
> > > Then, obviously also Andrea's check would be needed (otherwise is unlikely that
> > > somebody would have TMPDIR on vfat or exfat).

> > AFAICT, .all_filesystems and .needs_rofs are mutually exclusive at the
> > moment.

Also I wonder if having functionality for .all_filesystems + .needs_rofs
wouldn't be useful. @Cyril @Martin WDYT?

Also, there is fallback when prepare_and_mount_ro_fs() fails to use block
device. Although, I don't see the read only mount flags added in this fallback,
IMHO MS_RDONLY is only in prepare_and_mount_ro_fs(), therefore the fallback is
read write and we even didn't get TWARN, just plain TINFO (it should be either
TWARN or TINFO with "WARNING:" at least).

Kind regards,
Petr

lib/tst_test.c

static void prepare_device(void)
{
	...
	if (tst_test->needs_rofs) {
		/* If we failed to mount read-only tmpfs. Fallback to
		 * using a device with read-only filesystem.
		 */
		if (prepare_and_mount_ro_fs(NULL, tst_test->mntpoint, "tmpfs")) {
			tst_res(TINFO, "Can't mount tmpfs read-only, "
				"falling back to block device...");
			tst_test->needs_device = 1;
			tst_test->format_device = 1;
		}
	}

static int prepare_and_mount_ro_fs(const char *dev, const char *mntpoint,
				   const char *fs_type)
{
	char buf[PATH_MAX];

	if (mount(dev, mntpoint, fs_type, 0, NULL)) {
		tst_res(TINFO | TERRNO, "Can't mount %s at %s (%s)",
			dev, mntpoint, fs_type);
		return 1;
	}

	mntpoint_mounted = 1;

	snprintf(buf, sizeof(buf), "%s/dir/", mntpoint);
	SAFE_MKDIR(buf, 0777);

	snprintf(buf, sizeof(buf), "%s/file", mntpoint);
	SAFE_FILE_PRINTF(buf, "file content");
	SAFE_CHMOD(buf, 0777);

	SAFE_MOUNT(dev, mntpoint, fs_type, MS_REMOUNT | MS_RDONLY, NULL);

	return 0;
}

> Good point, I completely overlook .needs_rofs. That makes things clearer.

> ATM we have 3 other tests in syscalls/unlink. Not sure if all are filesystem
> specific (I would say yes, but not sure), but at least unlink05.c (tests
> deleting with unlink()) should be tested .all_filesystems. unlink07.c and
> unlink08.c test errno.

> Kind regards,
> Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp