Thread (25 messages) 25 messages, 7 authors, 2011-08-11

Re: subvolumes missing from "btrfs subvolume list" output

From: Goffredo Baroncelli <hidden>
Date: 2011-06-29 20:41:18

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:
quoted
On 06/29/2011 11:00 AM, Stephane Chazelas wrote:
quoted
2011-06-29 15:37:47 +0100, Stephane Chazelas:
[...]
quoted
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)
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help