All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Eric Sandeen <sandeen@sandeen.net>
Cc: linux-xfs@vger.kernel.org, fstests@vger.kernel.org, amir73il@gmail.com
Subject: Re: [PATCH 5/6] xfs_io: make various commands one-shot only
Date: Fri, 16 Dec 2016 11:53:26 +1100	[thread overview]
Message-ID: <20161216005326.GK4326@dastard> (raw)
In-Reply-To: <fec1a6dd-cebd-1ff4-e916-f696139b876e@sandeen.net>

On Thu, Dec 15, 2016 at 12:21:43PM -0600, Eric Sandeen wrote:
> On 12/6/16 9:47 PM, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > It makes no sense to iterate the file table for some xfs_io
> > commands. Some commands are already marked in this way, but lots of
> > them are not and this leads to bad behaviour. For example, the open
> > command will run until the process fd table is full and EMFILE is
> > returned rather than just opening the specified file once.
> 
> Ok, I'm not quite clear on when we should expect commands to be
> "oneshot"
> 
> With freeze, for example, this:
> 
> xfs_io -x -c freeze mnt/file mnt2/file
> 
> will freeze both filesystems today.  With this change,

xfs_freeze will ignore multiple filesystem options, so it only ever
passes a single file to xfs_io.  IOWs, it's behaviour will be
completely unchanged by making the xfs_io freeze command a one-shot
command.

And right now, I think that's the only case we have to care about.
unless someone is actually using xfs_io directly to freeze multiple
filesystems like this, then I think it's better to make it a
one-shot command.

> xfs_io -x -c freeze mnt/file mnt2/file
> 
> freezes the mnt2 filesystem but not mnt.  Is that desired?

It's exactly what the man page says the freeze command does.

> I guess the command /is/ documented as "freeze fs of /current/ file"
> but i wonder if that's just an accident of documentation.

If the man page documents it as operating on the current file,
but instead it freezes all the open files, then that's a bug that
needs fixing.

> ditto for i.e. the inode command, or resblks - why not
> iterate those?

Because they are aimed at single, specific filesystem operations
only. It just doesn't make sense to iterate them across all open
files inside xfs_io. If you have multiple filesystems youneed to
query/modify, then do an xfs_io call for each. 

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

  reply	other threads:[~2016-12-16  0:59 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-07  3:47 [PATCH 0/6] xfs_io: fix up command iteration Dave Chinner
2016-12-07  3:47 ` [PATCH 1/6] libxcmd: check CMD_FLAG_GLOBAL inside args_command() Dave Chinner
2016-12-07  3:47 ` [PATCH 2/6] libxcmd: rename args_command to command_iterator Dave Chinner
2016-12-07  3:47 ` [PATCH 3/6] libxcmd: merge command() and iterate_command() Dave Chinner
2016-12-07  3:47 ` [PATCH 4/6] libxcmd: don't check generic library commands Dave Chinner
2016-12-07  3:47 ` [PATCH 5/6] xfs_io: make various commands one-shot only Dave Chinner
2016-12-15 18:21   ` Eric Sandeen
2016-12-16  0:53     ` Dave Chinner [this message]
2016-12-16  1:50       ` Eric Sandeen
2016-12-16  4:21         ` Dave Chinner
2016-12-07  3:47 ` [PATCH 6/6] libxcmd: add non-iterating user commands Dave Chinner
2016-12-07  4:49   ` Amir Goldstein
2016-12-07  4:57     ` Amir Goldstein
2016-12-07 14:21     ` Amir Goldstein
2016-12-07 20:16       ` Dave Chinner
2016-12-08 10:14         ` Amir Goldstein
2016-12-08 22:22           ` Dave Chinner
2016-12-15 19:09     ` Eric Sandeen
2017-01-12  5:14 ` [PATCH 0/6] xfs_io: fix up command iteration Amir Goldstein
2017-01-12 12:52   ` Eric Sandeen
  -- strict thread matches above, loose matches on Subject: below --
2016-12-16  4:41 [PATCH v2 " Dave Chinner
2016-12-16  4:41 ` [PATCH 5/6] xfs_io: make various commands one-shot only Dave Chinner
2016-12-20  8:44   ` Christoph Hellwig

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=20161216005326.GK4326@dastard \
    --to=david@fromorbit.com \
    --cc=amir73il@gmail.com \
    --cc=fstests@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=sandeen@sandeen.net \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.