diff mbox series

[04/12] btrfs: create a helper function, check_fsid(), to verify the tempfsid

Message ID cd8342fb284a1983d7645698464debecf417e52a.1707969354.git.anand.jain@oracle.com (mailing list archive)
State New, archived
Headers show
Series btrfs: functional test cases for tempfsid | expand

Commit Message

Anand Jain Feb. 15, 2024, 6:34 a.m. UTC
check_fsid() provides a method to verify if the given device is mounted
with the tempfsid in the kernel. Function sb() is an internal only
function.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 common/btrfs | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

Comments

Filipe Manana Feb. 15, 2024, 12:13 p.m. UTC | #1
On Thu, Feb 15, 2024 at 6:35 AM Anand Jain <anand.jain@oracle.com> wrote:
>
> check_fsid() provides a method to verify if the given device is mounted
> with the tempfsid in the kernel. Function sb() is an internal only
> function.
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  common/btrfs | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
>
> diff --git a/common/btrfs b/common/btrfs
> index e1b29c613767..5cba9b16b4de 100644
> --- a/common/btrfs
> +++ b/common/btrfs
> @@ -792,3 +792,37 @@ _has_btrfs_sysfs_feature_attr()
>
>         test -e /sys/fs/btrfs/features/$feature_attr
>  }
> +
> +# Dump key members of the on-disk super-block from the given disk; helps debug
> +sb()
> +{
> +       local dev1=$1
> +       local parameters="device|devid|^metadata_uuid|^fsid|^incom|^generation|\
> +               ^flags| \|$| \)$|compat_flags|compat_ro_flags|dev_item.uuid"
> +
> +       $BTRFS_UTIL_PROG inspect-internal dump-super $dev1 | egrep "$parameters"
> +}

Don't add such a short function name that doesn't minimally indicate
what it does.... especially taking into account that it's polluting
the global namespace and isn't used anywhere else.
So this code should be in the function below, as it's only used once....

> +
> +check_fsid()
> +{
> +       local dev1=$1
> +       local fsid
> +
> +       # on disk fsid
> +       fsid=$(sb $dev1 | grep ^fsid | awk -d" " '{print $2}')

Please use $AWK_PROG instead.

Again this sb() function is pointless, even because here we only care
about the fsid line, yet the function is extracting a lot of other
lines without any users.

> +       echo -e "On disk fsid:\t\t$fsid" | sed -e "s/$fsid/FSID/g"
> +
> +       echo -e -n "Metadata uuid:\t\t"
> +       cat /sys/fs/btrfs/$fsid/metadata_uuid | sed -e "s/$fsid/FSID/g"

Ok, so why do we care about printing the fsid and metadata uuid lines
if we always replace the IDs with the constant FSID?
It seems pointless to print them... At the very least this deserves a
comment, a rationale on what this accomplishes.

Thanks.

> +
> +       # This returns the temp_fsid if set
> +       tempfsid=$(_btrfs_get_fsid $dev1)
> +       if [[ $tempfsid == $fsid ]]; then
> +               echo -e "Temp fsid:\t\tFSID"
> +       else
> +               echo -e "Temp fsid:\t\tTEMPFSID"
> +       fi
> +
> +       echo -e -n "Tempfsid status:\t"
> +       cat /sys/fs/btrfs/$tempfsid/temp_fsid
> +}
> --
> 2.39.3
>
>
Zorro Lang Feb. 16, 2024, 3:02 p.m. UTC | #2
On Thu, Feb 15, 2024 at 02:34:07PM +0800, Anand Jain wrote:
> check_fsid() provides a method to verify if the given device is mounted
> with the tempfsid in the kernel. Function sb() is an internal only
> function.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  common/btrfs | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/common/btrfs b/common/btrfs
> index e1b29c613767..5cba9b16b4de 100644
> --- a/common/btrfs
> +++ b/common/btrfs
> @@ -792,3 +792,37 @@ _has_btrfs_sysfs_feature_attr()
>  
>  	test -e /sys/fs/btrfs/features/$feature_attr
>  }
> +
> +# Dump key members of the on-disk super-block from the given disk; helps debug
> +sb()
> +{
> +	local dev1=$1
> +	local parameters="device|devid|^metadata_uuid|^fsid|^incom|^generation|\
> +		^flags| \|$| \)$|compat_flags|compat_ro_flags|dev_item.uuid"
> +
> +	$BTRFS_UTIL_PROG inspect-internal dump-super $dev1 | egrep "$parameters"

Please don't use "egrep", it's deprecated, might hit a warning output.
Replace it with "grep -E" :)

> +}
> +
> +check_fsid()
> +{
> +	local dev1=$1
> +	local fsid
> +
> +	# on disk fsid
> +	fsid=$(sb $dev1 | grep ^fsid | awk -d" " '{print $2}')
> +	echo -e "On disk fsid:\t\t$fsid" | sed -e "s/$fsid/FSID/g"
> +
> +	echo -e -n "Metadata uuid:\t\t"
> +	cat /sys/fs/btrfs/$fsid/metadata_uuid | sed -e "s/$fsid/FSID/g"
> +
> +	# This returns the temp_fsid if set
> +	tempfsid=$(_btrfs_get_fsid $dev1)
> +	if [[ $tempfsid == $fsid ]]; then
> +		echo -e "Temp fsid:\t\tFSID"
> +	else
> +		echo -e "Temp fsid:\t\tTEMPFSID"
> +	fi
> +
> +	echo -e -n "Tempfsid status:\t"
> +	cat /sys/fs/btrfs/$tempfsid/temp_fsid
> +}
> -- 
> 2.39.3
> 
>
diff mbox series

Patch

diff --git a/common/btrfs b/common/btrfs
index e1b29c613767..5cba9b16b4de 100644
--- a/common/btrfs
+++ b/common/btrfs
@@ -792,3 +792,37 @@  _has_btrfs_sysfs_feature_attr()
 
 	test -e /sys/fs/btrfs/features/$feature_attr
 }
+
+# Dump key members of the on-disk super-block from the given disk; helps debug
+sb()
+{
+	local dev1=$1
+	local parameters="device|devid|^metadata_uuid|^fsid|^incom|^generation|\
+		^flags| \|$| \)$|compat_flags|compat_ro_flags|dev_item.uuid"
+
+	$BTRFS_UTIL_PROG inspect-internal dump-super $dev1 | egrep "$parameters"
+}
+
+check_fsid()
+{
+	local dev1=$1
+	local fsid
+
+	# on disk fsid
+	fsid=$(sb $dev1 | grep ^fsid | awk -d" " '{print $2}')
+	echo -e "On disk fsid:\t\t$fsid" | sed -e "s/$fsid/FSID/g"
+
+	echo -e -n "Metadata uuid:\t\t"
+	cat /sys/fs/btrfs/$fsid/metadata_uuid | sed -e "s/$fsid/FSID/g"
+
+	# This returns the temp_fsid if set
+	tempfsid=$(_btrfs_get_fsid $dev1)
+	if [[ $tempfsid == $fsid ]]; then
+		echo -e "Temp fsid:\t\tFSID"
+	else
+		echo -e "Temp fsid:\t\tTEMPFSID"
+	fi
+
+	echo -e -n "Tempfsid status:\t"
+	cat /sys/fs/btrfs/$tempfsid/temp_fsid
+}