diff mbox series

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

Message ID 20230119105410.9236-1-jack@suse.cz (mailing list archive)
State New, archived
Headers show
Series fstests: Fix checking of UDF filesystems | expand

Commit Message

Jan Kara Jan. 19, 2023, 10:54 a.m. UTC
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(-)

Comments

David Disseldorp Jan. 20, 2023, 2:41 p.m. UTC | #1
Hi Jan,

On Thu, 19 Jan 2023 11:54:02 +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..de94e5a4cde4 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 ?+([0-9]+).*/\1/p'`

It looks as though mkudffs also accepts --blocksize= for this, so it
should also be handled here, IIUC.

Looks fine otherwise.

> +    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
Jan Kara Jan. 20, 2023, 3:45 p.m. UTC | #2
Hello!

On Fri 20-01-23 15:41:23, David Disseldorp wrote:
> On Thu, 19 Jan 2023 11:54:02 +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..de94e5a4cde4 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 ?+([0-9]+).*/\1/p'`
> 
> It looks as though mkudffs also accepts --blocksize= for this, so it
> should also be handled here, IIUC.

Thanks for review! I've fixed this.

									Honza
Zorro Lang Jan. 20, 2023, 7:08 p.m. UTC | #3
On Fri, Jan 20, 2023 at 04:45:10PM +0100, Jan Kara wrote:
> Hello!
> 
> On Fri 20-01-23 15:41:23, David Disseldorp wrote:
> > On Thu, 19 Jan 2023 11:54:02 +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..de94e5a4cde4 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 ?+([0-9]+).*/\1/p'`
> > 
> > It looks as though mkudffs also accepts --blocksize= for this, so it
> > should also be handled here, IIUC.

And _scratch_mkfs_sized need blocksize too. If udf support blocksize parameter,
you might like to take a look at that helper too, due to it always use 4096 for udf
blocksize.

Similar helpers are _scratch_mkfs_geom (maybe not suit for udf) and
_scratch_mkfs_blocksized.

Thanks,
Zorro

> 
> Thanks for review! I've fixed this.
> 
> 									Honza
> -- 
> Jan Kara <jack@suse.com>
> SUSE Labs, CR
>
Jan Kara Jan. 20, 2023, 7:22 p.m. UTC | #4
On Sat 21-01-23 03:08:15, Zorro Lang wrote:
> On Fri, Jan 20, 2023 at 04:45:10PM +0100, Jan Kara wrote:
> > Hello!
> > 
> > On Fri 20-01-23 15:41:23, David Disseldorp wrote:
> > > On Thu, 19 Jan 2023 11:54:02 +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..de94e5a4cde4 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 ?+([0-9]+).*/\1/p'`
> > > 
> > > It looks as though mkudffs also accepts --blocksize= for this, so it
> > > should also be handled here, IIUC.
> 
> And _scratch_mkfs_sized need blocksize too. If udf support blocksize parameter,
> you might like to take a look at that helper too, due to it always use 4096 for udf
> blocksize.
> 
> Similar helpers are _scratch_mkfs_geom (maybe not suit for udf) and
> _scratch_mkfs_blocksized.

Thanks for the tips, I'll have a look into those next.

									Honza
diff mbox series

Patch

diff --git a/common/rc b/common/rc
index 8060c03b7d18..de94e5a4cde4 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 ?+([0-9]+).*/\1/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