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

[Dave Chinner <david@fromorbit.com>]

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.
-- 
Dave Chinner
david@fromorbit.com