xfs
[Top] [All Lists]

Re: [PATCH] xfs_repair: automatically enable -f (file) mode when needed

To: Eric Sandeen <sandeen@xxxxxxxxxx>
Subject: Re: [PATCH] xfs_repair: automatically enable -f (file) mode when needed
From: Brian Foster <bfoster@xxxxxxxxxx>
Date: Fri, 19 Jun 2015 09:20:04 -0400
Cc: xfs-oss <xfs@xxxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <5583383D.4070906@xxxxxxxxxx>
References: <5583383D.4070906@xxxxxxxxxx>
User-agent: Mutt/1.5.23 (2014-03-12)
On Thu, Jun 18, 2015 at 04:29:33PM -0500, Eric Sandeen wrote:
> If we specify "-f" to xfs_repair, it recognizes that it's working
> on a file, and if the underlying filesystem sector size differs
> such that direct IO won't work, it disables direct IO.
> 
> It's odd, though, that we'd need to specify this, and the failure
> is non-obvious:
> 
> # xfs_repair /mnt/test/foo.img
> Phase 1 - find and verify superblock...
> xfs_repair: read failed: Invalid argument
> 
> I see no advantage to requirin the administrator to jump through
> this hoop; why not just detect that it's a file, and move on?
> 
> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
> ---
> 
> diff --git a/repair/xfs_repair.c b/repair/xfs_repair.c
> index 834697a..2d376be 100644
> --- a/repair/xfs_repair.c
> +++ b/repair/xfs_repair.c
> @@ -573,6 +573,18 @@ main(int argc, char **argv)
>               exit(1);
>       }
>  
> +     /* -f forces this, but let's be nice and autodetect it, as well. */
> +     if (!isa_file) {
> +             int             fd = libxfs_device_to_fd(x.ddev);
> +             struct stat64   statbuf;
> +
> +             if (fstat64(fd, &statbuf) < 0)
> +                     do_warn(_("%s:  couldn't stat \"%s\"\n"),
> +                             progname, fs_name);
> +             if (S_ISREG(statbuf.st_mode))
> +                     isa_file = 1;

We probably shouldn't query the statbuf if the stat call failed (who
knows what's in there). Otherwise this seems like a good change to me.

Brian

> +     }
> +
>       /*
>        * if the sector size of the filesystem we are trying to repair is
>        * smaller than that of the underlying filesystem (i.e. we are repairing
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs

<Prev in Thread] Current Thread [Next in Thread>