On 03/30/2018 06:46 AM, Misono Tomohiro wrote:
> On 2018/03/30 2:35, Goffredo Baroncelli wrote:
>> Hi Misono,
>
> Hello,
[...]
>> My conclusion, is that your work is not consistent with the current implementation; so I am suggesting to create a new subcommand ("btrfs sub tree" ?) where you can use, without constraint, your api.
>
> I agree that the current output of "sub list" is confusing first of all and
> it is not easy to keep consistent behavior between user and root.
>
> So, I think "sub list" (or new command) should be:
> - (default) list the subvolumes under the specified path, including subvolumes mounted below the specified path.
> Any user can do this (with appropriate permission checks).
> The path to be printed is the relative from the specified path.
> - (-a option) list all the subvolmumes in the filesystem. Only root can do this.
> The path to be printed is the absolute path from the toplevel subvolume.
>
> This would need some changes in progs code, but I think we can use the same new ioctls I proposed[1] for the default behavior.
>
> I'd like to update "sub list" command eventually rather than adding new command, even though this breaks the compatibility.
> I want to know what other developer/user think.
>
> [1] https://www.spinics.net/lists/linux-btrfs/msg76003.html
>
>>
>> Another small differences that I found is in the subcommand "btrfs sub show". This is not capable to show the snapshots of a subvolume; I think that, in "user mode", it should be reported that there is a "missing capabilities" problem than to show an empty list
>
> Yes, this is because that only the snapshots *under the specified path* are searched.
> This can be easily changed to search under the mount point, but this also results in
> slight change in print format of the path for root. I tried to keep as much consistency in this version.
When I perform a snapshot, I think the new snapshot more as a sister/brother than a child, so I put it at the same level instead of below the source subvolume. Moreover the path printed at the first line seems to be relative at the root of filesystem.
> Thanks,
> Tomohiro Misono
>
>>
>> Below an example of what I am say:
>>
>> ghigo@venice:/tmp$ btrfs sub cre a
>> ghigo@venice:/tmp$ btrfs sub snap a c
>>
>> ghigo@venice:/tmp$ # patched btrfs
>> ghigo@venice:/tmp$ /home/ghigo/btrfs/btrfs-progs/btrfs sub show a
>> tmp/a
>> Name: a
>> UUID: 0c19a7a4-a7f5-1849-9262-88ab3da504d0
>> Parent UUID: -
>> Received UUID: -
>> Creation time: 2018-03-28 22:48:09 +0200
>> Subvolume ID: 598
>> Generation: 696908
>> Gen at creation: 696907
>> Parent ID: 257
>> Top level ID: 257
>> Flags: -
>> Snapshot(s):
>> ^^^^^^^^^^^^^^^^^^^^
>>
>> ghigo@venice:/tmp$ # stock btrfs
>> ghigo@venice:/tmp$ sudo /home/ghigo/btrfs/btrfs-progs/btrfs sub show a
>> ^^^^^^
>> tmp/a
>> Name: a
>> UUID: 0c19a7a4-a7f5-1849-9262-88ab3da504d0
>> Parent UUID: -
>> Received UUID: -
>> Creation time: 2018-03-28 22:48:09 +0200
>> Subvolume ID: 598
>> Generation: 696908
>> Gen at creation: 696907
>> Parent ID: 257
>> Top level ID: 257
>> Flags: -
>> Snapshot(s):
>> debian/tmp/c
>> ^^^^^^^^^^^^^
>>
>>
>> BR
>> G.Baroncelli
>>
>>
>> -----
>> Test performed:
>>
>> ghigo@venice:/tmp$ sudo btrfs sub crea a
>> ghigo@venice:/tmp$ sudo btrfs sub crea a/a1
>> ghigo@venice:/tmp$ sudo btrfs sub crea a/a1/a2
>> ghigo@venice:/tmp$ sudo btrfs sub crea b
>> ghigo@venice:/tmp$ sudo btrfs sub crea b/b1
>> ghigo@venice:/tmp$ sudo chmod og-rwx b/.
>>
>>
>> # stock btrfs progs
>> ghigo@venice:/tmp$ sudo btrfs sub l .
>> ID 257 gen 696886 top level 5 path debian
>> ID 289 gen 587461 top level 257 path var/lib/machines
>> ID 299 gen 693561 top level 5 path boot
>> ID 582 gen 683965 top level 5 path i386
>> ID 592 gen 696884 top level 257 path tmp/a
>> ID 593 gen 696885 top level 592 path tmp/a/a1
>> ID 594 gen 696885 top level 593 path tmp/a/a1/a2
>> ID 595 gen 696887 top level 257 path tmp/b
>> ID 596 gen 696887 top level 595 path tmp/b/b1
>>
>> # patched btrfs progs
>> ghigo@venice:/tmp$ /home/ghigo/btrfs/btrfs-progs/btrfs sub lis .
>> ID 592 gen 696884 top level 257 path a
>> ID 593 gen 696885 top level 592 path a/a1
>> ID 594 gen 696885 top level 593 path a/a1/a2
>> ID 595 gen 0 top level 257 path b
>>
>>
>> ghigo@venice:/tmp$ /home/ghigo/btrfs/btrfs-progs/btrfs sub show a
>> tmp/a
>> Name: a
>> UUID: 17520c11-ec8b-5b49-a07e-37ba58113261
>> Parent UUID: -
>> Received UUID: -
>> Creation time: 2018-03-28 22:19:48 +0200
>> Subvolume ID: 592
>> Generation: 696884
>> Gen at creation: 696883
>> Parent ID: 257
>> Top level ID: 257
>> Flags: -
>> Snapshot(s):
>>
>>
>> On 03/19/2018 08:30 AM, Misono, Tomohiro wrote:
>>> changelog:
>>>
>>> v2 -> v3
>>> - use get_euid() to check the caller's privilege (and remove 3rd patch)
>>> - improve error handling
>>> v1 -> v2
>>> - add independent error handling patch (1st patch)
>>> - reimplement according to ioctl change
>>> - various cleanup
>>> ===
>>>
>>> This RFC implements user version of "subvolume list/show" using three new ioctls.
>>> The ioctl patch to the kernel can be found in the ML titled
>>> "[PATCH v3 0/3] btrfs: Add three new unprivileged ioctls to allow normal users to call "sub list/show" etc.
>>>
>>> 1th patch is independent and improvements of error handling
>>> 2nd-4th are some prepartion works.
>>> 5th patch is the main part.
>>> 6th-7th adds the test for "subvolume list"
>>>
>>> The main behavior differences between root and normal users are:
>>>
>>> - "sub list" list the subvolumes which exist under the specified path
>>> (including the path itself). The specified path itself is not needed to be
>>> a subvolume. Also If the subvolume cannot be opend but the parent
>>> directory can be, the information other than name or id would be zeroed out.
>>>
>>> - snapshot filed of "subvolume show" just lists
>>> the snapshots under the specified subvolume.
>>>
>>>
>>> This is a part of RFC I sent last December[1] whose aim is to improve normal users' usability.
>>> The remaining works of RFC are:
>>> - Allow "sub delete" for empty subvolume
>>> - Allow "qgroup show" to check quota limit
>>>
>>> [1] https://www.mail-archive.com/linux-btrfs@xxxxxxxxxxxxxxx/msg70991.html
>>>
>>>
>>> Tomohiro Misono (7):
>>> btrfs-progs: sub list: Call rb_free_nodes() in error path
>>> btrfs-progs: ioctl: Add 3 definitions of new unprivileged ioctl
>>> btrfs-progs: sub list: Pass specified path down to
>>> btrfs_list_subvols()
>>> btrfs-progs: fallback to open without O_NOATIME flag in
>>> find_mount_root()
>>> btrfs-progs: sub list: Allow normal user to call "subvolume list/show"
>>> btrfs-progs: test: Add helper function to check if test user exists
>>> btrfs-porgs: test: Add cli-test/009 to check subvolume list for both
>>> root and normal user
>>>
>>> btrfs-list.c | 376 +++++++++++++++++++++++++++--
>>> btrfs-list.h | 7 +-
>>> cmds-subvolume.c | 14 +-
>>> ioctl.h | 86 +++++++
>>> tests/cli-tests/009-subvolume-list/test.sh | 136 +++++++++++
>>> tests/common | 10 +
>>> utils.c | 13 +-
>>> 7 files changed, 609 insertions(+), 33 deletions(-)
>>> create mode 100755 tests/cli-tests/009-subvolume-list/test.sh
>>>
>>
>>
>
>
--
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5
--
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
