Hi Hugo On 06/29/2011 06:47 PM, Hugo Mills wrote: > On Wed, Jun 29, 2011 at 12:16:06PM -0400, Josef Bacik wrote: >> On 06/29/2011 11:00 AM, Stephane Chazelas wrote: >>> 2011-06-29 15:37:47 +0100, Stephane Chazelas: >>> [...] >>>> I found >>>> http://thread.gmane.org/gmane.comp.file-systems.btrfs/8123/focus=8208 >>>> >>>> which looks like the same issue, with Li Zefan saying he had a >>>> fix, but I couldn't find any mention that it was actually fixed. >>>> >>>> Has anybody got any update on that? >>> [...] >>> >>> I've found >>> http://thread.gmane.org/gmane.comp.file-systems.btrfs/8232 >>> >>> but no corresponding fix or ioctl.c >>> http://git.kernel.org/?p=linux/kernel/git/mason/btrfs-unstable.git;a=history;f=fs/btrfs/ioctl.c >>> >>> I'm under the impression that the issue has been forgotten >>> about. >>> >>> From what I managed to gather though, it seems that what's on >>> disk is correct, it's just the ioctl and/or "btrfs sub list" >>> that's wrong. Am I right? >> >> Yeah, did you apply the patch from that thread and verify that it fixes >> your problem? Thanks, > > Note that changing this API will probably break btrfs-gui's listing > of subvolumes... > > The issue with that patch is that there are two distinct behaviours > that people want or expect with the tree-search ioctl: > > (A) Return all items with keys which collate linearly between > (min_objectid, min_type, min_offset) and > (max_objectid, max_type, max_offset) > > i.e. treating keys as indivisible objects and sorting lexically, > as the trees do. > > (B) Return all items with keys (i, t, o) which fulfil the criteria > (min_objectid <= i <= max_objectid, > min_type <= t <= max_type, > min_offset <= o <= max_offset) > > i.e. treating keys as 3-tuples, and selecting from a rectilinear > subsset of the tuple space, which is natural for some > applications. > > Clearly, we can't do both with the same call (except for some > limited cases (*)). However, different users expect different > behaviours. The current behaviour is (A), which is the "natural" > behaviour for tree searches within the btrfs code, and is (IMO) the > right thing to be doing for an API like this. looking at the function copy_to_sk() it seems that the key advance is made on the following logic: if (key->offset < (u64)-1 && key->offset < sk->max_offset) key->offset++; else if (key->type < (u8)-1 && key->type < sk->max_type) { key->offset = 0; key->type++; } else if (key->objectid < (u64)-1 && key->objectid < sk->max_objectid) { key->offset = 0; key->type = 0; key->objectid++; which to me it seems a bit different from the case A. In fact (if I read the code correctly) *both* the following condition are always true (min_objectid, min_type, min_offset) <= key and key < (max_objectid, max_type, max_offset) and (key_objectid <= max_objectid and key_type <= max_type and key_offset <= max_offset) In conclusion the code is an hybrid between A and B. > > It sounds to me like the user of the API needs to be fixed, not the > ioctl itself -- possibly the author of the subvol scanning code > assumed (B) when they were getting (A). Note that there is at least > one other user of the ioctl outside btrfs-progs: btrfs-gui, which uses > the ioctl for several things, one of which is enumerating subvolumes > as btrfs-progs does. > > It should be possible to write an additional ioctl for behaviour > (B) which contains both min and max limits on each element of the key > 3-tuple, *and* the current search state. That would reduce developer > confusion (given appropriate comments or documentation to explain what > the difference between the two is). However, I'm not sufficiently > convinced that it's actually necessary right now. I may change my tune > after I've started doing some of the more complex bits I'd thought of > doing with btrfs-gui, but for now, it's perfectly possible to use the > existing API without too much hassle. > > Hugo. > > (*) The limited cases where both behaviours return the same set of > keys are: > > (i_0, 0, 0) to (i_1, -1UL, -1UL) > (i, t_0, 0) to (i, t_1, -1UL) > (i, t, o_0) to (i, t, o_1) > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
