RE: [PATCH RESEND 0/8] btrfs-progs: sub: Relax the privileges of "subvolume list/show"
Hi, > -Original Message- > From: Martin Steigerwald [mailto:mar...@lichtvoll.de] > Sent: Tuesday, November 27, 2018 6:48 PM > To: Misono, Tomohiro > Cc: linux-btrfs@vger.kernel.org > Subject: Re: [PATCH RESEND 0/8] btrfs-progs: sub: Relax the privileges > of "subvolume list/show" > > Misono Tomohiro - 27.11.18, 06:24: > > Importantly, in order to make output consistent for both root and > > non-privileged user, this changes the behavior of "subvolume list": > > - (default) Only list in subvolume under the specified path. > >Path needs to be a subvolume. > > Does that work recursively? Not in this version. Previous version has -f option which recursively search and list subbvolumes (only if they have the same btrfs fsid): https://lore.kernel.org/linux-btrfs/84d06767762b4285ddefec0392ee16e2d7e06f62.1529310485.git.misono.tomoh...@jp.fujitsu.com/ However, current "sub list" command already has many options and we cannot add new one randomly, I drop the patches which add new options in this version. (In other word, previous version can be divided in two parts: 1. Relax the privileges of sub list/show 2. Add new option to sub list And this version contains only 1). > > I wound find it quite unexpected if I did btrfs subvol list in or on the > root directory of a BTRFS filesystem would not display any subvolumes > on > that filesystem no matter where they are. Yes, I think output of -f option is more readable in such case. If agreement has been made about how "sub list" should really work, I could update/send patches again. Thanks, Misono
RE: [PATCH 00/10] btrfs-progs: my libbtrfsutil patch queue
> -Original Message- > From: David Sterba [mailto:dste...@suse.cz] > Sent: Tuesday, November 27, 2018 1:18 AM > To: Omar Sandoval > Cc: linux-btrfs@vger.kernel.org; kernel-t...@fb.com; Misono, Tomohiro > > Subject: Re: [PATCH 00/10] btrfs-progs: my libbtrfsutil patch queue > > On Tue, Nov 13, 2018 at 11:46:55PM -0800, Omar Sandoval wrote: > > From: Omar Sandoval > > > > Hi, > > > > This series contains my backlog of libbtrfsutil changes which I've been > > collecting over the past few weeks. > > > > Patches 1-4 are fixes. Patches 5-6 add functionality to the unit tests > > which is needed for patches 7-8. Patches 7-8 add support for the > > unprivileged ioctls added in Linux 4.18; more on those below. Patch > 9 > > bumps the library version. Patch 10 adds documentation for the available > > API along with examples. > > > > Patches 7-8 are based on Misono Tomohiro's previous patch series [1], > > with a few important changes. > > > > - Both subvolume_info() and create_subvolume_iterator() now have unit > > tests for the unprivileged case. > > - Both no longer explicitly check that top == 0 in the unprivileged > > case, since that will already fail with a clear permission error. > > - Unprivileged iteration is much simpler: it uses openat() instead of > > fchdir() and is based more closely on the original tree search > > variant. This fixes a bug in post-order iteration in Misono's version. > > - Unprivileged iteration does _not_ support passing in a non-subvolume > > path; if this behavior is desired, I'd like it to be a separate change > > with an explicit flag. > > Series merged to devel, thanks. I've added link from the main README now > that there's the API documentation. > > The test-libbtrfsutil is missing from the travis CI for some reason, I > was about to add it. So far the testing environment does not provide > 'umount' that knows about '-R' so the tests fail. I'll have a look if > there's a newer base image provided, otherwise a workaround would be > necessary. > > As for the unprivileged subvolume listing ioctls, the functionality in > the util library is self-contained and the interface is up to you to > design properly, so this does not depend on the 'btrfs subvolume list' > command. That one has unfortunately not bubbled high enough in my todo. Hello, I missed the mails and am sorry for late response. As mentioned libbtrfsuitl and other progs are mostly independent, the patches I submitted (once in devel with your review and some modification) can be cleanly applied to this version of libbtrfsutil. I will resend them for easier review/comment. Thanks, Misono
Re: [PATCH 1/3] btrfs-progs: check: lowmem: Factor out inline extent checking code in its own function
Hello,
fsck-test 006 fails for low-mem mode in current devel branch and bisect points
this.
On 2018/09/13 21:05, Nikolay Borisov wrote:
> Since the inline extent code can be largely self-sufficient, factor
> it out from check_file_extent. No functional changes.
>
> Signed-off-by: Nikolay Borisov
> ---
> check/mode-lowmem.c | 142 ++--
> 1 file changed, 83 insertions(+), 59 deletions(-)
>
> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
> index 1bce44f5658a..48c1537e7440 100644
> --- a/check/mode-lowmem.c
> +++ b/check/mode-lowmem.c
> @@ -1800,6 +1800,87 @@ static int repair_inline_ram_bytes(struct btrfs_root
> *root,
> return ret;
> }
>
> +
> +static int check_file_extent_inline(struct btrfs_root *root,
> + struct btrfs_path *path, u64 *size,
> + u64 *end)
> +{
> + u32 max_inline_extent_size = min_t(u32, root->fs_info->sectorsize - 1,
> + BTRFS_MAX_INLINE_DATA_SIZE(root->fs_info));
> + struct extent_buffer *node = path->nodes[0];
> + struct btrfs_item *e = btrfs_item_nr(0);
btrfs_item_nr(path->slots[0])
I think this fixes the problem.
Thanks.
Misono
> + struct btrfs_file_extent_item *fi;
> + struct btrfs_key fkey;
> + u64 extent_num_bytes;
> + u32 item_inline_len;
> + int ret;
> + int compressed = 0;
> + int err = 0;
> +
> + fi = btrfs_item_ptr(node, path->slots[0],
> + struct btrfs_file_extent_item);
> + item_inline_len = btrfs_file_extent_inline_item_len(node, e);
> + extent_num_bytes = btrfs_file_extent_ram_bytes(node, fi);
> + compressed = btrfs_file_extent_compression(node, fi);
> + btrfs_item_key_to_cpu(node, , path->slots[0]);
> +
> + if (extent_num_bytes == 0) {
> + error(
> +"root %llu EXTENT_DATA[%llu %llu] has empty inline extent",
> + root->objectid, fkey.objectid, fkey.offset);
> + err |= FILE_EXTENT_ERROR;
> + }
> +
> + if (compressed) {
> + if (extent_num_bytes > root->fs_info->sectorsize) {
> + error(
> +"root %llu EXTENT_DATA[%llu %llu] too large inline extent ram size, have
> %llu, max: %u",
> + root->objectid, fkey.objectid,
> + fkey.offset, extent_num_bytes,
> + root->fs_info->sectorsize - 1);
> + err |= FILE_EXTENT_ERROR;
> + }
> +
> + if (item_inline_len > max_inline_extent_size) {
> + error(
> +"root %llu EXTENT_DATA[%llu %llu] too large inline extent on-disk size, have
> %u, max: %u",
> + root->objectid, fkey.objectid,
> + fkey.offset, item_inline_len,
> + max_inline_extent_size);
> + err |= FILE_EXTENT_ERROR;
> + }
> +
> + } else {
> +
> + if (extent_num_bytes > max_inline_extent_size) {
> + error(
> +"root %llu EXTENT_DATA[%llu %llu] too large inline extent size, have %llu,
> max: %u",
> + root->objectid, fkey.objectid, fkey.offset,
> + extent_num_bytes, max_inline_extent_size);
> + err |= FILE_EXTENT_ERROR;
> + }
> +
> + }
> + if (!compressed && extent_num_bytes != item_inline_len) {
> + error(
> +"root %llu EXTENT_DATA[%llu %llu] wrong inline size, have: %llu, expected:
> %u",
> + root->objectid, fkey.objectid, fkey.offset,
> + extent_num_bytes, item_inline_len);
> + if (repair) {
> + ret = repair_inline_ram_bytes(root, path,
> + _num_bytes);
> + if (ret)
> + err |= FILE_EXTENT_ERROR;
> + } else {
> + err |= FILE_EXTENT_ERROR;
> + }
> + }
> + *end += extent_num_bytes;
> + *size += extent_num_bytes;
> +
> + return err;
> +}
> +
> /*
> * Check file extent datasum/hole, update the size of the file extents,
> * check and update the last offset of the file extent.
> @@ -1824,8 +1905,6 @@ static int check_file_extent(struct btrfs_root *root,
> struct btrfs_path *path,
> u64 csum_found; /* In byte size, sectorsize aligned */
> u64 search_start; /* Logical range start we search for csum */
> u64 search_len; /* Logical range len we search for csum */
> - u32 max_inline_extent_size = min_t(u32, root->fs_info->sectorsize - 1,
> - BTRFS_MAX_INLINE_DATA_SIZE(root->fs_info));
> unsigned int extent_type;
> unsigned int is_hole;
> int slot = path->slots[0];
> @@
Re: [PATCH 1/3] btrfs-progs: check: lowmem: Factor out inline extent checking code in its own function
Hello,
fsck-test 006 fails for low-mem mode in current devel branch and bisect points
this.
> Since the inline extent code can be largely self-sufficient, factor
> it out from check_file_extent. No functional changes.
>
> Signed-off-by: Nikolay Borisov
> ---
> check/mode-lowmem.c | 142 ++--
> 1 file changed, 83 insertions(+), 59 deletions(-)
>
> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
> index 1bce44f5658a..48c1537e7440 100644
> --- a/check/mode-lowmem.c
> +++ b/check/mode-lowmem.c
> @@ -1800,6 +1800,87 @@ static int repair_inline_ram_bytes(struct btrfs_root
> *root,
> return ret;
> }
>
> +
> +static int check_file_extent_inline(struct btrfs_root *root,
> + struct btrfs_path *path, u64 *size,
> + u64 *end)
> +{
> + u32 max_inline_extent_size = min_t(u32, root->fs_info->sectorsize - 1,
> + BTRFS_MAX_INLINE_DATA_SIZE(root->fs_info));
> + struct extent_buffer *node = path->nodes[0];
> + struct btrfs_item *e = btrfs_item_nr(0);
btrfs_item_nr(path->slots[0])
I think this fixes the problem.
Thanks.
Misono
> + struct btrfs_file_extent_item *fi;
> + struct btrfs_key fkey;
> + u64 extent_num_bytes;
> + u32 item_inline_len;
> + int ret;
> + int compressed = 0;
> + int err = 0;
> +
> + fi = btrfs_item_ptr(node, path->slots[0],
> + struct btrfs_file_extent_item);
> + item_inline_len = btrfs_file_extent_inline_item_len(node, e);
> + extent_num_bytes = btrfs_file_extent_ram_bytes(node, fi);
> + compressed = btrfs_file_extent_compression(node, fi);
> + btrfs_item_key_to_cpu(node, , path->slots[0]);
> +
> + if (extent_num_bytes == 0) {
> + error(
> +"root %llu EXTENT_DATA[%llu %llu] has empty inline extent",
> + root->objectid, fkey.objectid, fkey.offset);
> + err |= FILE_EXTENT_ERROR;
> + }
> +
> + if (compressed) {
> + if (extent_num_bytes > root->fs_info->sectorsize) {
> + error(
> +"root %llu EXTENT_DATA[%llu %llu] too large inline extent ram size, have
> %llu, max: %u",
> + root->objectid, fkey.objectid,
> + fkey.offset, extent_num_bytes,
> + root->fs_info->sectorsize - 1);
> + err |= FILE_EXTENT_ERROR;
> + }
> +
> + if (item_inline_len > max_inline_extent_size) {
> + error(
> +"root %llu EXTENT_DATA[%llu %llu] too large inline extent on-disk size, have
> %u, max: %u",
> + root->objectid, fkey.objectid,
> + fkey.offset, item_inline_len,
> + max_inline_extent_size);
> + err |= FILE_EXTENT_ERROR;
> + }
> +
> + } else {
> +
> + if (extent_num_bytes > max_inline_extent_size) {
> + error(
> +"root %llu EXTENT_DATA[%llu %llu] too large inline extent size, have %llu,
> max: %u",
> + root->objectid, fkey.objectid, fkey.offset,
> + extent_num_bytes, max_inline_extent_size);
> + err |= FILE_EXTENT_ERROR;
> + }
> +
> + }
> + if (!compressed && extent_num_bytes != item_inline_len) {
> + error(
> +"root %llu EXTENT_DATA[%llu %llu] wrong inline size, have: %llu, expected:
> %u",
> + root->objectid, fkey.objectid, fkey.offset,
> + extent_num_bytes, item_inline_len);
> + if (repair) {
> + ret = repair_inline_ram_bytes(root, path,
> + _num_bytes);
> + if (ret)
> + err |= FILE_EXTENT_ERROR;
> + } else {
> + err |= FILE_EXTENT_ERROR;
> + }
> + }
> + *end += extent_num_bytes;
> + *size += extent_num_bytes;
> +
> + return err;
> +}
> +
> /*
> * Check file extent datasum/hole, update the size of the file extents,
> * check and update the last offset of the file extent.
> @@ -1824,8 +1905,6 @@ static int check_file_extent(struct btrfs_root *root,
> struct btrfs_path *path,
> u64 csum_found; /* In byte size, sectorsize aligned */
> u64 search_start; /* Logical range start we search for csum */
> u64 search_len; /* Logical range len we search for csum */
> - u32 max_inline_extent_size = min_t(u32, root->fs_info->sectorsize - 1,
> - BTRFS_MAX_INLINE_DATA_SIZE(root->fs_info));
> unsigned int extent_type;
> unsigned int is_hole;
> int slot = path->slots[0];
> @@ -1838,63 +1917,8 @@ static int
