diff mbox series

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

Message ID 20240815091225.3237097-1-csokas.bence@prolan.hu
State New
Headers show
Series [1/2] fs-tests: integck: Refactor: split out common remount logic | expand

Commit Message

Csókás Bence Aug. 15, 2024, 9:12 a.m. UTC 
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@prolan.hu>
---

Notes:
    `--color-moved` is recommended

 tests/fs-tests/integrity/integck.c | 204 ++++++++++++++---------------
 1 file changed, 96 insertions(+), 108 deletions(-)

Comments

Zhihao Cheng Aug. 16, 2024, 12:41 p.m. UTC | #1 
在 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@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;
>
diff mbox series

Patch

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;
+	}
+
+	if (reatt)
+		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;
+		}
+	}
+
+	return 0;
+}
+
 /**
  * Re-mount the test file-system. This function randomly select how to
  * re-mount.
@@ -2633,41 +2724,9 @@  static int remount_tested_fs(void)
 			}
 		}
 
-		ret = umount(fsinfo.mount_point);
-		if (ret) {
-			pcv("cannot unmount %s", fsinfo.mount_point);
+		ret = umount_and_remount(1, 0, um_rorw);
+		if (ret)
 			return -1;
-		}
-
-		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;
-			}
-		}
 	}
 
 	if (rorw2) {
@@ -3143,46 +3202,6 @@  static void free_fs_info(struct dir_info *dir)
 	}
 }
 
-/*
- * 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;
-}
-
 /*
  * 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);
 
-	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;