[PATCH 1/2] fs-tests: integck: Refactor: split out common remount logic

[Zhihao Cheng]

在 2024/8/15 17:12, Csókás, Bence 写道:
> remount_tested_fs() and recover_tested_fs() both have
> almost the same code for umount'ing the target FS and
> mount'ing it back. Split this sequence into a new
> function called umount_and_remount().
> 
> Signed-off-by: Csókás, Bence <csokas.bence at prolan.hu>
> ---
> 
> Notes:
>      `--color-moved` is recommended
> 
>   tests/fs-tests/integrity/integck.c | 204 ++++++++++++++---------------
>   1 file changed, 96 insertions(+), 108 deletions(-)
> 
> diff --git a/tests/fs-tests/integrity/integck.c b/tests/fs-tests/integrity/integck.c
> index 0a7f142..6ce8c9d 100644
> --- a/tests/fs-tests/integrity/integck.c
> +++ b/tests/fs-tests/integrity/integck.c
> @@ -2577,6 +2577,97 @@ static int rm_minus_rf_dir(const char *dir_name)
>   	return 0;
>   }
>   
> +/*
> + * Detach the MTD device from UBI and attach it back. This function is used
> + * whed performing emulated power cut testing andthe power cuts are amulated by
> + * UBI, not by UBIFS. In this case, to recover from the emulated power cut we
> + * have to unmount UBIFS and re-attach the MTD device.
> + */
> +static int reattach(void)
> +{
> +	int err = 0;
> +	libubi_t libubi;
> +	struct ubi_attach_request req;
> +
> +	libubi = libubi_open();
> +	if (!libubi) {
> +		if (errno == 0)
> +			return errmsg("UBI is not present in the system");
> +		return sys_errmsg("cannot open libubi");
> +	}
> +
> +	err = ubi_detach_mtd(libubi, "/dev/ubi_ctrl", args.mtdn);
> +	if (err) {
> +		sys_errmsg("cannot detach mtd%d", args.mtdn);
> +		goto out;
> +	}
> +
> +	req.dev_num = UBI_DEV_NUM_AUTO;
> +	req.mtd_num = args.mtdn;
> +	req.vid_hdr_offset = 0;
> +	req.mtd_dev_node = NULL;
> +	req.max_beb_per1024 = 0;
> +
> +	err = ubi_attach(libubi, "/dev/ubi_ctrl", &req);
> +	if (err)
> +		sys_errmsg("cannot attach mtd%d", args.mtdn);
> +
> +out:
> +	libubi_close(libubi);
> +	return err;
> +}
> +
> +/**
> + * Unmount and mount back the test file-system.
> + */
> +static int umount_and_remount(int mounted, int reatt, int um_rorw)
> +{
> +	int ret = 0;
> +	unsigned long flags;
> +
> +	if (mounted)
> +		ret = umount(fsinfo.mount_point);
> +	if (ret) {
> +		pcv("cannot unmount %s", fsinfo.mount_point);
> +		return -1;
> +	}
> +

[...]
>   /*
>    * Recover the tested file-system from an emulated power cut failure by
>    * unmounting it and mounting it again.
> @@ -3206,41 +3225,10 @@ static int recover_tested_fs(void)
>   	 * while mounting in 'remount_tested_fs()'.
>   	 */
>   	mntent = get_tested_fs_mntent();
> -	if (mntent)
> -		CHECK(umount(fsinfo.mount_point) != -1);

I think the 'CHECK' should't be removed, because it may lead to a dead 
loop in the caller:

3303         /* Check whether we can re-mount the tested FS  */
3304         do {
3305                 ret = recover_tested_fs();
3306         } while (ret && args.power_cut_mode && errno == EROFS);
>   
> -	if (args.reattach)
> -		CHECK(reattach() == 0);
> -
> -	if (!um_rorw) {
> -		ret = mount(fsinfo.fsdev, fsinfo.mount_point,
> -			    fsinfo.fstype, fsinfo.mount_flags,
> -			    fsinfo.mount_opts);
> -		if (ret) {
> -			pcv("unmounted %s, but cannot mount it back R/W",
> -			    fsinfo.mount_point);
> -			return -1;
> -		}
> -	} else {
> -		ret = mount(fsinfo.fsdev, fsinfo.mount_point,
> -			    fsinfo.fstype, fsinfo.mount_flags | MS_RDONLY,
> -			    fsinfo.mount_opts);
> -		if (ret) {
> -			pcv("unmounted %s, but cannot mount it back R/O",
> -			    fsinfo.mount_point);
> -			return -1;
> -		}
> -
> -		flags = fsinfo.mount_flags | MS_REMOUNT;
> -		flags &= ~((unsigned long)MS_RDONLY);
> -		ret = mount(fsinfo.fsdev, fsinfo.mount_point,
> -			    fsinfo.fstype, flags, fsinfo.mount_opts);
> -		if (ret) {
> -			pcv("unmounted %s, mounted R/O, but cannot re-mount it R/W",
> -			     fsinfo.mount_point);
> -			return -1;
> -		}
> -	}
> +	ret = umount_and_remount(!!mntent, args.reattach, um_rorw);
> +	if (ret)
> +		return -1;
>   
>   	if (rorw2) {
>   		flags = fsinfo.mount_flags | MS_RDONLY | MS_REMOUNT;
>