Re: [PATCH] xfs_admin: revert online label setting ability

Eric Sandeen
2020-03-01T21:13:10Z

On 3/1/20 12:55 PM, Dave Chinner wrote:
> On Sun, Mar 01, 2020 at 09:50:03AM -0800, Eric Sandeen wrote:
>> The changes to xfs_admin which allowed online label setting via
>> ioctl had some unintended consequences in terms of changing command
>> order and processing. It's going to be somewhat tricky to fix, so
>> back it out for now.
>
> What are the symptoms and behaviour of these "unintended
> consequences"? And why are they tricky to fix?

Yeah, I should have probably said more in the commit log.

https://bugzilla.kernel.org/show_bug.cgi?id=206429

was the first clue,

"xfs_admin can't print both label and UUID for mounted filesystems"

The main problem is that if /any/ options that trigger xfs_io get specified,
they are the only ones that get run:

# Try making the changes online, if supported
if [ -n "$IO_OPTS" ] && mntpt="$(find_mntpt_for_arg "$1")"
then
eval xfs_io -x -p xfs_admin $IO_OPTS "$mntpt"
test "$?" -eq 0 && exit 0
fi

and the non-io / db opts don't get run at all.

So sure, we could then move on to the db commands, but we actually built them
all up along the way as well:

l) DB_OPTS=$DB_OPTS" -r -c label"
IO_OPTS=$IO_OPTS" -r -c label"
;;

so we'd need to keep those separate, and not re-run them in db.

And another thing that I struggled with was preserving the order; you'd
kind of expect that if you specify commands in a certain order
they'd be executed in that order, and that used to be true. Now it's not,
even if we don't exit in the "if IO_OPTS" case above.

So I experimented with building up an array of commands, invoking xfs_db
or xfs_io one command at a time as needed for each, and ... it just got worse
and worse, TBH.

It's broken now, and so far a clean solution isn't evident, and I hate to
leave it broken across another release.

-Eric