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

Dave Chinner
2020-03-02T03:10:28Z

On Sun, Mar 01, 2020 at 03:13:07PM -0600, Eric Sandeen wrote:
> 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.

And there's your new commit message. :)

Cheers,

Dave.