Re: [PATCH 02/12] Btrfs-progs: move printing subvol list outside of btrfs_list_subvols

2013-01-30 Thread Anand Jain 



Thanks for the review. Comments accepted. V5 sent out.

Anand


On 01/30/2013 11:27 AM, Wang Shilong wrote:

Hi,

To improve the code reuse its better to have btrfs_list_subvols
just return list of subvols witout printing

Signed-off-by: Anand Jain anand.j...@oracle.com
---
  btrfs-list.c | 28 ++--
  btrfs-list.h |  2 +-
  cmds-subvolume.c |  4 ++--
  3 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/btrfs-list.c b/btrfs-list.c
index cb42fbc..b404e1d 100644
--- a/btrfs-list.c
+++ b/btrfs-list.c
@@ -1439,15 +1439,11 @@ static void print_all_volume_info(struct root_lookup 
*sorted_tree,
}
  }

-int btrfs_list_subvols(int fd, struct btrfs_list_filter_set *filter_set,
-  struct btrfs_list_comparer_set *comp_set,
-  int is_tab_result)
+int btrfs_list_subvols(int fd, struct root_lookup *root_lookup)
  {
-   struct root_lookup root_lookup;
-   struct root_lookup root_sort;
int ret;

-   ret = __list_subvol_search(fd, root_lookup);
+   ret = __list_subvol_search(fd, root_lookup);
if (ret) {
fprintf(stderr, ERROR: can't perform the search - %s\n,
strerror(errno));
@@ -1458,16 +1454,28 @@ int btrfs_list_subvols(int fd, struct 
btrfs_list_filter_set *filter_set,
 * now we have an rbtree full of root_info objects, but we need to fill
 * in their path names within the subvol that is referencing each one.
 */
-   ret = __list_subvol_fill_paths(fd, root_lookup);
-   if (ret  0)
-   return ret;
+   ret = __list_subvol_fill_paths(fd, root_lookup);
+   return ret;
+}

+int btrfs_list_subvols_print(int fd, struct btrfs_list_filter_set *filter_set,
+  struct btrfs_list_comparer_set *comp_set,
+  int is_tab_result)
+{
+   struct root_lookup root_lookup;
+   struct root_lookup root_sort;
+   int ret;
+
+   ret = btrfs_list_subvols(fd, root_lookup);
+   if (ret)
+   return ret;
__filter_and_sort_subvol(root_lookup, root_sort, filter_set,
 comp_set, fd);

print_all_volume_info(root_sort, is_tab_result);
__free_all_subvolumn(root_lookup);

 Here we forget to free filter and comp_set before..i hope you can add it 
to your patchset..
 Maybe you can have patch 13...

 if (filter_set)
 btrfs_list_free_filter_set(filter_set);
 if (comp_set)
 btrfs_list_free_comparer_set(comp_set);

 Thanks,
 Wang

-   return ret;
+
+   return 0;
  }

  static int print_one_extent(int fd, struct btrfs_ioctl_search_header *sh,
diff --git a/btrfs-list.h b/btrfs-list.h
index cde4b3c..71fe0f3 100644
--- a/btrfs-list.h
+++ b/btrfs-list.h
@@ -98,7 +98,7 @@ int btrfs_list_setup_comparer(struct btrfs_list_comparer_set 
**comp_set,
  enum btrfs_list_comp_enum comparer,
  int is_descending);

-int btrfs_list_subvols(int fd, struct btrfs_list_filter_set *filter_set,
+int btrfs_list_subvols_print(int fd, struct btrfs_list_filter_set *filter_set,
   struct btrfs_list_comparer_set *comp_set,
int is_tab_result);
  int btrfs_list_find_updated_files(int fd, u64 root_id, u64 oldest_gen);
diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index e3cdb1e..c35dff7 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -406,7 +406,7 @@ static int cmd_subvol_list(int argc, char **argv)
BTRFS_LIST_FILTER_TOPID_EQUAL,
top_id);

-   ret = btrfs_list_subvols(fd, filter_set, comparer_set,
+   ret = btrfs_list_subvols_print(fd, filter_set, comparer_set,
is_tab_result);
if (ret)
return 19;
@@ -613,7 +613,7 @@ static int cmd_subvol_get_default(int argc, char **argv)
btrfs_list_setup_filter(filter_set, BTRFS_LIST_FILTER_ROOTID,
default_id);

-   ret = btrfs_list_subvols(fd, filter_set, NULL, 0);
+   ret = btrfs_list_subvols_print(fd, filter_set, NULL, 0);
if (ret)
return 19;
return 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


--
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 02/12] Btrfs-progs: move printing subvol list outside of btrfs_list_subvols

2013-01-29 Thread Wang Shilong 
Hi,
 To improve the code reuse its better to have btrfs_list_subvols
 just return list of subvols witout printing

 Signed-off-by: Anand Jain anand.j...@oracle.com
 ---
  btrfs-list.c | 28 ++--
  btrfs-list.h |  2 +-
  cmds-subvolume.c |  4 ++--
  3 files changed, 21 insertions(+), 13 deletions(-)

 diff --git a/btrfs-list.c b/btrfs-list.c
 index cb42fbc..b404e1d 100644
 --- a/btrfs-list.c
 +++ b/btrfs-list.c
 @@ -1439,15 +1439,11 @@ static void print_all_volume_info(struct root_lookup 
 *sorted_tree,
   }
  }
  
 -int btrfs_list_subvols(int fd, struct btrfs_list_filter_set *filter_set,
 -struct btrfs_list_comparer_set *comp_set,
 -int is_tab_result)
 +int btrfs_list_subvols(int fd, struct root_lookup *root_lookup)
  {
 - struct root_lookup root_lookup;
 - struct root_lookup root_sort;
   int ret;
  
 - ret = __list_subvol_search(fd, root_lookup);
 + ret = __list_subvol_search(fd, root_lookup);
   if (ret) {
   fprintf(stderr, ERROR: can't perform the search - %s\n,
   strerror(errno));
 @@ -1458,16 +1454,28 @@ int btrfs_list_subvols(int fd, struct 
 btrfs_list_filter_set *filter_set,
* now we have an rbtree full of root_info objects, but we need to fill
* in their path names within the subvol that is referencing each one.
*/
 - ret = __list_subvol_fill_paths(fd, root_lookup);
 - if (ret  0)
 - return ret;
 + ret = __list_subvol_fill_paths(fd, root_lookup);
 + return ret;
 +}
  
 +int btrfs_list_subvols_print(int fd, struct btrfs_list_filter_set 
 *filter_set,
 +struct btrfs_list_comparer_set *comp_set,
 +int is_tab_result)
 +{
 + struct root_lookup root_lookup;
 + struct root_lookup root_sort;
 + int ret;
 +
 + ret = btrfs_list_subvols(fd, root_lookup);
 + if (ret)
 + return ret;
   __filter_and_sort_subvol(root_lookup, root_sort, filter_set,
comp_set, fd);
  
   print_all_volume_info(root_sort, is_tab_result);
   __free_all_subvolumn(root_lookup);
Here we forget to free filter and comp_set before..i hope you can add it to 
your patchset..
Maybe you can have patch 13...

if (filter_set)
btrfs_list_free_filter_set(filter_set);
if (comp_set)
btrfs_list_free_comparer_set(comp_set);

Thanks,
Wang
 - return ret;
 +
 + return 0;
  }
  
  static int print_one_extent(int fd, struct btrfs_ioctl_search_header *sh,
 diff --git a/btrfs-list.h b/btrfs-list.h
 index cde4b3c..71fe0f3 100644
 --- a/btrfs-list.h
 +++ b/btrfs-list.h
 @@ -98,7 +98,7 @@ int btrfs_list_setup_comparer(struct 
 btrfs_list_comparer_set **comp_set,
 enum btrfs_list_comp_enum comparer,
 int is_descending);
  
 -int btrfs_list_subvols(int fd, struct btrfs_list_filter_set *filter_set,
 +int btrfs_list_subvols_print(int fd, struct btrfs_list_filter_set 
 *filter_set,
  struct btrfs_list_comparer_set *comp_set,
   int is_tab_result);
  int btrfs_list_find_updated_files(int fd, u64 root_id, u64 oldest_gen);
 diff --git a/cmds-subvolume.c b/cmds-subvolume.c
 index e3cdb1e..c35dff7 100644
 --- a/cmds-subvolume.c
 +++ b/cmds-subvolume.c
 @@ -406,7 +406,7 @@ static int cmd_subvol_list(int argc, char **argv)
   BTRFS_LIST_FILTER_TOPID_EQUAL,
   top_id);
  
 - ret = btrfs_list_subvols(fd, filter_set, comparer_set,
 + ret = btrfs_list_subvols_print(fd, filter_set, comparer_set,
   is_tab_result);
   if (ret)
   return 19;
 @@ -613,7 +613,7 @@ static int cmd_subvol_get_default(int argc, char **argv)
   btrfs_list_setup_filter(filter_set, BTRFS_LIST_FILTER_ROOTID,
   default_id);
  
 - ret = btrfs_list_subvols(fd, filter_set, NULL, 0);
 + ret = btrfs_list_subvols_print(fd, filter_set, NULL, 0);
   if (ret)
   return 19;
   return 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

[PATCH 02/12] Btrfs-progs: move printing subvol list outside of btrfs_list_subvols

2013-01-28 Thread Anand Jain 
To improve the code reuse its better to have btrfs_list_subvols
just return list of subvols witout printing

Signed-off-by: Anand Jain anand.j...@oracle.com
---
 btrfs-list.c | 28 ++--
 btrfs-list.h |  2 +-
 cmds-subvolume.c |  4 ++--
 3 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/btrfs-list.c b/btrfs-list.c
index cb42fbc..b404e1d 100644
--- a/btrfs-list.c
+++ b/btrfs-list.c
@@ -1439,15 +1439,11 @@ static void print_all_volume_info(struct root_lookup 
*sorted_tree,
}
 }
 
-int btrfs_list_subvols(int fd, struct btrfs_list_filter_set *filter_set,
-  struct btrfs_list_comparer_set *comp_set,
-  int is_tab_result)
+int btrfs_list_subvols(int fd, struct root_lookup *root_lookup)
 {
-   struct root_lookup root_lookup;
-   struct root_lookup root_sort;
int ret;
 
-   ret = __list_subvol_search(fd, root_lookup);
+   ret = __list_subvol_search(fd, root_lookup);
if (ret) {
fprintf(stderr, ERROR: can't perform the search - %s\n,
strerror(errno));
@@ -1458,16 +1454,28 @@ int btrfs_list_subvols(int fd, struct 
btrfs_list_filter_set *filter_set,
 * now we have an rbtree full of root_info objects, but we need to fill
 * in their path names within the subvol that is referencing each one.
 */
-   ret = __list_subvol_fill_paths(fd, root_lookup);
-   if (ret  0)
-   return ret;
+   ret = __list_subvol_fill_paths(fd, root_lookup);
+   return ret;
+}
 
+int btrfs_list_subvols_print(int fd, struct btrfs_list_filter_set *filter_set,
+  struct btrfs_list_comparer_set *comp_set,
+  int is_tab_result)
+{
+   struct root_lookup root_lookup;
+   struct root_lookup root_sort;
+   int ret;
+
+   ret = btrfs_list_subvols(fd, root_lookup);
+   if (ret)
+   return ret;
__filter_and_sort_subvol(root_lookup, root_sort, filter_set,
 comp_set, fd);
 
print_all_volume_info(root_sort, is_tab_result);
__free_all_subvolumn(root_lookup);
-   return ret;
+
+   return 0;
 }
 
 static int print_one_extent(int fd, struct btrfs_ioctl_search_header *sh,
diff --git a/btrfs-list.h b/btrfs-list.h
index cde4b3c..71fe0f3 100644
--- a/btrfs-list.h
+++ b/btrfs-list.h
@@ -98,7 +98,7 @@ int btrfs_list_setup_comparer(struct btrfs_list_comparer_set 
**comp_set,
  enum btrfs_list_comp_enum comparer,
  int is_descending);
 
-int btrfs_list_subvols(int fd, struct btrfs_list_filter_set *filter_set,
+int btrfs_list_subvols_print(int fd, struct btrfs_list_filter_set *filter_set,
   struct btrfs_list_comparer_set *comp_set,
int is_tab_result);
 int btrfs_list_find_updated_files(int fd, u64 root_id, u64 oldest_gen);
diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index e3cdb1e..c35dff7 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -406,7 +406,7 @@ static int cmd_subvol_list(int argc, char **argv)
BTRFS_LIST_FILTER_TOPID_EQUAL,
top_id);
 
-   ret = btrfs_list_subvols(fd, filter_set, comparer_set,
+   ret = btrfs_list_subvols_print(fd, filter_set, comparer_set,
is_tab_result);
if (ret)
return 19;
@@ -613,7 +613,7 @@ static int cmd_subvol_get_default(int argc, char **argv)
btrfs_list_setup_filter(filter_set, BTRFS_LIST_FILTER_ROOTID,
default_id);
 
-   ret = btrfs_list_subvols(fd, filter_set, NULL, 0);
+   ret = btrfs_list_subvols_print(fd, filter_set, NULL, 0);
if (ret)
return 19;
return 0;
-- 
1.8.1.227.g44fe835

--
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