* [PATCH] Btrfs-progs: Filter out deleting or already-deleted subvolumes in lookup_ino_path.
@ 2012-10-17 16:06 Alex Lyakas
2012-10-18 2:00 ` Miao Xie
0 siblings, 1 reply; 3+ messages in thread
From: Alex Lyakas @ 2012-10-17 16:06 UTC (permalink / raw)
To: linux-btrfs, Stefan Priebe, miaox
If a subvolume is deleted between __list_subvol_search() and
__list_subvol_fill_paths(),
the tool would have stopped with an error. So drop those subvolumes
that did not have ROOT_BACKREF, and those
for whom a path component to the parent root was missing.
(Note that BTRFS_IOC_INO_LOOKUP does not do a search if dirid is
BTRFS_FIRST_FREE_OBJECTID, so such
a subvolume will be considered as still existing, but subvolume
listing will not fail at least).
Reported-by: Stefan Priebe <s.priebe@profihost.ag>
Signed-off-by: Alex Lyakas <alex.btrfs@zadarastorage.com>
diff --git a/btrfs-list.c b/btrfs-list.c
index e5f0f96..faf5b1a 100644
--- a/btrfs-list.c
+++ b/btrfs-list.c
@@ -672,6 +672,13 @@ static int lookup_ino_path(int fd, struct root_info *ri)
if (ri->path)
return 0;
+ /*
+ * If the subvolume's ROOT_BACKREF does not exist anymore, consider it
+ * as deleted.
+ */
+ if (ri->ref_tree == 0)
+ return -ENOENT;
+
memset(&args, 0, sizeof(args));
args.treeid = ri->ref_tree;
args.objectid = ri->dir_id;
@@ -679,9 +686,15 @@ static int lookup_ino_path(int fd, struct root_info *ri)
ret = ioctl(fd, BTRFS_IOC_INO_LOOKUP, &args);
e = errno;
if (ret) {
- fprintf(stderr, "ERROR: Failed to lookup path for root %llu - %s\n",
- (unsigned long long)ri->ref_tree,
- strerror(e));
+ ret = -e;
+ /*
+ * If one of the path components is missing, we will conside
+ * the subvolume as deleted.
+ */
+ if (ret != -ENOENT)
+ fprintf(stderr, "ERROR: Failed to lookup path for root %llu - %s\n",
+ (unsigned long long)ri->ref_tree,
+ strerror(e));
return ret;
}
@@ -1290,19 +1303,32 @@ static void __filter_and_sort_subvol(struct
root_lookup *all_subvols,
}
}
-static int __list_subvol_fill_paths(int fd, struct root_lookup *root_lookup)
+static int __list_subvol_fill_paths(int fd, struct root_lookup *root_lookup,
+ struct root_lookup *root_lookup_final)
{
struct rb_node *n;
+ root_lookup_init(root_lookup_final);
+
n = rb_first(&root_lookup->root);
while (n) {
struct root_info *entry;
int ret;
entry = rb_entry(n, struct root_info, rb_node);
ret = lookup_ino_path(fd, entry);
- if(ret < 0)
+ if(ret < 0 && ret != -ENOENT)
return ret;
- n = rb_next(n);
+ rb_erase(&entry->rb_node, &root_lookup->root);
+ /*
+ * If lookup_ino_path() returned ENOENT, some of the path
+ * components are missing. Let's consider this subvolume
+ * as deleted then.
+ */
+ if (ret == -ENOENT)
+ __free_root_info(entry);
+ else
+ root_tree_insert(root_lookup_final, entry);
+ n = rb_first(&root_lookup->root);
}
return 0;
@@ -1446,6 +1472,7 @@ int btrfs_list_subvols(int fd, struct
btrfs_list_filter_set *filter_set,
int is_tab_result)
{
struct root_lookup root_lookup;
+ struct root_lookup root_lookup_final;
struct root_lookup root_sort;
int ret;
@@ -1460,15 +1487,15 @@ 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);
+ ret = __list_subvol_fill_paths(fd, &root_lookup, &root_lookup_final);
if (ret < 0)
return ret;
- __filter_and_sort_subvol(&root_lookup, &root_sort, filter_set,
+ __filter_and_sort_subvol(&root_lookup_final, &root_sort, filter_set,
comp_set, fd);
print_all_volume_info(&root_sort, is_tab_result);
- __free_all_subvolumn(&root_lookup);
+ __free_all_subvolumn(&root_lookup_final);
return ret;
}
@@ -1655,6 +1682,7 @@ int btrfs_list_find_updated_files(int fd, u64
root_id, u64 oldest_gen)
char *btrfs_list_path_for_root(int fd, u64 root)
{
struct root_lookup root_lookup;
+ struct root_lookup root_lookup_final;
struct rb_node *n;
char *ret_path = NULL;
int ret;
@@ -1664,16 +1692,16 @@ char *btrfs_list_path_for_root(int fd, u64 root)
if (ret < 0)
return ERR_PTR(ret);
- ret = __list_subvol_fill_paths(fd, &root_lookup);
+ ret = __list_subvol_fill_paths(fd, &root_lookup, &root_lookup_final);
if (ret < 0)
return ERR_PTR(ret);
- n = rb_last(&root_lookup.root);
+ n = rb_last(&root_lookup_final.root);
while (n) {
struct root_info *entry;
entry = rb_entry(n, struct root_info, rb_node);
- resolve_root(&root_lookup, entry, top_id);
+ resolve_root(&root_lookup_final, entry, top_id);
if (entry->root_id == root) {
ret_path = entry->full_path;
entry->full_path = NULL;
@@ -1681,7 +1709,7 @@ char *btrfs_list_path_for_root(int fd, u64 root)
n = rb_prev(n);
}
- __free_all_subvolumn(&root_lookup);
+ __free_all_subvolumn(&root_lookup_final);
return ret_path;
}
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] Btrfs-progs: Filter out deleting or already-deleted subvolumes in lookup_ino_path.
2012-10-17 16:06 [PATCH] Btrfs-progs: Filter out deleting or already-deleted subvolumes in lookup_ino_path Alex Lyakas
@ 2012-10-18 2:00 ` Miao Xie
2012-10-18 9:03 ` Alex Lyakas
0 siblings, 1 reply; 3+ messages in thread
From: Miao Xie @ 2012-10-18 2:00 UTC (permalink / raw)
To: Alex Lyakas; +Cc: linux-btrfs, Stefan Priebe
On Wed, 17 Oct 2012 18:06:52 +0200, Alex Lyakas wrote:
> -static int __list_subvol_fill_paths(int fd, struct root_lookup *root_lookup)
> +static int __list_subvol_fill_paths(int fd, struct root_lookup *root_lookup,
> + struct root_lookup *root_lookup_final)
> {
> struct rb_node *n;
>
> + root_lookup_init(root_lookup_final);
> +
> n = rb_first(&root_lookup->root);
> while (n) {
> struct root_info *entry;
> int ret;
> entry = rb_entry(n, struct root_info, rb_node);
> ret = lookup_ino_path(fd, entry);
> - if(ret < 0)
> + if(ret < 0 && ret != -ENOENT)
> return ret;
> - n = rb_next(n);
> + rb_erase(&entry->rb_node, &root_lookup->root);
> + /*
> + * If lookup_ino_path() returned ENOENT, some of the path
> + * components are missing. Let's consider this subvolume
> + * as deleted then.
> + */
> + if (ret == -ENOENT)
> + __free_root_info(entry);
> + else
> + root_tree_insert(root_lookup_final, entry);
> + n = rb_first(&root_lookup->root);
I don't think we need a final rb-tree since we have removed the deleted root entries from
the tree and freed them.
Beside that, this patch didn't fix the problem completely. Consider the following case:
Subv0
|-> Subv1
|-> Subv2
When we fill the path, we may deal with Subv2 firstly, and then deal with Subv1. But
if someone deletes Subv2 and Subv1 after we fill the path of Subv2 and before we fill
the path of Subv1, we may deal with Subv2 successfully, then we will find Subv1 has been
deleted, so we will clean up Subv2 entry. If so, we will fail to resolve the full path of
Subv2 because we can not find Subv1.
My partner made a patch which can fix the above problem, I will send it out later.
Thanks
Miao
> }
>
> return 0;
> @@ -1446,6 +1472,7 @@ int btrfs_list_subvols(int fd, struct
> btrfs_list_filter_set *filter_set,
> int is_tab_result)
> {
> struct root_lookup root_lookup;
> + struct root_lookup root_lookup_final;
> struct root_lookup root_sort;
> int ret;
>
> @@ -1460,15 +1487,15 @@ 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);
> + ret = __list_subvol_fill_paths(fd, &root_lookup, &root_lookup_final);
> if (ret < 0)
> return ret;
>
> - __filter_and_sort_subvol(&root_lookup, &root_sort, filter_set,
> + __filter_and_sort_subvol(&root_lookup_final, &root_sort, filter_set,
> comp_set, fd);
>
> print_all_volume_info(&root_sort, is_tab_result);
> - __free_all_subvolumn(&root_lookup);
> + __free_all_subvolumn(&root_lookup_final);
> return ret;
> }
>
> @@ -1655,6 +1682,7 @@ int btrfs_list_find_updated_files(int fd, u64
> root_id, u64 oldest_gen)
> char *btrfs_list_path_for_root(int fd, u64 root)
> {
> struct root_lookup root_lookup;
> + struct root_lookup root_lookup_final;
> struct rb_node *n;
> char *ret_path = NULL;
> int ret;
> @@ -1664,16 +1692,16 @@ char *btrfs_list_path_for_root(int fd, u64 root)
> if (ret < 0)
> return ERR_PTR(ret);
>
> - ret = __list_subvol_fill_paths(fd, &root_lookup);
> + ret = __list_subvol_fill_paths(fd, &root_lookup, &root_lookup_final);
> if (ret < 0)
> return ERR_PTR(ret);
>
> - n = rb_last(&root_lookup.root);
> + n = rb_last(&root_lookup_final.root);
> while (n) {
> struct root_info *entry;
>
> entry = rb_entry(n, struct root_info, rb_node);
> - resolve_root(&root_lookup, entry, top_id);
> + resolve_root(&root_lookup_final, entry, top_id);
> if (entry->root_id == root) {
> ret_path = entry->full_path;
> entry->full_path = NULL;
> @@ -1681,7 +1709,7 @@ char *btrfs_list_path_for_root(int fd, u64 root)
>
> n = rb_prev(n);
> }
> - __free_all_subvolumn(&root_lookup);
> + __free_all_subvolumn(&root_lookup_final);
>
> return ret_path;
> }
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] Btrfs-progs: Filter out deleting or already-deleted subvolumes in lookup_ino_path.
2012-10-18 2:00 ` Miao Xie
@ 2012-10-18 9:03 ` Alex Lyakas
0 siblings, 0 replies; 3+ messages in thread
From: Alex Lyakas @ 2012-10-18 9:03 UTC (permalink / raw)
To: miaox; +Cc: linux-btrfs, Stefan Priebe
Hi Miao,
On Thu, Oct 18, 2012 at 4:00 AM, Miao Xie <miaox@cn.fujitsu.com> wrote:
> On Wed, 17 Oct 2012 18:06:52 +0200, Alex Lyakas wrote:
>> -static int __list_subvol_fill_paths(int fd, struct root_lookup *root_lookup)
>> +static int __list_subvol_fill_paths(int fd, struct root_lookup *root_lookup,
>> + struct root_lookup *root_lookup_final)
>> {
>> struct rb_node *n;
>>
>> + root_lookup_init(root_lookup_final);
>> +
>> n = rb_first(&root_lookup->root);
>> while (n) {
>> struct root_info *entry;
>> int ret;
>> entry = rb_entry(n, struct root_info, rb_node);
>> ret = lookup_ino_path(fd, entry);
>> - if(ret < 0)
>> + if(ret < 0 && ret != -ENOENT)
>> return ret;
>> - n = rb_next(n);
>> + rb_erase(&entry->rb_node, &root_lookup->root);
>> + /*
>> + * If lookup_ino_path() returned ENOENT, some of the path
>> + * components are missing. Let's consider this subvolume
>> + * as deleted then.
>> + */
>> + if (ret == -ENOENT)
>> + __free_root_info(entry);
>> + else
>> + root_tree_insert(root_lookup_final, entry);
>> + n = rb_first(&root_lookup->root);
>
> I don't think we need a final rb-tree since we have removed the deleted root entries from
> the tree and freed them.
I just did not want to start the search from the beginning of the same
tree (although it will be cheaper, because lookup_ino_path has a
shortcut).
>
> Beside that, this patch didn't fix the problem completely. Consider the following case:
> Subv0
> |-> Subv1
> |-> Subv2
>
> When we fill the path, we may deal with Subv2 firstly, and then deal with Subv1. But
> if someone deletes Subv2 and Subv1 after we fill the path of Subv2 and before we fill
> the path of Subv1, we may deal with Subv2 successfully, then we will find Subv1 has been
> deleted, so we will clean up Subv2 entry.
I don't see where we will clean the Subv2 entry? Once we have filled
Subv2's path successfully in lookup_ino_path(), we consider it as
existing and won't check ever again, I think.
If so, we will fail to resolve the full path of
> Subv2 because we can not find Subv1.
Note that in your particular example, I think we will never fail to
fill the paths (even if all three subvolumes are deleted), because of
the following code in btrfs_search_path_in_tree():
if (dirid == BTRFS_FIRST_FREE_OBJECTID) {
name[0]='\0';
return 0;
}
You need to have a regular subdirectory in-between the two roots for
btrfs_search_path_in_tree() to really check the path. I noted that in
the commit message.
>
> My partner made a patch which can fix the above problem, I will send it out later.
Perfect!
Thanks,
Alex.
>
> Thanks
> Miao
>
>> }
>>
>> return 0;
>> @@ -1446,6 +1472,7 @@ int btrfs_list_subvols(int fd, struct
>> btrfs_list_filter_set *filter_set,
>> int is_tab_result)
>> {
>> struct root_lookup root_lookup;
>> + struct root_lookup root_lookup_final;
>> struct root_lookup root_sort;
>> int ret;
>>
>> @@ -1460,15 +1487,15 @@ 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);
>> + ret = __list_subvol_fill_paths(fd, &root_lookup, &root_lookup_final);
>> if (ret < 0)
>> return ret;
>>
>> - __filter_and_sort_subvol(&root_lookup, &root_sort, filter_set,
>> + __filter_and_sort_subvol(&root_lookup_final, &root_sort, filter_set,
>> comp_set, fd);
>>
>> print_all_volume_info(&root_sort, is_tab_result);
>> - __free_all_subvolumn(&root_lookup);
>> + __free_all_subvolumn(&root_lookup_final);
>> return ret;
>> }
>>
>> @@ -1655,6 +1682,7 @@ int btrfs_list_find_updated_files(int fd, u64
>> root_id, u64 oldest_gen)
>> char *btrfs_list_path_for_root(int fd, u64 root)
>> {
>> struct root_lookup root_lookup;
>> + struct root_lookup root_lookup_final;
>> struct rb_node *n;
>> char *ret_path = NULL;
>> int ret;
>> @@ -1664,16 +1692,16 @@ char *btrfs_list_path_for_root(int fd, u64 root)
>> if (ret < 0)
>> return ERR_PTR(ret);
>>
>> - ret = __list_subvol_fill_paths(fd, &root_lookup);
>> + ret = __list_subvol_fill_paths(fd, &root_lookup, &root_lookup_final);
>> if (ret < 0)
>> return ERR_PTR(ret);
>>
>> - n = rb_last(&root_lookup.root);
>> + n = rb_last(&root_lookup_final.root);
>> while (n) {
>> struct root_info *entry;
>>
>> entry = rb_entry(n, struct root_info, rb_node);
>> - resolve_root(&root_lookup, entry, top_id);
>> + resolve_root(&root_lookup_final, entry, top_id);
>> if (entry->root_id == root) {
>> ret_path = entry->full_path;
>> entry->full_path = NULL;
>> @@ -1681,7 +1709,7 @@ char *btrfs_list_path_for_root(int fd, u64 root)
>>
>> n = rb_prev(n);
>> }
>> - __free_all_subvolumn(&root_lookup);
>> + __free_all_subvolumn(&root_lookup_final);
>>
>> return ret_path;
>> }
>>
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-10-18 9:03 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-17 16:06 [PATCH] Btrfs-progs: Filter out deleting or already-deleted subvolumes in lookup_ino_path Alex Lyakas
2012-10-18 2:00 ` Miao Xie
2012-10-18 9:03 ` Alex Lyakas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).