Re: [PATCH] xfs_io: Implement inodes64 command - bug in XFS_IOC_FSINUMBERS?

[Brian Foster <bfoster@redhat.com>]

On Wed, Sep 23, 2015 at 12:28:34PM +0200, Carlos Maiolino wrote:
> Howdy folks,
> 
> I was working in implementing the suggested feature in my patch, about getting
> the next inode used after one is provided, and I hit something that I'm not really
> sure if this might be considered a bug, or just a work form.
> 
> XFS_IOC_FSINUMBERS, is supposed to be called with a zeroed
> xfs_fsop_bulkreq.lastip, so at each call, kernel will update this number to the
> last inode returned, and, the next call will return in xfs_inogrp.xi_startino,
> the next existing inode after .lastip.
> 
> So, I was expecting that, passing a non-zero .lastip at the first call, I would
> be able to get the next inode right after the one I passed through .lastip, but,
> after some tests and reading the code, I noticed that this is not the case.
> 
> The problem (not sure if I can really say it's a problem), is that, if the inode
> number passed, happens to be somewhere in the middle of an inode chunk, the
> whole chunk will not be printed, only the next inode chunk will start to be
> printed, hiding all information of the previous one.
> 
> I'm not sure if this is the desired behavior or not, but, I'd say that, if the
> inode passed in .lastip, is not the first in the chunk, the output should start
> for its own chunk, instead of the next one, but, I prefer to see you folks POV
> before starting to fix something that I'm not sure if it's actually broken :-)
> 

On a quick pass through, it seems like this is probably just the nature
of the granularity and typical use case of the inumbers interface. For
one, I think the lastip parameter is intended to be used as a cookie as
opposed to an inode-granularity search key. The return structures also
appear to describe inode records so there isn't any natural way to get a
partial record that I can see. Note that bulkstat does appear to support
an inode granularity starting point (see xfs_bulkstat_grab_ichunk()) as
the output format is inode granularity.

FWIW, I would probably try to handle something like the above in
userspace by working with the interface rather than modifying it. In
other words, subtract (at least) XFS_INODES_PER_CHUNK from the inode
number in question and use that as a starting point to ensure the output
contains the record for the inode if one exists.

It seems like you could change the inumbers behavior to return the
record that contains lastip + 1 if you really wanted to, we'd just have
to take care not to break the lastip behavior itself (e.g., we can't
just search for any record that contains lastip). What looks
interesting, and might justify doing something in this area, is the
behavior presumably caused by doing a greater than or equal to
(XFS_LOOKUP_GE) lookup in xfs_inumbers(). If lastino happens to be a
record startino, we'll actually return the record that contains that
inode (thus technically supporting a mid-record start). If the index in
the record is beyond that, we'll skip it and start at the next record.
Unless I misinterpret that, it seems like that should be fixed one way
or another.

Brian

> Cheers
> 
> -- 
> Carlos
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs