[PATCH] btrfs: fix invalid-free in btrfs_extent_same
If this condition ((BTRFS_I(src)->flags & BTRFS_INODE_NODATASUM) !=
(BTRFS_I(dst)->flags & BTRFS_INODE_NODATASUM))
is hit, we will go to free the uninitialized cmp.src_pages and
cmp.dst_pages.
Fixes: 67b07bd4bec5 ("Btrfs: reuse cmp workspace in EXTENT_SAME ioctl")
Signed-off-by: Lu Fengqi
---
fs/btrfs/ioctl.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index c2837a32d689..43ecbe620dea 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -3577,7 +3577,7 @@ static int btrfs_extent_same(struct inode *src, u64 loff,
u64 olen,
ret = btrfs_extent_same_range(src, loff, BTRFS_MAX_DEDUPE_LEN,
dst, dst_loff, &cmp);
if (ret)
- goto out_unlock;
+ goto out_free;
loff += BTRFS_MAX_DEDUPE_LEN;
dst_loff += BTRFS_MAX_DEDUPE_LEN;
@@ -3587,16 +3587,16 @@ static int btrfs_extent_same(struct inode *src, u64
loff, u64 olen,
ret = btrfs_extent_same_range(src, loff, tail_len, dst,
dst_loff, &cmp);
+out_free:
+ kvfree(cmp.src_pages);
+ kvfree(cmp.dst_pages);
+
out_unlock:
if (same_inode)
inode_unlock(src);
else
btrfs_double_inode_unlock(src, dst);
-out_free:
- kvfree(cmp.src_pages);
- kvfree(cmp.dst_pages);
-
return ret;
}
--
2.17.1
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] btrfs-progs: Check factor out root parsing from check_chunks_and_extents
On 19.06.2018 05:18, Su Yue wrote:
>
>
> On 06/18/2018 07:10 PM, Nikolay Borisov wrote:
>> check_chunks_and_extents does quite a number of distinct things. The
>> first of those is going through all root items in the root tree and
>> classify every root depending on whether they have a dropping operation
>> in progress or not. Lets factor out this code and move the variables
>> specific to this in a separate function so clean up
>> check_chunks_and_extents
>> a bit. Accidentally, this patch fixes some reference leaks since
>> in error conditions in the loop the code does "goto out" but at that
>> label we don't really release the path. Having this code extracted in a
>> separate function which always releases the path avoids this problem
>> entirely.
>>
>> Signed-off-by: Nikolay Borisov
>
> Code looks okay. One nitpick belows.
>
>> ---
>> check/main.c | 141
>> +++
>> 1 file changed, 85 insertions(+), 56 deletions(-)
>>
>> diff --git a/check/main.c b/check/main.c
>> index a4d6855dccbf..fb5c86df21c9 100644
>> --- a/check/main.c
>> +++ b/check/main.c
>> @@ -8069,6 +8069,89 @@ static int deal_root_from_list(struct list_head
>> *list,
>> return ret;
>> }
>> +/**
>> + * parse_tree_roots - Go over all roots in the tree root and add each
>> one to
>> + * a list.
>> + *
>> + * @fs_info - pointer to fs_info struct of the file system.
>> + *
>> + * @normal_trees - list to contains all roots which don't have a drop
>> + * operation in progress
>> + *
>> + * @dropping_trees - list containing all roots which have a drop
>> operation
>> + * pending
>> + *
>> + * Returns 0 on success or a negative value indicating an error.
>> + *
>> + */
>> +static int parse_tree_roots(struct btrfs_fs_info *fs_info,
>> + struct list_head *normal_trees,
>> + struct list_head *dropping_trees)
>> +{
>> + struct btrfs_path path;
>> + struct btrfs_key key;
>> + struct btrfs_key found_key;
>> + struct btrfs_root_item ri;
>> + struct extent_buffer *leaf;
>> + int slot;
>> + int ret = 0;
>> +
>> + btrfs_init_path(&path);
>> + key.offset = 0;
>> + key.objectid = 0;
>> + key.type = BTRFS_ROOT_ITEM_KEY;
>> + ret = btrfs_search_slot(NULL, fs_info->tree_root, &key, &path, 0,
>> 0);
>> + if (ret < 0)
>> + goto out;
>> + while (1) {
>> + leaf = path.nodes[0];
>> + slot = path.slots[0];
>> + if (slot >= btrfs_header_nritems(path.nodes[0])) {
>> + ret = btrfs_next_leaf(fs_info->tree_root, &path);
>> + if (ret != 0)
>> + break;
>> + leaf = path.nodes[0];
>> + slot = path.slots[0];
>> + }
>
> Nit:
> I know the segment is just moved. The @slot is unused.
It's used in the if (slot >= btrfs_header_nritems(path.nodes[0])) check.
I guess the definition could be moved inside the while to reduce the
scope but it's a minor thing.
>> + btrfs_item_key_to_cpu(leaf, &found_key, path.slots[0]);
>> + if (found_key.type == BTRFS_ROOT_ITEM_KEY) {
>> + unsigned long offset;
>> + u64 last_snapshot;
>> + u8 level;
>> +
>> + offset = btrfs_item_ptr_offset(leaf, path.slots[0]);
>> + read_extent_buffer(leaf, &ri, offset, sizeof(ri));
>> + last_snapshot = btrfs_root_last_snapshot(&ri);
>> + level = btrfs_root_level(&ri);
>> + if (btrfs_disk_key_objectid(&ri.drop_progress) == 0) {
>> + ret = add_root_item_to_list(normal_trees,
>> + found_key.objectid,
>> + btrfs_root_bytenr(&ri),
>> + last_snapshot, level,
>> + 0, NULL);
>> + if (ret < 0)
>> + break;
>> + } else {
>> + u64 objectid = found_key.objectid;
>> + btrfs_disk_key_to_cpu(&found_key,
>> + &ri.drop_progress);
>> + ret = add_root_item_to_list(dropping_trees,
>> + objectid,
>> + btrfs_root_bytenr(&ri),
>> + last_snapshot, level,
>> + ri.drop_level, &found_key);
>> + if (ret < 0)
>> + break;
>> + }
>> + }
>> + path.slots[0]++;
>> + }
>> +
>> +out:
>> + btrfs_release_path(&path);
>> + return ret;
>> +}
>> +
>> static int check_chunks_and_extents(struct btrfs_fs_info *fs_info)
>> {
>> struct rb_root dev_cache;
>> @@ -8082,20 +8165,13 @@ static int check_chunks_and_extents(struct
>> btrfs_fs_info *fs_info)
>> struct cache_tree nodes;
>> struct extent_io_tree excluded_extents;
>> struct cache_tree corrupt_blocks;
>> - struct btrfs_path path;
>> - struct btrfs_key key;
>> - struct btrfs_key found_key;
>> int ret, err = 0;
>>
Re: [PATCH 2/3] btrfs: Document __btrfs_inc_extent_ref
On 2018年06月18日 19:59, Nikolay Borisov wrote: > Here is a doc-only patch which tires to deobfuscate the terra-incognita > that arguments for delayed refs are. > > Signed-off-by: Nikolay Borisov Reviewed-by: Qu Wenruo > --- > fs/btrfs/extent-tree.c | 34 ++ > 1 file changed, 34 insertions(+) > > diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c > index 59645ced6fbc..39d0652bf3f3 100644 > --- a/fs/btrfs/extent-tree.c > +++ b/fs/btrfs/extent-tree.c > @@ -2207,6 +2207,40 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle > *trans, > return ret; > } > > +/* > + * __btrfs_inc_extent_ref - insert backreference for a given extent > + * > + * @trans: Handle of transaction > + * > + * @node:The delayed ref node used to get the bytenr/length for > + * extent whose references are incremented. > + * > + * @parent: If this is a shared extent (BTRFS_SHARED_DATA_REF_KEY/ > + * BTRFS_SHARED_BLOCK_REF_KEY) then it holds the logical > + * bytenr of the parent block. Since new extents are always > + * created with indirect references, this will only be the case > + * when relocating a shared extent. In that case, root_objectid > + * will be BTRFS_TREE_RELOC_OBJECTID. Otheriwse, parent must > + * be 0 > + * > + * @root_objectid: The id of the root where this modification has > originated, > + * this can be either one of the well-known metadata trees or > + * the subvolume id which references this extent. > + * > + * @owner: For data extents it is the inode number of the owning file. > + * For metadata extents this parameter holds the level in the > + * tree of the extent. @owner the naming itself is a little confusing, but it's the necessary evil. Or we will have too many parameters. Thanks, Qu > + * > + * @offset: For metadata extents the offset is ignored and is currently > + * always passed as 0. For data extents it is the fileoffset > + * this extent belongs to. > + * > + * @refs_to_add Number of references to add > + * > + * @extent_op Pointer to a structure, holding information necessary > when > + * updating a tree block's flags > + * > + */ > static int __btrfs_inc_extent_ref(struct btrfs_trans_handle *trans, > struct btrfs_delayed_ref_node *node, > u64 parent, u64 root_objectid, > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] btrfs: Remove fs_info argument from __btrfs_inc_extent_ref
On 2018年06月18日 19:59, Nikolay Borisov wrote:
> This function already takes a transaction which holds a reference to
> the fs_info struct. Use that reference and remove the extra arg. No
> functional changes.
>
> Signed-off-by: Nikolay Borisov
Reviewed-by: Qu Wenruo
Thanks,
Qu
> ---
> fs/btrfs/extent-tree.c | 15 ++-
> 1 file changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 4850e538ab10..59645ced6fbc 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -2208,12 +2208,12 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle
> *trans,
> }
>
> static int __btrfs_inc_extent_ref(struct btrfs_trans_handle *trans,
> - struct btrfs_fs_info *fs_info,
> struct btrfs_delayed_ref_node *node,
> u64 parent, u64 root_objectid,
> u64 owner, u64 offset, int refs_to_add,
> struct btrfs_delayed_extent_op *extent_op)
> {
> + struct btrfs_fs_info *fs_info = trans->fs_info;
> struct btrfs_path *path;
> struct extent_buffer *leaf;
> struct btrfs_extent_item *item;
> @@ -2297,10 +2297,9 @@ static int run_delayed_data_ref(struct
> btrfs_trans_handle *trans,
>ref->objectid, ref->offset,
>&ins, node->ref_mod);
> } else if (node->action == BTRFS_ADD_DELAYED_REF) {
> - ret = __btrfs_inc_extent_ref(trans, fs_info, node, parent,
> - ref_root, ref->objectid,
> - ref->offset, node->ref_mod,
> - extent_op);
> + ret = __btrfs_inc_extent_ref(trans, node, parent, ref_root,
> + ref->objectid, ref->offset,
> + node->ref_mod, extent_op);
> } else if (node->action == BTRFS_DROP_DELAYED_REF) {
> ret = __btrfs_free_extent(trans, fs_info, node, parent,
> ref_root, ref->objectid,
> @@ -2450,10 +2449,8 @@ static int run_delayed_tree_ref(struct
> btrfs_trans_handle *trans,
> BUG_ON(!extent_op || !extent_op->update_flags);
> ret = alloc_reserved_tree_block(trans, node, extent_op);
> } else if (node->action == BTRFS_ADD_DELAYED_REF) {
> - ret = __btrfs_inc_extent_ref(trans, fs_info, node,
> - parent, ref_root,
> - ref->level, 0, 1,
> - extent_op);
> + ret = __btrfs_inc_extent_ref(trans, node, parent, ref_root,
> + ref->level, 0, 1, extent_op);
> } else if (node->action == BTRFS_DROP_DELAYED_REF) {
> ret = __btrfs_free_extent(trans, fs_info, node,
> parent, ref_root,
>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] btrfs-progs: Fix wrong optind re-initialization to allow mixed option and non-option
In function handle_global_options(), we reset @optind to 1.
However according to man page of getopt(3) NOTES section, if we need to
rescan options later, @optind should be reset to 0 to initialize the
internal variables correctly.
This explains the reason why in cmd_check(), getopt_long() doesn't
handle the following command correctly:
"btrfs check /dev/data/btrfs --check-data-csum"
While mkfs.btrfs handles mixed non-option and option correctly:
"mkfs.btrfs -f /dev/data/disk1 --data raid1 /dev/data/disk2"
Cc: Paul Jones
Cc: Hugo Mills
Fixes: 010ceab56e06 ("btrfs-progs: rework option parser to use getopt for
global options")
Signed-off-by: Qu Wenruo
---
btrfs.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/btrfs.c b/btrfs.c
index 2d39f2ced3e8..ebc4a507165d 100644
--- a/btrfs.c
+++ b/btrfs.c
@@ -169,7 +169,7 @@ static int cmd_version(int argc, char **argv)
* Parse global options, between binary name and first non-option argument
* after processing all valid options (including those with arguments).
*
- * Returns index to argv where parsting stopped, optind is reset to 1
+ * Returns index to argv where parsting stopped, optind is reset to 0
*/
static int handle_global_options(int argc, char **argv)
{
@@ -205,7 +205,13 @@ static int handle_global_options(int argc, char **argv)
}
shift = optind;
- optind = 1;
+ /*
+* optind must to be init to 0 other than 1.
+* Since later subcommand will call getopt_long() again, we need
+* to re-initialize the internal variables correct.
+* Check getopt(3) NOTES sections.
+*/
+ optind = 0;
return shift;
}
--
2.17.1
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: About more loose parameter sequence requirement
On 2018年06月19日 12:34, Qu Wenruo wrote: > > > On 2018年06月18日 20:02, David Sterba wrote: >> On Mon, Jun 18, 2018 at 07:40:44PM +0800, Qu Wenruo wrote: >>> >>> >>> On 2018年06月18日 19:34, David Sterba wrote: On Thu, Jun 14, 2018 at 03:17:45PM +0800, Qu Wenruo wrote: > I understand that btrfs-progs introduced restrict parameter/option order > to distinguish global and sub-command parameter/option. > > However it's really annoying if one just want to append some new options > to previous command: > > E.g. > # btrfs check /dev/data/btrfs > # !! --check-data-csum > > The last command will fail as current btrfs-progs doesn't allow any > option after parameter. > > > Despite the requirement to distinguish global and subcommand > option/parameter, is there any other requirement for such restrict > option-first-parameter-last policy? I'd say that it's a common and recommended pattern. Getopt is able to reorder the parameters so mixed options and non-options are accepted, unless POSIXLY_CORRECT (see man getopt(3)) is not set. With the more strict requirement, 'btrfs' option parser works the same regardless of that. >>> >>> But currently it doesn't work. >>> Just as the example, btrfs check /dev/data/btrfs --check-data-sum won't >>> work. >>> It's different from a lot of other common programs. >> >> With POSIXLY_CORRECT set, it expectedly won't work. With POSIXLY_CORRECT >> unset, it would work in general, but not for 'btrfs'. >> >> As this is per-application decision I find it ok, besides that I also >> find the 'options anywhere' pattern bad. Does it really save you that >> much time while typing the commands with new arguments? There are >> movement shortcuts to jump by words, you get the previous command by >> up-arrow. About the same number of keystrokes. >> >> New code needs to be tested, documented and maintained, that's the cost >> I find too high for something that's convenience for people who are used >> to some shell builtins. >> > > Well, after some testing, the result looks pretty strange on my side. > > With the testing code (*), mostly just copied from check/main.c, it > works to detect the final --check-data-csum without problem. > > I'm originally planning to use '-' to make getopt_long() to keep the > original order, but the experiment I did shows it unnecessary. > > And furthermore, even changing the optstring of btrfs check > getopt_long() with leading '-', it still doesn't work as expected to > detect non-option parameter. > > Is there anything wrong/special in btrfs-progs getopt_long() usage? OK, indeed there is something wrong here. Just as man page of getopt_long(3) said: --- NOTES A program that scans multiple argument vectors, or rescans the same vector more than once, and wants to make use of GNU exten‐ sions such as '+' and '-' at the start of optstring, or changes the value of POSIXLY_CORRECT between scans, must reinitialize getopt() by resetting optind to 0, rather than the traditional value of 1. (Resetting to 0 forces the invocation of an internal initialization routine that rechecks POSIXLY_CORRECT and checks for GNU extensions in optstring.) --- So, "btrfs check /dev/data/btrfs --check-data-csum" should work. And the bug is, we reset @optind to 1, other than 0 when calling getopt_long() on the new argc/argv. I'll fix it soon. Thanks, Qu > > Thanks, > Qu signature.asc Description: OpenPGP digital signature
Re: About more loose parameter sequence requirement
On 2018年06月18日 20:02, David Sterba wrote:
> On Mon, Jun 18, 2018 at 07:40:44PM +0800, Qu Wenruo wrote:
>>
>>
>> On 2018年06月18日 19:34, David Sterba wrote:
>>> On Thu, Jun 14, 2018 at 03:17:45PM +0800, Qu Wenruo wrote:
I understand that btrfs-progs introduced restrict parameter/option order
to distinguish global and sub-command parameter/option.
However it's really annoying if one just want to append some new options
to previous command:
E.g.
# btrfs check /dev/data/btrfs
# !! --check-data-csum
The last command will fail as current btrfs-progs doesn't allow any
option after parameter.
Despite the requirement to distinguish global and subcommand
option/parameter, is there any other requirement for such restrict
option-first-parameter-last policy?
>>>
>>> I'd say that it's a common and recommended pattern. Getopt is able to
>>> reorder the parameters so mixed options and non-options are accepted,
>>> unless POSIXLY_CORRECT (see man getopt(3)) is not set. With the more
>>> strict requirement, 'btrfs' option parser works the same regardless of
>>> that.
>>
>> But currently it doesn't work.
>> Just as the example, btrfs check /dev/data/btrfs --check-data-sum won't
>> work.
>> It's different from a lot of other common programs.
>
> With POSIXLY_CORRECT set, it expectedly won't work. With POSIXLY_CORRECT
> unset, it would work in general, but not for 'btrfs'.
>
> As this is per-application decision I find it ok, besides that I also
> find the 'options anywhere' pattern bad. Does it really save you that
> much time while typing the commands with new arguments? There are
> movement shortcuts to jump by words, you get the previous command by
> up-arrow. About the same number of keystrokes.
>
> New code needs to be tested, documented and maintained, that's the cost
> I find too high for something that's convenience for people who are used
> to some shell builtins.
>
Well, after some testing, the result looks pretty strange on my side.
With the testing code (*), mostly just copied from check/main.c, it
works to detect the final --check-data-csum without problem.
I'm originally planning to use '-' to make getopt_long() to keep the
original order, but the experiment I did shows it unnecessary.
And furthermore, even changing the optstring of btrfs check
getopt_long() with leading '-', it still doesn't work as expected to
detect non-option parameter.
Is there anything wrong/special in btrfs-progs getopt_long() usage?
Thanks,
Qu
*:
---
#include
#include
#include
int main(int argc, char** argv)
{
char *real_argv[] = { "btrfs check", "device", "--check-data-csum" ,
NULL};
int real_argc = sizeof(real_argv) / sizeof(char *) - 1;
while (1) {
enum { GETOPT_VAL_REPAIR = 257, GETOPT_VAL_INIT_CSUM,
GETOPT_VAL_INIT_EXTENT, GETOPT_VAL_CHECK_CSUM,
GETOPT_VAL_READONLY, GETOPT_VAL_CHUNK_TREE,
GETOPT_VAL_MODE, GETOPT_VAL_CLEAR_SPACE_CACHE,
GETOPT_VAL_FORCE };
static const struct option long_options[] = {
{ "super", required_argument, NULL, 's' },
{ "repair", no_argument, NULL, GETOPT_VAL_REPAIR },
{ "readonly", no_argument, NULL, GETOPT_VAL_READONLY },
{ "init-csum-tree", no_argument, NULL,
GETOPT_VAL_INIT_CSUM },
{ "init-extent-tree", no_argument, NULL,
GETOPT_VAL_INIT_EXTENT },
{ "check-data-csum", no_argument, NULL,
GETOPT_VAL_CHECK_CSUM },
{ "backup", no_argument, NULL, 'b' },
{ "subvol-extents", required_argument, NULL, 'E' },
{ "qgroup-report", no_argument, NULL, 'Q' },
{ "tree-root", required_argument, NULL, 'r' },
{ "chunk-root", required_argument, NULL,
GETOPT_VAL_CHUNK_TREE },
{ "progress", no_argument, NULL, 'p' },
{ "mode", required_argument, NULL,
GETOPT_VAL_MODE },
{ "clear-space-cache", required_argument, NULL,
GETOPT_VAL_CLEAR_SPACE_CACHE},
{ "force", no_argument, NULL, GETOPT_VAL_FORCE },
{ NULL, 0, NULL, 0}
};
int c;
c = getopt_long(real_argc, real_argv, "as:br:pEQ",
long_options, NULL);
if (c < 0)
break;
switch (c) {
case 'a':
case 'b':
case 'Q':
case 'E':
case 'p':
Re: [PATCH] btrfs-progs: Check factor out root parsing from check_chunks_and_extents
On 06/18/2018 07:10 PM, Nikolay Borisov wrote:
check_chunks_and_extents does quite a number of distinct things. The
first of those is going through all root items in the root tree and
classify every root depending on whether they have a dropping operation
in progress or not. Lets factor out this code and move the variables
specific to this in a separate function so clean up check_chunks_and_extents
a bit. Accidentally, this patch fixes some reference leaks since
in error conditions in the loop the code does "goto out" but at that
label we don't really release the path. Having this code extracted in a
separate function which always releases the path avoids this problem
entirely.
Signed-off-by: Nikolay Borisov
Code looks okay. One nitpick belows.
---
check/main.c | 141 +++
1 file changed, 85 insertions(+), 56 deletions(-)
diff --git a/check/main.c b/check/main.c
index a4d6855dccbf..fb5c86df21c9 100644
--- a/check/main.c
+++ b/check/main.c
@@ -8069,6 +8069,89 @@ static int deal_root_from_list(struct list_head *list,
return ret;
}
+/**
+ * parse_tree_roots - Go over all roots in the tree root and add each one to
+ * a list.
+ *
+ * @fs_info- pointer to fs_info struct of the file system.
+ *
+ * @normal_trees - list to contains all roots which don't have a drop
+ * operation in progress
+ *
+ * @dropping_trees - list containing all roots which have a drop operation
+ * pending
+ *
+ * Returns 0 on success or a negative value indicating an error.
+ *
+ */
+static int parse_tree_roots(struct btrfs_fs_info *fs_info,
+ struct list_head *normal_trees,
+ struct list_head *dropping_trees)
+{
+ struct btrfs_path path;
+ struct btrfs_key key;
+ struct btrfs_key found_key;
+ struct btrfs_root_item ri;
+ struct extent_buffer *leaf;
+ int slot;
+ int ret = 0;
+
+ btrfs_init_path(&path);
+ key.offset = 0;
+ key.objectid = 0;
+ key.type = BTRFS_ROOT_ITEM_KEY;
+ ret = btrfs_search_slot(NULL, fs_info->tree_root, &key, &path, 0, 0);
+ if (ret < 0)
+ goto out;
+ while (1) {
+ leaf = path.nodes[0];
+ slot = path.slots[0];
+ if (slot >= btrfs_header_nritems(path.nodes[0])) {
+ ret = btrfs_next_leaf(fs_info->tree_root, &path);
+ if (ret != 0)
+ break;
+ leaf = path.nodes[0];
+ slot = path.slots[0];
+ }
Nit:
I know the segment is just moved. The @slot is unused.
+ btrfs_item_key_to_cpu(leaf, &found_key, path.slots[0]);
+ if (found_key.type == BTRFS_ROOT_ITEM_KEY) {
+ unsigned long offset;
+ u64 last_snapshot;
+ u8 level;
+
+ offset = btrfs_item_ptr_offset(leaf, path.slots[0]);
+ read_extent_buffer(leaf, &ri, offset, sizeof(ri));
+ last_snapshot = btrfs_root_last_snapshot(&ri);
+ level = btrfs_root_level(&ri);
+ if (btrfs_disk_key_objectid(&ri.drop_progress) == 0) {
+ ret = add_root_item_to_list(normal_trees,
+ found_key.objectid,
+ btrfs_root_bytenr(&ri),
+ last_snapshot, level,
+ 0, NULL);
+ if (ret < 0)
+ break;
+ } else {
+ u64 objectid = found_key.objectid;
+ btrfs_disk_key_to_cpu(&found_key,
+ &ri.drop_progress);
+ ret = add_root_item_to_list(dropping_trees,
+ objectid,
+ btrfs_root_bytenr(&ri),
+ last_snapshot, level,
+ ri.drop_level, &found_key);
+ if (ret < 0)
+ break;
+ }
+ }
+ path.slots[0]++;
+ }
+
+out:
+ btrfs_release_path(&path);
+ return ret;
+}
+
static int check_chunks_and_extents(struct btrfs_fs_info *fs_info)
{
struct rb_root dev_cache;
@@ -8082,20 +8165,13 @@ static int check_chunks_and_extents(struct
btrfs_fs_info *fs_info)
struct cache_tree nodes;
struct extent_io_tree excluded_extents;
struct cache_tree corrupt_blocks;
- struct btrfs_path path;
-
Re: [PATCH] btrfs-progs: check: Fix wrong root parameter of btrfs_next_leaf call
On 06/18/2018 07:10 PM, Nikolay Borisov wrote:
The first thing that check_chunks_and_extents does is to iterate all
the root items in the root tree and link them to either the "normal_list"
or "dropping_trees" list. If a leaf has to be crossed during this
operation btrfs_next_leaf is called to do that. However, currently it's
called with a wrong argument for its 'root' parameter. Since we are
iterating the root tree the passed root should be fs_info->tree_rot,
whereas right now we are passing the local variable 'root' which is
assigned to the fs_tree. As it stands, this bug is actually benign since
the passed root is only passed to reada_for_search, where it's used to
reference the fs_info. Nevertheless the code is wrong and at the very least
misleading, so fix it by passing the correct root.
Signed-off-by: Nikolay Borisov
Reviewed-by: Su Yue
---
check/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/check/main.c b/check/main.c
index 68da994f7ae0..a4d6855dccbf 100644
--- a/check/main.c
+++ b/check/main.c
@@ -8157,7 +8157,7 @@ static int check_chunks_and_extents(struct btrfs_fs_info
*fs_info)
leaf = path.nodes[0];
slot = path.slots[0];
if (slot >= btrfs_header_nritems(path.nodes[0])) {
- ret = btrfs_next_leaf(root, &path);
+ ret = btrfs_next_leaf(fs_info->tree_root, &path);
if (ret != 0)
break;
leaf = path.nodes[0];
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: About more loose parameter sequence requirement
On 2018年06月18日 20:02, David Sterba wrote: > On Mon, Jun 18, 2018 at 07:40:44PM +0800, Qu Wenruo wrote: >> >> >> On 2018年06月18日 19:34, David Sterba wrote: >>> On Thu, Jun 14, 2018 at 03:17:45PM +0800, Qu Wenruo wrote: I understand that btrfs-progs introduced restrict parameter/option order to distinguish global and sub-command parameter/option. However it's really annoying if one just want to append some new options to previous command: E.g. # btrfs check /dev/data/btrfs # !! --check-data-csum The last command will fail as current btrfs-progs doesn't allow any option after parameter. Despite the requirement to distinguish global and subcommand option/parameter, is there any other requirement for such restrict option-first-parameter-last policy? >>> >>> I'd say that it's a common and recommended pattern. Getopt is able to >>> reorder the parameters so mixed options and non-options are accepted, >>> unless POSIXLY_CORRECT (see man getopt(3)) is not set. With the more >>> strict requirement, 'btrfs' option parser works the same regardless of >>> that. >> >> But currently it doesn't work. >> Just as the example, btrfs check /dev/data/btrfs --check-data-sum won't >> work. >> It's different from a lot of other common programs. > > With POSIXLY_CORRECT set, it expectedly won't work. With POSIXLY_CORRECT > unset, it would work in general, but not for 'btrfs'. > > As this is per-application decision I find it ok, besides that I also > find the 'options anywhere' pattern bad. Does it really save you that > much time while typing the commands with new arguments? Personally speaking, yes. > There are > movement shortcuts to jump by words, you get the previous command by > up-arrow. About the same number of keystrokes. Not really, normally just "!! ", where I don't even need to move my fingers to arrow keys. (I'm all ears about extra bash shortcuts on this) > > New code needs to be tested, documented and maintained, that's the cost > I find too high for something that's convenience for people who are used > to some shell builtins. The biggest problem is, the behavior isn't even consistent across btrfs-progs. mkfs.btrfs accept such out-of-order parameters while btrfs not. And most common tools, like commands provided by coretutil, they don't care about the order. The only daily exception is 'scp', which I found it pretty unhandy. And just as Paul and Hugo, I think there are quite some users preferring out-of-order parameter/options. I also understand the maintenance burden, but at least let's try if there are better solution for this. Thanks, Qu signature.asc Description: OpenPGP digital signature
btrfs balance did not progress after 12H
So, I ran this: gargamel:/mnt/btrfs_pool2# btrfs balance start -dusage=60 -v . & [1] 24450 Dumping filters: flags 0x1, state 0x0, force is off DATA (flags 0x2): balancing, usage=60 gargamel:/mnt/btrfs_pool2# while :; do btrfs balance status .; sleep 60; done 0 out of about 0 chunks balanced (0 considered), -nan% left Balance on '.' is running 0 out of about 73 chunks balanced (2 considered), 100% left Balance on '.' is running After about 20mn, it changed to this: 1 out of about 73 chunks balanced (6724 considered), 99% left Balance on '.' is running Now, 12H later, it's still there, only 1 out of 73. gargamel:/mnt/btrfs_pool2# btrfs fi show . Label: 'dshelf2' uuid: 0f1a0c9f-4e54-4fa7-8736-fd50818ff73d Total devices 1 FS bytes used 12.72TiB devid1 size 14.55TiB used 13.81TiB path /dev/mapper/dshelf2 gargamel:/mnt/btrfs_pool2# btrfs fi df . Data, single: total=13.57TiB, used=12.60TiB System, DUP: total=32.00MiB, used=1.55MiB Metadata, DUP: total=121.50GiB, used=116.53GiB GlobalReserve, single: total=512.00MiB, used=848.00KiB kernel: 4.16.8 Is that expected? Should I be ready to wait days possibly for this balance to finish? Thanks, Marc -- "A mouse is a device used to point at the xterm you want to type in" - A.S.R. Microsoft is to operating systems what McDonalds is to gourmet cooking Home page: http://marc.merlins.org/ | PGP 7F55D5F27AAF9D08 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: About more loose parameter sequence requirement
> -Original Message- > From: linux-btrfs-ow...@vger.kernel.org ow...@vger.kernel.org> On Behalf Of Hugo Mills > Sent: Monday, 18 June 2018 9:44 PM > To: dste...@suse.cz; Qu Wenruo ; linux- > bt...@vger.kernel.org > Subject: Re: About more loose parameter sequence requirement > > On Mon, Jun 18, 2018 at 01:34:32PM +0200, David Sterba wrote: > > On Thu, Jun 14, 2018 at 03:17:45PM +0800, Qu Wenruo wrote: > > > I understand that btrfs-progs introduced restrict parameter/option > > > order to distinguish global and sub-command parameter/option. > > > > > > However it's really annoying if one just want to append some new > > > options to previous command: > > > > > > E.g. > > > # btrfs check /dev/data/btrfs > > > # !! --check-data-csum > > > > > > The last command will fail as current btrfs-progs doesn't allow any > > > option after parameter. > > > > > > > > > Despite the requirement to distinguish global and subcommand > > > option/parameter, is there any other requirement for such restrict > > > option-first-parameter-last policy? > > > > I'd say that it's a common and recommended pattern. Getopt is able to > > reorder the parameters so mixed options and non-options are accepted, > > unless POSIXLY_CORRECT (see man getopt(3)) is not set. With the more > > strict requirement, 'btrfs' option parser works the same regardless of > > that. > >I got bitten by this the other day. I put an option flag at the end of the > line, > after the mountpoint, and it refused to work. > >I would definitely prefer it if it parsed options in any position. (Or at > least, > any position after the group/command parameters). Same with me - I do it all the time. I type the arguments as I think of them, which is usually back-to-front of what is required. Eg. Btrfs check this mountpoint, oh yeah, and use this specific option = fail. Arrow arrow arrow arrow arrow Paul. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: About more loose parameter sequence requirement
On Mon, Jun 18, 2018 at 01:34:32PM +0200, David Sterba wrote: > On Thu, Jun 14, 2018 at 03:17:45PM +0800, Qu Wenruo wrote: > > I understand that btrfs-progs introduced restrict parameter/option order > > to distinguish global and sub-command parameter/option. > > > > However it's really annoying if one just want to append some new options > > to previous command: > > > > E.g. > > # btrfs check /dev/data/btrfs > > # !! --check-data-csum > > > > The last command will fail as current btrfs-progs doesn't allow any > > option after parameter. > > > > > > Despite the requirement to distinguish global and subcommand > > option/parameter, is there any other requirement for such restrict > > option-first-parameter-last policy? > > I'd say that it's a common and recommended pattern. Getopt is able to > reorder the parameters so mixed options and non-options are accepted, > unless POSIXLY_CORRECT (see man getopt(3)) is not set. With the more > strict requirement, 'btrfs' option parser works the same regardless of > that. I got bitten by this the other day. I put an option flag at the end of the line, after the mountpoint, and it refused to work. I would definitely prefer it if it parsed options in any position. (Or at least, any position after the group/command parameters). Hugo. > > If I could implement a enhanced getopt to allow more loose order inside > > subcomand while still can distinguish global option, will it be accepted > > (if it's quality is acceptable) ? > > I think it's not worth updating the parser just to support an IMHO > narrow usecase. -- Hugo Mills | Turning, pages turning in the widening bath, hugo@... carfax.org.uk | The spine cannot bear the humidity. http://carfax.org.uk/ | Books fall apart; the binding cannot hold. PGP: E2AB1DE4 | Page 129 is loosed upon the world. Zarf signature.asc Description: Digital signature
Re: About more loose parameter sequence requirement
On Mon, Jun 18, 2018 at 07:40:44PM +0800, Qu Wenruo wrote: > > > On 2018年06月18日 19:34, David Sterba wrote: > > On Thu, Jun 14, 2018 at 03:17:45PM +0800, Qu Wenruo wrote: > >> I understand that btrfs-progs introduced restrict parameter/option order > >> to distinguish global and sub-command parameter/option. > >> > >> However it's really annoying if one just want to append some new options > >> to previous command: > >> > >> E.g. > >> # btrfs check /dev/data/btrfs > >> # !! --check-data-csum > >> > >> The last command will fail as current btrfs-progs doesn't allow any > >> option after parameter. > >> > >> > >> Despite the requirement to distinguish global and subcommand > >> option/parameter, is there any other requirement for such restrict > >> option-first-parameter-last policy? > > > > I'd say that it's a common and recommended pattern. Getopt is able to > > reorder the parameters so mixed options and non-options are accepted, > > unless POSIXLY_CORRECT (see man getopt(3)) is not set. With the more > > strict requirement, 'btrfs' option parser works the same regardless of > > that. > > But currently it doesn't work. > Just as the example, btrfs check /dev/data/btrfs --check-data-sum won't > work. > It's different from a lot of other common programs. With POSIXLY_CORRECT set, it expectedly won't work. With POSIXLY_CORRECT unset, it would work in general, but not for 'btrfs'. As this is per-application decision I find it ok, besides that I also find the 'options anywhere' pattern bad. Does it really save you that much time while typing the commands with new arguments? There are movement shortcuts to jump by words, you get the previous command by up-arrow. About the same number of keystrokes. New code needs to be tested, documented and maintained, that's the cost I find too high for something that's convenience for people who are used to some shell builtins. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] btrfs: Document __btrfs_inc_extent_ref
Here is a doc-only patch which tires to deobfuscate the terra-incognita that arguments for delayed refs are. Signed-off-by: Nikolay Borisov --- fs/btrfs/extent-tree.c | 34 ++ 1 file changed, 34 insertions(+) diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c index 59645ced6fbc..39d0652bf3f3 100644 --- a/fs/btrfs/extent-tree.c +++ b/fs/btrfs/extent-tree.c @@ -2207,6 +2207,40 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans, return ret; } +/* + * __btrfs_inc_extent_ref - insert backreference for a given extent + * + * @trans: Handle of transaction + * + * @node: The delayed ref node used to get the bytenr/length for + * extent whose references are incremented. + * + * @parent:If this is a shared extent (BTRFS_SHARED_DATA_REF_KEY/ + * BTRFS_SHARED_BLOCK_REF_KEY) then it holds the logical + * bytenr of the parent block. Since new extents are always + * created with indirect references, this will only be the case + * when relocating a shared extent. In that case, root_objectid + * will be BTRFS_TREE_RELOC_OBJECTID. Otheriwse, parent must + * be 0 + * + * @root_objectid: The id of the root where this modification has originated, + * this can be either one of the well-known metadata trees or + * the subvolume id which references this extent. + * + * @owner: For data extents it is the inode number of the owning file. + * For metadata extents this parameter holds the level in the + * tree of the extent. + * + * @offset:For metadata extents the offset is ignored and is currently + * always passed as 0. For data extents it is the fileoffset + * this extent belongs to. + * + * @refs_to_add Number of references to add + * + * @extent_op Pointer to a structure, holding information necessary when + * updating a tree block's flags + * + */ static int __btrfs_inc_extent_ref(struct btrfs_trans_handle *trans, struct btrfs_delayed_ref_node *node, u64 parent, u64 root_objectid, -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] btrfs: Remove fs_info argument from __btrfs_inc_extent_ref
This function already takes a transaction which holds a reference to
the fs_info struct. Use that reference and remove the extra arg. No
functional changes.
Signed-off-by: Nikolay Borisov
---
fs/btrfs/extent-tree.c | 15 ++-
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 4850e538ab10..59645ced6fbc 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -2208,12 +2208,12 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle
*trans,
}
static int __btrfs_inc_extent_ref(struct btrfs_trans_handle *trans,
- struct btrfs_fs_info *fs_info,
struct btrfs_delayed_ref_node *node,
u64 parent, u64 root_objectid,
u64 owner, u64 offset, int refs_to_add,
struct btrfs_delayed_extent_op *extent_op)
{
+ struct btrfs_fs_info *fs_info = trans->fs_info;
struct btrfs_path *path;
struct extent_buffer *leaf;
struct btrfs_extent_item *item;
@@ -2297,10 +2297,9 @@ static int run_delayed_data_ref(struct
btrfs_trans_handle *trans,
ref->objectid, ref->offset,
&ins, node->ref_mod);
} else if (node->action == BTRFS_ADD_DELAYED_REF) {
- ret = __btrfs_inc_extent_ref(trans, fs_info, node, parent,
-ref_root, ref->objectid,
-ref->offset, node->ref_mod,
-extent_op);
+ ret = __btrfs_inc_extent_ref(trans, node, parent, ref_root,
+ref->objectid, ref->offset,
+node->ref_mod, extent_op);
} else if (node->action == BTRFS_DROP_DELAYED_REF) {
ret = __btrfs_free_extent(trans, fs_info, node, parent,
ref_root, ref->objectid,
@@ -2450,10 +2449,8 @@ static int run_delayed_tree_ref(struct
btrfs_trans_handle *trans,
BUG_ON(!extent_op || !extent_op->update_flags);
ret = alloc_reserved_tree_block(trans, node, extent_op);
} else if (node->action == BTRFS_ADD_DELAYED_REF) {
- ret = __btrfs_inc_extent_ref(trans, fs_info, node,
-parent, ref_root,
-ref->level, 0, 1,
-extent_op);
+ ret = __btrfs_inc_extent_ref(trans, node, parent, ref_root,
+ref->level, 0, 1, extent_op);
} else if (node->action == BTRFS_DROP_DELAYED_REF) {
ret = __btrfs_free_extent(trans, fs_info, node,
parent, ref_root,
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] btrfs: Fix comment in lookup_inline_extent_backref
The comment wrongfully states that the owner parameter is the level of
the parent block. In fact owner is the level of the current block and
by adding 1 to it we can eventually get to the parent/root.
Signed-off-by: Nikolay Borisov
---
fs/btrfs/extent-tree.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 39d0652bf3f3..a470a07d4f2a 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -1635,7 +1635,7 @@ int lookup_inline_extent_backref(struct
btrfs_trans_handle *trans,
extra_size = -1;
/*
-* Owner is our parent level, so we can just add one to get the level
+* Owner is our level, so we can just add one to get the level
* for the block we are interested in.
*/
if (skinny_metadata && owner < BTRFS_FIRST_FREE_OBJECTID) {
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] Cleanups around extent increment
Hello, This series improves the functions involved around extent reference increment. The first patch just removes a redundant argument, the second one documents the parameters of __btrfs_inc_extent_ref. It can be considered v2 of the standalone version which Jeff had some input to. The final patch fixes a comment in lookup_inline_extent_backref which transpired while Jeff was revieweing the documentation patch. Nikolay Borisov (3): btrfs: Remove fs_info argument from __btrfs_inc_extent_ref btrfs: Document __btrfs_inc_extent_ref btrfs: Fix comment in lookup_inline_extent_backref fs/btrfs/extent-tree.c | 51 -- 1 file changed, 41 insertions(+), 10 deletions(-) -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: About more loose parameter sequence requirement
On 2018年06月18日 19:34, David Sterba wrote: > On Thu, Jun 14, 2018 at 03:17:45PM +0800, Qu Wenruo wrote: >> I understand that btrfs-progs introduced restrict parameter/option order >> to distinguish global and sub-command parameter/option. >> >> However it's really annoying if one just want to append some new options >> to previous command: >> >> E.g. >> # btrfs check /dev/data/btrfs >> # !! --check-data-csum >> >> The last command will fail as current btrfs-progs doesn't allow any >> option after parameter. >> >> >> Despite the requirement to distinguish global and subcommand >> option/parameter, is there any other requirement for such restrict >> option-first-parameter-last policy? > > I'd say that it's a common and recommended pattern. Getopt is able to > reorder the parameters so mixed options and non-options are accepted, > unless POSIXLY_CORRECT (see man getopt(3)) is not set. With the more > strict requirement, 'btrfs' option parser works the same regardless of > that. But currently it doesn't work. Just as the example, btrfs check /dev/data/btrfs --check-data-sum won't work. It's different from a lot of other common programs. > >> If I could implement a enhanced getopt to allow more loose order inside >> subcomand while still can distinguish global option, will it be accepted >> (if it's quality is acceptable) ? > > I think it's not worth updating the parser just to support an IMHO > narrow usecase. I'll try to use the getopt() and keeps the current structure if possible. Thanks, Qu > signature.asc Description: OpenPGP digital signature
Re: About more loose parameter sequence requirement
On Thu, Jun 14, 2018 at 03:17:45PM +0800, Qu Wenruo wrote: > I understand that btrfs-progs introduced restrict parameter/option order > to distinguish global and sub-command parameter/option. > > However it's really annoying if one just want to append some new options > to previous command: > > E.g. > # btrfs check /dev/data/btrfs > # !! --check-data-csum > > The last command will fail as current btrfs-progs doesn't allow any > option after parameter. > > > Despite the requirement to distinguish global and subcommand > option/parameter, is there any other requirement for such restrict > option-first-parameter-last policy? I'd say that it's a common and recommended pattern. Getopt is able to reorder the parameters so mixed options and non-options are accepted, unless POSIXLY_CORRECT (see man getopt(3)) is not set. With the more strict requirement, 'btrfs' option parser works the same regardless of that. > If I could implement a enhanced getopt to allow more loose order inside > subcomand while still can distinguish global option, will it be accepted > (if it's quality is acceptable) ? I think it's not worth updating the parser just to support an IMHO narrow usecase. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] btrfs: Deduplicate extent_buffer init code
When a new extent buffer is allocated there are a few mandatory fields
which need to be set in order for the buffer to be sane: level,
generation, bytenr, backref_rev, owner and FSID/UUID. Currently this
is open coded in the callers of btrfs_alloc_tree_block, meaning it's
fairly high in the abstraction hierarchy of operations. This patch
solves this by simply moving this init code in btrfs_init_new_buffer,
since this is the function which initializes a newly allocated
extent buffer. No functional changes.
Signed-off-by: Nikolay Borisov
---
fs/btrfs/ctree.c | 29 +
fs/btrfs/disk-io.c | 14 --
fs/btrfs/extent-tree.c | 16
fs/btrfs/ioctl.c | 8
4 files changed, 13 insertions(+), 54 deletions(-)
diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 4bc326df472e..6879697520d5 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -3359,17 +3359,7 @@ static noinline int insert_new_root(struct
btrfs_trans_handle *trans,
root_add_used(root, fs_info->nodesize);
- memzero_extent_buffer(c, 0, sizeof(struct btrfs_header));
btrfs_set_header_nritems(c, 1);
- btrfs_set_header_level(c, level);
- btrfs_set_header_bytenr(c, c->start);
- btrfs_set_header_generation(c, trans->transid);
- btrfs_set_header_backref_rev(c, BTRFS_MIXED_BACKREF_REV);
- btrfs_set_header_owner(c, root->root_key.objectid);
-
- write_extent_buffer_fsid(c, fs_info->fsid);
- write_extent_buffer_chunk_tree_uuid(c, fs_info->chunk_tree_uuid);
-
btrfs_set_node_key(c, &lower_key, 0);
btrfs_set_node_blockptr(c, 0, lower->start);
lower_gen = btrfs_header_generation(lower);
@@ -3498,15 +3488,7 @@ static noinline int split_node(struct btrfs_trans_handle
*trans,
return PTR_ERR(split);
root_add_used(root, fs_info->nodesize);
-
- memzero_extent_buffer(split, 0, sizeof(struct btrfs_header));
- btrfs_set_header_level(split, btrfs_header_level(c));
- btrfs_set_header_bytenr(split, split->start);
- btrfs_set_header_generation(split, trans->transid);
- btrfs_set_header_backref_rev(split, BTRFS_MIXED_BACKREF_REV);
- btrfs_set_header_owner(split, root->root_key.objectid);
- write_extent_buffer_fsid(split, fs_info->fsid);
- write_extent_buffer_chunk_tree_uuid(split, fs_info->chunk_tree_uuid);
+ ASSERT(btrfs_header_level(c) == level);
ret = tree_mod_log_eb_copy(fs_info, split, c, 0, mid, c_nritems - mid);
if (ret) {
@@ -4292,15 +4274,6 @@ static noinline int split_leaf(struct btrfs_trans_handle
*trans,
root_add_used(root, fs_info->nodesize);
- memzero_extent_buffer(right, 0, sizeof(struct btrfs_header));
- btrfs_set_header_bytenr(right, right->start);
- btrfs_set_header_generation(right, trans->transid);
- btrfs_set_header_backref_rev(right, BTRFS_MIXED_BACKREF_REV);
- btrfs_set_header_owner(right, root->root_key.objectid);
- btrfs_set_header_level(right, 0);
- write_extent_buffer_fsid(right, fs_info->fsid);
- write_extent_buffer_chunk_tree_uuid(right, fs_info->chunk_tree_uuid);
-
if (split == 0) {
if (mid <= slot) {
btrfs_set_header_nritems(right, 0);
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 205092dc9390..80a46aabf14c 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1292,15 +1292,7 @@ struct btrfs_root *btrfs_create_tree(struct
btrfs_trans_handle *trans,
goto fail;
}
- memzero_extent_buffer(leaf, 0, sizeof(struct btrfs_header));
- btrfs_set_header_bytenr(leaf, leaf->start);
- btrfs_set_header_generation(leaf, trans->transid);
- btrfs_set_header_backref_rev(leaf, BTRFS_MIXED_BACKREF_REV);
- btrfs_set_header_owner(leaf, objectid);
root->node = leaf;
-
- write_extent_buffer_fsid(leaf, fs_info->fsid);
- write_extent_buffer_chunk_tree_uuid(leaf, fs_info->chunk_tree_uuid);
btrfs_mark_buffer_dirty(leaf);
root->commit_root = btrfs_root_node(root);
@@ -1374,14 +1366,8 @@ static struct btrfs_root *alloc_log_tree(struct
btrfs_trans_handle *trans,
return ERR_CAST(leaf);
}
- memzero_extent_buffer(leaf, 0, sizeof(struct btrfs_header));
- btrfs_set_header_bytenr(leaf, leaf->start);
- btrfs_set_header_generation(leaf, trans->transid);
- btrfs_set_header_backref_rev(leaf, BTRFS_MIXED_BACKREF_REV);
- btrfs_set_header_owner(leaf, BTRFS_TREE_LOG_OBJECTID);
root->node = leaf;
- write_extent_buffer_fsid(root->node, fs_info->fsid);
btrfs_mark_buffer_dirty(root->node);
btrfs_tree_unlock(root->node);
return root;
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index fb44ec27bdfd..9cafa9c0c924 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -8318,7 +8318,7 @@ int btr
[PATCH] btrfs-progs: Check factor out root parsing from check_chunks_and_extents
check_chunks_and_extents does quite a number of distinct things. The
first of those is going through all root items in the root tree and
classify every root depending on whether they have a dropping operation
in progress or not. Lets factor out this code and move the variables
specific to this in a separate function so clean up check_chunks_and_extents
a bit. Accidentally, this patch fixes some reference leaks since
in error conditions in the loop the code does "goto out" but at that
label we don't really release the path. Having this code extracted in a
separate function which always releases the path avoids this problem
entirely.
Signed-off-by: Nikolay Borisov
---
check/main.c | 141 +++
1 file changed, 85 insertions(+), 56 deletions(-)
diff --git a/check/main.c b/check/main.c
index a4d6855dccbf..fb5c86df21c9 100644
--- a/check/main.c
+++ b/check/main.c
@@ -8069,6 +8069,89 @@ static int deal_root_from_list(struct list_head *list,
return ret;
}
+/**
+ * parse_tree_roots - Go over all roots in the tree root and add each one to
+ * a list.
+ *
+ * @fs_info- pointer to fs_info struct of the file system.
+ *
+ * @normal_trees - list to contains all roots which don't have a drop
+ * operation in progress
+ *
+ * @dropping_trees - list containing all roots which have a drop operation
+ * pending
+ *
+ * Returns 0 on success or a negative value indicating an error.
+ *
+ */
+static int parse_tree_roots(struct btrfs_fs_info *fs_info,
+ struct list_head *normal_trees,
+ struct list_head *dropping_trees)
+{
+ struct btrfs_path path;
+ struct btrfs_key key;
+ struct btrfs_key found_key;
+ struct btrfs_root_item ri;
+ struct extent_buffer *leaf;
+ int slot;
+ int ret = 0;
+
+ btrfs_init_path(&path);
+ key.offset = 0;
+ key.objectid = 0;
+ key.type = BTRFS_ROOT_ITEM_KEY;
+ ret = btrfs_search_slot(NULL, fs_info->tree_root, &key, &path, 0, 0);
+ if (ret < 0)
+ goto out;
+ while (1) {
+ leaf = path.nodes[0];
+ slot = path.slots[0];
+ if (slot >= btrfs_header_nritems(path.nodes[0])) {
+ ret = btrfs_next_leaf(fs_info->tree_root, &path);
+ if (ret != 0)
+ break;
+ leaf = path.nodes[0];
+ slot = path.slots[0];
+ }
+ btrfs_item_key_to_cpu(leaf, &found_key, path.slots[0]);
+ if (found_key.type == BTRFS_ROOT_ITEM_KEY) {
+ unsigned long offset;
+ u64 last_snapshot;
+ u8 level;
+
+ offset = btrfs_item_ptr_offset(leaf, path.slots[0]);
+ read_extent_buffer(leaf, &ri, offset, sizeof(ri));
+ last_snapshot = btrfs_root_last_snapshot(&ri);
+ level = btrfs_root_level(&ri);
+ if (btrfs_disk_key_objectid(&ri.drop_progress) == 0) {
+ ret = add_root_item_to_list(normal_trees,
+ found_key.objectid,
+ btrfs_root_bytenr(&ri),
+ last_snapshot, level,
+ 0, NULL);
+ if (ret < 0)
+ break;
+ } else {
+ u64 objectid = found_key.objectid;
+ btrfs_disk_key_to_cpu(&found_key,
+ &ri.drop_progress);
+ ret = add_root_item_to_list(dropping_trees,
+ objectid,
+ btrfs_root_bytenr(&ri),
+ last_snapshot, level,
+ ri.drop_level, &found_key);
+ if (ret < 0)
+ break;
+ }
+ }
+ path.slots[0]++;
+ }
+
+out:
+ btrfs_release_path(&path);
+ return ret;
+}
+
static int check_chunks_and_extents(struct btrfs_fs_info *fs_info)
{
struct rb_root dev_cache;
@@ -8082,20 +8165,13 @@ static int check_chunks_and_extents(struct
btrfs_fs_info *fs_info)
struct cache_tree nodes;
struct extent_io_tree excluded_extents;
struct cache_tree corrupt_blocks;
- struct btrfs_path path;
- struct btrfs_key key;
- struct btrfs_key found_key;
int ret, err = 0;
struct block_info *bits;
int bits_nr;
- struct extent_buf
[PATCH] btrfs-progs: check: Fix wrong root parameter of btrfs_next_leaf call
The first thing that check_chunks_and_extents does is to iterate all
the root items in the root tree and link them to either the "normal_list"
or "dropping_trees" list. If a leaf has to be crossed during this
operation btrfs_next_leaf is called to do that. However, currently it's
called with a wrong argument for its 'root' parameter. Since we are
iterating the root tree the passed root should be fs_info->tree_rot,
whereas right now we are passing the local variable 'root' which is
assigned to the fs_tree. As it stands, this bug is actually benign since
the passed root is only passed to reada_for_search, where it's used to
reference the fs_info. Nevertheless the code is wrong and at the very least
misleading, so fix it by passing the correct root.
Signed-off-by: Nikolay Borisov
---
check/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/check/main.c b/check/main.c
index 68da994f7ae0..a4d6855dccbf 100644
--- a/check/main.c
+++ b/check/main.c
@@ -8157,7 +8157,7 @@ static int check_chunks_and_extents(struct btrfs_fs_info
*fs_info)
leaf = path.nodes[0];
slot = path.slots[0];
if (slot >= btrfs_header_nritems(path.nodes[0])) {
- ret = btrfs_next_leaf(root, &path);
+ ret = btrfs_next_leaf(fs_info->tree_root, &path);
if (ret != 0)
break;
leaf = path.nodes[0];
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Btrfs: sync log after logging new name
On Fri, Jun 15, 2018 at 05:19:07PM +0100, Filipe Manana wrote:
> On Fri, Jun 15, 2018 at 4:54 PM, David Sterba wrote:
> > On Mon, Jun 11, 2018 at 07:24:28PM +0100, fdman...@kernel.org wrote:
> >> From: Filipe Manana
> >> Fixes: 12fcfd22fe5b ("Btrfs: tree logging unlink/rename fixes")
> >> Reported-by: Vijay Chidambaram
> >> Signed-off-by: Filipe Manana
> >
> > There are some warnings and possible lock up caused by this patch, the
> > 1/2 alone is ok but 1/2 + 2/2 leads to the following warnings. I checked
> > twice, the patch base was the pull request ie. without any other 4.18
> > stuff.
>
> Are you sure it's this patch?
> On top of for-4.18 it didn't cause any problems here, plus the trace
> below has nothing to do with renames, hard links or fsync at all -
> everything seems stuck on waiting for IO from dev replace.
It was a false alert, sorry. Strange that the warnings appeared only in
the VM running both patches and not otherwise.
Though the test did not directly use rename, the possible error scenario
I had in mind was some leftover from locking, error handling or state
that blocked umount of 011.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] btrfs: Fix a C compliance issue
On 18.06.2018 12:26, David Sterba wrote:
> On Sat, Jun 16, 2018 at 01:28:13PM +0300, Nikolay Borisov wrote:
>>
>>
>> On 16.06.2018 01:36, Bart Van Assche wrote:
>>> The C programming language does not allow to use preprocessor statements
>>> inside macro arguments (pr_info() is defined as a macro). Hence rework
>>> the pr_info() statement in btrfs_print_mod_info() such that it becomes
>>> compliant. This patch allows tools like sparse to analyze the BTRFS
>>> source code.
>>>
>>> Fixes: 62e855771dac ("btrfs: convert printk(KERN_* to use pr_* calls")
>>> Signed-off-by: Bart Van Assche
>>> Cc: Jeff Mahoney
>>> Cc: David Sterba
>>> ---
>>> fs/btrfs/super.c | 6 +++---
>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
>>> index 81107ad49f3a..dd4980df5b8e 100644
>>> --- a/fs/btrfs/super.c
>>> +++ b/fs/btrfs/super.c
>>> @@ -2369,7 +2369,7 @@ static __cold void btrfs_interface_exit(void)
>>>
>>> static void __init btrfs_print_mod_info(void)
>>> {
>>> - pr_info("Btrfs loaded, crc32c=%s"
>>> + static const char fmt[] = KERN_INFO "Btrfs loaded, crc32c=%s"
>>> #ifdef CONFIG_BTRFS_DEBUG
>>> ", debug=on"
>>> #endif
>>> @@ -2382,8 +2382,8 @@ static void __init btrfs_print_mod_info(void)
>>> #ifdef CONFIG_BTRFS_FS_REF_VERIFY
>>> ", ref-verify=on"
>>> #endif
>>> - "\n",
>>> - crc32c_impl());
>>> + "\n";
>>> + printk(fmt, crc32c_impl());
>>
>> I'd rather not see more printk being added. Nothing prevents from having
>> the fmt string being passed to pr_info.
>
> So you mean to do
>
> + static const char fmt[] = "Btrfs loaded, crc32c=%s"
> + pr_info(fmt);
Pretty much, something along the lines of
pr_info(fmt, crc32c_impl).
printk requires having the KERN_INFO in the format string, which I see
no point in doing, correct me if I'm wrong?
>
> ?
>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] btrfs: Fix a C compliance issue
On Sat, Jun 16, 2018 at 01:28:13PM +0300, Nikolay Borisov wrote:
>
>
> On 16.06.2018 01:36, Bart Van Assche wrote:
> > The C programming language does not allow to use preprocessor statements
> > inside macro arguments (pr_info() is defined as a macro). Hence rework
> > the pr_info() statement in btrfs_print_mod_info() such that it becomes
> > compliant. This patch allows tools like sparse to analyze the BTRFS
> > source code.
> >
> > Fixes: 62e855771dac ("btrfs: convert printk(KERN_* to use pr_* calls")
> > Signed-off-by: Bart Van Assche
> > Cc: Jeff Mahoney
> > Cc: David Sterba
> > ---
> > fs/btrfs/super.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> > index 81107ad49f3a..dd4980df5b8e 100644
> > --- a/fs/btrfs/super.c
> > +++ b/fs/btrfs/super.c
> > @@ -2369,7 +2369,7 @@ static __cold void btrfs_interface_exit(void)
> >
> > static void __init btrfs_print_mod_info(void)
> > {
> > - pr_info("Btrfs loaded, crc32c=%s"
> > + static const char fmt[] = KERN_INFO "Btrfs loaded, crc32c=%s"
> > #ifdef CONFIG_BTRFS_DEBUG
> > ", debug=on"
> > #endif
> > @@ -2382,8 +2382,8 @@ static void __init btrfs_print_mod_info(void)
> > #ifdef CONFIG_BTRFS_FS_REF_VERIFY
> > ", ref-verify=on"
> > #endif
> > - "\n",
> > - crc32c_impl());
> > + "\n";
> > + printk(fmt, crc32c_impl());
>
> I'd rather not see more printk being added. Nothing prevents from having
> the fmt string being passed to pr_info.
So you mean to do
+ static const char fmt[] = "Btrfs loaded, crc32c=%s"
+ pr_info(fmt);
?
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 03/20] btrfs-progs: libbtrfsutil: Factor out btrfs_util_subvolume_info_fd()
Factor out main logic of btrfs_util_subvolume_info_fd().
This is a preparation work to relax the root privilege of this function.
No functional change happens.
Signed-off-by: Misono Tomohiro
---
libbtrfsutil/subvolume.c | 45 ++---
1 file changed, 26 insertions(+), 19 deletions(-)
diff --git a/libbtrfsutil/subvolume.c b/libbtrfsutil/subvolume.c
index 867b3e10..0d7ef5bf 100644
--- a/libbtrfsutil/subvolume.c
+++ b/libbtrfsutil/subvolume.c
@@ -295,8 +295,8 @@ PUBLIC enum btrfs_util_error
btrfs_util_subvolume_info(const char *path,
return err;
}
-PUBLIC enum btrfs_util_error btrfs_util_subvolume_info_fd(int fd, uint64_t id,
- struct
btrfs_util_subvolume_info *subvol)
+static enum btrfs_util_error get_subvolume_info_root(int fd, uint64_t id,
+struct
btrfs_util_subvolume_info *subvol)
{
struct btrfs_ioctl_search_args search = {
.key = {
@@ -310,27 +310,10 @@ PUBLIC enum btrfs_util_error
btrfs_util_subvolume_info_fd(int fd, uint64_t id,
.nr_items = 0,
},
};
- enum btrfs_util_error err;
size_t items_pos = 0, buf_off = 0;
bool need_root_item = true, need_root_backref = true;
int ret;
- if (id == 0) {
- err = btrfs_util_is_subvolume_fd(fd);
- if (err)
- return err;
-
- err = btrfs_util_subvolume_id_fd(fd, &id);
- if (err)
- return err;
- }
-
- if ((id < BTRFS_FIRST_FREE_OBJECTID && id != BTRFS_FS_TREE_OBJECTID) ||
- id > BTRFS_LAST_FREE_OBJECTID) {
- errno = ENOENT;
- return BTRFS_UTIL_ERROR_SUBVOLUME_NOT_FOUND;
- }
-
search.key.min_objectid = search.key.max_objectid = id;
if (subvol) {
@@ -400,6 +383,30 @@ PUBLIC enum btrfs_util_error
btrfs_util_subvolume_info_fd(int fd, uint64_t id,
return BTRFS_UTIL_OK;
}
+PUBLIC enum btrfs_util_error btrfs_util_subvolume_info_fd(int fd, uint64_t id,
+ struct
btrfs_util_subvolume_info *subvol)
+{
+ enum btrfs_util_error err;
+
+ if (id == 0) {
+ err = btrfs_util_is_subvolume_fd(fd);
+ if (err)
+ return err;
+
+ err = btrfs_util_subvolume_id_fd(fd, &id);
+ if (err)
+ return err;
+ }
+
+ if ((id < BTRFS_FIRST_FREE_OBJECTID && id != BTRFS_FS_TREE_OBJECTID) ||
+ id > BTRFS_LAST_FREE_OBJECTID) {
+ errno = ENOENT;
+ return BTRFS_UTIL_ERROR_SUBVOLUME_NOT_FOUND;
+ }
+
+ return get_subvolume_info_root(fd, id, subvol);
+}
+
PUBLIC enum btrfs_util_error btrfs_util_get_subvolume_read_only_fd(int fd,
bool
*read_only_ret)
{
--
2.14.4
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 10/20] btrfs-progs: sub list: Add -A option to output path in absolute path
By default, the printed path is relative to the specified path.
Let's add option to print absolute path with -A option.
[Example]
$ mkfs.btrfs -f $DEV
$ mount $DEV /mnt
$ btrfs subvolume create /mnt/AAA
$ btrfs subvolume list /mnt
ID 256 gen 6 top level 5 path AAA
$ btrfs subvolume list -A /mnt
ID 256 gen 6 top level 5 path /mnt/AAA
Signed-off-by: Misono Tomohiro
---
Documentation/btrfs-subvolume.asciidoc | 2 +
cmds-subvolume.c | 73 +-
2 files changed, 66 insertions(+), 9 deletions(-)
diff --git a/Documentation/btrfs-subvolume.asciidoc
b/Documentation/btrfs-subvolume.asciidoc
index 99fff977..fec4b769 100644
--- a/Documentation/btrfs-subvolume.asciidoc
+++ b/Documentation/btrfs-subvolume.asciidoc
@@ -148,6 +148,8 @@ list deleted subvolumes that are not yet cleaned.
Other;;
-t
print the result as a table.
+-A
+print path in absolute path.
Sorting;;
-G [+|-]
diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index 23596c17..ea341d50 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -1156,7 +1156,8 @@ static void get_subvols_info(struct subvol_list **subvols,
struct btrfs_list_filter_set_v2 *filter_set,
int fd,
int tree_id,
-size_t *capacity)
+size_t *capacity,
+const char *prefix)
{
struct btrfs_util_subvolume_iterator *iter;
enum btrfs_util_error err;
@@ -1206,7 +1207,10 @@ static void get_subvols_info(struct subvol_list
**subvols,
goto out;
}
- subvol.path = strdup(".");
+ if (prefix)
+ subvol.path = strdup(prefix);
+ else
+ subvol.path = strdup(".");
if (!filters_match(&subvol, filter_set)) {
free(subvol.path);
} else {
@@ -1231,6 +1235,29 @@ skip:
goto out;
}
+ if (prefix) {
+ char *temp = subvol.path;
+
+ subvol.path = malloc(strlen(prefix) +
+strlen(subvol.path) + 2);
+ if (!subvol.path) {
+ error("out of memory");
+ subvol.path = temp;
+ ret = -1;
+ goto out;
+ }
+
+ strcpy(subvol.path, prefix);
+ if (strlen(prefix) == 1) {
+ strcpy(subvol.path + 1, temp);
+ } else {
+ subvol.path[strlen(prefix)] = '/';
+ strcpy(subvol.path + strlen(prefix) + 1, temp);
+ }
+
+ free(temp);
+ }
+
if (!filters_match(&subvol, filter_set)) {
free(subvol.path);
} else {
@@ -1252,6 +1279,7 @@ out:
static struct subvol_list *btrfs_list_subvols(int fd,
int is_list_all,
+ int absolute_path,
const char *path,
struct btrfs_list_filter_set_v2
*filter_set)
{
@@ -1265,11 +1293,24 @@ static struct subvol_list *btrfs_list_subvols(int fd,
}
subvols->num = 0;
- if (is_list_all)
+ if (is_list_all) {
get_subvols_info(&subvols, filter_set, fd,
- BTRFS_FS_TREE_OBJECTID, &capacity);
- else
- get_subvols_info(&subvols, filter_set, fd, 0, &capacity);
+ BTRFS_FS_TREE_OBJECTID, &capacity, NULL);
+ } else {
+ char *fullpath;
+
+ fullpath = realpath(path, NULL);
+ if (!fullpath) {
+ error("cannot find real path for '%s': %m", path);
+ free_subvol_list(subvols);
+ return NULL;
+ }
+
+ get_subvols_info(&subvols, filter_set, fd, 0, &capacity,
+ (absolute_path ? fullpath : NULL));
+
+ free(fullpath);
+ }
return subvols;
}
@@ -1279,6 +1320,7 @@ static int btrfs_list_subvols_print_v2(int fd,
struct btrfs_list_comparer_set_v2 *comp_set,
enum btrfs_list_layout layout,
int is_list_all,
+ int absolute_path,
const char *path,
const char *raw_prefix)
{
@@ -1287,7 +1329,8 @@ static int btrfs_list_subvols_print_v2(int
[PATCH v2 00/20] btrfs-progs: Rework of "subvolume list/show" and relax the root privileges of them
Changelog v1 -> v2: generally update whole patch set, especially: - rebased to progs 4.17 - Improve error handling - Update man/help/commit message - Add/Update several options of sub list: -f ... follow mounted subvolumes -a ... remove meaningless filter -A ... print path in absolute path --nosort ... output results incrementally Please see below examples = github: https://github.com/t-msn/btrfs-progs/tree/rework-sub-list Hello, This series requires some new ioctls which are now in kernel 4.18-rc1. The aim of this series is to relax the root privileges of "sub list/show" while keeping as much output consistency between root and non-privileged user. For "subvolume list", default output has been changed from current btrfs-progs (in both old and new kernel) and some options are newly added. For "subvolume show", root's output is the same as before but there are some difference from non-privileged user's output. Please see below examples. * Behavior summary of new "sub list" - default (no option) - lists subvolumes below the specified path (inc. path itself) - If new ioctls exists - the path can be non-subvolume directory - non-privileged user can call it (subvolumes to which the user cannot access will be skipped) - -f - follow mounted subvolume below the specified path and list them too (only if it is the same filesystem) - -a - updated to remove filter. i.e. the output is the same as current progs without option (require root privileges) - -A - print path in absolute path -- nosort - output results incrementally without loading information to memory [Example] $ mkfs.btrfs -f $DEV $ mkfs.btrfs -f $DEV2 $ mount $DEV $MNT $ btrfs subvolume create $MNT/AAA $ btrfs subvolume create $MNT/BBB $ btrfs subvolume create $MNT/CCC $ btrfs subvolume create $MNT/DDD $ mkdir $MNT/AAA/bbb $ mkdir $MNT/AAA/ccc $ mkdir $MNT/AAA/other $ umount $MNT $ mount -o subvol=AAA $DEV $MNT $ mount -o subvol=BBB $DEV $MNT/bbb $ mount -o subvol=CCC $DEV $MNT/ccc $ mount -o $DEV2 $MNT/other $ btrfs subvolume list $MNT # print subvolumes below the path ID 256 gen 10 top level 5 path . $ btrfs subvolume list -A $MNT # print path in absolute path ID 256 gen 10 top level 5 path /mnt $ btrfs subvolume list -f $MNT # follow mounted subvolumes too ID 256 gen 10 top level 5 path . ID 258 gen 7 top level 5 path bbb ID 259 gen 8 top level 5 path ccc $ btrfs subvolume list -a $MNT # print all subvolumes in the fs. same output as progs<=4.17 without option ID 256 gen 10 top level 5 path AAA ID 258 gen 7 top level 5 path BBB ID 259 gen 8 top level 5 path CCC ID 260 gen 9 top level 5 path DDD More details are in each commit log. * Behavior summary of new "sub show" - No change for root's output - If new ioctls exists, non-privileged user can call it - In that case, path to be shown is absolute path (for root, it is relative to top-level subvolume) Also, snapshots to be shown are to which the user can access from current mount point. (for root, all snapshots in the fs) * Patch structure The first several patches update libbtrfsutil and the latter patches update sub list/show command. 1st patch is independent and updates man doc of btrfs-subvolume 2nd-6th update the libbtrfsutil using new ioctls: - Relax the privileges of following functions if kernel supports new ioctls and @top/@id is zero (i.e. the given path/fd is used instead of arbitrary subvolume id). - util_subvolume_info() - subvolume iterator related ones (util_subvolume_iterator_next() etc.) - For subvolume iterator, if kernel supports new ioctls and @top is zero, non-subvolume directory can be specified as a start point. Also, subvolume which cannot be accessed (either because of permission error or not found (may happen if other volume is mounted in the path) will be skipped for non-privileged user. - Code path of root and non-privileged user is different. While root uses TREE_SEARCH ioctl as before, non-privileged user uses newly added ioctls. However, There is only one exception and when subvolume iterator is created from non-subvolume directory, code path of both is the same (and thus both use new ioctls). 7th patch update the "sub list" to use libbtrfsutil (no behavior change yet) This is a copy of non-merged following patch originally written by Omar Sandoval: btrfs-progs: use libbtrfsutil for subvolume list [1] expect this commit keeps libbtrfs implementation which above commit tries to remove. (I suspect that the part of the reason that the original patch has not been merged is it removes libbtrfs and this commits modify this. but I'm completely fine with the original patch instead of this.) 8th-15th patch update the behavior of "sub list" 16th-17
[PATCH v2 04/20] btrfs-porgs: libbtrfsutil: Relax the privileges of util_subvolume_info()
This commit relaxes the privileges of util_subvolume_info() if kernel
supports new ioctl (BTRFS_IOC_GET_SUBVOL_INFO) and @id is zero
(i.e. when getting the information of the given path/fd).
For older kernel (< 4.18), the behavior is the same.
Signed-off-by: Misono Tomohiro
---
libbtrfsutil/btrfsutil.h | 7 +-
libbtrfsutil/errors.c| 4
libbtrfsutil/subvolume.c | 58
3 files changed, 68 insertions(+), 1 deletion(-)
diff --git a/libbtrfsutil/btrfsutil.h b/libbtrfsutil/btrfsutil.h
index 6d655f49..3f21d163 100644
--- a/libbtrfsutil/btrfsutil.h
+++ b/libbtrfsutil/btrfsutil.h
@@ -63,6 +63,8 @@ enum btrfs_util_error {
BTRFS_UTIL_ERROR_SYNC_FAILED,
BTRFS_UTIL_ERROR_START_SYNC_FAILED,
BTRFS_UTIL_ERROR_WAIT_SYNC_FAILED,
+ BTRFS_UTIL_ERROR_INVALID_ARGUMENT_FOR_USER,
+ BTRFS_UTIL_ERROR_GET_SUBVOL_INFO_FAILED,
};
/**
@@ -266,7 +268,10 @@ struct btrfs_util_subvolume_info {
* to check whether the subvolume exists; %BTRFS_UTIL_ERROR_SUBVOLUME_NOT_FOUND
* will be returned if it does not.
*
- * This requires appropriate privilege (CAP_SYS_ADMIN).
+ * This requires appropriate privilege (CAP_SYS_ADMIN) for kernel < 4.18.
+ * From kernel >= 4.18 which supports BTRFS_IOC_GET_SUGBVOL_INFO,
+ * non-privileged user with appropriate permission for @path can use this too
+ * (in that case @id must be zero).
*
* Return: %BTRFS_UTIL_OK on success, non-zero error code on failure.
*/
diff --git a/libbtrfsutil/errors.c b/libbtrfsutil/errors.c
index 634edc65..f196fa71 100644
--- a/libbtrfsutil/errors.c
+++ b/libbtrfsutil/errors.c
@@ -45,6 +45,10 @@ static const char * const error_messages[] = {
[BTRFS_UTIL_ERROR_SYNC_FAILED] = "Could not sync filesystem",
[BTRFS_UTIL_ERROR_START_SYNC_FAILED] = "Could not start filesystem
sync",
[BTRFS_UTIL_ERROR_WAIT_SYNC_FAILED] = "Could not wait for filesystem
sync",
+ [BTRFS_UTIL_ERROR_INVALID_ARGUMENT_FOR_USER] =
+ "Non-root user cannot specify subvolume id",
+ [BTRFS_UTIL_ERROR_GET_SUBVOL_INFO_FAILED] =
+ "Could not get subvolume information by BTRFS_IOC_GET_SUBVOL_INFO",
};
PUBLIC const char *btrfs_util_strerror(enum btrfs_util_error err)
diff --git a/libbtrfsutil/subvolume.c b/libbtrfsutil/subvolume.c
index 0d7ef5bf..e94c7079 100644
--- a/libbtrfsutil/subvolume.c
+++ b/libbtrfsutil/subvolume.c
@@ -31,6 +31,14 @@
#include "btrfsutil_internal.h"
+static bool is_root(void)
+{
+ uid_t uid;
+
+ uid = geteuid();
+ return (uid == 0);
+}
+
/*
* This intentionally duplicates btrfs_util_is_subvolume_fd() instead of
opening
* a file descriptor and calling it, because fstat() and fstatfs() don't accept
@@ -383,11 +391,61 @@ static enum btrfs_util_error get_subvolume_info_root(int
fd, uint64_t id,
return BTRFS_UTIL_OK;
}
+static enum btrfs_util_error get_subvolume_info_user(int fd,
+struct
btrfs_util_subvolume_info *subvol)
+{
+ struct btrfs_ioctl_get_subvol_info_args info;
+ int ret;
+
+ ret = ioctl(fd, BTRFS_IOC_GET_SUBVOL_INFO, &info);
+ if (ret < 0)
+ return BTRFS_UTIL_ERROR_GET_SUBVOL_INFO_FAILED;
+
+ subvol->id = info.treeid;
+ subvol->parent_id = info.parent_id;
+ subvol->dir_id = info.dirid;
+ subvol->flags = info.flags;
+ subvol->generation = info.generation;
+
+ memcpy(subvol->uuid, info.uuid, sizeof(subvol->uuid));
+ memcpy(subvol->parent_uuid, info.parent_uuid,
+ sizeof(subvol->parent_uuid));
+ memcpy(subvol->received_uuid, info.received_uuid,
+ sizeof(subvol->received_uuid));
+
+ subvol->ctransid = info.ctransid;
+ subvol->otransid = info.otransid;
+ subvol->stransid = info.stransid;
+ subvol->rtransid = info.rtransid;
+
+ subvol->ctime.tv_sec = info.ctime.sec;
+ subvol->ctime.tv_nsec = info.ctime.nsec;
+ subvol->otime.tv_sec = info.otime.sec;
+ subvol->otime.tv_nsec = info.otime.nsec;
+ subvol->stime.tv_sec = info.stime.sec;
+ subvol->stime.tv_nsec = info.stime.nsec;
+ subvol->rtime.tv_sec = info.rtime.sec;
+ subvol->rtime.tv_nsec = info.rtime.nsec;
+
+ return BTRFS_UTIL_OK;
+}
+
PUBLIC enum btrfs_util_error btrfs_util_subvolume_info_fd(int fd, uint64_t id,
struct
btrfs_util_subvolume_info *subvol)
{
enum btrfs_util_error err;
+ if (!is_root()) {
+ if (id != 0)
+ return BTRFS_UTIL_ERROR_INVALID_ARGUMENT_FOR_USER;
+
+ err = btrfs_util_is_subvolume_fd(fd);
+ if (err)
+ return err;
+
+ return get_subvolume_info_user(fd, subvol);
+ }
+
if (id == 0) {
err = btrfs_util_is_subvolume_fd(fd);
if (err)
--
2.
[PATCH v2 13/20] btrfs-progs: sub list: Update -a option and remove meaningless filter
Currently, -a option add filter and change subvolume path as follows:
- If a subvolume is a child of the specified path, nothing changes
- otherwise, adds to head
This is rather meaningless, so let's remove this filter.
As a result, the behavior of -a option becomes the same as
default behavior of sub list in progs <= 4.17
[Example]
$ mkfs.btrfs -f $DEV
$ mount $DEV /mnt
$ btrfs subvolume create /mnt/AAA
$ btrfs subvolume create /mnt/AAA/BBB
$ btrfs subvolume create /mnt/ZZZ
$ btrfs subvolume list -a /mnt
ID 256 gen 9 top level 5 path AAA
ID 257 gen 9 top level 256 path AAA/BBB
ID 258 gen 10 top level 5 path ZZZ
** output of progs <= 4.17
$ btrfs subvolume list -a /mnt
ID 256 gen 9 top level 5 path AAA
ID 257 gen 9 top level 256 path /AAA/BBB
ID 258 gen 10 top level 5 path ZZZ
Signed-off-by: Misono Tomohiro
---
Documentation/btrfs-subvolume.asciidoc | 6 --
cmds-subvolume.c | 35 --
2 files changed, 8 insertions(+), 33 deletions(-)
diff --git a/Documentation/btrfs-subvolume.asciidoc
b/Documentation/btrfs-subvolume.asciidoc
index cd91385a..20fae1e1 100644
--- a/Documentation/btrfs-subvolume.asciidoc
+++ b/Documentation/btrfs-subvolume.asciidoc
@@ -118,8 +118,10 @@ Path filtering;;
-o
print only subvolumes below specified .
-a
-print all the subvolumes in the filesystem and distinguish between
-absolute and relative path with respect to the given .
+print all the subvolumes in the filesystem, including subvolumes
+which cannot be accessed from current mount point.
+path to be shown is relative to the top-level subvolume
+(require root privileges).
-f
follow mounted subvolumes below recursively and list them too
(only if it is the same filesystem). If top-level subvolume is mounted
diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index 802f5c5e..dab266aa 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -759,28 +759,6 @@ static int filter_topid_equal(struct listed_subvol
*subvol, uint64_t data)
return subvol->info.parent_id == data;
}
-static int filter_full_path(struct listed_subvol *subvol, uint64_t data)
-{
- /*
-* This implements the same behavior as before the conversion to
-* libbtrfsutil, which is mostly nonsensical.
-*/
- if (subvol->info.parent_id != data) {
- char *tmp;
- int ret;
-
- ret = asprintf(&tmp, "/%s", subvol->path);
- if (ret == -1) {
- error("out of memory");
- exit(1);
- }
-
- free(subvol->path);
- subvol->path = tmp;
- }
- return 1;
-}
-
static int filter_by_parent(struct listed_subvol *subvol, uint64_t data)
{
return !uuid_compare(subvol->info.parent_uuid,
@@ -798,7 +776,6 @@ static btrfs_list_filter_func_v2 all_filter_funcs[] = {
[BTRFS_LIST_FILTER_CGEN_LESS] = filter_cgen_less,
[BTRFS_LIST_FILTER_CGEN_EQUAL] = filter_cgen_equal,
[BTRFS_LIST_FILTER_TOPID_EQUAL] = filter_topid_equal,
- [BTRFS_LIST_FILTER_FULL_PATH] = filter_full_path,
[BTRFS_LIST_FILTER_BY_PARENT] = filter_by_parent,
};
@@ -1574,9 +1551,9 @@ static const char * const cmd_subvol_list_usage[] = {
"",
"Path filtering:",
"-o print only subvolumes below specified path",
- "-a print all the subvolumes in the filesystem and",
- " distinguish absolute and relative path with respect",
- " to the given (require root privileges)",
+ "-a print all the subvolumes in the filesystem.",
+ " path to be shown is relative to the top-level",
+ " subvolume (require root privileges)",
"-f follow mounted subvolumes below the specified path",
" and list them too (only if it is the same filesystem)",
"",
@@ -1783,11 +1760,7 @@ static int cmd_subvol_list(int argc, char **argv)
if (ret)
goto out;
- if (is_list_all)
- btrfs_list_setup_filter_v2(&filter_set,
- BTRFS_LIST_FILTER_FULL_PATH,
- top_id);
- else if (is_only_in_path)
+ if (is_only_in_path)
btrfs_list_setup_filter_v2(&filter_set,
BTRFS_LIST_FILTER_TOPID_EQUAL,
top_id);
--
2.14.4
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 01/20] btrfs-progs: doc: Update man btrfs subvolume
Some information is obsolete and updated as follows:
- Add missing explanations of some options
- Remove outdated explanation of "subvolrootid" mount option
- Reorder/group options of "sub list" to corresponds help message
- Add explanation about different meaning of parent in "parent ID/UUID"
- Fix indent/misspelling
- Add missing comma
Signed-off-by: Misono Tomohiro
---
Documentation/btrfs-subvolume.asciidoc | 77 --
1 file changed, 55 insertions(+), 22 deletions(-)
diff --git a/Documentation/btrfs-subvolume.asciidoc
b/Documentation/btrfs-subvolume.asciidoc
index a8c4af4b..f3eb4e26 100644
--- a/Documentation/btrfs-subvolume.asciidoc
+++ b/Documentation/btrfs-subvolume.asciidoc
@@ -77,13 +77,16 @@ safely stored on the device.
`Options`
+
-c|--commit-after
-wait for transaction commit at the end of the operation
+wait for transaction commit at the end of the operation.
+
-C|--commit-each
-wait for transaction commit after deleting each subvolume
+wait for transaction commit after deleting each subvolume.
++
+-v|--verbose
+verbose output of operations.
*find-new* ::
-List the recently modified files in a subvolume, after ID.
+List the recently modified files in a subvolume, after generation.
*get-default* ::
Get the default subvolume of the filesystem .
@@ -93,40 +96,54 @@ The output format is similar to *subvolume list* command.
*list* [options] [-G [\+|-]] [-C [+|-]]
[--sort=rootid,gen,ogen,path] ::
List the subvolumes present in the filesystem .
+
-For every subvolume the following information is shown by default. +
-ID top level path +
-where path is the relative path of the subvolume to the top level subvolume.
+For every subvolume the following information is shown by default:
++
+ID gen top level path
++
+where ID is subvolume's id, gen is an internal counter which is updated
+every transaction, top level is the same as parent subvolume's id, and
+path is the relative path of the subvolume to the top level subvolume.
The subvolume's ID may be used by the subvolume set-default command,
or at mount time via the subvolid= option.
-If `-p` is given, then parent is added to the output between ID
-and top level. The parent's ID may be used at mount time via the
-`subvolrootid=` option.
+
`Options`
+
--p
-print parent ID.
+Path filtering;;
+-o
+print only subvolumes below specified .
-a
print all the subvolumes in the filesystem and distinguish between
absolute and relative path with respect to the given .
+
+Field selection;;
+-p
+print the parent ID
+('parent' here means the subvolume which contains this subvolume).
-c
print the ogeneration of the subvolume, aliases: ogen or origin generation.
-g
-print the generation of the subvolume.
--o
-print only subvolumes below specified .
+print the generation of the subvolume (default).
-u
print the UUID of the subvolume.
-q
-print the parent uuid of subvolumes (and snapshots).
+print the parent UUID of the subvolume
+('parent' here means subvolume of which this subvolume is a snapshot).
-R
-print the UUID of the sent subvolume, where the subvolume is the result of a
receive operation
--t
-print the result as a table.
+print the UUID of the sent subvolume, where the subvolume is the result of a
receive operation.
+
+Type filtering;;
-s
only snapshot subvolumes in the filesystem will be listed.
-r
only readonly subvolumes in the filesystem will be listed.
+-d
+list deleted subvolumes that are not yet cleaned.
+
+Other;;
+-t
+print the result as a table.
+
+Sorting;;
-G [+|-]
list subvolumes in the filesystem that its generation is
>=, \<= or = value. \'\+' means >= value, \'-' means \<= value, If there is
@@ -144,9 +161,9 @@ for --sort you can combine some items together by \',',
just like
*set-default* [| ]::
Set the default subvolume for the (mounted) filesystem.
-
++
Set the default subvolume for the (mounted) filesystem at . This will
hide
-the top-level subvolume (ie. the one mounted with 'subvol=/' or 'subvolid=5').
+the top-level subvolume (i.e. the one mounted with 'subvol=/' or 'subvolid=5').
Takes action on next mount.
+
There are two ways how to specify the subvolume, by or by the
@@ -154,10 +171,22 @@ path.
The id can be obtained from *btrfs subvolume list*, *btrfs subvolume show* or
*btrfs inspect-internal rootid*.
-*show* ::
+*show* [options] |::
Show information of a given subvolume in the .
++
+`Options`
++
+-r|--rootid
+rootid of the subvolume.
+-u|--uuid:::
+UUID of the subvolume.
+
++
+If no option is specified, subvolume information of is shown,
+otherwise the subvolume information of rootid or UUID in the filesystem
+is shown.
-*snapshot* [-r] |[/]::
+*snapshot* [-r|-i ] |[/]::
Create a snapshot of the subvolume with the
name in the directory.
+
@@ -168,6 +197,10 @@ If is not a subvolume, btrfs returns an error.
+
-r
Make the new snapsho
[PATCH v2 12/20] btrfs-progs: sub list: Add --nosort option to output incrementally without sort
Currently, "subvolume list" loads all the subvolume information into
memory first in order to sort them. This may cause a performance problem
if there are a lot of subvolumes.
This commit adds --nosort option to output subvolume information
incrementally without sort to avoid consuming memory.
Signed-off-by: Misono Tomohiro
---
Documentation/btrfs-subvolume.asciidoc | 5 ++
cmds-subvolume.c | 140 ++---
2 files changed, 100 insertions(+), 45 deletions(-)
diff --git a/Documentation/btrfs-subvolume.asciidoc
b/Documentation/btrfs-subvolume.asciidoc
index b2461398..cd91385a 100644
--- a/Documentation/btrfs-subvolume.asciidoc
+++ b/Documentation/btrfs-subvolume.asciidoc
@@ -170,6 +170,11 @@ you can add \'\+' or \'-' in front of each items, \'+'
means ascending,
+
for --sort you can combine some items together by \',', just like
--sort=+ogen,-gen,path,rootid.
++
+--nosort
+Output the results incrementally without sort. This avoids loading all
+subvolume information to memory and can be useful when there is a lot
+of subvolumes.
*set-default* [| ]::
Set the default subvolume for the (mounted) filesystem.
diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index 4ebe0377..802f5c5e 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -1036,6 +1036,23 @@ static void print_all_subvol_info_tab_head(void)
}
}
+static void print_one_subvol_info(struct listed_subvol *subvol,
+ enum btrfs_list_layout layout,
+ const char *raw_prefix)
+{
+ switch (layout) {
+ case BTRFS_LIST_LAYOUT_DEFAULT:
+ print_one_subvol_info_default(subvol);
+ break;
+ case BTRFS_LIST_LAYOUT_TABLE:
+ print_one_subvol_info_table(subvol);
+ break;
+ case BTRFS_LIST_LAYOUT_RAW:
+ print_one_subvol_info_raw(subvol, raw_prefix);
+ break;
+ }
+}
+
static void print_all_subvol_info(struct subvol_list *subvols,
enum btrfs_list_layout layout,
const char *raw_prefix)
@@ -1048,17 +1065,7 @@ static void print_all_subvol_info(struct subvol_list
*subvols,
for (i = 0; i < subvols->num; i++) {
struct listed_subvol *subvol = &subvols->subvols[i];
- switch (layout) {
- case BTRFS_LIST_LAYOUT_DEFAULT:
- print_one_subvol_info_default(subvol);
- break;
- case BTRFS_LIST_LAYOUT_TABLE:
- print_one_subvol_info_table(subvol);
- break;
- case BTRFS_LIST_LAYOUT_RAW:
- print_one_subvol_info_raw(subvol, raw_prefix);
- break;
- }
+ print_one_subvol_info(subvol, layout, raw_prefix);
}
}
@@ -1159,7 +1166,9 @@ static void get_subvols_info(struct subvol_list **subvols,
int tree_id,
size_t *capacity,
const char *prefix,
-int show_top)
+int show_top,
+enum btrfs_list_layout layout,
+const char *raw_prefix)
{
struct btrfs_util_subvolume_iterator *iter;
enum btrfs_util_error err;
@@ -1216,9 +1225,14 @@ static void get_subvols_info(struct subvol_list
**subvols,
if (!filters_match(&subvol, filter_set)) {
free(subvol.path);
} else {
- ret = add_subvol(subvols, &subvol, capacity);
- if (ret)
- goto out;
+ if (*subvols == NULL) {
+ print_one_subvol_info(&subvol,
+ layout, raw_prefix);
+ } else {
+ ret = add_subvol(subvols, &subvol, capacity);
+ if (ret)
+ goto out;
+ }
}
}
@@ -1263,9 +1277,14 @@ skip:
if (!filters_match(&subvol, filter_set)) {
free(subvol.path);
} else {
- ret = add_subvol(subvols, &subvol, capacity);
- if (ret)
- goto out;
+ if (*subvols == NULL) {
+ print_one_subvol_info(&subvol,
+ layout, raw_prefix);
+ } else {
+ ret = add_subvol(subvols, &subvol, capacity);
+ if (ret)
+ goto out;
+ }
}
}
@@ -1275,7 +
[PATCH v2 05/20] btrfs-progs: libbtrfsuitl: Factor out btrfs_util_subvolume_iterator_next()
Factor out the main logic of btrfs_util_subvolume_iterator_next().
This is a preparation work to relax the root privilege of this function.
No functional change happens.
Signed-off-by: Misono Tomohiro
---
libbtrfsutil/subvolume.c | 13 ++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/libbtrfsutil/subvolume.c b/libbtrfsutil/subvolume.c
index e94c7079..73471d4f 100644
--- a/libbtrfsutil/subvolume.c
+++ b/libbtrfsutil/subvolume.c
@@ -1255,9 +1255,9 @@ static enum btrfs_util_error build_subvol_path(struct
btrfs_util_subvolume_itera
return BTRFS_UTIL_OK;
}
-PUBLIC enum btrfs_util_error btrfs_util_subvolume_iterator_next(struct
btrfs_util_subvolume_iterator *iter,
- char **path_ret,
- uint64_t
*id_ret)
+static enum btrfs_util_error subvolume_iterator_next_root(struct
btrfs_util_subvolume_iterator *iter,
+ char **path_ret,
+ uint64_t *id_ret)
{
struct search_stack_entry *top;
const struct btrfs_ioctl_search_header *header;
@@ -1331,6 +1331,13 @@ out:
return BTRFS_UTIL_OK;
}
+PUBLIC enum btrfs_util_error btrfs_util_subvolume_iterator_next(struct
btrfs_util_subvolume_iterator *iter,
+ char **path_ret,
+ uint64_t
*id_ret)
+{
+ return subvolume_iterator_next_root(iter, path_ret, id_ret);
+}
+
PUBLIC enum btrfs_util_error btrfs_util_subvolume_iterator_next_info(struct
btrfs_util_subvolume_iterator *iter,
char
**path_ret,
struct
btrfs_util_subvolume_info *subvol)
--
2.14.4
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 02/20] btrfs-progs: ioctl/libbtrfsutil: Add 3 definitions of new unprivileged ioctl
Copy and add 3 definitions of new unprivileged ioctl
(BTRFS_IOC_GET_SUBVOL_INFO, BTRFS_IOC_GET_SUBVOL_ROOTREF and
BTRFS_IOC_INO_LOOKUP_USER) from kernel. They will be used to implement
the user version of "btrfs subvolume list/show" etc.
Signed-off-by: Misono Tomohiro
---
ioctl.h | 99
libbtrfsutil/btrfs.h | 97 ++
2 files changed, 196 insertions(+)
diff --git a/ioctl.h b/ioctl.h
index 709e996f..cce55dbd 100644
--- a/ioctl.h
+++ b/ioctl.h
@@ -320,6 +320,22 @@ struct btrfs_ioctl_ino_lookup_args {
};
BUILD_ASSERT(sizeof(struct btrfs_ioctl_ino_lookup_args) == 4096);
+#define BTRFS_INO_LOOKUP_USER_PATH_MAX (4080 - BTRFS_VOL_NAME_MAX - 1)
+struct btrfs_ioctl_ino_lookup_user_args {
+ /* in, inode number containing the subvolume of 'subvolid' */
+ __u64 dirid;
+ /* in */
+ __u64 treeid;
+ /* out, name of the subvolume of 'treeid' */
+ char name[BTRFS_VOL_NAME_MAX + 1];
+ /*
+* out, constructed path from the directory with which the ioctl is
+* called to dirid
+*/
+ char path[BTRFS_INO_LOOKUP_USER_PATH_MAX];
+};
+BUILD_ASSERT(sizeof(struct btrfs_ioctl_ino_lookup_user_args) == 4096);
+
struct btrfs_ioctl_search_key {
/* which root are we searching. 0 is the tree of tree roots */
__u64 tree_id;
@@ -672,6 +688,83 @@ BUILD_ASSERT(sizeof(struct btrfs_ioctl_send_args_64) ==
72);
#define BTRFS_IOC_SEND_64_COMPAT_DEFINED 1
+/*
+ * Information about a fs tree root.
+ *
+ * All items are filled by the ioctl
+ */
+struct btrfs_ioctl_get_subvol_info_args {
+ /* Id of this subvolume */
+ __u64 treeid;
+
+ /* Name of this subvolume, used to get the real name at mount point */
+ char name[BTRFS_VOL_NAME_MAX + 1];
+
+ /*
+* Id of the subvolume which contains this subvolume.
+* Zero for top-level subvolume or a deleted subvolume.
+*/
+ __u64 parent_id;
+
+ /*
+* Inode number of the directory which contains this subvolume.
+* Zero for top-level subvolume or a deleted subvolume
+*/
+ __u64 dirid;
+
+ /* Latest transaction id of this subvolume */
+ __u64 generation;
+
+ /* Flags of this subvolume */
+ __u64 flags;
+
+ /* UUID of this subvolume */
+ __u8 uuid[BTRFS_UUID_SIZE];
+
+ /*
+* UUID of the subvolume of which this subvolume is a snapshot.
+* All zero for a non-snapshot subvolume.
+*/
+ __u8 parent_uuid[BTRFS_UUID_SIZE];
+
+ /*
+* UUID of the subvolume from which this subvolume was received.
+* All zero for non-received subvolume.
+*/
+ __u8 received_uuid[BTRFS_UUID_SIZE];
+
+ /* Transaction id indicating when change/create/send/receive happened */
+ __u64 ctransid;
+ __u64 otransid;
+ __u64 stransid;
+ __u64 rtransid;
+ /* Time corresponding to c/o/s/rtransid */
+ struct btrfs_ioctl_timespec ctime;
+ struct btrfs_ioctl_timespec otime;
+ struct btrfs_ioctl_timespec stime;
+ struct btrfs_ioctl_timespec rtime;
+
+ /* Must be zero */
+ __u64 reserved[8];
+};
+
+#define BTRFS_MAX_ROOTREF_BUFFER_NUM 255
+struct btrfs_ioctl_get_subvol_rootref_args {
+ /* in/out, minimum id of rootref's treeid to be searched */
+ __u64 min_treeid;
+
+ /* out */
+ struct {
+ __u64 treeid;
+ __u64 dirid;
+ } rootref[BTRFS_MAX_ROOTREF_BUFFER_NUM];
+
+ /* out, number of found items */
+ __u8 num_items;
+ __u8 align[7];
+};
+BUILD_ASSERT(sizeof(struct btrfs_ioctl_get_subvol_rootref_args) == 4096);
+
/* Error codes as returned by the kernel */
enum btrfs_err_code {
notused,
@@ -828,6 +921,12 @@ static inline char *btrfs_err_str(enum btrfs_err_code
err_code)
struct btrfs_ioctl_feature_flags[3])
#define BTRFS_IOC_RM_DEV_V2_IOW(BTRFS_IOCTL_MAGIC, 58, \
struct btrfs_ioctl_vol_args_v2)
+#define BTRFS_IOC_GET_SUBVOL_INFO _IOR(BTRFS_IOCTL_MAGIC, 60, \
+ struct btrfs_ioctl_get_subvol_info_args)
+#define BTRFS_IOC_GET_SUBVOL_ROOTREF _IOWR(BTRFS_IOCTL_MAGIC, 61, \
+ struct btrfs_ioctl_get_subvol_rootref_args)
+#define BTRFS_IOC_INO_LOOKUP_USER _IOWR(BTRFS_IOCTL_MAGIC, 62, \
+ struct btrfs_ioctl_ino_lookup_user_args)
#ifdef __cplusplus
}
#endif
diff --git a/libbtrfsutil/btrfs.h b/libbtrfsutil/btrfs.h
index c293f6bf..95740de2 100644
--- a/libbtrfsutil/btrfs.h
+++ b/libbtrfsutil/btrfs.h
@@ -421,6 +421,21 @@ struct btrfs_ioctl_ino_lookup_args {
char name[BTRFS_INO_LOOKUP_PATH_MAX];
};
+#define BTRFS_INO_LOOKUP_USER_PATH_MAX (4080-BTRFS_VOL_NAME_MA
[PATCH v2 11/20] btrfs-progs: sub list: Add -f option to follow mounted subvolumes below the path
Add -f option to follow mounted subvolumes below the specified path, only
if it is the same filesystem.
[Example]
$ mkfs.btrfs -f $DEV
$ mkfs.btrfs -f $DEV2
$ mount $DEV /mnt
$ btrfs subvolume create /mnt/AAA
$ btrfs subvolume create /mnt/BBB
$ btrfs subvolume create /mnt/CCC
$ mkdir /mnt/AAA/bbb
$ mkdir /mnt/AAA/ccc
$ mkdir /mnt/AAA/other
$ umount /mnt
$ mount -o subvol=AAA $DEV /mnt
$ mount -o subvol=BBB $DEV /mnt/bbb
$ mount -o subvol=CCC $DEV /mnt/ccc
$ mount -o $DEV2 /mnt/other
$ btrfs subvolume list /mnt
ID 256 gen 9 top level 5 path .
$ btrfs subvolume list -f /mnt
ID 256 gen 9 top level 5 path .
ID 258 gen 7 top level 5 path bbb
ID 259 gen 8 top level 5 path ccc
Note that this option lists top-level subvolume if it is mounted in the
way.
Signed-off-by: Misono Tomohiro
---
Documentation/btrfs-subvolume.asciidoc | 4 ++
cmds-subvolume.c | 116 +++--
2 files changed, 113 insertions(+), 7 deletions(-)
diff --git a/Documentation/btrfs-subvolume.asciidoc
b/Documentation/btrfs-subvolume.asciidoc
index fec4b769..b2461398 100644
--- a/Documentation/btrfs-subvolume.asciidoc
+++ b/Documentation/btrfs-subvolume.asciidoc
@@ -120,6 +120,10 @@ print only subvolumes below specified .
-a
print all the subvolumes in the filesystem and distinguish between
absolute and relative path with respect to the given .
+-f
+follow mounted subvolumes below recursively and list them too
+(only if it is the same filesystem). If top-level subvolume is mounted
+in the way, it is also listed.
Field selection;;
-p
diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index ea341d50..4ebe0377 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -28,6 +28,7 @@
#include
#include
#include
+#include
#include
@@ -1157,7 +1158,8 @@ static void get_subvols_info(struct subvol_list **subvols,
int fd,
int tree_id,
size_t *capacity,
-const char *prefix)
+const char *prefix,
+int show_top)
{
struct btrfs_util_subvolume_iterator *iter;
enum btrfs_util_error err;
@@ -1195,7 +1197,7 @@ static void get_subvols_info(struct subvol_list **subvols,
ret = -1;
goto out;
}
- if (id == BTRFS_FS_TREE_OBJECTID) {
+ if (!show_top && id == BTRFS_FS_TREE_OBJECTID) {
/* Skip top level subvolume */
ret = 0;
goto skip;
@@ -1280,6 +1282,7 @@ out:
static struct subvol_list *btrfs_list_subvols(int fd,
int is_list_all,
int absolute_path,
+ int follow_mount,
const char *path,
struct btrfs_list_filter_set_v2
*filter_set)
{
@@ -1295,7 +1298,8 @@ static struct subvol_list *btrfs_list_subvols(int fd,
if (is_list_all) {
get_subvols_info(&subvols, filter_set, fd,
- BTRFS_FS_TREE_OBJECTID, &capacity, NULL);
+ BTRFS_FS_TREE_OBJECTID, &capacity, NULL,
+ false);
} else {
char *fullpath;
@@ -1307,8 +1311,92 @@ static struct subvol_list *btrfs_list_subvols(int fd,
}
get_subvols_info(&subvols, filter_set, fd, 0, &capacity,
- (absolute_path ? fullpath : NULL));
+ (absolute_path ? fullpath : NULL), false);
+ if (subvols == NULL) {
+ free(fullpath);
+ return NULL;
+ }
+
+ /* Follow mounted subvolumes below @path */
+ if (follow_mount) {
+ struct mntent *mnt;
+ FILE *f;
+ DIR *dirstream;
+ u8 fsid[BTRFS_FSID_SIZE];
+ u8 fsid2[BTRFS_FSID_SIZE];
+ char *c;
+ int fd2;
+ int ret;
+
+ ret = get_fsid(path, fsid, 0);
+ if (ret < 0) {
+ error("failed to get fsid: %m");
+ free(fullpath);
+ free_subvol_list(subvols);
+ return NULL;
+ }
+
+ f = setmntent("/proc/self/mounts", "r");
+ if (f == NULL) {
+ error("failed to read mount entry: %m");
+ free(fullpath);
+ free_subvol_list(subvols);
+
[PATCH v2 15/20] btrfs-progs: sub list: Update help message of -d option
Explicitly states that -d requires root privileges.
Also, update some option handling with regard to -d option.
Signed-off-by: Misono Tomohiro
---
Documentation/btrfs-subvolume.asciidoc | 3 ++-
cmds-subvolume.c | 8
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/Documentation/btrfs-subvolume.asciidoc
b/Documentation/btrfs-subvolume.asciidoc
index 0381c92c..2db1d479 100644
--- a/Documentation/btrfs-subvolume.asciidoc
+++ b/Documentation/btrfs-subvolume.asciidoc
@@ -149,7 +149,8 @@ only snapshot subvolumes in the filesystem will be listed.
-r
only readonly subvolumes in the filesystem will be listed.
-d
-list deleted subvolumes that are not yet cleaned.
+list deleted subvolumes that are not yet cleaned
+(require root privileges).
Other;;
-t
diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index 552c6dea..ef39789a 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -1569,6 +1569,7 @@ static const char * const cmd_subvol_list_usage[] = {
"-s list only snapshots",
"-r list readonly subvolumes (including snapshots)",
"-d list deleted subvolumes that are not yet cleaned",
+ " (require root privileges)",
"",
"Other:",
"-t print the result as a table",
@@ -1744,6 +1745,13 @@ static int cmd_subvol_list(int argc, char **argv)
goto out;
}
+ if (filter_set->only_deleted &&
+ (is_list_all || absolute_path || follow_mount)) {
+ ret = -1;
+ error("cannot use -d with -a/f/A option");
+ goto out;
+ }
+
subvol = argv[optind];
fd = btrfs_open_dir(subvol, &dirstream, 1);
if (fd < 0) {
--
2.14.4
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 06/20] btrfs-progs: libbtrfsutil: Relax the privileges of subvolume iterator
This commit relax the privileges of subvolume iterator when the kernel
supports new ioctls (BTRFS_IOC_GET_SUBVOL_ROOTREF and
BTRFS_IOC_INO_LOOKUP_USER).
For older kernel (< 4.18), the behavior is the same.
Non-privileged user can use subvolume iterator only if new ioctls are
supported and btrfs_util_create_subvolume_iterator() is called with @top
zero (i.e. if the iterator is created from given path/fd). In this case:
- an iterator can be created from non-subvolume directory
- subvolume will be skipped if
- it does not exist nor has different id from the found subvolume
by INO_LOOKUP_USER (may happen if a dir in the path is being mounted)
- it cannot be opened due to permission error
Note that this commit also allows root user to specify non-subvolume
direcotry when @top is zero. Only in that case, root's code path is
the same as user.
Signed-off-by: Misono Tomohiro
---
libbtrfsutil/btrfsutil.h | 18 ++-
libbtrfsutil/errors.c| 6 +
libbtrfsutil/subvolume.c | 380 +++
3 files changed, 377 insertions(+), 27 deletions(-)
diff --git a/libbtrfsutil/btrfsutil.h b/libbtrfsutil/btrfsutil.h
index 3f21d163..49454a63 100644
--- a/libbtrfsutil/btrfsutil.h
+++ b/libbtrfsutil/btrfsutil.h
@@ -65,6 +65,10 @@ enum btrfs_util_error {
BTRFS_UTIL_ERROR_WAIT_SYNC_FAILED,
BTRFS_UTIL_ERROR_INVALID_ARGUMENT_FOR_USER,
BTRFS_UTIL_ERROR_GET_SUBVOL_INFO_FAILED,
+ BTRFS_UTIL_ERROR_GET_SUBVOL_ROOTREF_FAILED,
+ BTRFS_UTIL_ERROR_INO_LOOKUP_USER_FAILED,
+ BTRFS_UTIL_ERROR_DUP_FAILED,
+ BTRFS_UTIL_ERROR_CHDIR_FAILED,
};
/**
@@ -510,6 +514,14 @@ struct btrfs_util_subvolume_iterator;
* @flags: Bitmask of BTRFS_UTIL_SUBVOLUME_ITERATOR_* flags.
* @ret: Returned iterator.
*
+ * Using subvolume iterator requires appropriate privilege (CAP_SYS_ADMIN) for
+ * kernel < 4.18. From kenrel >= 4.18 which supports
+ * BTRFS_IOC_GET_SUBVOL_ROOTREF and BTRFS_IOC_INO_LOOKUP_USER, non-previleged
+ * user can use it too (in that case @top must be zero). Also from kernel
+ * >=4.18, if @top is zero, the specified path can be non-subvolume directory
+ * and subvolumes which cannot be accessed will be skipped (either due to
+ * permission error or path is hidden by other mount).
+ *
* The returned iterator must be freed with
* btrfs_util_destroy_subvolume_iterator().
*
@@ -558,7 +570,8 @@ int btrfs_util_subvolume_iterator_fd(const struct
btrfs_util_subvolume_iterator
* Must be freed with free().
* @id_ret: Returned subvolume ID. May be %NULL.
*
- * This requires appropriate privilege (CAP_SYS_ADMIN).
+ * This requires appropriate privilege (CAP_SYS_ADMIN) for kernel < 4.18.
+ * See the comment of btrfs_util_create_subvolume_iterator()
*
* Return: %BTRFS_UTIL_OK on success, %BTRFS_UTIL_ERROR_STOP_ITERATION if there
* are no more subvolumes, non-zero error code on failure.
@@ -577,7 +590,8 @@ enum btrfs_util_error
btrfs_util_subvolume_iterator_next(struct btrfs_util_subvo
* This convenience function basically combines
* btrfs_util_subvolume_iterator_next() and btrfs_util_subvolume_info().
*
- * This requires appropriate privilege (CAP_SYS_ADMIN).
+ * This requires appropriate privilege (CAP_SYS_ADMIN) for kernel < 4.18.
+ * See the comment of btrfs_util_create_subvolume_iterator()
*
* Return: See btrfs_util_subvolume_iterator_next().
*/
diff --git a/libbtrfsutil/errors.c b/libbtrfsutil/errors.c
index f196fa71..c77407bf 100644
--- a/libbtrfsutil/errors.c
+++ b/libbtrfsutil/errors.c
@@ -49,6 +49,12 @@ static const char * const error_messages[] = {
"Non-root user cannot specify subvolume id",
[BTRFS_UTIL_ERROR_GET_SUBVOL_INFO_FAILED] =
"Could not get subvolume information by BTRFS_IOC_GET_SUBVOL_INFO",
+ [BTRFS_UTIL_ERROR_GET_SUBVOL_ROOTREF_FAILED] =
+ "Could not get rootref information by BTRRFS_IOC_GET_SUBVOL_ROOTREF",
+ [BTRFS_UTIL_ERROR_INO_LOOKUP_USER_FAILED] =
+ "Could not resolve subvolume path by BTRFS_IOC_INO_LOOKUP_USER",
+ [BTRFS_UTIL_ERROR_DUP_FAILED] = "Could not dup",
+ [BTRFS_UTIL_ERROR_CHDIR_FAILED] = "Could not chdir",
};
PUBLIC const char *btrfs_util_strerror(enum btrfs_util_error err)
diff --git a/libbtrfsutil/subvolume.c b/libbtrfsutil/subvolume.c
index 73471d4f..ae39d5c8 100644
--- a/libbtrfsutil/subvolume.c
+++ b/libbtrfsutil/subvolume.c
@@ -760,13 +760,21 @@ PUBLIC enum btrfs_util_error
btrfs_util_create_subvolume_fd(int parent_fd,
#define BTRFS_UTIL_SUBVOLUME_ITERATOR_CLOSE_FD (1 << 30)
struct search_stack_entry {
+ /* below are used for subvolume_iterator_next_user */
+ uint64_t id;
+ struct btrfs_ioctl_get_subvol_rootref_args rootref_args;
+ /* below is used for subvolume_iterator_next_root */
struct btrfs_ioctl_search_args search;
+ /* below are used for both */
size_t items_pos, buf_off;
size_t path_len;
};
struct btrfs_util_subvolume_iterator {
+
[PATCH v2 07/20] btrfs-progs: sub list: Use libbtrfsuitl for subvolume list
This is a copy of non-merged following patch originally written
by Omar Sandoval:
btrfs-progs: use libbtrfsutil for subvolume list
expect this commit keeps libbtrfs implementation which above commit
tries to remove (therefore this adds suffix _v2 for struct/function).
Original Author: Omar Sandoval
Signed-off-by: Misono Tomohiro
---
cmds-subvolume.c | 963 +--
1 file changed, 936 insertions(+), 27 deletions(-)
diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index 45363a5a..c54a8003 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -404,6 +404,915 @@ keep_fd:
return ret;
}
+#define BTRFS_LIST_NFILTERS_INCREASE (2 * BTRFS_LIST_FILTER_MAX)
+#define BTRFS_LIST_NCOMPS_INCREASE (2 * BTRFS_LIST_COMP_MAX)
+
+struct listed_subvol {
+ struct btrfs_util_subvolume_info info;
+ char *path;
+};
+
+struct subvol_list {
+ size_t num;
+ struct listed_subvol subvols[];
+};
+
+typedef int (*btrfs_list_filter_func_v2)(struct listed_subvol *, uint64_t);
+typedef int (*btrfs_list_comp_func_v2)(const struct listed_subvol *,
+ const struct listed_subvol *,
+ int);
+
+struct btrfs_list_filter_v2 {
+ btrfs_list_filter_func_v2 filter_func;
+ u64 data;
+};
+
+struct btrfs_list_comparer_v2 {
+ btrfs_list_comp_func_v2 comp_func;
+ int is_descending;
+};
+
+struct btrfs_list_filter_set_v2 {
+ int total;
+ int nfilters;
+ int only_deleted;
+ struct btrfs_list_filter_v2 filters[0];
+};
+
+struct btrfs_list_comparer_set_v2 {
+ int total;
+ int ncomps;
+ struct btrfs_list_comparer_v2 comps[0];
+};
+
+static struct {
+ char*name;
+ char*column_name;
+ int need_print;
+} btrfs_list_columns[] = {
+ {
+ .name = "ID",
+ .column_name= "ID",
+ .need_print = 0,
+ },
+ {
+ .name = "gen",
+ .column_name= "Gen",
+ .need_print = 0,
+ },
+ {
+ .name = "cgen",
+ .column_name= "CGen",
+ .need_print = 0,
+ },
+ {
+ .name = "parent",
+ .column_name= "Parent",
+ .need_print = 0,
+ },
+ {
+ .name = "top level",
+ .column_name= "Top Level",
+ .need_print = 0,
+ },
+ {
+ .name = "otime",
+ .column_name= "OTime",
+ .need_print = 0,
+ },
+ {
+ .name = "parent_uuid",
+ .column_name= "Parent UUID",
+ .need_print = 0,
+ },
+ {
+ .name = "received_uuid",
+ .column_name= "Received UUID",
+ .need_print = 0,
+ },
+ {
+ .name = "uuid",
+ .column_name= "UUID",
+ .need_print = 0,
+ },
+ {
+ .name = "path",
+ .column_name= "Path",
+ .need_print = 0,
+ },
+ {
+ .name = NULL,
+ .column_name= NULL,
+ .need_print = 0,
+ },
+};
+
+static btrfs_list_filter_func_v2 all_filter_funcs[];
+static btrfs_list_comp_func_v2 all_comp_funcs[];
+
+static void btrfs_list_setup_print_column_v2(enum btrfs_list_column_enum
column)
+{
+ int i;
+
+ ASSERT(0 <= column && column <= BTRFS_LIST_ALL);
+
+ if (column < BTRFS_LIST_ALL) {
+ btrfs_list_columns[column].need_print = 1;
+ return;
+ }
+
+ for (i = 0; i < BTRFS_LIST_ALL; i++)
+ btrfs_list_columns[i].need_print = 1;
+}
+
+static int comp_entry_with_rootid_v2(const struct listed_subvol *entry1,
+ const struct listed_subvol *entry2,
+ int is_descending)
+{
+ int ret;
+
+ if (entry1->info.id > entry2->info.id)
+ ret = 1;
+ else if (entry1->info.id < entry2->info.id)
+ ret = -1;
+ else
+ ret = 0;
+
+ return is_descending ? -ret : ret;
+}
+
+static int comp_entry_with_gen_v2(const struct listed_subvol *entry1,
+ const struct listed_subvol *entry2,
+ int is_descending)
+{
+ int ret;
+
+ if (entry1->info.generation > entry2->info.generation)
+ ret = 1;
+ else if (entry1->info.generation < entry2->info.generation)
+ ret = -1;
+ else
+ ret = 0;
+
+ return is_descending ? -ret : ret;
+}
+
+static int comp_entry_with_ogen_v2(const struct listed_subvol *entry1,
+
[PATCH v2 17/20] btrfs-progs: sub show: Allow non-privileged user to call "subvolume show"
Allow non-privileged user to call subvolume show if new ioctls
(BTRFS_IOC_GET_SUBVOL_INFO/BTRFS_IOC_GET_SUBVOL_ROOTREF,
BTRFS_IOC_INO_LOOKUP_USER, from kernel 4.18) are available.
Non-privileged user still cannot use -r or -u option.
The behavior for root user is the same as before.
There are some output differences between root and user:
root ... subvolume path is from top-level subvolume
list all snapshots in the fs (inc. non-accessible ones)
user ... subvolume path is absolute path
list snapshots under the mountpoint
(only to which the user has appropriate access right)
[Example]
$ sudo mkfs.btrfs -f $DEV
$ sudo mount $DEV /mnt
$ sudo btrfs subvolume create /mnt/AAA
$ sudo btrfs subvolume snapshot /mnt/AAA /mnt/snap1
$ sudo btrfs subvolume snapshot /mnt/AAA /mnt/AAA/snap2
$ sudo umount /mnt
$ sudo mount -o subvol=AAA $DEV /mnt
# root
$ sudo btrfs subvolume show /mnt
AAA
Name:AAA
UUID:15e80697-2ffb-0b4b-8e1e-e0873a7cf944
...
Snapshot(s):
AAA/snap2
snap1
# non-privileged user
$ btrfs subvolume show /mnt
/mnt
Name:AAA
UUID:15e80697-2ffb-0b4b-8e1e-e0873a7cf944
...
Snapshot(s):
/mnt/snap2
Signed-off-by: Misono Tomohiro
---
Documentation/btrfs-subvolume.asciidoc | 11 +++-
cmds-subvolume.c | 107 +
2 files changed, 105 insertions(+), 13 deletions(-)
diff --git a/Documentation/btrfs-subvolume.asciidoc
b/Documentation/btrfs-subvolume.asciidoc
index 2db1d479..fc57e6bc 100644
--- a/Documentation/btrfs-subvolume.asciidoc
+++ b/Documentation/btrfs-subvolume.asciidoc
@@ -194,12 +194,19 @@ The id can be obtained from *btrfs subvolume list*,
*btrfs subvolume show* or
*show* [options] |::
Show information of a given subvolume in the .
+
+This command had required root privileges. From kernel 4.18,
+non-privileged user can call this unless -r/-u option is not used.
+Note that for root, output path is relative to the top-level subvolume
+while absolute path is shown for non-privileged user.
+Also for root, snapshots filed lists all the snapshots in the fs while
+only snapshots under mount point are shown for non-privileged user.
++
`Options`
+
-r|--rootid
-rootid of the subvolume.
+rootid of the subvolume (require root privileges).
-u|--uuid:::
-UUID of the subvolume.
+UUID of the subvolume (require root privileges).
+
If no option is specified, subvolume information of is shown,
diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index ef39789a..4df418b0 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -2095,8 +2095,12 @@ static int cmd_subvol_find_new(int argc, char **argv)
static const char * const cmd_subvol_show_usage[] = {
"btrfs subvolume show [options] |",
"Show more information about the subvolume",
- "-r|--rootid rootid of the subvolume",
- "-u|--uuid uuid of the subvolume",
+ "",
+ "This command had required root privileges. From kernel 4.18,",
+ "non-privileged user can call this unless -r/-u option is not used.",
+ "",
+ "-r|--rootid rootid of the subvolume (require root privileges)",
+ "-u|--uuid uuid of the subvolume (require root privileges)",
"",
"If no option is specified, will be shown, otherwise",
"the rootid or uuid are resolved relative to the path.",
@@ -2109,8 +2113,10 @@ static int cmd_subvol_show(int argc, char **argv)
char uuidparse[BTRFS_UUID_UNPARSED_SIZE];
char *fullpath = NULL;
int fd = -1;
+ int fd_mnt = -1;
int ret = 1;
DIR *dirstream1 = NULL;
+ DIR *dirstream_mnt = NULL;
int by_rootid = 0;
int by_uuid = 0;
u64 rootid_arg = 0;
@@ -2118,7 +2124,10 @@ static int cmd_subvol_show(int argc, char **argv)
struct btrfs_util_subvolume_iterator *iter;
struct btrfs_util_subvolume_info subvol;
char *subvol_path = NULL;
+ char *subvol_name = NULL;
+ char *mount_point = NULL;
enum btrfs_util_error err;
+ bool root;
while (1) {
int c;
@@ -2155,6 +2164,12 @@ static int cmd_subvol_show(int argc, char **argv)
usage(cmd_subvol_show_usage);
}
+ root = is_root();
+ if (!root && (by_rootid || by_uuid)) {
+ error("only root can use -r or -u options");
+ return -1;
+ }
+
fullpath = realpath(argv[optind], NULL);
if (!fullpath) {
error("cannot find real path for '%s': %m", argv[optind]);
@@ -2209,19 +2224,53 @@ static int cmd_subvol_show(int argc, char **argv)
goto out;
}
- err = btrfs_util_subvolume_path_fd(fd, subvol.id, &subvol_path);
- if (err) {
- error_btrfs_util(err);
- g
[PATCH v2 09/20] btrfs-progs: sub list: Change the default behavior of "subvolume list" and allow non-privileged user to call it
Change the default behavior of "subvolume list" and allow non-privileged
user to call it as well.
>From this commit, by default it only lists subvolumes under the specified
path (incl. the path itself except top-level subvolume). Also, if kernel
supports new ioctls (BTRFS_IOC_GET_SUBVOL_INFO/BTRFS_IOC_GET_SUBVOL_ROOTREF
and BTRFS_IOC_INO_LOOKUP_USER, which are avilable from 4.18),
- the specified path can be non-subvolume directory.
- non-privileged user can also call it (subvolumes which the user
does not have access right will be skiped).
Note that root user can list all the subvolume in the fs with -a option.
[Example]
$ mkfs.btrfs -f $DEV
$ mount $DEV /mnt
$ btrfs subvolume create /mnt/AAA
$ btrfs subvolume create /mnt/AAA/BBB
$ mkdir /mnt/AAA/BBB/dir
$ btrfs subvolume create /mnt/AAA/BBB/dir/CCC
$ btrfs subvolume create /mnt/ZZZ
$ umount /mnt
$ mount -o subvol=AAA $DEV /mnt
$ btrfs subvolume list /mnt
ID 256 gen 11 top level 5 path .
ID 257 gen 8 top level 256 path BBB
ID 258 gen 8 top level 257 path BBB/dir/CCC
$ btrfs subvolume list /mnt/BBB
ID 257 gen 8 top level 256 path .
ID 258 gen 8 top level 257 path dir/CCC
$ btrfs subvolume list /mnt/BBB/dir
ID 258 gen 8 top level 257 path CCC
** output of progs <= 4.17
$ mount -o subvol=AAA $DEV /mnt
$ btrfs subvolume list /mnt
ID 256 gen 11 top level 5 path AAA
ID 257 gen 8 top level 256 path BBB
ID 258 gen 8 top level 257 path BBB/dir/CCC
ID 259 gen 11 top level 256 path ZZZ
Signed-off-by: Misono Tomohiro
---
Documentation/btrfs-subvolume.asciidoc | 8 +-
cmds-subvolume.c | 144 +
2 files changed, 119 insertions(+), 33 deletions(-)
diff --git a/Documentation/btrfs-subvolume.asciidoc
b/Documentation/btrfs-subvolume.asciidoc
index f3eb4e26..99fff977 100644
--- a/Documentation/btrfs-subvolume.asciidoc
+++ b/Documentation/btrfs-subvolume.asciidoc
@@ -95,6 +95,12 @@ The output format is similar to *subvolume list* command.
*list* [options] [-G [\+|-]] [-C [+|-]]
[--sort=rootid,gen,ogen,path] ::
List the subvolumes present in the filesystem .
+By default, this only lists the subvolumes under ,
+including itself (except top-level subvolume).
++
+This command had required root privileges. From kernel 4.18,
+non privileged user can call this too. Also from kernel 4.18,
+It is possible to specify non-subvolume directory as .
+
For every subvolume the following information is shown by default:
+
@@ -102,7 +108,7 @@ ID gen top level path
+
where ID is subvolume's id, gen is an internal counter which is updated
every transaction, top level is the same as parent subvolume's id, and
-path is the relative path of the subvolume to the top level subvolume.
+path is the relative path of the subvolume to the specified path.
The subvolume's ID may be used by the subvolume set-default command,
or at mount time via the subvolid= option.
+
diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index 279a6e51..23596c17 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -1125,9 +1125,37 @@ out:
return subvols;
}
+static int add_subvol(struct subvol_list **subvols,
+ struct listed_subvol *subvol,
+ size_t *capacity)
+{
+ if ((*subvols)->num >= *capacity) {
+ struct subvol_list *new_subvols;
+ size_t new_capacity = max_t(size_t, 1, *capacity * 2);
+
+ new_subvols = realloc(*subvols,
+ sizeof(*new_subvols) +
+ new_capacity *
+ sizeof(new_subvols->subvols[0]));
+ if (!new_subvols) {
+ error("out of memory");
+ return -1;
+ }
+
+ *subvols = new_subvols;
+ *capacity = new_capacity;
+ }
+
+ (*subvols)->subvols[(*subvols)->num] = *subvol;
+ (*subvols)->num++;
+
+ return 0;
+}
+
static void get_subvols_info(struct subvol_list **subvols,
struct btrfs_list_filter_set_v2 *filter_set,
int fd,
+int tree_id,
size_t *capacity)
{
struct btrfs_util_subvolume_iterator *iter;
@@ -1135,7 +1163,7 @@ static void get_subvols_info(struct subvol_list **subvols,
int ret = -1;
err = btrfs_util_create_subvolume_iterator_fd(fd,
- BTRFS_FS_TREE_OBJECTID, 0,
+ tree_id, 0,
&iter);
if (err) {
iter = NULL;
@@ -1143,6 +1171,52 @@ static void get_subvols_info(struct subvol_list
**subvols,
goto out;
}
+ /*
+* Subvolume iterator does not include the information of the
+* specified path/fd. So, add it first.
+
[PATCH v2 14/20] btrfs-progs: sub list: Update help message of -o option
Currently "sub list -o" lists only child subvolumes of the specified
path. So, update help message and variable name more appropriately.
Signed-off-by: Misono Tomohiro
---
Documentation/btrfs-subvolume.asciidoc | 2 +-
cmds-subvolume.c | 10 +-
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/Documentation/btrfs-subvolume.asciidoc
b/Documentation/btrfs-subvolume.asciidoc
index 20fae1e1..0381c92c 100644
--- a/Documentation/btrfs-subvolume.asciidoc
+++ b/Documentation/btrfs-subvolume.asciidoc
@@ -116,7 +116,7 @@ or at mount time via the subvolid= option.
+
Path filtering;;
-o
-print only subvolumes below specified .
+print only subvolumes which the subvolume of contains.
-a
print all the subvolumes in the filesystem, including subvolumes
which cannot be accessed from current mount point.
diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index dab266aa..552c6dea 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -1550,7 +1550,7 @@ static const char * const cmd_subvol_list_usage[] = {
"It is possible to specify non-subvolume directory as .",
"",
"Path filtering:",
- "-o print only subvolumes below specified path",
+ "-o print only subvolumes which the subvolume of
contains",
"-a print all the subvolumes in the filesystem.",
" path to be shown is relative to the top-level",
" subvolume (require root privileges)",
@@ -1605,7 +1605,7 @@ static int cmd_subvol_list(int argc, char **argv)
int follow_mount = 0;
int sort = 0;
int no_sort = 0;
- int is_only_in_path = 0;
+ int is_only_child = 0;
int absolute_path = 0;
DIR *dirstream = NULL;
enum btrfs_list_layout layout = BTRFS_LIST_LAYOUT_DEFAULT;
@@ -1651,7 +1651,7 @@ static int cmd_subvol_list(int argc, char **argv)
btrfs_list_setup_print_column_v2(BTRFS_LIST_GENERATION);
break;
case 'o':
- is_only_in_path = 1;
+ is_only_child = 1;
break;
case 't':
layout = BTRFS_LIST_LAYOUT_TABLE;
@@ -1732,7 +1732,7 @@ static int cmd_subvol_list(int argc, char **argv)
goto out;
}
- if (follow_mount && (is_list_all || is_only_in_path)) {
+ if (follow_mount && (is_list_all || is_only_child)) {
ret = -1;
error("cannot use -f with -a or -o option");
goto out;
@@ -1760,7 +1760,7 @@ static int cmd_subvol_list(int argc, char **argv)
if (ret)
goto out;
- if (is_only_in_path)
+ if (is_only_child)
btrfs_list_setup_filter_v2(&filter_set,
BTRFS_LIST_FILTER_TOPID_EQUAL,
top_id);
--
2.14.4
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 18/20] btrfs-progs: test: Add helper function to check if test user exists
Test user 'progs-test' will be used to test the behavior of normal user.
In order to pass this check, add the user by "useradd -M progs-test".
Note that progs-test should not have root privileges.
Signed-off-by: Misono Tomohiro
---
tests/common | 10 ++
1 file changed, 10 insertions(+)
diff --git a/tests/common b/tests/common
index 7e4e09df..7a37a4cd 100644
--- a/tests/common
+++ b/tests/common
@@ -314,6 +314,16 @@ check_global_prereq()
fi
}
+check_testuser()
+{
+ id -u progs-test > /dev/null 2>&1
+ if [ $? -ne 0 ]; then
+ _not_run "Need to add user \"progs-test\""
+ fi
+ # Note that progs-test should not have root privileges
+ # otherwise test may not run as expected
+}
+
check_image()
{
local image
--
2.14.4
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 20/20] btrfs-progs: test: Add cli-test/010 to check "subvolume list -f" option
Signed-off-by: Misono Tomohiro
---
tests/cli-tests/010-subvolume-list-follow/test.sh | 86 +++
1 file changed, 86 insertions(+)
create mode 100755 tests/cli-tests/010-subvolume-list-follow/test.sh
diff --git a/tests/cli-tests/010-subvolume-list-follow/test.sh
b/tests/cli-tests/010-subvolume-list-follow/test.sh
new file mode 100755
index ..8fb746c6
--- /dev/null
+++ b/tests/cli-tests/010-subvolume-list-follow/test.sh
@@ -0,0 +1,86 @@
+#!/bin/bash
+# test for "subvolume list -f"
+
+source "$TEST_TOP/common"
+
+check_prereq mkfs.btrfs
+check_prereq btrfs
+
+setup_root_helper
+setup_loopdevs 2
+prepare_loopdevs
+dev1=${loopdevs[1]}
+dev2=${loopdevs[2]}
+
+# test if the ids returned by "sub list" match expected ids
+# $1 ... option for subvolume list
+# $2 ... PATH to be specified by sub list command
+# $3~ ... expected return ids
+test_list()
+{
+ result=$(run_check_stdout $SUDO "$TOP/btrfs" subvolume list $1 "$2" | \
+ awk '{print $2}' | xargs | sort -n)
+
+ shift
+ shift
+ expected=($(echo "$@" | tr " " "\n" | sort -n))
+ expected=$(IFS=" "; echo "${expected[*]}")
+
+ if [ "$result" != "$expected" ]; then
+ echo "result : $result"
+ echo "expected: $expected"
+ _fail "ids returned by sub list does not match expected ids"
+ fi
+}
+
+run_check $SUDO_HELPER "$TOP/mkfs.btrfs" -f "$dev1"
+run_check $SUDO_HELPER "$TOP/mkfs.btrfs" -f "$dev2"
+
+run_check $SUDO_HELPER mount "$dev1" "$TEST_MNT"
+cd "$TEST_MNT"
+
+# Create some subvolumes and directories
+# (id 5)
+# |- AAA (id 256)
+# | |- top
+# | |- bbb
+# | -- ccc
+# |
+# |- BBB (id 258)
+# -- CCC (id 259)
+run_check $SUDO_HELPER "$TOP/btrfs" subvolume create AAA
+run_check $SUDO_HELPER "$TOP/btrfs" subvolume create BBB
+run_check $SUDO_HELPER "$TOP/btrfs" subvolume create CCC
+run_check $SUDO_HELPER mkdir AAA/top
+run_check $SUDO_HELPER mkdir AAA/bbb
+run_check $SUDO_HELPER mkdir AAA/ccc
+
+test_list "" "." "256 258 259"
+test_list "-f" "." "256 258 259"
+cd ..
+run_check $SUDO_HELPER umount "$TEST_MNT"
+
+# Mount as follows:
+#
+# "TEST_MNT" (AAA)
+# |- top (FS_TREE)
+# | |- AAA
+# | |- BBB
+# | -- CCC
+# |
+# |- bbb (BBB)
+# -- ccc (CCC)
+run_check $SUDO_HELPER mount -o subvol=AAA "$dev1" "$TEST_MNT"
+run_check $SUDO_HELPER mount "$dev1" "$TEST_MNT/top"
+run_check $SUDO_HELPER mount -o subvol=BBB "$dev1" "$TEST_MNT/bbb"
+run_check $SUDO_HELPER mount -o subvol=CCC "$dev1" "$TEST_MNT/ccc"
+
+cd "$TEST_MNT"
+test_list "" "." "256"
+# With -f option, subvolume AAA/BBB/CCC will be counted twice.
+# Also, it will list FS_TREE (5) if it is mounted below the specified path.
+test_list "-f" "." "5 256 256 258 258 259 259"
+
+cd ..
+run_check $SUDO_HELPER umount -R "$TEST_MNT"
+cleanup_loopdevs
--
2.14.4
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 19/20] btrfs-porgs: test: Add cli-test/009 to check subvolume list for both root and normal user
Signed-off-by: Misono Tomohiro
---
tests/cli-tests/009-subvolume-list/test.sh | 134 +
1 file changed, 134 insertions(+)
create mode 100755 tests/cli-tests/009-subvolume-list/test.sh
diff --git a/tests/cli-tests/009-subvolume-list/test.sh
b/tests/cli-tests/009-subvolume-list/test.sh
new file mode 100755
index ..bf6c31d6
--- /dev/null
+++ b/tests/cli-tests/009-subvolume-list/test.sh
@@ -0,0 +1,134 @@
+#!/bin/bash
+# test for "subvolume list" both for root and normal user
+
+source "$TEST_TOP/common"
+
+check_testuser
+check_prereq mkfs.btrfs
+check_prereq btrfs
+
+setup_root_helper
+prepare_test_dev
+
+
+# test if the ids returned by "sub list" match expected ids
+# $1 ... indicate run as root or test user
+# $2 ... PATH to be specified by sub list command
+# $3~ ... expected return ids
+test_list()
+{
+ local SUDO
+ if [ $1 -eq 1 ]; then
+ SUDO=$SUDO_HELPER
+ else
+ SUDO="sudo -u progs-test"
+ fi
+
+ result=$(run_check_stdout $SUDO "$TOP/btrfs" subvolume list "$2" | \
+ awk '{print $2}' | xargs | sort -n)
+
+ shift
+ shift
+ expected=($(echo "$@" | tr " " "\n" | sort -n))
+ expected=$(IFS=" "; echo "${expected[*]}")
+
+ if [ "$result" != "$expected" ]; then
+ echo "result : $result"
+ echo "expected: $expected"
+ _fail "ids returned by sub list does not match expected ids"
+ fi
+}
+
+run_check $SUDO_HELPER "$TOP/mkfs.btrfs" -f "$TEST_DEV"
+run_check_mount_test_dev
+cd "$TEST_MNT"
+
+# create subvolumes and directories and make some non-readable
+# by user 'progs-test'
+run_check $SUDO_HELPER "$TOP/btrfs" subvolume create sub1
+run_check $SUDO_HELPER "$TOP/btrfs" subvolume create sub1/subsub1
+run_check $SUDO_HELPER mkdir sub1/dir
+
+run_check $SUDO_HELPER "$TOP/btrfs" subvolume create sub2
+run_check $SUDO_HELPER mkdir -p sub2/dir/dirdir
+run_check $SUDO_HELPER "$TOP/btrfs" subvolume create sub2/dir/subsub2
+run_check $SUDO_HELPER "$TOP/btrfs" subvolume create sub2/dir/dirdir/subsubX
+
+run_check $SUDO_HELPER "$TOP/btrfs" subvolume create sub3
+run_check $SUDO_HELPER "$TOP/btrfs" subvolume create sub3/subsub3
+run_check $SUDO_HELPER mkdir sub3/dir
+run_check $SUDO_HELPER "$TOP/btrfs" subvolume create sub3/dir/subsubY
+run_check $SUDO_HELPER chmod o-r sub3
+
+run_check $SUDO_HELPER "$TOP/btrfs" subvolume create sub4
+run_check $SUDO_HELPER "$TOP/btrfs" subvolume create sub4/subsub4
+run_check $SUDO_HELPER mkdir sub4/dir
+run_check $SUDO_HELPER "$TOP/btrfs" subvolume create sub4/dir/subsubZ
+run_check $SUDO_HELPER setfacl -m u:progs-test:- sub4/dir
+
+run_check $SUDO_HELPER touch "file"
+
+# expected result for root at mount point:
+#
+# ID 256 gen 8 top level 5 path sub1
+# ID 258 gen 7 top level 256 path sub1/subsub1
+# ID 259 gen 10 top level 5 path sub2
+# ID 260 gen 9 top level 259 path sub2/dir/subsub2
+# ID 261 gen 10 top level 259 path sub2/dir/dirdir/subsubX
+# ID 262 gen 14 top level 5 path sub3
+# ID 263 gen 12 top level 262 path sub3/subsub3
+# ID 264 gen 13 top level 262 path sub3/dir/subsubY
+# ID 265 gen 17 top level 5 path sub4
+# ID 266 gen 15 top level 265 path sub4/subsub4
+# ID 267 gen 16 top level 265 path sub4/dir/subsubZ
+
+# check for root for both absolute/relative path
+all=(256 258 259 260 261 262 263 264 265 266 267)
+test_list 1 "$TEST_MNT" "${all[@]}"
+test_list 1 "$TEST_MNT/sub1" "256 258"
+test_list 1 "$TEST_MNT/sub1/dir" ""
+test_list 1 "$TEST_MNT/sub2" "259 260 261"
+test_list 1 "$TEST_MNT/sub2/dir" "260 261"
+test_list 1 "$TEST_MNT/sub3" "262 263 264"
+test_list 1 "$TEST_MNT/sub4" "265 266 267"
+run_mustfail "should fail for file" \
+ $SUDO_HELPER "$TOP/btrfs" subvolume list "$TEST_MNT/file"
+
+test_list 1 "." "${all[@]}"
+test_list 1 "sub1" "256 258"
+test_list 1 "sub1/dir" ""
+test_list 1 "sub2" "259 260 261"
+test_list 1 "sub2/dir" "260 261"
+test_list 1 "sub3" "262 263 264"
+test_list 1 "sub4" "265 266 267"
+run_mustfail "should fail for file" \
+ $SUDO_HELPER "$TOP/btrfs" subvolume list "file"
+
+# check for normal user for both absolute/relative path
+test_list 0 "$TEST_MNT" "256 258 259 260 261 265 266"
+test_list 0 "$TEST_MNT/sub1" "256 258"
+test_list 0 "$TEST_MNT/sub1/dir" ""
+test_list 0 "$TEST_MNT/sub2" "259 260 261"
+test_list 0 "$TEST_MNT/sub2/dir" "260 261"
+run_mustfail "should raise permission error" \
+ sudo -u progs-test "$TOP/btrfs" subvolume list "$TEST_MNT/sub3"
+test_list 0 "$TEST_MNT/sub4" "265 266"
+run_mustfail "should raise permission error" \
+ sudo -u progs-test "$TOP/btrfs" subvolume list "$TEST_MNT/sub4/dir"
+run_mustfail "should fail for file" \
+ sudo -u progs-test "$TOP/btrfs" subvolume list "$TEST_MNT/file"
+
+test_list 0 "." "256 258 259 260 261 265 266"
+test_list 0 "sub1/dir" ""
+test_list 0 "sub2" "259 260 261"
+test_list 0 "sub2/dir" "260 261"
+run_mustfail "should raise permission error" \
+ sudo -u progs-te
[PATCH v2 16/20] btrfs-progs: utils: Fallback to open without O_NOATIME flag in find_mount_root():
O_NOATIME flag requires effective UID of process matches file's owner or has CAP_FOWNER capabilities. Fallback to open without O_NOATIME flag so that non-privileged user can also call find_mount_root(). This is a preparation work to allow non-privileged user to call "subvolume show". Signed-off-by: Misono Tomohiro --- utils.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/utils.c b/utils.c index 21de09d3..1a4366e8 100644 --- a/utils.c +++ b/utils.c @@ -2048,6 +2048,9 @@ int find_mount_root(const char *path, char **mount_root) char *longest_match = NULL; fd = open(path, O_RDONLY | O_NOATIME); + if (fd < 0 && errno == EPERM) + fd = open(path, O_RDONLY); + if (fd < 0) return -errno; close(fd); -- 2.14.4 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 08/20] btrfs-progs: sub list: factor out main part of btrfs_list_subvols
No functional changes.
This is a preparation work for reworking "subvolume list".
Signed-off-by: Misono Tomohiro
---
cmds-subvolume.c | 50 ++
1 file changed, 30 insertions(+), 20 deletions(-)
diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index c54a8003..279a6e51 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -1125,22 +1125,15 @@ out:
return subvols;
}
-static struct subvol_list *btrfs_list_subvols(int fd,
- struct btrfs_list_filter_set_v2
*filter_set)
+static void get_subvols_info(struct subvol_list **subvols,
+struct btrfs_list_filter_set_v2 *filter_set,
+int fd,
+size_t *capacity)
{
- struct subvol_list *subvols;
- size_t capacity = 0;
struct btrfs_util_subvolume_iterator *iter;
enum btrfs_util_error err;
int ret = -1;
- subvols = malloc(sizeof(*subvols));
- if (!subvols) {
- error("out of memory");
- return NULL;
- }
- subvols->num = 0;
-
err = btrfs_util_create_subvolume_iterator_fd(fd,
BTRFS_FS_TREE_OBJECTID, 0,
&iter);
@@ -1168,11 +1161,11 @@ static struct subvol_list *btrfs_list_subvols(int fd,
continue;
}
- if (subvols->num >= capacity) {
+ if ((*subvols)->num >= *capacity) {
struct subvol_list *new_subvols;
- size_t new_capacity = max_t(size_t, 1, capacity * 2);
+ size_t new_capacity = max_t(size_t, 1, *capacity * 2);
- new_subvols = realloc(subvols,
+ new_subvols = realloc(*subvols,
sizeof(*new_subvols) +
new_capacity *
sizeof(new_subvols->subvols[0]));
@@ -1181,12 +1174,12 @@ static struct subvol_list *btrfs_list_subvols(int fd,
goto out;
}
- subvols = new_subvols;
- capacity = new_capacity;
+ *subvols = new_subvols;
+ *capacity = new_capacity;
}
- subvols->subvols[subvols->num] = subvol;
- subvols->num++;
+ (*subvols)->subvols[(*subvols)->num] = subvol;
+ (*subvols)->num++;
}
ret = 0;
@@ -1194,9 +1187,26 @@ out:
if (iter)
btrfs_util_destroy_subvolume_iterator(iter);
if (ret) {
- free_subvol_list(subvols);
- subvols = NULL;
+ free_subvol_list(*subvols);
+ *subvols = NULL;
+ }
+}
+
+static struct subvol_list *btrfs_list_subvols(int fd,
+ struct btrfs_list_filter_set_v2
*filter_set)
+{
+ struct subvol_list *subvols;
+ size_t capacity = 0;
+
+ subvols = malloc(sizeof(*subvols));
+ if (!subvols) {
+ error("out of memory");
+ return NULL;
}
+ subvols->num = 0;
+
+ get_subvols_info(&subvols, filter_set, fd, &capacity);
+
return subvols;
}
--
2.14.4
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
