public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Eric Sandeen <sandeen@redhat.com>
To: Brian Foster <bfoster@redhat.com>
Cc: xfs-oss <xfs@oss.sgi.com>
Subject: Re: [PATCH] xfs_repair: automatically enable -f (file) mode when needed
Date: Fri, 19 Jun 2015 10:19:49 -0500	[thread overview]
Message-ID: <55843315.1050800@redhat.com> (raw)
In-Reply-To: <20150619132004.GD12833@bfoster.bfoster>

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

  reply	other threads:[~2015-06-19 15:19 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-18 21:29 [PATCH] xfs_repair: automatically enable -f (file) mode when needed Eric Sandeen
2015-06-19 13:20 ` Brian Foster
2015-06-19 15:19   ` Eric Sandeen [this message]
2015-06-19 16:41 ` [PATCH V2] " Eric Sandeen
2015-06-24 11:38   ` Brian Foster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55843315.1050800@redhat.com \
    --to=sandeen@redhat.com \
    --cc=bfoster@redhat.com \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox