[PATCH] xfs/020: use -f option for xfs_repair a fs image

[PATCH] xfs/020: use -f option for xfs_repair a fs image

[Zorro Lang]

This case creates a 60t image file on TEST_DIR. Generally
this's no problem. But if the image sector size is smaller
than the TEST_DEV sector size, for example we create the
image on hard 4k TEST_DEV, the test will fail.

So use -f for xfs_repair will helpful.

Signed-off-by: Zorro Lang <zlang@redhat.com>
---
 tests/xfs/020 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/xfs/020 b/tests/xfs/020
index dc305c1..120492d 100755
--- a/tests/xfs/020
+++ b/tests/xfs/020
@@ -55,7 +55,7 @@ fsfile=$TEST_DIR/fsfile.$seq
 rm -f $fsfile
 
 $MKFS_PROG -t xfs -d size=60t,file,name=$fsfile >/dev/null
-$XFS_REPAIR_PROG -o ag_stride=32 -t 1 $fsfile >/dev/null 2>&1
+$XFS_REPAIR_PROG -f -o ag_stride=32 -t 1 $fsfile >/dev/null 2>&1
 
 status=$?
 exit
-- 
1.9.3

Re: [PATCH] xfs/020: use -f option for xfs_repair a fs image

[Eric Sandeen]

On 8/12/15 12:33 PM, Zorro Lang wrote:
> This case creates a 60t image file on TEST_DIR. Generally
> this's no problem. But if the image sector size is smaller
> than the TEST_DEV sector size, for example we create the
> image on hard 4k TEST_DEV, the test will fail.
> 
> So use -f for xfs_repair will helpful.
> 
> Signed-off-by: Zorro Lang <zlang@redhat.com>
> ---
>  tests/xfs/020 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/xfs/020 b/tests/xfs/020
> index dc305c1..120492d 100755
> --- a/tests/xfs/020
> +++ b/tests/xfs/020
> @@ -55,7 +55,7 @@ fsfile=$TEST_DIR/fsfile.$seq
>  rm -f $fsfile
>  
>  $MKFS_PROG -t xfs -d size=60t,file,name=$fsfile >/dev/null
> -$XFS_REPAIR_PROG -o ag_stride=32 -t 1 $fsfile >/dev/null 2>&1
> +$XFS_REPAIR_PROG -f -o ag_stride=32 -t 1 $fsfile >/dev/null 2>&1

Agreed, but we should audit all tests for this problem, I think.

FWIW, the latest upstream will do this automatically...

So for this change, it's fine:

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

but will you see if other tests should be fixed as well?

-Eric

Re: [PATCH] xfs/020: use -f option for xfs_repair a fs image

[Zirong Lang]

----- 原始邮件 -----
> 发件人: "Eric Sandeen" <sandeen@sandeen.net>
> 收件人: "Zorro Lang" <zlang@redhat.com>, fstests@vger.kernel.org
> 抄送: eguan@redhat.com
> 发送时间: 星期四, 2015年 8 月 13日 上午 1:37:48
> 主题: Re: [PATCH] xfs/020: use -f option for xfs_repair a fs image
> 
> On 8/12/15 12:33 PM, Zorro Lang wrote:
> > This case creates a 60t image file on TEST_DIR. Generally
> > this's no problem. But if the image sector size is smaller
> > than the TEST_DEV sector size, for example we create the
> > image on hard 4k TEST_DEV, the test will fail.
> > 
> > So use -f for xfs_repair will helpful.
> > 
> > Signed-off-by: Zorro Lang <zlang@redhat.com>
> > ---
> >  tests/xfs/020 | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tests/xfs/020 b/tests/xfs/020
> > index dc305c1..120492d 100755
> > --- a/tests/xfs/020
> > +++ b/tests/xfs/020
> > @@ -55,7 +55,7 @@ fsfile=$TEST_DIR/fsfile.$seq
> >  rm -f $fsfile
> >  
> >  $MKFS_PROG -t xfs -d size=60t,file,name=$fsfile >/dev/null
> > -$XFS_REPAIR_PROG -o ag_stride=32 -t 1 $fsfile >/dev/null 2>&1
> > +$XFS_REPAIR_PROG -f -o ag_stride=32 -t 1 $fsfile >/dev/null 2>&1
> 
> Agreed, but we should audit all tests for this problem, I think.
> 
> FWIW, the latest upstream will do this automatically...
> 
> So for this change, it's fine:
> 
> Reviewed-by: Eric Sandeen <sandeen@redhat.com>

Thanks for review.

> 
> but will you see if other tests should be fixed as well?

I saw you fixed one in commit ddd55d5.

Sure, I will help to find all cases should be fixed.

Maybe add -f automatically for xfs_repair will be better, if it
try to repair an image file?

Or maybe add a function likes mkfs_image_file(), try to find the
sector size of the device contain the image file at first?

Thanks,
Zorro


> 
> -Eric
> 
> 

Re: [PATCH] xfs/020: use -f option for xfs_repair a fs image

[Zirong Lang]

----- 原始邮件 -----
> 发件人: "Eric Sandeen" <sandeen@sandeen.net>
> 收件人: "Zorro Lang" <zlang@redhat.com>, fstests@vger.kernel.org
> 抄送: eguan@redhat.com
> 发送时间: 星期四, 2015年 8 月 13日 上午 1:37:48
> 主题: Re: [PATCH] xfs/020: use -f option for xfs_repair a fs image
> 
> On 8/12/15 12:33 PM, Zorro Lang wrote:
> > This case creates a 60t image file on TEST_DIR. Generally
> > this's no problem. But if the image sector size is smaller
> > than the TEST_DEV sector size, for example we create the
> > image on hard 4k TEST_DEV, the test will fail.
> > 
> > So use -f for xfs_repair will helpful.
> > 
> > Signed-off-by: Zorro Lang <zlang@redhat.com>
> > ---
> >  tests/xfs/020 | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tests/xfs/020 b/tests/xfs/020
> > index dc305c1..120492d 100755
> > --- a/tests/xfs/020
> > +++ b/tests/xfs/020
> > @@ -55,7 +55,7 @@ fsfile=$TEST_DIR/fsfile.$seq
> >  rm -f $fsfile
> >  
> >  $MKFS_PROG -t xfs -d size=60t,file,name=$fsfile >/dev/null
> > -$XFS_REPAIR_PROG -o ag_stride=32 -t 1 $fsfile >/dev/null 2>&1
> > +$XFS_REPAIR_PROG -f -o ag_stride=32 -t 1 $fsfile >/dev/null 2>&1
> 
> Agreed, but we should audit all tests for this problem, I think.
> 
> FWIW, the latest upstream will do this automatically...
> 
> So for this change, it's fine:
> 
> Reviewed-by: Eric Sandeen <sandeen@redhat.com>
> 
> but will you see if other tests should be fixed as well?

Hi Eric,

I have checked all xfstests cases about xfs_repair, only xfs/020 and
xfs/032 try to repair a file image.

This patch is about xfs/020. 

But for xfs/032, I think there is no big problem in it. Because always
copy device to a file, if the device's sector size is 4k, the file
image will follow it. Except the TEST_DEV sector size not equal to
SCRATCH_DEV sector size. But yes, add -f will be safe.

Do you need I re-send a patch to add -f in xfs/020 and xfs/032 both?

Or you have another idea to fix this problem, likes add a new function?

Thanks,
Zorro Lang

> 
> -Eric
> 
> 

Re: [PATCH] xfs/020: use -f option for xfs_repair a fs image

[Eryu Guan]

On Thu, Aug 13, 2015 at 11:15:00AM -0400, Zirong Lang wrote:
> 
> 
> ----- 原始邮件 -----
> > 发件人: "Eric Sandeen" <sandeen@sandeen.net>
> > 收件人: "Zorro Lang" <zlang@redhat.com>, fstests@vger.kernel.org
> > 抄送: eguan@redhat.com
> > 发送时间: 星期四, 2015年 8 月 13日 上午 1:37:48
> > 主题: Re: [PATCH] xfs/020: use -f option for xfs_repair a fs image
> > 
> > On 8/12/15 12:33 PM, Zorro Lang wrote:
> > > This case creates a 60t image file on TEST_DIR. Generally
> > > this's no problem. But if the image sector size is smaller
> > > than the TEST_DEV sector size, for example we create the
> > > image on hard 4k TEST_DEV, the test will fail.
> > > 
> > > So use -f for xfs_repair will helpful.
> > > 
> > > Signed-off-by: Zorro Lang <zlang@redhat.com>
> > > ---
> > >  tests/xfs/020 | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/tests/xfs/020 b/tests/xfs/020
> > > index dc305c1..120492d 100755
> > > --- a/tests/xfs/020
> > > +++ b/tests/xfs/020
> > > @@ -55,7 +55,7 @@ fsfile=$TEST_DIR/fsfile.$seq
> > >  rm -f $fsfile
> > >  
> > >  $MKFS_PROG -t xfs -d size=60t,file,name=$fsfile >/dev/null
> > > -$XFS_REPAIR_PROG -o ag_stride=32 -t 1 $fsfile >/dev/null 2>&1
> > > +$XFS_REPAIR_PROG -f -o ag_stride=32 -t 1 $fsfile >/dev/null 2>&1
> > 
> > Agreed, but we should audit all tests for this problem, I think.
> > 
> > FWIW, the latest upstream will do this automatically...
> > 
> > So for this change, it's fine:
> > 
> > Reviewed-by: Eric Sandeen <sandeen@redhat.com>
> > 
> > but will you see if other tests should be fixed as well?
> 
> Hi Eric,
> 
> I have checked all xfstests cases about xfs_repair, only xfs/020 and
> xfs/032 try to repair a file image.
> 
> This patch is about xfs/020. 
> 
> But for xfs/032, I think there is no big problem in it. Because always
> copy device to a file, if the device's sector size is 4k, the file
> image will follow it. Except the TEST_DEV sector size not equal to
> SCRATCH_DEV sector size. But yes, add -f will be safe.
> 
> Do you need I re-send a patch to add -f in xfs/020 and xfs/032 both?

I think the current version of this patch is OK, as xfs/020 is the only
affected case.

Thanks,
Eryu