Skip to contentSkip to conversation

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

PATCH In review Eric Sandeen 3 participants 5 messages Jun 18, 2015 – Jun 24, 2015

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 xfs_repair: automatically enable -f (file) mode when needed

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?

---

repair/xfs_repair.c +12 -0
index 834697a..2d376be 100644
@@ -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
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#1 lore raw
Quoted 25 lines · 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?

---

repair/xfs_repair.c +0 -0
index 834697a..2d376be 100644
@@ -573,6 +573,18 @@ main(int argc, char **argv)
 		exit(1);
 	}
 

repair/xfs_repair.c main(int argc, char **argv) · line 585

Quoted 10 lines
repair/xfs_repair.c +10 -0
+	/* -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

Quoted 10 lines
repair/xfs_repair.c +2 -0
+	}
+
 	/*
 	 * 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
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#2 lore raw
Quoted 26 lines · Brian Foster
Quoted 25 lines · 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?

---

repair/xfs_repair.c +0 -0
index 834697a..2d376be 100644
@@ -573,6 +573,18 @@ main(int argc, char **argv)
 		exit(1);
 	}
 
Quoted 13 lines

repair/xfs_repair.c main(int argc, char **argv) · line 585

Quoted 10 lines
repair/xfs_repair.c +10 -0
+	/* -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

Quoted 8 lines

Brian

Quoted 6 lines
repair/xfs_repair.c +2 -0
+	}
+
 	/*
 	 * 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
Quoted 4 lines
Quoted 4 lines

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#3 lore raw

PATCH v2 1/1 xfs_repair: automatically enable -f (file) mode when needed

In reply to 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?

---

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

repair/xfs_repair.c +12 -0
index 834697a..2d376be 100644
@@ -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
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Reviewed-by on v2 1/1
#4 lore raw
Quoted 11 lines · 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
Quoted 5 lines

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?

---

Quoted 28 lines

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

repair/xfs_repair.c +12 -0
index 834697a..2d376be 100644
@@ -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
Quoted 4 lines

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs