diff mbox

[6/6] overlay: deduplicate code in overlay mount helpers

Message ID 1506599710-9751-1-git-send-email-amir73il@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Amir Goldstein Sept. 28, 2017, 11:55 a.m. UTC 
factor out helpers _overlay_base_mount() and _overlay_base_umount()
to reduce code duplication.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---

Eryu,

Appologies for the rolling series, but since you said you are looking
into re-factoring mount option handling, I figured you may want to take
this additional cleanup as well.

Beyond reducing code duplication, this change puts the difference in
mount options for base test fs and base scratch fs in wrapper functions
and leaves the common code free of those differences.

Amir.

 common/overlay | 66 ++++++++++++++++++++++++++--------------------------------
 1 file changed, 29 insertions(+), 37 deletions(-)

Comments

Eryu Guan Sept. 29, 2017, 1:44 a.m. UTC | #1 
On Thu, Sep 28, 2017 at 02:55:10PM +0300, Amir Goldstein wrote:
> factor out helpers _overlay_base_mount() and _overlay_base_umount()
> to reduce code duplication.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
> 
> Eryu,
> 
> Appologies for the rolling series, but since you said you are looking
> into re-factoring mount option handling, I figured you may want to take
> this additional cleanup as well.

No problem, thanks for all the work! However, I haven't got some time to
test this patchset yet (but will start testing today), and the mount
option handling seems quite complicated from my quick look last week, I
may need some time to think more about it and play with this patchset.
But it's public holiday next week here in China, I may take more time
than expected :)

Thanks,
Eryu

> 
> Beyond reducing code duplication, this change puts the difference in
> mount options for base test fs and base scratch fs in wrapper functions
> and leaves the common code free of those differences.
> 
> Amir.
> 
>  common/overlay | 66 ++++++++++++++++++++++++++--------------------------------
>  1 file changed, 29 insertions(+), 37 deletions(-)
> 
> diff --git a/common/overlay b/common/overlay
> index c3bb9ee..79097ba 100644
> --- a/common/overlay
> +++ b/common/overlay
> @@ -53,23 +53,31 @@ _overlay_mount()
>  				$* $dir $mnt
>  }
>  
> -_overlay_base_test_mount()
> +_overlay_base_mount()
>  {
> -	if [ -z "$OVL_BASE_TEST_DEV" -o -z "$OVL_BASE_TEST_DIR" ] || \
> -		_check_mounted_on OVL_BASE_TEST_DEV $OVL_BASE_TEST_DEV \
> -				OVL_BASE_TEST_DIR $OVL_BASE_TEST_DIR
> -	then
> +	local devname=$1
> +	local mntname=$2
> +	local dev=$3
> +	local mnt=$4
> +	shift 4
> +
> +	if [ -z "$dev" -o -z "$mnt" ] || \
> +		_check_mounted_on $devname $dev $mntname $mnt; then
>  		# no base fs or already mounted
>  		return 0
> -	elif [ $? -ne 1 ]
> -	then
> +	elif [ $? -ne 1 ]; then
>  		# base fs mounted but not on mount point
>  		return 1
>  	fi
>  
> -	_mount $TEST_FS_MOUNT_OPTS \
> -		$SELINUX_MOUNT_OPTIONS \
> -		$OVL_BASE_TEST_DEV $OVL_BASE_TEST_DIR
> +	_mount $* $dev $mnt
> +}
> +
> +_overlay_base_test_mount()
> +{
> +	_overlay_base_mount OVL_BASE_TEST_DEV OVL_BASE_TEST_DIR \
> +			"$OVL_BASE_TEST_DEV" "$OVL_BASE_TEST_DIR" \
> +			$TEST_FS_MOUNT_OPTS $SELINUX_MOUNT_OPTIONS
>  }
>  
>  _overlay_test_mount()
> @@ -80,28 +88,9 @@ _overlay_test_mount()
>  
>  _overlay_base_scratch_mount()
>  {
> -	if [ -z "$OVL_BASE_SCRATCH_DEV" -o -z "$OVL_BASE_SCRATCH_MNT" ] || \
> -		_check_mounted_on OVL_BASE_SCRATCH_DEV $OVL_BASE_SCRATCH_DEV \
> -				OVL_BASE_SCRATCH_MNT $OVL_BASE_SCRATCH_MNT
> -	then
> -		# no base fs or already mounted
> -		return 0
> -	elif [ $? -ne 1 ]
> -	then
> -		# base fs mounted but not on mount point
> -		return 1
> -	fi
> -
> -	_mount $OVL_BASE_MOUNT_OPTIONS \
> -		$SELINUX_MOUNT_OPTIONS \
> -		$OVL_BASE_SCRATCH_DEV $OVL_BASE_SCRATCH_MNT
> -}
> -
> -_overlay_base_scratch_unmount()
> -{
> -	[ -n "$OVL_BASE_SCRATCH_DEV" -a -n "$OVL_BASE_SCRATCH_MNT" ] || return 0
> -
> -	$UMOUNT_PROG $OVL_BASE_SCRATCH_MNT
> +	_overlay_base_mount OVL_BASE_SCRATCH_DEV OVL_BASE_SCRATCH_MNT \
> +			"$OVL_BASE_SCRATCH_DEV" "$OVL_BASE_SCRATCH_MNT" \
> +			$OVL_BASE_MOUNT_OPTIONS $SELINUX_MOUNT_OPTIONS
>  }
>  
>  _overlay_scratch_mount()
> @@ -110,23 +99,26 @@ _overlay_scratch_mount()
>  		_overlay_mount $OVL_BASE_SCRATCH_MNT $SCRATCH_MNT $*
>  }
>  
> -_overlay_base_test_unmount()
> +_overlay_base_unmount()
>  {
> -	[ -n "$OVL_BASE_TEST_DEV" -a -n "$OVL_BASE_TEST_DIR" ] || return 0
> +	local dev=$1
> +	local mnt=$2
> +
> +	[ -n "$dev" -a -n "$mnt" ] || return 0
>  
> -	$UMOUNT_PROG $OVL_BASE_TEST_DIR
> +	$UMOUNT_PROG $mnt
>  }
>  
>  _overlay_test_unmount()
>  {
>  	$UMOUNT_PROG $TEST_DIR
> -	_overlay_base_test_unmount
> +	_overlay_base_unmount "$OVL_BASE_TEST_DEV" "$OVL_BASE_TEST_DIR"
>  }
>  
>  _overlay_scratch_unmount()
>  {
>  	$UMOUNT_PROG $SCRATCH_MNT
> -	_overlay_base_scratch_unmount
> +	_overlay_base_unmount "$OVL_BASE_SCRATCH_DEV" "$OVL_BASE_SCRATCH_MNT"
>  }
>  
>  # Require a specific overlayfs feature
> -- 
> 2.7.4
> 
--
To unsubscribe from this list: send the line "unsubscribe fstests" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/common/overlay b/common/overlay
index c3bb9ee..79097ba 100644
--- a/common/overlay
+++ b/common/overlay
@@ -53,23 +53,31 @@  _overlay_mount()
 				$* $dir $mnt
 }
 
-_overlay_base_test_mount()
+_overlay_base_mount()
 {
-	if [ -z "$OVL_BASE_TEST_DEV" -o -z "$OVL_BASE_TEST_DIR" ] || \
-		_check_mounted_on OVL_BASE_TEST_DEV $OVL_BASE_TEST_DEV \
-				OVL_BASE_TEST_DIR $OVL_BASE_TEST_DIR
-	then
+	local devname=$1
+	local mntname=$2
+	local dev=$3
+	local mnt=$4
+	shift 4
+
+	if [ -z "$dev" -o -z "$mnt" ] || \
+		_check_mounted_on $devname $dev $mntname $mnt; then
 		# no base fs or already mounted
 		return 0
-	elif [ $? -ne 1 ]
-	then
+	elif [ $? -ne 1 ]; then
 		# base fs mounted but not on mount point
 		return 1
 	fi
 
-	_mount $TEST_FS_MOUNT_OPTS \
-		$SELINUX_MOUNT_OPTIONS \
-		$OVL_BASE_TEST_DEV $OVL_BASE_TEST_DIR
+	_mount $* $dev $mnt
+}
+
+_overlay_base_test_mount()
+{
+	_overlay_base_mount OVL_BASE_TEST_DEV OVL_BASE_TEST_DIR \
+			"$OVL_BASE_TEST_DEV" "$OVL_BASE_TEST_DIR" \
+			$TEST_FS_MOUNT_OPTS $SELINUX_MOUNT_OPTIONS
 }
 
 _overlay_test_mount()
@@ -80,28 +88,9 @@  _overlay_test_mount()
 
 _overlay_base_scratch_mount()
 {
-	if [ -z "$OVL_BASE_SCRATCH_DEV" -o -z "$OVL_BASE_SCRATCH_MNT" ] || \
-		_check_mounted_on OVL_BASE_SCRATCH_DEV $OVL_BASE_SCRATCH_DEV \
-				OVL_BASE_SCRATCH_MNT $OVL_BASE_SCRATCH_MNT
-	then
-		# no base fs or already mounted
-		return 0
-	elif [ $? -ne 1 ]
-	then
-		# base fs mounted but not on mount point
-		return 1
-	fi
-
-	_mount $OVL_BASE_MOUNT_OPTIONS \
-		$SELINUX_MOUNT_OPTIONS \
-		$OVL_BASE_SCRATCH_DEV $OVL_BASE_SCRATCH_MNT
-}
-
-_overlay_base_scratch_unmount()
-{
-	[ -n "$OVL_BASE_SCRATCH_DEV" -a -n "$OVL_BASE_SCRATCH_MNT" ] || return 0
-
-	$UMOUNT_PROG $OVL_BASE_SCRATCH_MNT
+	_overlay_base_mount OVL_BASE_SCRATCH_DEV OVL_BASE_SCRATCH_MNT \
+			"$OVL_BASE_SCRATCH_DEV" "$OVL_BASE_SCRATCH_MNT" \
+			$OVL_BASE_MOUNT_OPTIONS $SELINUX_MOUNT_OPTIONS
 }
 
 _overlay_scratch_mount()
@@ -110,23 +99,26 @@  _overlay_scratch_mount()
 		_overlay_mount $OVL_BASE_SCRATCH_MNT $SCRATCH_MNT $*
 }
 
-_overlay_base_test_unmount()
+_overlay_base_unmount()
 {
-	[ -n "$OVL_BASE_TEST_DEV" -a -n "$OVL_BASE_TEST_DIR" ] || return 0
+	local dev=$1
+	local mnt=$2
+
+	[ -n "$dev" -a -n "$mnt" ] || return 0
 
-	$UMOUNT_PROG $OVL_BASE_TEST_DIR
+	$UMOUNT_PROG $mnt
 }
 
 _overlay_test_unmount()
 {
 	$UMOUNT_PROG $TEST_DIR
-	_overlay_base_test_unmount
+	_overlay_base_unmount "$OVL_BASE_TEST_DEV" "$OVL_BASE_TEST_DIR"
 }
 
 _overlay_scratch_unmount()
 {
 	$UMOUNT_PROG $SCRATCH_MNT
-	_overlay_base_scratch_unmount
+	_overlay_base_unmount "$OVL_BASE_SCRATCH_DEV" "$OVL_BASE_SCRATCH_MNT"
 }
 
 # Require a specific overlayfs feature