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

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

[Eric Sandeen]

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@redhat.com>
---

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;
+	}
+
 	/*
 	 * 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@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

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

[Brian Foster]

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@redhat.com>
> ---
> 
> 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@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

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

[Eric Sandeen]

On 6/19/15 8:20 AM, Brian Foster wrote:
> 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@redhat.com>
>> ---
>>
>> 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.

yeargh, right, last-minute decision to not fail on the failed stat.

Sigh.  V2 coming.

(I think Jan is working on a more wholesale cleanup of this crap, but in the
meantime I think maybe a couple targeted fixes to get things going are probably
ok).

-Eric

> 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@oss.sgi.com
>> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

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

[Eric Sandeen]

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 requiring 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@redhat.com>
---

V2: don't check statbuf if stat fails, thanks Brian :)

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);
+		else if (S_ISREG(statbuf.st_mode))
+			isa_file = 1;
+	}
+
 	/*
 	 * 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@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

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

[Brian Foster]

On Fri, Jun 19, 2015 at 11:41:01AM -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 requiring 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@redhat.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

> 
> V2: don't check statbuf if stat fails, thanks Brian :)
> 
> 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);
> +		else if (S_ISREG(statbuf.st_mode))
> +			isa_file = 1;
> +	}
> +
>  	/*
>  	 * 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@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs