On Wed, Dec 7, 2016 at 10:16 PM, Dave Chinner <david@xxxxxxxxxxxxx> wrote: > On Wed, Dec 07, 2016 at 04:21:31PM +0200, Amir Goldstein wrote: ... >> >> 1. Do you think that xfs_io should error on -c "open foo" to force >> explicit -C "open foo"? > > No. > >> it can't be breaking any existing users, because -c "open foo" is >> already very broken. > > Maybe so, but there are users of it. e.g: > > $ git grep open |grep XFS_IO > tests/overlay/001: $XFS_IO_PROG -c "open" $f >>$seqres.full > $ > > This is precisely why I made oneshot commands just work silently > with "-c" - who knows what will break if we start rejecting commands > that otherwise work just fine when there is only one open file.... > Interesting example. Here -c "open" is actually an 'alias' for -c "stat", which could have been non one shot. Nevertheless, I see your point. >> 2. You marked mmap ONE_SHOT, but not all the m* commands. >> Stands to reason that they should all be marked ONE_SHOT. because iterating >> the file table has nothing to do with the m* commands. no? > > It is not clear to me what the correct thing to do here is, I don't > have the time right now to look into it, so I didn't > change mread/mwrite/msync behaviour. If it's broken before it is > still broken now. > Very well, but I am not sure why mmap should be marked one shot? Possibly, mmap caught your filters because it is CMD_NOFILE_OK, but in fact, the only case where mmap with no open files works is after mmaping files and closing all open files. Currently, this command: $ xfs_io -c "mmap 0 4" -C "mmap" foo bar Results in: [000] 0x7f17319ae000 - 0x7f17319ae004 rwx bar (0 : 4) IMO, it would be more useful if it resulted in: 000 0x7f27e289d000 - 0x7f27e289d004 rwx foo (0 : 4) [001] 0x7f27e289c000 - 0x7f27e289c004 rwx bar (0 : 4) Which will allow following up with more specific -C m* commands and it would be more consistent with the result of: $ xfs_io -C "file" foo bar 000 foo (foreign,non-sync,non-direct,read-write) [001] bar (foreign,non-sync,non-direct,read-write) >> About the fix to overlay/016. >> How would you prefer to address the conditional availability of xfs_io -C? >> >> 1. new helper _require_xfs_io_one_shot_command >> 2. _require_xfs_io_command "open" (which internally checks for xfs_io -C "open") >> 3. third option? > > I don't really care - #2 is probably neatest. If what you do is too > ugly to live then I'll let you know. > I trust that you will :-) Will post it soon. Thanks, Amir. -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
