Re: [PATCH V4 5/7] Btrfs-progs: restructure list_subvolumes

2012-09-20 Thread David Sterba
On Tue, Sep 18, 2012 at 07:06:49PM +0800, Miao Xie wrote:
 The current code of list_subvols() has very bad scalability, if we want to
 add new filter conditions or new sort methods, we have to modify lots of code.

I've briefly skimmed through the patch, not a short one, IMO the right
way to go.

david
--
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 V4 5/7] Btrfs-progs: restructure list_subvolumes

2012-09-18 Thread Miao Xie
The current code of list_subvols() has very bad scalability, if we want to
add new filter conditions or new sort methods, we have to modify lots of code.

Beside that, the most code of list_snapshots() is similar to list_subvols(),

So I restructure list_subvols(), and split the subvolume filter function,
the subvolume sort function and the output function from list_subvols().
In order to implement it, we defined some importtant structures:
struct btrfs_list_filter {
btrfs_list_filter_func filter_func;
void *data;
};

struct btrfs_list_comparer {
btrfs_list_comp_func comp_func;
int is_descending;
};

struct {
char*name;
char*column_name;
int need_print;
} btrfs_list_columns[];

If we want to add a new filter condition, we can choose a suitable filter
function, or implement a new filter function[1], and add it into a set of
the filters, and then pass the filter set into list_subvols(). We also can
mix several filters (just add those filters into the set, and pass the set
into list_subvols()) if the users specify two or more filter conditions.

The subvolume sort function is similar to the subvolume filter function. The
differentiation is the order of comparers in the array which is passed into
list_subvols() show us the priority of the sort methods.

The output function is different with the above two functions, we define a
array to manage all the columns that can be outputed, and use a member variant
(-need_print) to control the output of the relative column. Some columns are
outputed by default. But we can change it according to the requirement of the
users.

After appling this patch, we needn't implement a independent list_snapshots()
function, just pass a filter function which is used to identify the snapshot
into list_subvols().

[1]: If we implement new filter functions or compare functions, we must add
them into the array all_filter_funcs or the array all_comp_funcs, and modify
the relative enum variants(btrfs_list_filter_enum, btrfs_list_comp_enum).

Signed-off-by: Miao Xie mi...@cn.fujitsu.com
---
Changelog v3 - v4:
- add the filter set and comparer set which are used to manage the filters and
  comparers. And the memory space of these two set are allocated dynamically,
  in this way, the users can specify lots of filters and comparers, not be 
limited
  by the size of the array.

Changelog v1 - v3:
- new patch.
---
 btrfs-list.c | 1004 --
 btrfs-list.h |   73 -
 cmds-inspect.c   |2 +-
 cmds-subvolume.c |   59 +++-
 send-utils.c |3 +-
 5 files changed, 712 insertions(+), 429 deletions(-)

diff --git a/btrfs-list.c b/btrfs-list.c
index ed28021..bace903 100644
--- a/btrfs-list.c
+++ b/btrfs-list.c
@@ -37,6 +37,9 @@
 #include uuid/uuid.h
 #include btrfs-list.h
 
+#define BTRFS_LIST_NFILTERS_INCREASE   (2 * BTRFS_LIST_FILTER_MAX)
+#define BTRFS_LIST_NCOMPS_INCREASE (2 * BTRFS_LIST_COMP_MAX)
+
 /* we store all the roots we find in an rbtree so that we can
  * search for them later.
  */
@@ -49,19 +52,28 @@ struct root_lookup {
  */
 struct root_info {
struct rb_node rb_node;
+   struct rb_node sort_node;
 
/* this root's id */
u64 root_id;
 
+   /* equal the offset of the root's key */
+   u64 root_offset;
+
/* the id of the root that references this one */
u64 ref_tree;
 
/* the dir id we're in from ref_tree */
u64 dir_id;
 
+   u64 top_id;
+
/* generation when the root is created or last updated */
u64 gen;
 
+   /* creation generation of this root in sec*/
+   u64 ogen;
+
/* creation time of this root in sec*/
time_t otime;
 
@@ -73,35 +85,254 @@ struct root_info {
char *path;
 
/* the name of this root in the directory it lives in */
-   char name[];
+   char *name;
+
+   char *full_path;
 };
 
+struct {
+   char*name;
+   char*column_name;
+   int need_print;
+} btrfs_list_columns[] = {
+   {
+   .name   = ID,
+   .column_name= ID,
+   .need_print = 1,
+   },
+   {
+   .name   = gen,
+   .column_name= Gen,
+   .need_print = 1,
+   },
+   {
+   .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 = 1,
+   },
+   {
+   .name   = otime,
+   .column_name= OTime,
+   .need_print = 0,
+   },
+   {
+   .name   = uuid,
+   .column_name=