public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Eric Sandeen <sandeen@redhat.com>
Cc: linux-xfs <linux-xfs@vger.kernel.org>,
	"Darrick J. Wong" <darrick.wong@oracle.com>
Subject: Re: [PATCH V2] xfs_admin: revert online label setting ability
Date: Tue, 3 Mar 2020 12:18:09 +1100	[thread overview]
Message-ID: <20200303011809.GQ10776@dread.disaster.area> (raw)
In-Reply-To: <b4d2d6cf-dbae-2a2e-7580-3b6fd13aad9a@redhat.com>

On Mon, Mar 02, 2020 at 08:35:02AM -0600, Eric Sandeen wrote:
> "xfs_admin can't print both label and UUID for mounted filesystems"
> https://bugzilla.kernel.org/show_bug.cgi?id=206429
> 
> alerted us to the problem that if /any/ options that use xfs_io get
> specified to xfs_admin, 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 thanks to the exit, the xfs_db operations don't get run at all.
> 
> We could move on to the xfs_db commands after executing the xfs_io
> commands, but we build them all up in parallel at this time:
> 
>         l)      DB_OPTS=$DB_OPTS" -r -c label"
>                 IO_OPTS=$IO_OPTS" -r -c label"
>                 ;;
> 
> so we'd need to keep track of these, and not re-run them in xfs_db.
> 
> Another issue is that prior to this commit, we'd run commands in
> command line order.
> 
> 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 got overly
> complicated.
> 
> It's broken now, and so far a clean solution isn't evident, and I hate to
> leave it broken across another release.  So revert it for now.
> 
> Reverts: 3f153e051a ("xfs_admin: enable online label getting and setting")
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---

Looks fine, I understand why now :)

Reviewed-by: Dave Chinner <dchinner@redhat.com>
-- 
Dave Chinner
david@fromorbit.com

      reply	other threads:[~2020-03-03  1:18 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-01 17:50 [PATCH] xfs_admin: revert online label setting ability Eric Sandeen
2020-03-01 20:55 ` Dave Chinner
2020-03-01 21:13   ` Eric Sandeen
2020-03-02  3:10     ` Dave Chinner
2020-03-02 14:35 ` [PATCH V2] " Eric Sandeen
2020-03-03  1:18   ` Dave Chinner [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200303011809.GQ10776@dread.disaster.area \
    --to=david@fromorbit.com \
    --cc=darrick.wong@oracle.com \
    --cc=linux-xfs@vger.kernel.org \
    --cc=sandeen@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox