[PATCH v2 0/2] fstests: Fix checking of UDF filesystems

[PATCH v2 0/2] fstests: Fix checking of UDF filesystems

[Jan Kara]

Hello,

these two patches fix the usage of udf_test program in _check_udf_filesystem
so that it can reliably check udf filesystems. It provides it proper arguments
and also unmounts the filesystem prior to running the check.

Changes since v1:
* Added Reviewed-by tag
* Accept also --blocksize version of the option

								Honza
Previous versions:
Link: http://lore.kernel.org/r/20230119104618.28701-1-jack@suse.cz # v1

[PATCH v2 1/2] common: Provide blocksize and ecclength to udf fsck

[Jan Kara]

udf_test program used for verifying filesystem is not able to determine
filesystem blocksize. Provide it in the options together with disabling
ecclength as it is not used on harddrives.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 common/rc | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/common/rc b/common/rc
index 8060c03b7d18..bf3effa4c1ed 100644
--- a/common/rc
+++ b/common/rc
@@ -3075,9 +3075,13 @@ _check_udf_filesystem()
     fi
 
     local device=$1
-    local opt_arg=""
+    local blksz=`echo $MKFS_OPTIONS | sed -rn 's/.*(-b|--blocksize)[ =]?+([0-9]+).*/\2/p'`
+    if [ -z "$blksz" ]; then
+	blksz=512
+    fi
+    local opt_arg="-ecclength 1 -blocksize $blksz"
     if [ $# -eq 2 ]; then
-	opt_arg="-lastvalidblock $(( $2 - 1 ))"
+	opt_arg+=" -lastvalidblock $(( $2 - 1 ))"
     fi
 
     rm -f $seqres.checkfs
-- 
2.35.3

Re: [PATCH v2 1/2] common: Provide blocksize and ecclength to udf fsck

[David Disseldorp]

On Fri, 20 Jan 2023 16:50:29 +0100, Jan Kara wrote:

> udf_test program used for verifying filesystem is not able to determine
> filesystem blocksize. Provide it in the options together with disabling
> ecclength as it is not used on harddrives.
> 
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  common/rc | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/common/rc b/common/rc
> index 8060c03b7d18..bf3effa4c1ed 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -3075,9 +3075,13 @@ _check_udf_filesystem()
>      fi
>  
>      local device=$1
> -    local opt_arg=""
> +    local blksz=`echo $MKFS_OPTIONS | sed -rn 's/.*(-b|--blocksize)[ =]?+([0-9]+).*/\2/p'`
> +    if [ -z "$blksz" ]; then
> +	blksz=512
> +    fi
> +    local opt_arg="-ecclength 1 -blocksize $blksz"
>      if [ $# -eq 2 ]; then
> -	opt_arg="-lastvalidblock $(( $2 - 1 ))"
> +	opt_arg+=" -lastvalidblock $(( $2 - 1 ))"
>      fi
>  
>      rm -f $seqres.checkfs

Looks fine - I suppose it's safe to assume the --media-type=hd default
blksz value.
Reviewed-by: David Disseldorp <ddiss@suse.de>

Re: [PATCH v2 1/2] common: Provide blocksize and ecclength to udf fsck

[Jan Kara]

On Fri 20-01-23 17:34:54, David Disseldorp wrote:
> On Fri, 20 Jan 2023 16:50:29 +0100, Jan Kara wrote:
> 
> > udf_test program used for verifying filesystem is not able to determine
> > filesystem blocksize. Provide it in the options together with disabling
> > ecclength as it is not used on harddrives.
> > 
> > Signed-off-by: Jan Kara <jack@suse.cz>
> > ---
> >  common/rc | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/common/rc b/common/rc
> > index 8060c03b7d18..bf3effa4c1ed 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -3075,9 +3075,13 @@ _check_udf_filesystem()
> >      fi
> >  
> >      local device=$1
> > -    local opt_arg=""
> > +    local blksz=`echo $MKFS_OPTIONS | sed -rn 's/.*(-b|--blocksize)[ =]?+([0-9]+).*/\2/p'`
> > +    if [ -z "$blksz" ]; then
> > +	blksz=512
> > +    fi
> > +    local opt_arg="-ecclength 1 -blocksize $blksz"
> >      if [ $# -eq 2 ]; then
> > -	opt_arg="-lastvalidblock $(( $2 - 1 ))"
> > +	opt_arg+=" -lastvalidblock $(( $2 - 1 ))"
> >      fi
> >  
> >      rm -f $seqres.checkfs
> 
> Looks fine - I suppose it's safe to assume the --media-type=hd default
> blksz value.
> Reviewed-by: David Disseldorp <ddiss@suse.de>

Thanks for review! Yeah, I didn't really find a value in trying to handle
anything else. If a real usecase comes, we can always tweak this.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[PATCH v2 2/2] common: Unmount udf filesystem prior checking

[Jan Kara]

_check_udf_filesystem forgot to unmount the filesystem prior to checking
it. That was leading to check failures.

Reviewed-by: David Disseldorp <ddiss@suse.de>
Signed-off-by: Jan Kara <jack@suse.cz>
---
 common/rc | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/common/rc b/common/rc
index bf3effa4c1ed..7f80afe7aeec 100644
--- a/common/rc
+++ b/common/rc
@@ -3074,6 +3074,12 @@ _check_udf_filesystem()
 	return
     fi
 
+    # Is the filesystem mounted?
+    local type=`_fs_type $device`
+    if [ "$type" = "$FSTYP" ]; then
+	local mountpoint=`_umount_or_remount_ro $device`
+    fi
+
     local device=$1
     local blksz=`echo $MKFS_OPTIONS | sed -rn 's/.*(-b|--blocksize)[ =]?+([0-9]+).*/\2/p'`
     if [ -z "$blksz" ]; then
@@ -3090,6 +3096,10 @@ _check_udf_filesystem()
 	_udf_test_known_error_filter | \
 	grep -E -iv "Error count:.*[0-9]+.*total occurrences:.*[0-9]+|Warning count:.*[0-9]+.*total occurrences:.*[0-9]+" && \
         echo "Warning UDF Verifier reported errors see $seqres.checkfs." && return 1
+    # Remount the filesystem
+    if [ "$type" = "$FSTYP" ]; then
+	_mount_or_remount_rw "$MOUNT_OPTIONS" $device $mountpoint
+    fi
     return 0
 }
 
-- 
2.35.3