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@xxxxxxxxxxxxx
