Thread (28 messages) 28 messages, 5 authors, 2017-01-20

Re: [PATCH v3 4/5] xfs_db: sanitize geometry on load

From: Eric Sandeen <hidden>
Date: 2017-01-14 02:35:04

On 1/13/17 7:35 AM, Brian Foster wrote:
quoted
+		fprintf(stderr,
+_("%s: device %s AG count is insane, but could be %u.  Limiting reads to AG 0.\n"),
+			progname, fsdevice, dblocks / sbp->sb_agblocks);
+	} else {
+		fprintf(stderr,
+_("%s: device %s AG count is insane.  Limiting reads to AG 0.\n"),
+			progname, fsdevice);
+	}
For reasons like the above, I think xfs_db shouldn't be in the business
of repair like validation (xfs_check notwithstanding). That said,
dropping into a fixed single AG mode seems less risky than trying to
surmise a valid geometry. I'd get rid of the "this might be your
agcount" messaging entirely though and just replace it with something
that explicitly states the filesystem is corrupted, the runtime geometry
is invalid and that the user should probably run xfs_repair before doing
anything.
So keep in mind that xfs_db is for people with super xfs powers. (*)

I wouldn't suggest repair, I'd start with 1 ag to avoid the OOM, state 
that clearly, and punt the problem to the admin with no other specific
suggestions.
I still like the idea of the single AG mode thing as a command line flag
rather than default behavior because it requires user acknowledgement,
but this is a debug tool after all, so I'll defer to Eric on that. I do
think that if we create this kind of invalid runtime mode, this should
be split into two patches. First, a bugfix patch for the core OOM
problem (i.e., detect a wacky superblock and exit). Second, replace the
exit with the single AG runtime mode thing.
Well, the problem with a flag, I think, is that you might have already
unwittingly OOMed your box to find out that you need it.
Rebooting to try again with a flag sucks.

(*) unless you are invoking it via xfs_admin.sh, dammit.  We sure wouldn't
want xfs_admin to exit happily, having updated only one AG.  Dammit!

Perhaps it should set exitcode, and then xfs_admin could do something
like:

	xfs_db -c quit $DEV

first, and check that db is able to initialize sanely before using it again
to perform normal admin functions.

-Eric
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help