[PATCH v2 00/10] undelete subvolume offline version

[PATCH v2 00/10] undelete subvolume offline version

[Lu Fengqi]

This patchset will add undelete-subvol subcommand for btrfs rescue. This
enhancement allows undeleting a subvolume on an unmounted filesystem.

Patchset can be fetched from github:
https://github.com/littleroad/btrfs-progs.git undelete

v1->v2: add -s option to allow user specify the subvolume which will be
recovered.

The first six patches are not modified.
7th-8th add the -s option to specify subvol_id instead of recovering all
subvolumes.
9th add shell quoting to test script.
10th just add the -s option documentation.

Lu Fengqi (10):
  btrfs-progs: copy btrfs_del_orphan_item from kernel
  btrfs-progs: extract btrfs_link_subvol from btrfs_mksubvol
  btrfs-progs: use btrfs_find_free_dir_index to find free inode index
  btrfs-progs: undelete-subvol: introduce is_subvol_intact
  btrfs-progs: undelete-subvol: introduce recover_dead_root
  btrfs-progs: undelete-subvol: introduce link_subvol_to_lostfound
  btrfs-progs: undelete-subvol: introduce btrfs_undelete_intact_subvols
  btrfs-progs: undelete-subvol: add undelete-subvol subcommand
  btrfs-progs: tests: add testcase for undelete-subvol
  btrfs-progs: undelete-subvol: update completion and documentation

 Documentation/btrfs-rescue.asciidoc                |  12 +
 Makefile                                           |   3 +-
 btrfs-completion                                   |   2 +-
 cmds-rescue.c                                      |  70 ++++++
 convert/main.c                                     |  57 +++++
 ctree.h                                            |   8 +-
 inode.c                                            |  99 ++++----
 .../030-undelete-subvol/deleted_subvolume.img      | Bin 0 -> 4096 bytes
 .../030-undelete-subvol/drop_progress.raw.xz       | Bin 0 -> 23452 bytes
 tests/misc-tests/030-undelete-subvol/test.sh       |  34 +++
 undelete-subvol.c                                  | 254 +++++++++++++++++++++
 undelete-subvol.h                                  |  19 ++
 12 files changed, 511 insertions(+), 47 deletions(-)
 create mode 100644 tests/misc-tests/030-undelete-subvol/deleted_subvolume.img
 create mode 100644 tests/misc-tests/030-undelete-subvol/drop_progress.raw.xz
 create mode 100755 tests/misc-tests/030-undelete-subvol/test.sh
 create mode 100644 undelete-subvol.c
 create mode 100644 undelete-subvol.h

-- 
2.16.2

[PATCH v2 01/10] btrfs-progs: copy btrfs_del_orphan_item from kernel

[Lu Fengqi]

Add btrfs_del_orphan_item for the later subvolume undelete.

Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
---
 ctree.h |  2 ++
 inode.c | 30 ++++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/ctree.h b/ctree.h
index fa861ba0b4c3..0fc31dd8d998 100644
--- a/ctree.h
+++ b/ctree.h
@@ -2793,6 +2793,8 @@ int btrfs_unlink(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 int btrfs_add_orphan_item(struct btrfs_trans_handle *trans,
 			  struct btrfs_root *root, struct btrfs_path *path,
 			  u64 ino);
+int btrfs_del_orphan_item(struct btrfs_trans_handle *trans,
+			  struct btrfs_root *root, u64 offset);
 int btrfs_mkdir(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 		char *name, int namelen, u64 parent_ino, u64 *ino, int mode);
 struct btrfs_root *btrfs_mksubvol(struct btrfs_root *root, const char *base,
diff --git a/inode.c b/inode.c
index 2398bca4a109..8d0812c7cf50 100644
--- a/inode.c
+++ b/inode.c
@@ -262,6 +262,36 @@ int btrfs_add_orphan_item(struct btrfs_trans_handle *trans,
 	return btrfs_insert_empty_item(trans, root, path, &key, 0);
 }
 
+int btrfs_del_orphan_item(struct btrfs_trans_handle *trans,
+			  struct btrfs_root *root, u64 offset)
+{
+	struct btrfs_path *path;
+	struct btrfs_key key;
+	int ret = 0;
+
+	key.objectid = BTRFS_ORPHAN_OBJECTID;
+	key.type = BTRFS_ORPHAN_ITEM_KEY;
+	key.offset = offset;
+
+	path = btrfs_alloc_path();
+	if (!path)
+		return -ENOMEM;
+
+	ret = btrfs_search_slot(trans, root, &key, path, -1, 1);
+	if (ret < 0)
+		goto out;
+	if (ret) {
+		ret = -ENOENT;
+		goto out;
+	}
+
+	ret = btrfs_del_item(trans, root, path);
+
+out:
+	btrfs_free_path(path);
+	return ret;
+}
+
 /*
  * Unlink an inode, which will remove its backref and corresponding dir_index/
  * dir_item if any of them exists.
-- 
2.16.2

[PATCH v2 02/10] btrfs-progs: extract btrfs_link_subvol from btrfs_mksubvol

[Lu Fengqi]

The original btrfs_mksubvol is too specific to specify the directory that
the subvolume will link to. Furthermore, in this transaction, we don't only
need to create root_ref/dir-item, but also update the refs or flags of
root_item. Extract a generic btrfs_link_subvol that allow the caller pass a
trans argument for later subvolume undelete.

No functional changes for the btrfs_mksubvol.

Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
---
 convert/main.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 ctree.h        |  5 +++--
 inode.c        | 46 +++++++++++++++++++++-------------------------
 3 files changed, 81 insertions(+), 27 deletions(-)

diff --git a/convert/main.c b/convert/main.c
index 6bdfab40d0b0..dd6b42a1f2b1 100644
--- a/convert/main.c
+++ b/convert/main.c
@@ -1002,6 +1002,63 @@ err:
 	return ret;
 }
 
+/*
+ * Link the subvolume specified by @root_objectid to the root_dir of @root.
+ *
+ * @root		the root of the file tree which the subvolume will
+ *			be linked to.
+ * @subvol_name		the name of the subvolume which will be linked.
+ * @root_objectid	specify the subvolume which will be linked.
+ * @convert		the flag to determine whether to try to resolve
+ *			the name conflict.
+ *
+ * Return the root of the subvolume if success, otherwise return NULL.
+ */
+static struct btrfs_root *btrfs_mksubvol(struct btrfs_root *root,
+					 const char *subvol_name,
+					 u64 root_objectid,
+					 bool convert)
+{
+	struct btrfs_trans_handle *trans;
+	struct btrfs_root *subvol_root = NULL;
+	struct btrfs_key key;
+	u64 dirid = btrfs_root_dirid(&root->root_item);
+	int ret;
+
+
+	trans = btrfs_start_transaction(root, 1);
+	if (IS_ERR(trans)) {
+		error("unable to start transaction");
+		goto fail;
+	}
+
+	ret = btrfs_link_subvol(trans, root, subvol_name, root_objectid, dirid,
+				true);
+	if (ret) {
+		error("unable to link subvolume %s", subvol_name);
+		goto fail;
+	}
+
+	ret = btrfs_commit_transaction(trans, root);
+	if (ret) {
+		error("transaction commit failed: %d", ret);
+		goto fail;
+	}
+
+	key.objectid = root_objectid;
+	key.offset = (u64)-1;
+	key.type = BTRFS_ROOT_ITEM_KEY;
+
+	subvol_root = btrfs_read_fs_root(root->fs_info, &key);
+	if (!subvol_root) {
+		error("unable to link subvolume %s", subvol_name);
+		goto fail;
+	}
+
+fail:
+	return subvol_root;
+}
+
 /*
  * Migrate super block to its default position and zero 0 ~ 16k
  */
diff --git a/ctree.h b/ctree.h
index 0fc31dd8d998..4bff0b821472 100644
--- a/ctree.h
+++ b/ctree.h
@@ -2797,8 +2797,9 @@ int btrfs_del_orphan_item(struct btrfs_trans_handle *trans,
 			  struct btrfs_root *root, u64 offset);
 int btrfs_mkdir(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 		char *name, int namelen, u64 parent_ino, u64 *ino, int mode);
-struct btrfs_root *btrfs_mksubvol(struct btrfs_root *root, const char *base,
-				  u64 root_objectid, bool convert);
+int btrfs_link_subvol(struct btrfs_trans_handle *trans, struct btrfs_root *root,
+		      const char *base, u64 root_objectid, u64 dirid,
+		      bool convert);
 
 /* file.c */
 int btrfs_get_extent(struct btrfs_trans_handle *trans,
diff --git a/inode.c b/inode.c
index 8d0812c7cf50..478036562652 100644
--- a/inode.c
+++ b/inode.c
@@ -606,19 +606,28 @@ out:
 	return ret;
 }
 
-struct btrfs_root *btrfs_mksubvol(struct btrfs_root *root,
-				  const char *base, u64 root_objectid,
-				  bool convert)
+/*
+ * Link the subvolume specified by @root_objectid to the directory specified by
+ * @dirid on the file tree specified by @root.
+ *
+ * @root		the root of the file tree where the directory on.
+ * @base		the name of the subvolume which will be linked.
+ * @root_objectid	specify the subvolume which will be linked.
+ * @dirid		specify the directory which the subvolume will be
+ *			linked to.
+ * @convert		the flag to determine whether to try to resolve
+ *			the name conflict.
+ */
+int btrfs_link_subvol(struct btrfs_trans_handle *trans, struct btrfs_root *root,
+		      const char *base, u64 root_objectid, u64 dirid,
+		      bool convert)
 {
-	struct btrfs_trans_handle *trans;
 	struct btrfs_fs_info *fs_info = root->fs_info;
 	struct btrfs_root *tree_root = fs_info->tree_root;
-	struct btrfs_root *new_root = NULL;
 	struct btrfs_path path;
 	struct btrfs_inode_item *inode_item;
 	struct extent_buffer *leaf;
 	struct btrfs_key key;
-	u64 dirid = btrfs_root_dirid(&root->root_item);
 	u64 index = 2;
 	char buf[BTRFS_NAME_LEN + 1]; /* for snprintf null */
 	int len;
@@ -627,8 +636,9 @@ struct btrfs_root *btrfs_mksubvol(struct btrfs_root *root,
 
 	len = strlen(base);
 	if (len == 0 || len > BTRFS_NAME_LEN)
-		return NULL;
+		return -EINVAL;
 
+	/* find the free dir_index */
 	btrfs_init_path(&path);
 	key.objectid = dirid;
 	key.type = BTRFS_DIR_INDEX_KEY;
@@ -649,12 +659,7 @@ struct btrfs_root *btrfs_mksubvol(struct btrfs_root *root,
 	}
 	btrfs_release_path(&path);
 
-	trans = btrfs_start_transaction(root, 1);
-	if (IS_ERR(trans)) {
-		error("unable to start transaction");
-		goto fail;
-	}
-
+	/* add the dir_item/dir_index */
 	key.objectid = dirid;
 	key.offset = 0;
 	key.type =  BTRFS_INODE_ITEM_KEY;
@@ -675,6 +680,7 @@ struct btrfs_root *btrfs_mksubvol(struct btrfs_root *root,
 
 	memcpy(buf, base, len);
 	if (convert) {
+		/* try to resolve name conflict by adding the number suffix */
 		for (i = 0; i < 1024; i++) {
 			ret = btrfs_insert_dir_item(trans, root, buf, len,
 					dirid, &key, BTRFS_FT_DIR, index);
@@ -719,18 +725,8 @@ struct btrfs_root *btrfs_mksubvol(struct btrfs_root *root,
 		goto fail;
 	}
 
-	ret = btrfs_commit_transaction(trans, root);
-	if (ret) {
-		error("transaction commit failed: %d", ret);
-		goto fail;
-	}
 
-	new_root = btrfs_read_fs_root(fs_info, &key);
-	if (IS_ERR(new_root)) {
-		error("unable to fs read root: %lu", PTR_ERR(new_root));
-		new_root = NULL;
-	}
 fail:
-	btrfs_init_path(&path);
-	return new_root;
+	btrfs_release_path(&path);
+	return ret;
 }
-- 
2.16.2

Re: [PATCH v2 02/10] btrfs-progs: extract btrfs_link_subvol from btrfs_mksubvol

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 7385 bytes --]



On 2018年03月27日 15:06, Lu Fengqi wrote:
> The original btrfs_mksubvol is too specific to specify the directory that
> the subvolume will link to. Furthermore, in this transaction, we don't only
> need to create root_ref/dir-item, but also update the refs or flags of
> root_item. Extract a generic btrfs_link_subvol that allow the caller pass a
> trans argument for later subvolume undelete.

The idea makes sense.

Although some small nitpicks inlined below.

> 
> No functional changes for the btrfs_mksubvol.
> 
> Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
> ---
>  convert/main.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  ctree.h        |  5 +++--
>  inode.c        | 46 +++++++++++++++++++++-------------------------
>  3 files changed, 81 insertions(+), 27 deletions(-)
> 
> diff --git a/convert/main.c b/convert/main.c
> index 6bdfab40d0b0..dd6b42a1f2b1 100644
> --- a/convert/main.c
> +++ b/convert/main.c
> @@ -1002,6 +1002,63 @@ err:
>  	return ret;
>  }
>  
> +/*
> + * Link the subvolume specified by @root_objectid to the root_dir of @root.

However the function name is btrfs_mksubvol(), not btrfs_link_subvol().
After reading the code, it turns out that btrfs_mksubvol() is just an
less generic btrfs_link_subvol().

> + * @root		the root of the file tree which the subvolume will
> + *			be linked to.

Here you're using btrfs_root for source subvolume.

> + * @subvol_name		the name of the subvolume which will be linked.
> + * @root_objectid	specify the subvolume which will be linked.

But here you're specifying subvolume id as destination.

It would be much better to unify both parameter to the same structure.
And personally I prefer both btrfs_root.

> + * @convert		the flag to determine whether to try to resolve
> + *			the name conflict.

This parameter is not used in this function, and the name "convert"
doesn't reflect the name conflict check.

Personally speaking, I would like the parameters to be something like
btrfs_mksubolv(struct btrfs_root *dst_root,
               struct btrfs_root *src_root)

> + *
> + * Return the root of the subvolume if success, otherwise return NULL.
> + */
> +static struct btrfs_root *btrfs_mksubvol(struct btrfs_root *root,
> +					 const char *subvol_name,
> +					 u64 root_objectid,
> +					 bool convert)
> +{
> +	struct btrfs_trans_handle *trans;
> +	struct btrfs_root *subvol_root = NULL;
> +	struct btrfs_key key;
> +	u64 dirid = btrfs_root_dirid(&root->root_item);
> +	int ret;
> +
> +
> +	trans = btrfs_start_transaction(root, 1);
> +	if (IS_ERR(trans)) {
> +		error("unable to start transaction");
> +		goto fail;
> +	}
> +
> +	ret = btrfs_link_subvol(trans, root, subvol_name, root_objectid, dirid,
> +				true);
> +	if (ret) {
> +		error("unable to link subvolume %s", subvol_name);
> +		goto fail;
> +	}
> +
> +	ret = btrfs_commit_transaction(trans, root);
> +	if (ret) {
> +		error("transaction commit failed: %d", ret);
> +		goto fail;
> +	}
> +
> +	key.objectid = root_objectid;
> +	key.offset = (u64)-1;
> +	key.type = BTRFS_ROOT_ITEM_KEY;
> +
> +	subvol_root = btrfs_read_fs_root(root->fs_info, &key);
> +	if (!subvol_root) {
> +		error("unable to link subvolume %s", subvol_name);
> +		goto fail;
> +	}
> +
> +fail:
> +	return subvol_root;
> +}
> +
>  /*
>   * Migrate super block to its default position and zero 0 ~ 16k
>   */
> diff --git a/ctree.h b/ctree.h
> index 0fc31dd8d998..4bff0b821472 100644
> --- a/ctree.h
> +++ b/ctree.h
> @@ -2797,8 +2797,9 @@ int btrfs_del_orphan_item(struct btrfs_trans_handle *trans,
>  			  struct btrfs_root *root, u64 offset);
>  int btrfs_mkdir(struct btrfs_trans_handle *trans, struct btrfs_root *root,
>  		char *name, int namelen, u64 parent_ino, u64 *ino, int mode);
> -struct btrfs_root *btrfs_mksubvol(struct btrfs_root *root, const char *base,
> -				  u64 root_objectid, bool convert);
> +int btrfs_link_subvol(struct btrfs_trans_handle *trans, struct btrfs_root *root,
> +		      const char *base, u64 root_objectid, u64 dirid,
> +		      bool convert);
>  
>  /* file.c */
>  int btrfs_get_extent(struct btrfs_trans_handle *trans,
> diff --git a/inode.c b/inode.c
> index 8d0812c7cf50..478036562652 100644
> --- a/inode.c
> +++ b/inode.c
> @@ -606,19 +606,28 @@ out:
>  	return ret;
>  }
>  
> -struct btrfs_root *btrfs_mksubvol(struct btrfs_root *root,
> -				  const char *base, u64 root_objectid,
> -				  bool convert)
> +/*
> + * Link the subvolume specified by @root_objectid to the directory specified by
> + * @dirid on the file tree specified by @root.
> + *
> + * @root		the root of the file tree where the directory on.
> + * @base		the name of the subvolume which will be linked.
> + * @root_objectid	specify the subvolume which will be linked.

Same nitpick about parameter here.

Thanks,
Qu

> + * @dirid		specify the directory which the subvolume will be
> + *			linked to.
> + * @convert		the flag to determine whether to try to resolve
> + *			the name conflict.
> + */
> +int btrfs_link_subvol(struct btrfs_trans_handle *trans, struct btrfs_root *root,
> +		      const char *base, u64 root_objectid, u64 dirid,
> +		      bool convert)
>  {
> -	struct btrfs_trans_handle *trans;
>  	struct btrfs_fs_info *fs_info = root->fs_info;
>  	struct btrfs_root *tree_root = fs_info->tree_root;
> -	struct btrfs_root *new_root = NULL;
>  	struct btrfs_path path;
>  	struct btrfs_inode_item *inode_item;
>  	struct extent_buffer *leaf;
>  	struct btrfs_key key;
> -	u64 dirid = btrfs_root_dirid(&root->root_item);
>  	u64 index = 2;
>  	char buf[BTRFS_NAME_LEN + 1]; /* for snprintf null */
>  	int len;
> @@ -627,8 +636,9 @@ struct btrfs_root *btrfs_mksubvol(struct btrfs_root *root,
>  
>  	len = strlen(base);
>  	if (len == 0 || len > BTRFS_NAME_LEN)
> -		return NULL;
> +		return -EINVAL;
>  
> +	/* find the free dir_index */
>  	btrfs_init_path(&path);
>  	key.objectid = dirid;
>  	key.type = BTRFS_DIR_INDEX_KEY;
> @@ -649,12 +659,7 @@ struct btrfs_root *btrfs_mksubvol(struct btrfs_root *root,
>  	}
>  	btrfs_release_path(&path);
>  
> -	trans = btrfs_start_transaction(root, 1);
> -	if (IS_ERR(trans)) {
> -		error("unable to start transaction");
> -		goto fail;
> -	}
> -
> +	/* add the dir_item/dir_index */
>  	key.objectid = dirid;
>  	key.offset = 0;
>  	key.type =  BTRFS_INODE_ITEM_KEY;
> @@ -675,6 +680,7 @@ struct btrfs_root *btrfs_mksubvol(struct btrfs_root *root,
>  
>  	memcpy(buf, base, len);
>  	if (convert) {
> +		/* try to resolve name conflict by adding the number suffix */
>  		for (i = 0; i < 1024; i++) {
>  			ret = btrfs_insert_dir_item(trans, root, buf, len,
>  					dirid, &key, BTRFS_FT_DIR, index);
> @@ -719,18 +725,8 @@ struct btrfs_root *btrfs_mksubvol(struct btrfs_root *root,
>  		goto fail;
>  	}
>  
> -	ret = btrfs_commit_transaction(trans, root);
> -	if (ret) {
> -		error("transaction commit failed: %d", ret);
> -		goto fail;
> -	}
>  
> -	new_root = btrfs_read_fs_root(fs_info, &key);
> -	if (IS_ERR(new_root)) {
> -		error("unable to fs read root: %lu", PTR_ERR(new_root));
> -		new_root = NULL;
> -	}
>  fail:
> -	btrfs_init_path(&path);
> -	return new_root;
> +	btrfs_release_path(&path);
> +	return ret;
>  }
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 02/10] btrfs-progs: extract btrfs_link_subvol from btrfs_mksubvol

[Lu Fengqi]

On Wed, Apr 18, 2018 at 01:02:43PM +0800, Qu Wenruo wrote:
>
>
>On 2018年03月27日 15:06, Lu Fengqi wrote:
>> The original btrfs_mksubvol is too specific to specify the directory that
>> the subvolume will link to. Furthermore, in this transaction, we don't only
>> need to create root_ref/dir-item, but also update the refs or flags of
>> root_item. Extract a generic btrfs_link_subvol that allow the caller pass a
>> trans argument for later subvolume undelete.
>
>The idea makes sense.
>
>Although some small nitpicks inlined below.

Sorry I've taken so long to reply.

>
>> 
>> No functional changes for the btrfs_mksubvol.
>> 
>> Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
>> ---
>>  convert/main.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  ctree.h        |  5 +++--
>>  inode.c        | 46 +++++++++++++++++++++-------------------------
>>  3 files changed, 81 insertions(+), 27 deletions(-)
>> 
>> diff --git a/convert/main.c b/convert/main.c
>> index 6bdfab40d0b0..dd6b42a1f2b1 100644
>> --- a/convert/main.c
>> +++ b/convert/main.c
>> @@ -1002,6 +1002,63 @@ err:
>>  	return ret;
>>  }
>>  
>> +/*
>> + * Link the subvolume specified by @root_objectid to the root_dir of @root.
>
>However the function name is btrfs_mksubvol(), not btrfs_link_subvol().
>After reading the code, it turns out that btrfs_mksubvol() is just an
>less generic btrfs_link_subvol().

The function name is really confusing. I will rename this function and
reword this comment to make it clear.

>
>> + * @root		the root of the file tree which the subvolume will
>> + *			be linked to.
>
>Here you're using btrfs_root for source subvolume.
>
>> + * @subvol_name		the name of the subvolume which will be linked.
>> + * @root_objectid	specify the subvolume which will be linked.
>
>But here you're specifying subvolume id as destination.
>
>It would be much better to unify both parameter to the same structure.
>And personally I prefer both btrfs_root.

Unfortunately, btrfs_root can't pass the subvolume name string.

>
>> + * @convert		the flag to determine whether to try to resolve
>> + *			the name conflict.
>
>This parameter is not used in this function, and the name "convert"
>doesn't reflect the name conflict check.
>

Yeah, I keep it to follow the original btrfs_mksubvol, but it is really
redundant and I will remove it. Of course, I will also rename it at
btrfs_link_subvol.

>Personally speaking, I would like the parameters to be something like
>btrfs_mksubolv(struct btrfs_root *dst_root,
>               struct btrfs_root *src_root)

How about the following defination?
link_subvolv_for_convert(struct btrfs_root *root,
			 const char *subvol_name, u64 root_objectid)

-- 
Thanks,
Lu

>
>> + *
>> + * Return the root of the subvolume if success, otherwise return NULL.
>> + */
>> +static struct btrfs_root *btrfs_mksubvol(struct btrfs_root *root,
>> +					 const char *subvol_name,
>> +					 u64 root_objectid,
>> +					 bool convert)
>> +{
>> +	struct btrfs_trans_handle *trans;
>> +	struct btrfs_root *subvol_root = NULL;
>> +	struct btrfs_key key;
>> +	u64 dirid = btrfs_root_dirid(&root->root_item);
>> +	int ret;
>> +
>> +
>> +	trans = btrfs_start_transaction(root, 1);
>> +	if (IS_ERR(trans)) {
>> +		error("unable to start transaction");
>> +		goto fail;
>> +	}
>> +
>> +	ret = btrfs_link_subvol(trans, root, subvol_name, root_objectid, dirid,
>> +				true);
>> +	if (ret) {
>> +		error("unable to link subvolume %s", subvol_name);
>> +		goto fail;
>> +	}
>> +
>> +	ret = btrfs_commit_transaction(trans, root);
>> +	if (ret) {
>> +		error("transaction commit failed: %d", ret);
>> +		goto fail;
>> +	}
>> +
>> +	key.objectid = root_objectid;
>> +	key.offset = (u64)-1;
>> +	key.type = BTRFS_ROOT_ITEM_KEY;
>> +
>> +	subvol_root = btrfs_read_fs_root(root->fs_info, &key);
>> +	if (!subvol_root) {
>> +		error("unable to link subvolume %s", subvol_name);
>> +		goto fail;
>> +	}
>> +
>> +fail:
>> +	return subvol_root;
>> +}
>> +
>>  /*
>>   * Migrate super block to its default position and zero 0 ~ 16k
>>   */
>> diff --git a/ctree.h b/ctree.h
>> index 0fc31dd8d998..4bff0b821472 100644
>> --- a/ctree.h
>> +++ b/ctree.h
>> @@ -2797,8 +2797,9 @@ int btrfs_del_orphan_item(struct btrfs_trans_handle *trans,
>>  			  struct btrfs_root *root, u64 offset);
>>  int btrfs_mkdir(struct btrfs_trans_handle *trans, struct btrfs_root *root,
>>  		char *name, int namelen, u64 parent_ino, u64 *ino, int mode);
>> -struct btrfs_root *btrfs_mksubvol(struct btrfs_root *root, const char *base,
>> -				  u64 root_objectid, bool convert);
>> +int btrfs_link_subvol(struct btrfs_trans_handle *trans, struct btrfs_root *root,
>> +		      const char *base, u64 root_objectid, u64 dirid,
>> +		      bool convert);
>>  
>>  /* file.c */
>>  int btrfs_get_extent(struct btrfs_trans_handle *trans,
>> diff --git a/inode.c b/inode.c
>> index 8d0812c7cf50..478036562652 100644
>> --- a/inode.c
>> +++ b/inode.c
>> @@ -606,19 +606,28 @@ out:
>>  	return ret;
>>  }
>>  
>> -struct btrfs_root *btrfs_mksubvol(struct btrfs_root *root,
>> -				  const char *base, u64 root_objectid,
>> -				  bool convert)
>> +/*
>> + * Link the subvolume specified by @root_objectid to the directory specified by
>> + * @dirid on the file tree specified by @root.
>> + *
>> + * @root		the root of the file tree where the directory on.
>> + * @base		the name of the subvolume which will be linked.
>> + * @root_objectid	specify the subvolume which will be linked.
>
>Same nitpick about parameter here.
>
>Thanks,
>Qu
>
>> + * @dirid		specify the directory which the subvolume will be
>> + *			linked to.
>> + * @convert		the flag to determine whether to try to resolve
>> + *			the name conflict.
>> + */
>> +int btrfs_link_subvol(struct btrfs_trans_handle *trans, struct btrfs_root *root,
>> +		      const char *base, u64 root_objectid, u64 dirid,
>> +		      bool convert)
>>  {
>> -	struct btrfs_trans_handle *trans;
>>  	struct btrfs_fs_info *fs_info = root->fs_info;
>>  	struct btrfs_root *tree_root = fs_info->tree_root;
>> -	struct btrfs_root *new_root = NULL;
>>  	struct btrfs_path path;
>>  	struct btrfs_inode_item *inode_item;
>>  	struct extent_buffer *leaf;
>>  	struct btrfs_key key;
>> -	u64 dirid = btrfs_root_dirid(&root->root_item);
>>  	u64 index = 2;
>>  	char buf[BTRFS_NAME_LEN + 1]; /* for snprintf null */
>>  	int len;
>> @@ -627,8 +636,9 @@ struct btrfs_root *btrfs_mksubvol(struct btrfs_root *root,
>>  
>>  	len = strlen(base);
>>  	if (len == 0 || len > BTRFS_NAME_LEN)
>> -		return NULL;
>> +		return -EINVAL;
>>  
>> +	/* find the free dir_index */
>>  	btrfs_init_path(&path);
>>  	key.objectid = dirid;
>>  	key.type = BTRFS_DIR_INDEX_KEY;
>> @@ -649,12 +659,7 @@ struct btrfs_root *btrfs_mksubvol(struct btrfs_root *root,
>>  	}
>>  	btrfs_release_path(&path);
>>  
>> -	trans = btrfs_start_transaction(root, 1);
>> -	if (IS_ERR(trans)) {
>> -		error("unable to start transaction");
>> -		goto fail;
>> -	}
>> -
>> +	/* add the dir_item/dir_index */
>>  	key.objectid = dirid;
>>  	key.offset = 0;
>>  	key.type =  BTRFS_INODE_ITEM_KEY;
>> @@ -675,6 +680,7 @@ struct btrfs_root *btrfs_mksubvol(struct btrfs_root *root,
>>  
>>  	memcpy(buf, base, len);
>>  	if (convert) {
>> +		/* try to resolve name conflict by adding the number suffix */
>>  		for (i = 0; i < 1024; i++) {
>>  			ret = btrfs_insert_dir_item(trans, root, buf, len,
>>  					dirid, &key, BTRFS_FT_DIR, index);
>> @@ -719,18 +725,8 @@ struct btrfs_root *btrfs_mksubvol(struct btrfs_root *root,
>>  		goto fail;
>>  	}
>>  
>> -	ret = btrfs_commit_transaction(trans, root);
>> -	if (ret) {
>> -		error("transaction commit failed: %d", ret);
>> -		goto fail;
>> -	}
>>  
>> -	new_root = btrfs_read_fs_root(fs_info, &key);
>> -	if (IS_ERR(new_root)) {
>> -		error("unable to fs read root: %lu", PTR_ERR(new_root));
>> -		new_root = NULL;
>> -	}
>>  fail:
>> -	btrfs_init_path(&path);
>> -	return new_root;
>> +	btrfs_release_path(&path);
>> +	return ret;
>>  }
>> 
>

[PATCH v2 03/10] btrfs-progs: use btrfs_find_free_dir_index to find free inode index

[Lu Fengqi]

Since we have an existing function to find free inode index, we can
reuse it here.

Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
---
 inode.c | 27 +++++++--------------------
 1 file changed, 7 insertions(+), 20 deletions(-)

diff --git a/inode.c b/inode.c
index 478036562652..86905365dfd8 100644
--- a/inode.c
+++ b/inode.c
@@ -628,7 +628,7 @@ int btrfs_link_subvol(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 	struct btrfs_inode_item *inode_item;
 	struct extent_buffer *leaf;
 	struct btrfs_key key;
-	u64 index = 2;
+	u64 index;
 	char buf[BTRFS_NAME_LEN + 1]; /* for snprintf null */
 	int len;
 	int i;
@@ -638,28 +638,15 @@ int btrfs_link_subvol(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 	if (len == 0 || len > BTRFS_NAME_LEN)
 		return -EINVAL;
 
-	/* find the free dir_index */
-	btrfs_init_path(&path);
-	key.objectid = dirid;
-	key.type = BTRFS_DIR_INDEX_KEY;
-	key.offset = (u64)-1;
-
-	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
-	if (ret <= 0) {
-		error("search for DIR_INDEX dirid %llu failed: %d",
-				(unsigned long long)dirid, ret);
+	/* add the dir_item/dir_index */
+	ret = btrfs_find_free_dir_index(root, dirid, &index);
+	if (ret < 0) {
+		error("unable to find free dir index dirid %llu failed: %d",
+		      (unsigned long long)dirid, ret);
 		goto fail;
 	}
 
-	if (path.slots[0] > 0) {
-		path.slots[0]--;
-		btrfs_item_key_to_cpu(path.nodes[0], &key, path.slots[0]);
-		if (key.objectid == dirid && key.type == BTRFS_DIR_INDEX_KEY)
-			index = key.offset + 1;
-	}
-	btrfs_release_path(&path);
-
-	/* add the dir_item/dir_index */
+	btrfs_init_path(&path);
 	key.objectid = dirid;
 	key.offset = 0;
 	key.type =  BTRFS_INODE_ITEM_KEY;
-- 
2.16.2

Re: [PATCH v2 03/10] btrfs-progs: use btrfs_find_free_dir_index to find free inode index

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 2014 bytes --]



On 2018年03月27日 15:06, Lu Fengqi wrote:
> Since we have an existing function to find free inode index, we can
> reuse it here.
> 
> Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>

Looks good.

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>  inode.c | 27 +++++++--------------------
>  1 file changed, 7 insertions(+), 20 deletions(-)
> 
> diff --git a/inode.c b/inode.c
> index 478036562652..86905365dfd8 100644
> --- a/inode.c
> +++ b/inode.c
> @@ -628,7 +628,7 @@ int btrfs_link_subvol(struct btrfs_trans_handle *trans, struct btrfs_root *root,
>  	struct btrfs_inode_item *inode_item;
>  	struct extent_buffer *leaf;
>  	struct btrfs_key key;
> -	u64 index = 2;
> +	u64 index;
>  	char buf[BTRFS_NAME_LEN + 1]; /* for snprintf null */
>  	int len;
>  	int i;
> @@ -638,28 +638,15 @@ int btrfs_link_subvol(struct btrfs_trans_handle *trans, struct btrfs_root *root,
>  	if (len == 0 || len > BTRFS_NAME_LEN)
>  		return -EINVAL;
>  
> -	/* find the free dir_index */
> -	btrfs_init_path(&path);
> -	key.objectid = dirid;
> -	key.type = BTRFS_DIR_INDEX_KEY;
> -	key.offset = (u64)-1;
> -
> -	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
> -	if (ret <= 0) {
> -		error("search for DIR_INDEX dirid %llu failed: %d",
> -				(unsigned long long)dirid, ret);
> +	/* add the dir_item/dir_index */
> +	ret = btrfs_find_free_dir_index(root, dirid, &index);
> +	if (ret < 0) {
> +		error("unable to find free dir index dirid %llu failed: %d",
> +		      (unsigned long long)dirid, ret);
>  		goto fail;
>  	}
>  
> -	if (path.slots[0] > 0) {
> -		path.slots[0]--;
> -		btrfs_item_key_to_cpu(path.nodes[0], &key, path.slots[0]);
> -		if (key.objectid == dirid && key.type == BTRFS_DIR_INDEX_KEY)
> -			index = key.offset + 1;
> -	}
> -	btrfs_release_path(&path);
> -
> -	/* add the dir_item/dir_index */
> +	btrfs_init_path(&path);
>  	key.objectid = dirid;
>  	key.offset = 0;
>  	key.type =  BTRFS_INODE_ITEM_KEY;
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v2 04/10] btrfs-progs: undelete-subvol: introduce is_subvol_intact

[Lu Fengqi]

The function is used to determine whether the subvolume is intact.

Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
---
 Makefile          |  3 ++-
 undelete-subvol.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 undelete-subvol.h | 17 +++++++++++++++++
 3 files changed, 72 insertions(+), 1 deletion(-)
 create mode 100644 undelete-subvol.c
 create mode 100644 undelete-subvol.h

diff --git a/Makefile b/Makefile
index 6065522a615f..cb38984c7386 100644
--- a/Makefile
+++ b/Makefile
@@ -116,7 +116,8 @@ objects = ctree.o disk-io.o kernel-lib/radix-tree.o extent-tree.o print-tree.o \
 	  qgroup.o free-space-cache.o kernel-lib/list_sort.o props.o \
 	  kernel-shared/ulist.o qgroup-verify.o backref.o string-table.o task-utils.o \
 	  inode.o file.o find-root.o free-space-tree.o help.o send-dump.o \
-	  fsfeatures.o kernel-lib/tables.o kernel-lib/raid56.o transaction.o
+	  fsfeatures.o kernel-lib/tables.o kernel-lib/raid56.o transaction.o \
+	  undelete-subvol.o
 cmds_objects = cmds-subvolume.o cmds-filesystem.o cmds-device.o cmds-scrub.o \
 	       cmds-inspect.o cmds-balance.o cmds-send.o cmds-receive.o \
 	       cmds-quota.o cmds-qgroup.o cmds-replace.o check/main.o \
diff --git a/undelete-subvol.c b/undelete-subvol.c
new file mode 100644
index 000000000000..00fcc4895778
--- /dev/null
+++ b/undelete-subvol.c
@@ -0,0 +1,53 @@
+/*
+ * Copyright (C) 2018 Fujitsu.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License v2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ */
+
+#include "ctree.h"
+
+/*
+ * Determines whether the subvolume is intact, according to the drop_progress
+ * of the root item specified by subvol_id.
+ *
+ * Return true if the subvolume is intact, otherwise return false.
+ */
+static bool is_subvol_intact(struct btrfs_root *root, u64 subvol_id)
+{
+	struct btrfs_path path;
+	struct btrfs_key key;
+	struct extent_buffer *leaf;
+	struct btrfs_root_item root_item;
+	u64 offset;
+	int ret;
+
+	key.objectid = subvol_id;
+	key.type = BTRFS_ROOT_ITEM_KEY;
+	key.offset = 0;
+
+	btrfs_init_path(&path);
+	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
+	if (ret) {
+		ret = false;
+		goto out;
+	}
+
+	leaf = path.nodes[0];
+	offset = btrfs_item_ptr_offset(leaf, path.slots[0]);
+
+	read_extent_buffer(leaf, &root_item, offset, sizeof(root_item));
+
+	/* the subvolume is intact if the objectid of drop_progress == 0 */
+	ret = btrfs_disk_key_objectid(&root_item.drop_progress) ? false : true;
+
+out:
+	btrfs_release_path(&path);
+	return ret;
+}
diff --git a/undelete-subvol.h b/undelete-subvol.h
new file mode 100644
index 000000000000..7cfd100cce37
--- /dev/null
+++ b/undelete-subvol.h
@@ -0,0 +1,17 @@
+/*
+ * Copyright (C) 2018 Fujitsu.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License v2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ */
+
+#ifndef __BTRFS_UNDELETE_SUBVOLUME_H__
+#define __BTRFS_UNDELETE_SUBVOLUME_H__
+
+#endif
-- 
2.16.2

Re: [PATCH v2 04/10] btrfs-progs: undelete-subvol: introduce is_subvol_intact

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 4572 bytes --]



On 2018年03月27日 15:06, Lu Fengqi wrote:
> The function is used to determine whether the subvolume is intact.
> 
> Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
> ---
>  Makefile          |  3 ++-
>  undelete-subvol.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  undelete-subvol.h | 17 +++++++++++++++++
>  3 files changed, 72 insertions(+), 1 deletion(-)
>  create mode 100644 undelete-subvol.c
>  create mode 100644 undelete-subvol.h
> 
> diff --git a/Makefile b/Makefile
> index 6065522a615f..cb38984c7386 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -116,7 +116,8 @@ objects = ctree.o disk-io.o kernel-lib/radix-tree.o extent-tree.o print-tree.o \
>  	  qgroup.o free-space-cache.o kernel-lib/list_sort.o props.o \
>  	  kernel-shared/ulist.o qgroup-verify.o backref.o string-table.o task-utils.o \
>  	  inode.o file.o find-root.o free-space-tree.o help.o send-dump.o \
> -	  fsfeatures.o kernel-lib/tables.o kernel-lib/raid56.o transaction.o
> +	  fsfeatures.o kernel-lib/tables.o kernel-lib/raid56.o transaction.o \
> +	  undelete-subvol.o
>  cmds_objects = cmds-subvolume.o cmds-filesystem.o cmds-device.o cmds-scrub.o \
>  	       cmds-inspect.o cmds-balance.o cmds-send.o cmds-receive.o \
>  	       cmds-quota.o cmds-qgroup.o cmds-replace.o check/main.o \
> diff --git a/undelete-subvol.c b/undelete-subvol.c
> new file mode 100644
> index 000000000000..00fcc4895778
> --- /dev/null
> +++ b/undelete-subvol.c
> @@ -0,0 +1,53 @@
> +/*
> + * Copyright (C) 2018 Fujitsu.  All rights reserved.

IIRC David will remove all such copy right line.
Is there some principle about this, David?

> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public
> + * License v2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + */
> +
> +#include "ctree.h"
> +
> +/*
> + * Determines whether the subvolume is intact, according to the drop_progress
> + * of the root item specified by subvol_id.
> + *
> + * Return true if the subvolume is intact, otherwise return false.
> + */
> +static bool is_subvol_intact(struct btrfs_root *root, u64 subvol_id)

Here we have 2 parameters which can represent a root, so please explain
the meaning of each parameter.

And after looking into the code, @root should be tree root, and in that
case, I prefer passing @fs_info here to get rid of any confusion.

> +{
> +	struct btrfs_path path;
> +	struct btrfs_key key;
> +	struct extent_buffer *leaf;
> +	struct btrfs_root_item root_item;
> +	u64 offset;
> +	int ret;
> +
> +	key.objectid = subvol_id;
> +	key.type = BTRFS_ROOT_ITEM_KEY;
> +	key.offset = 0;
> +
> +	btrfs_init_path(&path);
> +	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);

Instead of setting up keys and search manually, what about using
btrfs_read_fs_root()?

> +	if (ret) {
> +		ret = false;
> +		goto out;
> +	}
> +
> +	leaf = path.nodes[0];
> +	offset = btrfs_item_ptr_offset(leaf, path.slots[0]);
> +
> +	read_extent_buffer(leaf, &root_item, offset, sizeof(root_item));
> +
> +	/* the subvolume is intact if the objectid of drop_progress == 0 */
> +	ret = btrfs_disk_key_objectid(&root_item.drop_progress) ? false : true;
> +
> +out:
> +	btrfs_release_path(&path);
> +	return ret;
> +}
> diff --git a/undelete-subvol.h b/undelete-subvol.h
> new file mode 100644
> index 000000000000..7cfd100cce37
> --- /dev/null
> +++ b/undelete-subvol.h
> @@ -0,0 +1,17 @@
> +/*
> + * Copyright (C) 2018 Fujitsu.  All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public
> + * License v2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + */
> +
> +#ifndef __BTRFS_UNDELETE_SUBVOLUME_H__
I think the empty header is not really needed here.
What about introducing it when we have something here?

Thanks,
Qu

> +#define __BTRFS_UNDELETE_SUBVOLUME_H__
> +
> +#endif
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 04/10] btrfs-progs: undelete-subvol: introduce is_subvol_intact

[Lu Fengqi]

On Wed, Apr 18, 2018 at 01:12:10PM +0800, Qu Wenruo wrote:
>
>
>On 2018年03月27日 15:06, Lu Fengqi wrote:
>> The function is used to determine whether the subvolume is intact.
>> 
>> Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
>> ---
>>  Makefile          |  3 ++-
>>  undelete-subvol.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  undelete-subvol.h | 17 +++++++++++++++++
>>  3 files changed, 72 insertions(+), 1 deletion(-)
>>  create mode 100644 undelete-subvol.c
>>  create mode 100644 undelete-subvol.h
>> 
>> diff --git a/Makefile b/Makefile
>> index 6065522a615f..cb38984c7386 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -116,7 +116,8 @@ objects = ctree.o disk-io.o kernel-lib/radix-tree.o extent-tree.o print-tree.o \
>>  	  qgroup.o free-space-cache.o kernel-lib/list_sort.o props.o \
>>  	  kernel-shared/ulist.o qgroup-verify.o backref.o string-table.o task-utils.o \
>>  	  inode.o file.o find-root.o free-space-tree.o help.o send-dump.o \
>> -	  fsfeatures.o kernel-lib/tables.o kernel-lib/raid56.o transaction.o
>> +	  fsfeatures.o kernel-lib/tables.o kernel-lib/raid56.o transaction.o \
>> +	  undelete-subvol.o
>>  cmds_objects = cmds-subvolume.o cmds-filesystem.o cmds-device.o cmds-scrub.o \
>>  	       cmds-inspect.o cmds-balance.o cmds-send.o cmds-receive.o \
>>  	       cmds-quota.o cmds-qgroup.o cmds-replace.o check/main.o \
>> diff --git a/undelete-subvol.c b/undelete-subvol.c
>> new file mode 100644
>> index 000000000000..00fcc4895778
>> --- /dev/null
>> +++ b/undelete-subvol.c
>> @@ -0,0 +1,53 @@
>> +/*
>> + * Copyright (C) 2018 Fujitsu.  All rights reserved.
>
>IIRC David will remove all such copy right line.
>Is there some principle about this, David?

Are you referring to this patchset that replace GPL boilerplate by SPDX?
https://patchwork.kernel.org/patch/10321621/

However, I haven't seen a similar patch in btrfs-progs.

>
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public
>> + * License v2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * General Public License for more details.
>> + */
>> +
>> +#include "ctree.h"
>> +
>> +/*
>> + * Determines whether the subvolume is intact, according to the drop_progress
>> + * of the root item specified by subvol_id.
>> + *
>> + * Return true if the subvolume is intact, otherwise return false.
>> + */
>> +static bool is_subvol_intact(struct btrfs_root *root, u64 subvol_id)
>
>Here we have 2 parameters which can represent a root, so please explain
>the meaning of each parameter.
>
>And after looking into the code, @root should be tree root, and in that
>case, I prefer passing @fs_info here to get rid of any confusion.

Make sense.

>
>> +{
>> +	struct btrfs_path path;
>> +	struct btrfs_key key;
>> +	struct extent_buffer *leaf;
>> +	struct btrfs_root_item root_item;
>> +	u64 offset;
>> +	int ret;
>> +
>> +	key.objectid = subvol_id;
>> +	key.type = BTRFS_ROOT_ITEM_KEY;
>> +	key.offset = 0;
>> +
>> +	btrfs_init_path(&path);
>> +	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
>
>Instead of setting up keys and search manually, what about using
>btrfs_read_fs_root()?

Indeed looks better.

>
>> +	if (ret) {
>> +		ret = false;
>> +		goto out;
>> +	}
>> +
>> +	leaf = path.nodes[0];
>> +	offset = btrfs_item_ptr_offset(leaf, path.slots[0]);
>> +
>> +	read_extent_buffer(leaf, &root_item, offset, sizeof(root_item));
>> +
>> +	/* the subvolume is intact if the objectid of drop_progress == 0 */
>> +	ret = btrfs_disk_key_objectid(&root_item.drop_progress) ? false : true;
>> +
>> +out:
>> +	btrfs_release_path(&path);
>> +	return ret;
>> +}
>> diff --git a/undelete-subvol.h b/undelete-subvol.h
>> new file mode 100644
>> index 000000000000..7cfd100cce37
>> --- /dev/null
>> +++ b/undelete-subvol.h
>> @@ -0,0 +1,17 @@
>> +/*
>> + * Copyright (C) 2018 Fujitsu.  All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public
>> + * License v2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * General Public License for more details.
>> + */
>> +
>> +#ifndef __BTRFS_UNDELETE_SUBVOLUME_H__
>I think the empty header is not really needed here.
>What about introducing it when we have something here?

That's OK.

Thanks for your review.

--
Thanks,
Lu

>
>Thanks,
>Qu
>
>> +#define __BTRFS_UNDELETE_SUBVOLUME_H__
>> +
>> +#endif
>> 
>

Re: [PATCH v2 04/10] btrfs-progs: undelete-subvol: introduce is_subvol_intact

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 5910 bytes --]



On 2018年05月07日 10:03, Lu Fengqi wrote:
> On Wed, Apr 18, 2018 at 01:12:10PM +0800, Qu Wenruo wrote:
>>
>>
>> On 2018年03月27日 15:06, Lu Fengqi wrote:
>>> The function is used to determine whether the subvolume is intact.
>>>
>>> Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
>>> ---
>>>  Makefile          |  3 ++-
>>>  undelete-subvol.c | 53
>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  undelete-subvol.h | 17 +++++++++++++++++
>>>  3 files changed, 72 insertions(+), 1 deletion(-)
>>>  create mode 100644 undelete-subvol.c
>>>  create mode 100644 undelete-subvol.h
>>>
>>> diff --git a/Makefile b/Makefile
>>> index 6065522a615f..cb38984c7386 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -116,7 +116,8 @@ objects =tree.o disk-io.o kernel-lib/radix-tree.o
>>> extent-tree.o print-tree.o \
>>>        qgroup.o free-space-cache.o kernel-lib/list_sort.o props.o \
>>>        kernel-shared/ulist.o qgroup-verify.o backref.o string-table.o
>>> task-utils.o \
>>>        inode.o file.o find-root.o free-space-tree.o help.o send-dump.o \
>>> -      fsfeatures.o kernel-lib/tables.o kernel-lib/raid56.o
>>> transaction.o
>>> +      fsfeatures.o kernel-lib/tables.o kernel-lib/raid56.o
>>> transaction.o \
>>> +      undelete-subvol.o
>>>  cmds_objects =mds-subvolume.o cmds-filesystem.o cmds-device.o
>>> cmds-scrub.o \
>>>             cmds-inspect.o cmds-balance.o cmds-send.o cmds-receive.o \
>>>             cmds-quota.o cmds-qgroup.o cmds-replace.o check/main.o \
>>> diff --git a/undelete-subvol.c b/undelete-subvol.c
>>> new file mode 100644
>>> index 000000000000..00fcc4895778
>>> --- /dev/null
>>> +++ b/undelete-subvol.c
>>> @@ -0,0 +1,53 @@
>>> +/*
>>> + * Copyright (C) 2018 Fujitsu.  All rights reserved.
>>
>> IIRC David will remove all such copy right line.
>> Is there some principle about this, David?
> 
> Are you referring to this patchset that replace GPL boilerplate by SPDX?
> https://patchwork.kernel.org/patch/10321621/
> 
> However, I haven't seen a similar patch in btrfs-progs.
> 

Nope, I mean David will remove the Copyright (C) line when applying.
Although I'm not completely sure.

Thanks,
Qu

>>
>>> + *
>>> + * This program is free software; you can redistribute it and/or
>>> + * modify it under the terms of the GNU General Public
>>> + * License v2 as published by the Free Software Foundation.
>>> + *
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>> + * General Public License for more details.
>>> + */
>>> +
>>> +#include "ctree.h"
>>> +
>>> +/*
>>> + * Determines whether the subvolume is intact, according to the
>>> drop_progress
>>> + * of the root item specified by subvol_id.
>>> + *
>>> + * Return true if the subvolume is intact, otherwise return false.
>>> + */
>>> +static bool is_subvol_intact(struct btrfs_root *root, u64 subvol_id)
>>
>> Here we have 2 parameters which can represent a root, so please explain
>> the meaning of each parameter.
>>
>> And after looking into the code, @root should be tree root, and in that
>> case, I prefer passing @fs_info here to get rid of any confusion.
> 
> Make sense.
> 
>>
>>> +{
>>> +    struct btrfs_path path;
>>> +    struct btrfs_key key;
>>> +    struct extent_buffer *leaf;
>>> +    struct btrfs_root_item root_item;
>>> +    u64 offset;
>>> +    int ret;
>>> +
>>> +    key.objectid =ubvol_id;
>>> +    key.type =TRFS_ROOT_ITEM_KEY;
>>> +    key.offset =;
>>> +
>>> +    btrfs_init_path(&path);
>>> +    ret =trfs_search_slot(NULL, root, &key, &path, 0, 0);
>>
>> Instead of setting up keys and search manually, what about using
>> btrfs_read_fs_root()?
> 
> Indeed looks better.
> 
>>
>>> +    if (ret) {
>>> +        ret =alse;
>>> +        goto out;
>>> +    }
>>> +
>>> +    leaf =ath.nodes[0];
>>> +    offset =trfs_item_ptr_offset(leaf, path.slots[0]);
>>> +
>>> +    read_extent_buffer(leaf, &root_item, offset, sizeof(root_item));
>>> +
>>> +    /* the subvolume is intact if the objectid of drop_progress =0 */
>>> +    ret =trfs_disk_key_objectid(&root_item.drop_progress) ? false :
>>> true;
>>> +
>>> +out:
>>> +    btrfs_release_path(&path);
>>> +    return ret;
>>> +}
>>> diff --git a/undelete-subvol.h b/undelete-subvol.h
>>> new file mode 100644
>>> index 000000000000..7cfd100cce37
>>> --- /dev/null
>>> +++ b/undelete-subvol.h
>>> @@ -0,0 +1,17 @@
>>> +/*
>>> + * Copyright (C) 2018 Fujitsu.  All rights reserved.
>>> + *
>>> + * This program is free software; you can redistribute it and/or
>>> + * modify it under the terms of the GNU General Public
>>> + * License v2 as published by the Free Software Foundation.
>>> + *
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>> + * General Public License for more details.
>>> + */
>>> +
>>> +#ifndef __BTRFS_UNDELETE_SUBVOLUME_H__
>> I think the empty header is not really needed here.
>> What about introducing it when we have something here?
> 
> That's OK.
> 
> Thanks for your review.
> 
> -- 
> Thanks,
> Lu
> 
>>
>> Thanks,
>> Qu
>>
>>> +#define __BTRFS_UNDELETE_SUBVOLUME_H__
>>> +
>>> +#endif
>>>
>>
> 
> 
> 
> 
> 
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 04/10] btrfs-progs: undelete-subvol: introduce is_subvol_intact

[David Sterba]

On Mon, May 07, 2018 at 10:20:57AM +0800, Qu Wenruo wrote:
> >>> +++ b/undelete-subvol.c
> >>> @@ -0,0 +1,53 @@
> >>> +/*
> >>> + * Copyright (C) 2018 Fujitsu.  All rights reserved.
> >>
> >> IIRC David will remove all such copy right line.
> >> Is there some principle about this, David?
> > 
> > Are you referring to this patchset that replace GPL boilerplate by SPDX?
> > https://patchwork.kernel.org/patch/10321621/
> > 
> > However, I haven't seen a similar patch in btrfs-progs.
> > 
> 
> Nope, I mean David will remove the Copyright (C) line when applying.
> Although I'm not completely sure.

Removing copyright notices should not be done without an ack from all
parties and I don't remove the copyright notices from patches if
present.

I personally do not understand and see the point of the explicit
mentions if we have the signed-off by in git history. Also, when the
file is touched by many different people over time, the line 'copyright
by Evil Company' looks like all the credit goes to the single entitiy.

There's a point if the code is copied from another source, like the
test/sha* code that's from an RFC.

So, I decided to not care too much about the copyright line as long as
there's a signed-off and authorship is GPL compatible.

Re: [PATCH v2 04/10] btrfs-progs: undelete-subvol: introduce is_subvol_intact

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 1675 bytes --]



On 2018年05月07日 19:40, David Sterba wrote:
> On Mon, May 07, 2018 at 10:20:57AM +0800, Qu Wenruo wrote:
>>>>> +++ b/undelete-subvol.c
>>>>> @@ -0,0 +1,53 @@
>>>>> +/*
>>>>> + * Copyright (C) 2018 Fujitsu.  All rights reserved.
>>>>
>>>> IIRC David will remove all such copy right line.
>>>> Is there some principle about this, David?
>>>
>>> Are you referring to this patchset that replace GPL boilerplate by SPDX?
>>> https://patchwork.kernel.org/patch/10321621/
>>>
>>> However, I haven't seen a similar patch in btrfs-progs.
>>>
>>
>> Nope, I mean David will remove the Copyright (C) line when applying.
>> Although I'm not completely sure.
> 
> Removing copyright notices should not be done without an ack from all
> parties and I don't remove the copyright notices from patches if
> present.
> 
> I personally do not understand and see the point of the explicit
> mentions if we have the signed-off by in git history. Also, when the
> file is touched by many different people over time, the line 'copyright
> by Evil Company' looks like all the credit goes to the single entitiy.

That seems to be the point. :)

> 
> There's a point if the code is copied from another source, like the
> test/sha* code that's from an RFC.
> 
> So, I decided to not care too much about the copyright line as long as
> there's a signed-off and authorship is GPL compatible.

Glad that the policy is now clear.

Thanks,
Qu

> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v2 05/10] btrfs-progs: undelete-subvol: introduce recover_dead_root

[Lu Fengqi]

The function will find the root_item specified by the subvol_id,
clear the BTRFS_ROOT_SUBVOL_DEAD flag and set root_refs to one.

Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
---
 ctree.h           |  1 +
 undelete-subvol.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+)

diff --git a/ctree.h b/ctree.h
index 4bff0b821472..7c0b8150bc4e 100644
--- a/ctree.h
+++ b/ctree.h
@@ -184,6 +184,7 @@ static int btrfs_csum_sizes[] = { 4 };
 #define BTRFS_FT_MAX		9
 
 #define BTRFS_ROOT_SUBVOL_RDONLY	(1ULL << 0)
+#define BTRFS_ROOT_SUBVOL_DEAD		(1ULL << 48)
 
 /*
  * the key defines the order in the tree, and so it also defines (optimal)
diff --git a/undelete-subvol.c b/undelete-subvol.c
index 00fcc4895778..781057df2b84 100644
--- a/undelete-subvol.c
+++ b/undelete-subvol.c
@@ -12,6 +12,9 @@
  */
 
 #include "ctree.h"
+#include "transaction.h"
+#include "disk-io.h"
+#include "messages.h"
 
 /*
  * Determines whether the subvolume is intact, according to the drop_progress
@@ -51,3 +54,55 @@ out:
 	btrfs_release_path(&path);
 	return ret;
 }
+
+/*
+ * Clear BTRFS_ROOT_SUBVOL_DEAD flag and set the root_refs to one.
+ *
+ * @root	the root of root tree.
+ * @subvol_id	specify the root_item which will be modified.
+ *
+ * Return 0 if no error occurred.
+ */
+static int recover_dead_root(struct btrfs_trans_handle *trans,
+			     struct btrfs_root *root, u64 subvol_id)
+{
+	struct btrfs_key key;
+	struct btrfs_path path;
+	struct extent_buffer *leaf;
+	struct btrfs_root_item root_item;
+	u64 root_flags;
+	u64 offset;
+	int ret;
+
+	key.objectid = subvol_id;
+	key.type = BTRFS_ROOT_ITEM_KEY;
+	key.offset = 0;
+
+	btrfs_init_path(&path);
+	ret = btrfs_search_slot(trans, root, &key, &path, 0, 0);
+	if (ret) {
+		error("couldn't find ROOT_ITEM for %llu failed: %d",
+				subvol_id, ret);
+		goto out;
+	}
+
+	leaf = path.nodes[0];
+
+	offset = btrfs_item_ptr_offset(leaf, path.slots[0]);
+	read_extent_buffer(leaf, &root_item, offset, sizeof(root_item));
+
+	/* Clear BTRFS_ROOT_SUBVOL_DEAD */
+	root_flags = btrfs_root_flags(&root_item);
+	btrfs_set_root_flags(&root_item,
+			     root_flags & ~BTRFS_ROOT_SUBVOL_DEAD);
+
+	/* Increase the refs */
+	btrfs_set_root_refs(&root_item, 1);
+
+	write_extent_buffer(leaf, &root_item, offset, sizeof(root_item));
+	btrfs_mark_buffer_dirty(leaf);
+
+out:
+	btrfs_release_path(&path);
+	return ret;
+}
-- 
2.16.2

Re: [PATCH v2 05/10] btrfs-progs: undelete-subvol: introduce recover_dead_root

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 2913 bytes --]



On 2018年03月27日 15:06, Lu Fengqi wrote:
> The function will find the root_item specified by the subvol_id,
> clear the BTRFS_ROOT_SUBVOL_DEAD flag and set root_refs to one.

Sorry I didn't point it out in btrfs_link_subvol() patch, but at least
to me, refs should only be modified by btrfs_link_subvol(), just like
btrfs_unlink().

Thanks,
Qu

> 
> Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
> ---
>  ctree.h           |  1 +
>  undelete-subvol.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 56 insertions(+)
> 
> diff --git a/ctree.h b/ctree.h
> index 4bff0b821472..7c0b8150bc4e 100644
> --- a/ctree.h
> +++ b/ctree.h
> @@ -184,6 +184,7 @@ static int btrfs_csum_sizes[] = { 4 };
>  #define BTRFS_FT_MAX		9
>  
>  #define BTRFS_ROOT_SUBVOL_RDONLY	(1ULL << 0)
> +#define BTRFS_ROOT_SUBVOL_DEAD		(1ULL << 48)
>  
>  /*
>   * the key defines the order in the tree, and so it also defines (optimal)
> diff --git a/undelete-subvol.c b/undelete-subvol.c
> index 00fcc4895778..781057df2b84 100644
> --- a/undelete-subvol.c
> +++ b/undelete-subvol.c
> @@ -12,6 +12,9 @@
>   */
>  
>  #include "ctree.h"
> +#include "transaction.h"
> +#include "disk-io.h"
> +#include "messages.h"
>  
>  /*
>   * Determines whether the subvolume is intact, according to the drop_progress
> @@ -51,3 +54,55 @@ out:
>  	btrfs_release_path(&path);
>  	return ret;
>  }
> +
> +/*
> + * Clear BTRFS_ROOT_SUBVOL_DEAD flag and set the root_refs to one.
> + *
> + * @root	the root of root tree.
> + * @subvol_id	specify the root_item which will be modified.
> + *
> + * Return 0 if no error occurred.
> + */
> +static int recover_dead_root(struct btrfs_trans_handle *trans,
> +			     struct btrfs_root *root, u64 subvol_id)
> +{
> +	struct btrfs_key key;
> +	struct btrfs_path path;
> +	struct extent_buffer *leaf;
> +	struct btrfs_root_item root_item;
> +	u64 root_flags;
> +	u64 offset;
> +	int ret;
> +
> +	key.objectid = subvol_id;
> +	key.type = BTRFS_ROOT_ITEM_KEY;
> +	key.offset = 0;
> +
> +	btrfs_init_path(&path);
> +	ret = btrfs_search_slot(trans, root, &key, &path, 0, 0);
> +	if (ret) {
> +		error("couldn't find ROOT_ITEM for %llu failed: %d",
> +				subvol_id, ret);
> +		goto out;
> +	}
> +
> +	leaf = path.nodes[0];
> +
> +	offset = btrfs_item_ptr_offset(leaf, path.slots[0]);
> +	read_extent_buffer(leaf, &root_item, offset, sizeof(root_item));
> +
> +	/* Clear BTRFS_ROOT_SUBVOL_DEAD */
> +	root_flags = btrfs_root_flags(&root_item);
> +	btrfs_set_root_flags(&root_item,
> +			     root_flags & ~BTRFS_ROOT_SUBVOL_DEAD);
> +
> +	/* Increase the refs */
> +	btrfs_set_root_refs(&root_item, 1);
> +
> +	write_extent_buffer(leaf, &root_item, offset, sizeof(root_item));
> +	btrfs_mark_buffer_dirty(leaf);
> +
> +out:
> +	btrfs_release_path(&path);
> +	return ret;
> +}
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 05/10] btrfs-progs: undelete-subvol: introduce recover_dead_root

[Lu Fengqi]

On Wed, Apr 18, 2018 at 01:16:55PM +0800, Qu Wenruo wrote:
>
>
>On 2018年03月27日 15:06, Lu Fengqi wrote:
>> The function will find the root_item specified by the subvol_id,
>> clear the BTRFS_ROOT_SUBVOL_DEAD flag and set root_refs to one.
>
>Sorry I didn't point it out in btrfs_link_subvol() patch, but at least
>to me, refs should only be modified by btrfs_link_subvol(), just like
>btrfs_unlink().

This has bothered me a long time. The function create_subvol has set
root_refs to 1 before linking the subvolume in the kernel or
btrfs_progs.

I would like to modify the refs to 1 in btrfs_link_subvol as you said,
regardless of the root_refs of the subvolume is already 1 (only when
link_subvol_for_convert is caller).

-- 
Thanks,
Lu

>
>Thanks,
>Qu
>
>> 
>> Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
>> ---
>>  ctree.h           |  1 +
>>  undelete-subvol.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 56 insertions(+)
>> 
>> diff --git a/ctree.h b/ctree.h
>> index 4bff0b821472..7c0b8150bc4e 100644
>> --- a/ctree.h
>> +++ b/ctree.h
>> @@ -184,6 +184,7 @@ static int btrfs_csum_sizes[] = { 4 };
>>  #define BTRFS_FT_MAX		9
>>  
>>  #define BTRFS_ROOT_SUBVOL_RDONLY	(1ULL << 0)
>> +#define BTRFS_ROOT_SUBVOL_DEAD		(1ULL << 48)
>>  
>>  /*
>>   * the key defines the order in the tree, and so it also defines (optimal)
>> diff --git a/undelete-subvol.c b/undelete-subvol.c
>> index 00fcc4895778..781057df2b84 100644
>> --- a/undelete-subvol.c
>> +++ b/undelete-subvol.c
>> @@ -12,6 +12,9 @@
>>   */
>>  
>>  #include "ctree.h"
>> +#include "transaction.h"
>> +#include "disk-io.h"
>> +#include "messages.h"
>>  
>>  /*
>>   * Determines whether the subvolume is intact, according to the drop_progress
>> @@ -51,3 +54,55 @@ out:
>>  	btrfs_release_path(&path);
>>  	return ret;
>>  }
>> +
>> +/*
>> + * Clear BTRFS_ROOT_SUBVOL_DEAD flag and set the root_refs to one.
>> + *
>> + * @root	the root of root tree.
>> + * @subvol_id	specify the root_item which will be modified.
>> + *
>> + * Return 0 if no error occurred.
>> + */
>> +static int recover_dead_root(struct btrfs_trans_handle *trans,
>> +			     struct btrfs_root *root, u64 subvol_id)
>> +{
>> +	struct btrfs_key key;
>> +	struct btrfs_path path;
>> +	struct extent_buffer *leaf;
>> +	struct btrfs_root_item root_item;
>> +	u64 root_flags;
>> +	u64 offset;
>> +	int ret;
>> +
>> +	key.objectid = subvol_id;
>> +	key.type = BTRFS_ROOT_ITEM_KEY;
>> +	key.offset = 0;
>> +
>> +	btrfs_init_path(&path);
>> +	ret = btrfs_search_slot(trans, root, &key, &path, 0, 0);
>> +	if (ret) {
>> +		error("couldn't find ROOT_ITEM for %llu failed: %d",
>> +				subvol_id, ret);
>> +		goto out;
>> +	}
>> +
>> +	leaf = path.nodes[0];
>> +
>> +	offset = btrfs_item_ptr_offset(leaf, path.slots[0]);
>> +	read_extent_buffer(leaf, &root_item, offset, sizeof(root_item));
>> +
>> +	/* Clear BTRFS_ROOT_SUBVOL_DEAD */
>> +	root_flags = btrfs_root_flags(&root_item);
>> +	btrfs_set_root_flags(&root_item,
>> +			     root_flags & ~BTRFS_ROOT_SUBVOL_DEAD);
>> +
>> +	/* Increase the refs */
>> +	btrfs_set_root_refs(&root_item, 1);
>> +
>> +	write_extent_buffer(leaf, &root_item, offset, sizeof(root_item));
>> +	btrfs_mark_buffer_dirty(leaf);
>> +
>> +out:
>> +	btrfs_release_path(&path);
>> +	return ret;
>> +}
>> 
>

[PATCH v2 06/10] btrfs-progs: undelete-subvol: introduce link_subvol_to_lostfound

[Lu Fengqi]

The function will create lost+found directory, link the deleted
subvolume specified by the subvol_id to the directory, update the
information of root_item and cleanup the associated orphan item.

Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
---
 undelete-subvol.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 76 insertions(+)

diff --git a/undelete-subvol.c b/undelete-subvol.c
index 781057df2b84..9243e35545c5 100644
--- a/undelete-subvol.c
+++ b/undelete-subvol.c
@@ -106,3 +106,79 @@ out:
 	btrfs_release_path(&path);
 	return ret;
 }
+
+/*
+ * Recover a subvolume specified by subvol_id, and link it to the lost+found
+ * directory.
+ *
+ * @root: the root of the root tree.
+ * @subvol_id: specify the subvolume which will be linked, and also be the part
+ * of the subvolume name.
+ *
+ * Return 0 if no error occurred.
+ */
+static int link_subvol_to_lostfound(struct btrfs_root *root, u64 subvol_id)
+{
+	struct btrfs_trans_handle *trans;
+	struct btrfs_fs_info *fs_info = root->fs_info;
+	struct btrfs_root *fs_root = fs_info->fs_root;
+	char buf[BTRFS_NAME_LEN + 1] = {0}; /* 1 for snprintf null */
+	char *dir_name = "lost+found";
+	u64 lost_found_ino = 0;
+	u32 mode = 0700;
+	int ret;
+
+	/*
+	 * For link subvolume to lost+found,
+	 * 2 for parent(256)'s dir_index and dir_item
+	 * 2 for lost+found dir's inode_item and inode_ref
+	 * 2 for lost+found dir's dir_index and dir_item for the subvolume
+	 * 2 for the subvolume's root_ref and root_backref
+	 */
+	trans = btrfs_start_transaction(fs_root, 8);
+	if (IS_ERR(trans)) {
+		error("unable to start transaction");
+		ret = PTR_ERR(trans);
+		goto out;
+	}
+
+	/* Create lost+found directory */
+	ret = btrfs_mkdir(trans, fs_root, dir_name, strlen(dir_name),
+			  BTRFS_FIRST_FREE_OBJECTID, &lost_found_ino,
+			  mode);
+	if (ret < 0) {
+		error("failed to create '%s' dir: %d", dir_name, ret);
+		goto out;
+	}
+
+	/* Link the subvolume to lost+found directory */
+	snprintf(buf, BTRFS_NAME_LEN + 1, "sub%llu", subvol_id);
+	ret = btrfs_link_subvol(trans, fs_root, buf, subvol_id, lost_found_ino,
+				false);
+	if (ret) {
+		error("failed to link the subvol %llu: %d", subvol_id, ret);
+		goto out;
+	}
+
+	/* Clear root flags BTRFS_ROOT_SUBVOL_DEAD and increase root refs */
+	ret = recover_dead_root(trans, root, subvol_id);
+	if (ret)
+		goto out;
+
+	/* Delete the orphan item after undeletion is completed. */
+	ret = btrfs_del_orphan_item(trans, root, subvol_id);
+	if (ret) {
+		error("failed to delete the orphan_item for %llu: %d",
+				subvol_id, ret);
+		goto out;
+	}
+
+	ret = btrfs_commit_transaction(trans, fs_root);
+	if (ret) {
+		error("transaction commit failed: %d", ret);
+		goto out;
+	}
+
+out:
+	return ret;
+}
-- 
2.16.2

Re: [PATCH v2 06/10] btrfs-progs: undelete-subvol: introduce link_subvol_to_lostfound

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 3219 bytes --]



On 2018年03月27日 15:06, Lu Fengqi wrote:
> The function will create lost+found directory, link the deleted
> subvolume specified by the subvol_id to the directory, update the
> information of root_item and cleanup the associated orphan item.
> 
> Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
> ---
>  undelete-subvol.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 76 insertions(+)
> 
> diff --git a/undelete-subvol.c b/undelete-subvol.c
> index 781057df2b84..9243e35545c5 100644
> --- a/undelete-subvol.c
> +++ b/undelete-subvol.c
> @@ -106,3 +106,79 @@ out:
>  	btrfs_release_path(&path);
>  	return ret;
>  }
> +
> +/*
> + * Recover a subvolume specified by subvol_id, and link it to the lost+found
> + * directory.
> + *
> + * @root: the root of the root tree.

In that case, @fs_info would case less confusion.

Otherwise looks good.

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> + * @subvol_id: specify the subvolume which will be linked, and also be the part
> + * of the subvolume name.
> + *
> + * Return 0 if no error occurred.
> + */
> +static int link_subvol_to_lostfound(struct btrfs_root *root, u64 subvol_id)
> +{
> +	struct btrfs_trans_handle *trans;
> +	struct btrfs_fs_info *fs_info = root->fs_info;
> +	struct btrfs_root *fs_root = fs_info->fs_root;
> +	char buf[BTRFS_NAME_LEN + 1] = {0}; /* 1 for snprintf null */
> +	char *dir_name = "lost+found";
> +	u64 lost_found_ino = 0;
> +	u32 mode = 0700;
> +	int ret;
> +
> +	/*
> +	 * For link subvolume to lost+found,
> +	 * 2 for parent(256)'s dir_index and dir_item
> +	 * 2 for lost+found dir's inode_item and inode_ref
> +	 * 2 for lost+found dir's dir_index and dir_item for the subvolume
> +	 * 2 for the subvolume's root_ref and root_backref
> +	 */
> +	trans = btrfs_start_transaction(fs_root, 8);
> +	if (IS_ERR(trans)) {
> +		error("unable to start transaction");
> +		ret = PTR_ERR(trans);
> +		goto out;
> +	}
> +
> +	/* Create lost+found directory */
> +	ret = btrfs_mkdir(trans, fs_root, dir_name, strlen(dir_name),
> +			  BTRFS_FIRST_FREE_OBJECTID, &lost_found_ino,
> +			  mode);
> +	if (ret < 0) {
> +		error("failed to create '%s' dir: %d", dir_name, ret);
> +		goto out;
> +	}
> +
> +	/* Link the subvolume to lost+found directory */
> +	snprintf(buf, BTRFS_NAME_LEN + 1, "sub%llu", subvol_id);
> +	ret = btrfs_link_subvol(trans, fs_root, buf, subvol_id, lost_found_ino,
> +				false);
> +	if (ret) {
> +		error("failed to link the subvol %llu: %d", subvol_id, ret);
> +		goto out;
> +	}
> +
> +	/* Clear root flags BTRFS_ROOT_SUBVOL_DEAD and increase root refs */
> +	ret = recover_dead_root(trans, root, subvol_id);
> +	if (ret)
> +		goto out;
> +
> +	/* Delete the orphan item after undeletion is completed. */
> +	ret = btrfs_del_orphan_item(trans, root, subvol_id);
> +	if (ret) {
> +		error("failed to delete the orphan_item for %llu: %d",
> +				subvol_id, ret);
> +		goto out;
> +	}
> +
> +	ret = btrfs_commit_transaction(trans, fs_root);
> +	if (ret) {
> +		error("transaction commit failed: %d", ret);
> +		goto out;
> +	}
> +
> +out:
> +	return ret;
> +}
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 06/10] btrfs-progs: undelete-subvol: introduce link_subvol_to_lostfound

[Lu Fengqi]

On 04/18/2018 01:21 PM, Qu Wenruo wrote:
> 
> 
> On 2018年03月27日 15:06, Lu Fengqi wrote:
>> The function will create lost+found directory, link the deleted
>> subvolume specified by the subvol_id to the directory, update the
>> information of root_item and cleanup the associated orphan item.
>>
>> Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
>> ---
>>   undelete-subvol.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 76 insertions(+)
>>
>> diff --git a/undelete-subvol.c b/undelete-subvol.c
>> index 781057df2b84..9243e35545c5 100644
>> --- a/undelete-subvol.c
>> +++ b/undelete-subvol.c
>> @@ -106,3 +106,79 @@ out:
>>   	btrfs_release_path(&path);
>>   	return ret;
>>   }
>> +
>> +/*
>> + * Recover a subvolume specified by subvol_id, and link it to the lost+found
>> + * directory.
>> + *
>> + * @root: the root of the root tree.
> 
> In that case, @fs_info would case less confusion.
> 
> Otherwise looks good.
> 
> Reviewed-by: Qu Wenruo <wqu@suse.com>


Make sense.

-- 
Thanks,
Lu

> 
> Thanks,
> Qu
> 
>> + * @subvol_id: specify the subvolume which will be linked, and also be the part
>> + * of the subvolume name.
>> + *
>> + * Return 0 if no error occurred.
>> + */
>> +static int link_subvol_to_lostfound(struct btrfs_root *root, u64 subvol_id)
>> +{
>> +	struct btrfs_trans_handle *trans;
>> +	struct btrfs_fs_info *fs_info = root->fs_info;
>> +	struct btrfs_root *fs_root = fs_info->fs_root;
>> +	char buf[BTRFS_NAME_LEN + 1] = {0}; /* 1 for snprintf null */
>> +	char *dir_name = "lost+found";
>> +	u64 lost_found_ino = 0;
>> +	u32 mode = 0700;
>> +	int ret;
>> +
>> +	/*
>> +	 * For link subvolume to lost+found,
>> +	 * 2 for parent(256)'s dir_index and dir_item
>> +	 * 2 for lost+found dir's inode_item and inode_ref
>> +	 * 2 for lost+found dir's dir_index and dir_item for the subvolume
>> +	 * 2 for the subvolume's root_ref and root_backref
>> +	 */
>> +	trans = btrfs_start_transaction(fs_root, 8);
>> +	if (IS_ERR(trans)) {
>> +		error("unable to start transaction");
>> +		ret = PTR_ERR(trans);
>> +		goto out;
>> +	}
>> +
>> +	/* Create lost+found directory */
>> +	ret = btrfs_mkdir(trans, fs_root, dir_name, strlen(dir_name),
>> +			  BTRFS_FIRST_FREE_OBJECTID, &lost_found_ino,
>> +			  mode);
>> +	if (ret < 0) {
>> +		error("failed to create '%s' dir: %d", dir_name, ret);
>> +		goto out;
>> +	}
>> +
>> +	/* Link the subvolume to lost+found directory */
>> +	snprintf(buf, BTRFS_NAME_LEN + 1, "sub%llu", subvol_id);
>> +	ret = btrfs_link_subvol(trans, fs_root, buf, subvol_id, lost_found_ino,
>> +				false);
>> +	if (ret) {
>> +		error("failed to link the subvol %llu: %d", subvol_id, ret);
>> +		goto out;
>> +	}
>> +
>> +	/* Clear root flags BTRFS_ROOT_SUBVOL_DEAD and increase root refs */
>> +	ret = recover_dead_root(trans, root, subvol_id);
>> +	if (ret)
>> +		goto out;
>> +
>> +	/* Delete the orphan item after undeletion is completed. */
>> +	ret = btrfs_del_orphan_item(trans, root, subvol_id);
>> +	if (ret) {
>> +		error("failed to delete the orphan_item for %llu: %d",
>> +				subvol_id, ret);
>> +		goto out;
>> +	}
>> +
>> +	ret = btrfs_commit_transaction(trans, fs_root);
>> +	if (ret) {
>> +		error("transaction commit failed: %d", ret);
>> +		goto out;
>> +	}
>> +
>> +out:
>> +	return ret;
>> +}
>>
> 

[PATCH v2 07/10] btrfs-progs: undelete-subvol: introduce btrfs_undelete_intact_subvols

[Lu Fengqi]

The function default will traverse the all orphan items on the tree root,
and recover the all intact subvolumes. If subvol_id is specified, then only
the corresponding subvolume will be recovered.

Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
---
v2: add subvol_id argumenta to specify subvol_id instead of recovering
all subvolumes.

 undelete-subvol.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 undelete-subvol.h |  2 ++
 2 files changed, 72 insertions(+)

diff --git a/undelete-subvol.c b/undelete-subvol.c
index 9243e35545c5..5b494ca086ab 100644
--- a/undelete-subvol.c
+++ b/undelete-subvol.c
@@ -15,6 +15,7 @@
 #include "transaction.h"
 #include "disk-io.h"
 #include "messages.h"
+#include "undelete-subvol.h"
 
 /*
  * Determines whether the subvolume is intact, according to the drop_progress
@@ -182,3 +183,72 @@ static int link_subvol_to_lostfound(struct btrfs_root *root, u64 subvol_id)
 out:
 	return ret;
 }
+
+/*
+ * Traverse all orphan items on the root tree, restore them to the lost+found
+ * directory if the corresponding subvolumes are still intact left on the disk.
+ *
+ * @root	the root of the root tree.
+ * @subvol_id	if not set to 0, skip other subvolumes and only recover the
+ *		subvolume specified by @subvol_id.
+ *
+ * Return 0 if no error occurred even if no subvolume was recovered.
+ */
+int btrfs_undelete_intact_subvols(struct btrfs_root *root, u64 subvol_id)
+{
+	struct btrfs_key key;
+	struct btrfs_path path;
+	u64 found_count = 0;
+	u64 recovered_count = 0;
+	int ret = 0;
+
+	key.objectid = BTRFS_ORPHAN_OBJECTID;
+	key.type = BTRFS_ORPHAN_ITEM_KEY;
+	key.offset = subvol_id ? subvol_id + 1 : (u64)-1;
+
+	btrfs_init_path(&path);
+	while (subvol_id != key.offset) {
+		ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
+		if (ret < 0) {
+			error("search ORPHAN_ITEM for %llu failed.\n",
+			      key.offset);
+			break;
+		}
+
+		path.slots[0]--;
+		btrfs_item_key_to_cpu(path.nodes[0], &key, path.slots[0]);
+
+		btrfs_release_path(&path);
+
+		/* No more BTRFS_ORPHAN_ITEM, so we don't need to continue. */
+		if (key.type != BTRFS_ORPHAN_ITEM_KEY) {
+			ret = 0;
+			break;
+		}
+
+		/* If subvol_id is non-zero, skip other deleted subvolume. */
+		if (subvol_id && subvol_id != key.offset) {
+			ret = -ENOENT;
+			break;
+		}
+
+		if (!is_subvol_intact(root, key.offset))
+			continue;
+
+		/* Here we can confirm there is an intact subvolume. */
+		found_count++;
+		ret = link_subvol_to_lostfound(root, key.offset);
+		if (ret == 0) {
+			recovered_count++;
+			printf(
+		"Recovered subvolume %llu to lost+found successfully.\n",
+				key.offset);
+		}
+
+	}
+
+	printf("Found %llu subvols left intact\n", found_count);
+	printf("Recovered %llu subvols\n", found_count);
+
+	return ret;
+}
diff --git a/undelete-subvol.h b/undelete-subvol.h
index 7cfd100cce37..f773210c46fe 100644
--- a/undelete-subvol.h
+++ b/undelete-subvol.h
@@ -14,4 +14,6 @@
 #ifndef __BTRFS_UNDELETE_SUBVOLUME_H__
 #define __BTRFS_UNDELETE_SUBVOLUME_H__
 
+int btrfs_undelete_intact_subvols(struct btrfs_root *root, u64 subvol_id);
+
 #endif
-- 
2.16.2

Re: [PATCH v2 07/10] btrfs-progs: undelete-subvol: introduce btrfs_undelete_intact_subvols

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 3971 bytes --]



On 2018年03月27日 15:06, Lu Fengqi wrote:
> The function default will traverse the all orphan items on the tree root,
> and recover the all intact subvolumes. If subvol_id is specified, then only
> the corresponding subvolume will be recovered.
> 
> Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
> ---
> v2: add subvol_id argumenta to specify subvol_id instead of recovering
> all subvolumes.
> 
>  undelete-subvol.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  undelete-subvol.h |  2 ++
>  2 files changed, 72 insertions(+)
> 
> diff --git a/undelete-subvol.c b/undelete-subvol.c
> index 9243e35545c5..5b494ca086ab 100644
> --- a/undelete-subvol.c
> +++ b/undelete-subvol.c
> @@ -15,6 +15,7 @@
>  #include "transaction.h"
>  #include "disk-io.h"
>  #include "messages.h"
> +#include "undelete-subvol.h"
>  
>  /*
>   * Determines whether the subvolume is intact, according to the drop_progress
> @@ -182,3 +183,72 @@ static int link_subvol_to_lostfound(struct btrfs_root *root, u64 subvol_id)
>  out:
>  	return ret;
>  }
> +
> +/*
> + * Traverse all orphan items on the root tree, restore them to the lost+found
> + * directory if the corresponding subvolumes are still intact left on the disk.
> + *
> + * @root	the root of the root tree.

Same comment here.

> + * @subvol_id	if not set to 0, skip other subvolumes and only recover the
> + *		subvolume specified by @subvol_id.
> + *
> + * Return 0 if no error occurred even if no subvolume was recovered.
> + */
> +int btrfs_undelete_intact_subvols(struct btrfs_root *root, u64 subvol_id)

I prefer to remove the word "_intact".

> +{
> +	struct btrfs_key key;
> +	struct btrfs_path path;
> +	u64 found_count = 0;
> +	u64 recovered_count = 0;
> +	int ret = 0;
> +
> +	key.objectid = BTRFS_ORPHAN_OBJECTID;
> +	key.type = BTRFS_ORPHAN_ITEM_KEY;
> +	key.offset = subvol_id ? subvol_id + 1 : (u64)-1;
> +
> +	btrfs_init_path(&path);

I would prefer to do btrfs_search_slot() here, and then use
btrfs_previous_item() to iterate, other than calling btrfs_search_slot()
several times.

> +	while (subvol_id != key.offset) {
> +		ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
> +		if (ret < 0) {
> +			error("search ORPHAN_ITEM for %llu failed.\n",
> +			      key.offset);
> +			break;
> +		}
> +
> +		path.slots[0]--;

Btrfs_previous_item() is much better here.

Especially when above btrfs_search_slot() could return 0 (key found) and
path.slots[0] could be 0.
That's also the reason why I prefer btrfs_search_slot() then
btrfs_previous_item() in the loop.

Thanks,
Qu

> +		btrfs_item_key_to_cpu(path.nodes[0], &key, path.slots[0]);
> +
> +		btrfs_release_path(&path);
> +
> +		/* No more BTRFS_ORPHAN_ITEM, so we don't need to continue. */
> +		if (key.type != BTRFS_ORPHAN_ITEM_KEY) {
> +			ret = 0;
> +			break;
> +		}
> +
> +		/* If subvol_id is non-zero, skip other deleted subvolume. */
> +		if (subvol_id && subvol_id != key.offset) {
> +			ret = -ENOENT;
> +			break;
> +		}
> +
> +		if (!is_subvol_intact(root, key.offset))
> +			continue;
> +
> +		/* Here we can confirm there is an intact subvolume. */
> +		found_count++;
> +		ret = link_subvol_to_lostfound(root, key.offset);
> +		if (ret == 0) {
> +			recovered_count++;
> +			printf(
> +		"Recovered subvolume %llu to lost+found successfully.\n",
> +				key.offset);
> +		}
> +
> +	}
> +
> +	printf("Found %llu subvols left intact\n", found_count);
> +	printf("Recovered %llu subvols\n", found_count);
> +
> +	return ret;
> +}
> diff --git a/undelete-subvol.h b/undelete-subvol.h
> index 7cfd100cce37..f773210c46fe 100644
> --- a/undelete-subvol.h
> +++ b/undelete-subvol.h
> @@ -14,4 +14,6 @@
>  #ifndef __BTRFS_UNDELETE_SUBVOLUME_H__
>  #define __BTRFS_UNDELETE_SUBVOLUME_H__
>  
> +int btrfs_undelete_intact_subvols(struct btrfs_root *root, u64 subvol_id);
> +
>  #endif
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 07/10] btrfs-progs: undelete-subvol: introduce btrfs_undelete_intact_subvols

[Lu Fengqi]

On Wed, Apr 18, 2018 at 01:28:23PM +0800, Qu Wenruo wrote:
>
>
>On 2018年03月27日 15:06, Lu Fengqi wrote:
>> The function default will traverse the all orphan items on the tree root,
>> and recover the all intact subvolumes. If subvol_id is specified, then only
>> the corresponding subvolume will be recovered.
>> 
>> Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
>> ---
>> v2: add subvol_id argumenta to specify subvol_id instead of recovering
>> all subvolumes.
>> 
>>  undelete-subvol.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  undelete-subvol.h |  2 ++
>>  2 files changed, 72 insertions(+)
>> 
>> diff --git a/undelete-subvol.c b/undelete-subvol.c
>> index 9243e35545c5..5b494ca086ab 100644
>> --- a/undelete-subvol.c
>> +++ b/undelete-subvol.c
>> @@ -15,6 +15,7 @@
>>  #include "transaction.h"
>>  #include "disk-io.h"
>>  #include "messages.h"
>> +#include "undelete-subvol.h"
>>  
>>  /*
>>   * Determines whether the subvolume is intact, according to the drop_progress
>> @@ -182,3 +183,72 @@ static int link_subvol_to_lostfound(struct btrfs_root *root, u64 subvol_id)
>>  out:
>>  	return ret;
>>  }
>> +
>> +/*
>> + * Traverse all orphan items on the root tree, restore them to the lost+found
>> + * directory if the corresponding subvolumes are still intact left on the disk.
>> + *
>> + * @root	the root of the root tree.
>
>Same comment here.

Make sense.

>
>> + * @subvol_id	if not set to 0, skip other subvolumes and only recover the
>> + *		subvolume specified by @subvol_id.
>> + *
>> + * Return 0 if no error occurred even if no subvolume was recovered.
>> + */
>> +int btrfs_undelete_intact_subvols(struct btrfs_root *root, u64 subvol_id)
>
>I prefer to remove the word "_intact".

I also like the shorter function name.

>
>> +{
>> +	struct btrfs_key key;
>> +	struct btrfs_path path;
>> +	u64 found_count = 0;
>> +	u64 recovered_count = 0;
>> +	int ret = 0;
>> +
>> +	key.objectid = BTRFS_ORPHAN_OBJECTID;
>> +	key.type = BTRFS_ORPHAN_ITEM_KEY;
>> +	key.offset = subvol_id ? subvol_id + 1 : (u64)-1;
>> +
>> +	btrfs_init_path(&path);
>
>I would prefer to do btrfs_search_slot() here, and then use
>btrfs_previous_item() to iterate, other than calling btrfs_search_slot()
>several times.

This loop will perform some additions and deletions of the items on the 
tree, so we have to call btrfs_search_slot in the loop.

>
>> +	while (subvol_id != key.offset) {
>> +		ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
>> +		if (ret < 0) {
>> +			error("search ORPHAN_ITEM for %llu failed.\n",
>> +			      key.offset);
>> +			break;
>> +		}
>> +
>> +		path.slots[0]--;
>
>Btrfs_previous_item() is much better here.
>
>Especially when above btrfs_search_slot() could return 0 (key found) and
>path.slots[0] could be 0.
>That's also the reason why I prefer btrfs_search_slot() then
>btrfs_previous_item() in the loop.

Make sense.

-- 
Thanks,
Lu

>
>Thanks,
>Qu
>
>> +		btrfs_item_key_to_cpu(path.nodes[0], &key, path.slots[0]);
>> +
>> +		btrfs_release_path(&path);
>> +
>> +		/* No more BTRFS_ORPHAN_ITEM, so we don't need to continue. */
>> +		if (key.type != BTRFS_ORPHAN_ITEM_KEY) {
>> +			ret = 0;
>> +			break;
>> +		}
>> +
>> +		/* If subvol_id is non-zero, skip other deleted subvolume. */
>> +		if (subvol_id && subvol_id != key.offset) {
>> +			ret = -ENOENT;
>> +			break;
>> +		}
>> +
>> +		if (!is_subvol_intact(root, key.offset))
>> +			continue;
>> +
>> +		/* Here we can confirm there is an intact subvolume. */
>> +		found_count++;
>> +		ret = link_subvol_to_lostfound(root, key.offset);
>> +		if (ret == 0) {
>> +			recovered_count++;
>> +			printf(
>> +		"Recovered subvolume %llu to lost+found successfully.\n",
>> +				key.offset);
>> +		}
>> +
>> +	}
>> +
>> +	printf("Found %llu subvols left intact\n", found_count);
>> +	printf("Recovered %llu subvols\n", found_count);
>> +
>> +	return ret;
>> +}
>> diff --git a/undelete-subvol.h b/undelete-subvol.h
>> index 7cfd100cce37..f773210c46fe 100644
>> --- a/undelete-subvol.h
>> +++ b/undelete-subvol.h
>> @@ -14,4 +14,6 @@
>>  #ifndef __BTRFS_UNDELETE_SUBVOLUME_H__
>>  #define __BTRFS_UNDELETE_SUBVOLUME_H__
>>  
>> +int btrfs_undelete_intact_subvols(struct btrfs_root *root, u64 subvol_id);
>> +
>>  #endif
>> 
>

[PATCH v2 08/10] btrfs-progs: undelete-subvol: add undelete-subvol subcommand

[Lu Fengqi]

Add the undelete-subvol subcommand for btrfs rescue. This subcommand is
used to recover deleted subvolume left intact on the device.

Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
---
v2: add -s option to specify subvol_id.

 cmds-rescue.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)

diff --git a/cmds-rescue.c b/cmds-rescue.c
index c40088ad374e..c5132126e922 100644
--- a/cmds-rescue.c
+++ b/cmds-rescue.c
@@ -25,6 +25,7 @@
 #include "disk-io.h"
 #include "commands.h"
 #include "utils.h"
+#include "undelete-subvol.h"
 #include "help.h"
 
 static const char * const rescue_cmd_group_usage[] = {
@@ -248,6 +249,73 @@ out:
 	return !!ret;
 }
 
+static const char * const cmd_rescue_undelete_subvol_usage[] = {
+	"btrfs rescue undelete-subvol [-s <subvolid>] <device>",
+	"Undelete deleted subvolume",
+	"All deleted subvolume that still left intact on the device will be",
+	"recovered. If -s <subvolid> option is given, then just recover the",
+	"subvolume which specified by <subvolid>.",
+	"",
+	"-s <subvolid>	specify the subvolume which will be recovered.",
+	NULL
+};
+
+static int cmd_rescue_undelete_subvol(int argc, char **argv)
+{
+	struct btrfs_fs_info *fs_info;
+	char *devname;
+	u64 subvol_id = 0;
+	int ret;
+
+	while (1) {
+		int c = getopt(argc, argv, "s:");
+
+		if (c < 0)
+			break;
+		switch (c) {
+		case 's':
+			subvol_id = arg_strtou64(optarg);
+			if (!is_fstree(subvol_id)) {
+				error("%llu is not a valid subvolume id",
+						subvol_id);
+				ret = -EINVAL;
+				goto out;
+			}
+			break;
+		default:
+			usage(cmd_rescue_undelete_subvol_usage);
+		}
+	}
+
+	if (check_argc_exact(argc - optind, 1))
+		usage(cmd_rescue_undelete_subvol_usage);
+
+	devname = argv[optind];
+	ret = check_mounted(devname);
+	if (ret < 0) {
+		error("could not check mount status: %s", strerror(-ret));
+		goto out;
+	} else if (ret) {
+		error("%s is currently mounted", devname);
+		ret = -EBUSY;
+		goto out;
+	}
+
+	fs_info = open_ctree_fs_info(devname, 0, 0, 0, OPEN_CTREE_WRITES |
+				     OPEN_CTREE_PARTIAL);
+	if (!fs_info) {
+		error("could not open btrfs");
+		ret = -EIO;
+		goto out;
+	}
+
+	ret = btrfs_undelete_intact_subvols(fs_info->tree_root, subvol_id);
+
+	close_ctree(fs_info->tree_root);
+out:
+	return ret;
+}
+
 static const char rescue_cmd_group_info[] =
 "toolbox for specific rescue operations";
 
@@ -260,6 +328,8 @@ const struct cmd_group rescue_cmd_group = {
 		{ "zero-log", cmd_rescue_zero_log, cmd_rescue_zero_log_usage, NULL, 0},
 		{ "fix-device-size", cmd_rescue_fix_device_size,
 			cmd_rescue_fix_device_size_usage, NULL, 0},
+		{ "undelete-subvol", cmd_rescue_undelete_subvol,
+			cmd_rescue_undelete_subvol_usage, NULL, 0},
 		NULL_CMD_STRUCT
 	}
 };
-- 
2.16.2

Re: [PATCH v2 08/10] btrfs-progs: undelete-subvol: add undelete-subvol subcommand

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 3388 bytes --]



On 2018年03月27日 15:06, Lu Fengqi wrote:
> Add the undelete-subvol subcommand for btrfs rescue. This subcommand is
> used to recover deleted subvolume left intact on the device.
> 
> Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
> ---
> v2: add -s option to specify subvol_id.
> 
>  cmds-rescue.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 70 insertions(+)
> 
> diff --git a/cmds-rescue.c b/cmds-rescue.c
> index c40088ad374e..c5132126e922 100644
> --- a/cmds-rescue.c
> +++ b/cmds-rescue.c
> @@ -25,6 +25,7 @@
>  #include "disk-io.h"
>  #include "commands.h"
>  #include "utils.h"
> +#include "undelete-subvol.h"
>  #include "help.h"
>  
>  static const char * const rescue_cmd_group_usage[] = {
> @@ -248,6 +249,73 @@ out:
>  	return !!ret;
>  }
>  
> +static const char * const cmd_rescue_undelete_subvol_usage[] = {
> +	"btrfs rescue undelete-subvol [-s <subvolid>] <device>",
> +	"Undelete deleted subvolume",
> +	"All deleted subvolume that still left intact on the device will be",
> +	"recovered. If -s <subvolid> option is given, then just recover the",
> +	"subvolume which specified by <subvolid>.",
> +	"",
> +	"-s <subvolid>	specify the subvolume which will be recovered.",
> +	NULL
> +};
> +
> +static int cmd_rescue_undelete_subvol(int argc, char **argv)
> +{
> +	struct btrfs_fs_info *fs_info;
> +	char *devname;
> +	u64 subvol_id = 0;
> +	int ret;
> +
> +	while (1) {
> +		int c = getopt(argc, argv, "s:");
> +
> +		if (c < 0)
> +			break;
> +		switch (c) {
> +		case 's':
> +			subvol_id = arg_strtou64(optarg);
> +			if (!is_fstree(subvol_id)) {
> +				error("%llu is not a valid subvolume id",
> +						subvol_id);
> +				ret = -EINVAL;
> +				goto out;
> +			}
> +			break;
> +		default:
> +			usage(cmd_rescue_undelete_subvol_usage);
> +		}
> +	}
> +
> +	if (check_argc_exact(argc - optind, 1))
> +		usage(cmd_rescue_undelete_subvol_usage);
> +
> +	devname = argv[optind];
> +	ret = check_mounted(devname);
> +	if (ret < 0) {
> +		error("could not check mount status: %s", strerror(-ret));
> +		goto out;
> +	} else if (ret) {
> +		error("%s is currently mounted", devname);
> +		ret = -EBUSY;
> +		goto out;
> +	}
> +
> +	fs_info = open_ctree_fs_info(devname, 0, 0, 0, OPEN_CTREE_WRITES |
> +				     OPEN_CTREE_PARTIAL);

I'm not sure if using OPEN_CTREE_PARTIAL here is a good idea.

As the undelete-subvol looks like a tool to revert stupid user mistake,
and we expect a healthy fs to be provided.
And for corrupted fs, we may make the case worse.

Thanks,
Qu

> +	if (!fs_info) {
> +		error("could not open btrfs");
> +		ret = -EIO;
> +		goto out;
> +	}
> +
> +	ret = btrfs_undelete_intact_subvols(fs_info->tree_root, subvol_id);
> +
> +	close_ctree(fs_info->tree_root);
> +out:
> +	return ret;
> +}
> +
>  static const char rescue_cmd_group_info[] =
>  "toolbox for specific rescue operations";
>  
> @@ -260,6 +328,8 @@ const struct cmd_group rescue_cmd_group = {
>  		{ "zero-log", cmd_rescue_zero_log, cmd_rescue_zero_log_usage, NULL, 0},
>  		{ "fix-device-size", cmd_rescue_fix_device_size,
>  			cmd_rescue_fix_device_size_usage, NULL, 0},
> +		{ "undelete-subvol", cmd_rescue_undelete_subvol,
> +			cmd_rescue_undelete_subvol_usage, NULL, 0},
>  		NULL_CMD_STRUCT
>  	}
>  };
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 08/10] btrfs-progs: undelete-subvol: add undelete-subvol subcommand

[Lu Fengqi]

On 04/18/2018 01:32 PM, Qu Wenruo wrote:
> 
> 
> On 2018年03月27日 15:06, Lu Fengqi wrote:
>> Add the undelete-subvol subcommand for btrfs rescue. This subcommand is
>> used to recover deleted subvolume left intact on the device.
>>
>> Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
>> ---
>> v2: add -s option to specify subvol_id.
>>
>>   cmds-rescue.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 70 insertions(+)
>>
>> diff --git a/cmds-rescue.c b/cmds-rescue.c
>> index c40088ad374e..c5132126e922 100644
>> --- a/cmds-rescue.c
>> +++ b/cmds-rescue.c
>> @@ -25,6 +25,7 @@
>>   #include "disk-io.h"
>>   #include "commands.h"
>>   #include "utils.h"
>> +#include "undelete-subvol.h"
>>   #include "help.h"
>>   
>>   static const char * const rescue_cmd_group_usage[] = {
>> @@ -248,6 +249,73 @@ out:
>>   	return !!ret;
>>   }
>>   
>> +static const char * const cmd_rescue_undelete_subvol_usage[] = {
>> +	"btrfs rescue undelete-subvol [-s <subvolid>] <device>",
>> +	"Undelete deleted subvolume",
>> +	"All deleted subvolume that still left intact on the device will be",
>> +	"recovered. If -s <subvolid> option is given, then just recover the",
>> +	"subvolume which specified by <subvolid>.",
>> +	"",
>> +	"-s <subvolid>	specify the subvolume which will be recovered.",
>> +	NULL
>> +};
>> +
>> +static int cmd_rescue_undelete_subvol(int argc, char **argv)
>> +{
>> +	struct btrfs_fs_info *fs_info;
>> +	char *devname;
>> +	u64 subvol_id = 0;
>> +	int ret;
>> +
>> +	while (1) {
>> +		int c = getopt(argc, argv, "s:");
>> +
>> +		if (c < 0)
>> +			break;
>> +		switch (c) {
>> +		case 's':
>> +			subvol_id = arg_strtou64(optarg);
>> +			if (!is_fstree(subvol_id)) {
>> +				error("%llu is not a valid subvolume id",
>> +						subvol_id);
>> +				ret = -EINVAL;
>> +				goto out;
>> +			}
>> +			break;
>> +		default:
>> +			usage(cmd_rescue_undelete_subvol_usage);
>> +		}
>> +	}
>> +
>> +	if (check_argc_exact(argc - optind, 1))
>> +		usage(cmd_rescue_undelete_subvol_usage);
>> +
>> +	devname = argv[optind];
>> +	ret = check_mounted(devname);
>> +	if (ret < 0) {
>> +		error("could not check mount status: %s", strerror(-ret));
>> +		goto out;
>> +	} else if (ret) {
>> +		error("%s is currently mounted", devname);
>> +		ret = -EBUSY;
>> +		goto out;
>> +	}
>> +
>> +	fs_info = open_ctree_fs_info(devname, 0, 0, 0, OPEN_CTREE_WRITES |
>> +				     OPEN_CTREE_PARTIAL);
> 
> I'm not sure if using OPEN_CTREE_PARTIAL here is a good idea.
> 
> As the undelete-subvol looks like a tool to revert stupid user mistake,
> and we expect a healthy fs to be provided.
> And for corrupted fs, we may make the case worse.

Make sense. The flag shouldn't be set here.

-- 
Thanks,
Lu

> 
> Thanks,
> Qu
> 
>> +	if (!fs_info) {
>> +		error("could not open btrfs");
>> +		ret = -EIO;
>> +		goto out;
>> +	}
>> +
>> +	ret = btrfs_undelete_intact_subvols(fs_info->tree_root, subvol_id);
>> +
>> +	close_ctree(fs_info->tree_root);
>> +out:
>> +	return ret;
>> +}
>> +
>>   static const char rescue_cmd_group_info[] =
>>   "toolbox for specific rescue operations";
>>   
>> @@ -260,6 +328,8 @@ const struct cmd_group rescue_cmd_group = {
>>   		{ "zero-log", cmd_rescue_zero_log, cmd_rescue_zero_log_usage, NULL, 0},
>>   		{ "fix-device-size", cmd_rescue_fix_device_size,
>>   			cmd_rescue_fix_device_size_usage, NULL, 0},
>> +		{ "undelete-subvol", cmd_rescue_undelete_subvol,
>> +			cmd_rescue_undelete_subvol_usage, NULL, 0},
>>   		NULL_CMD_STRUCT
>>   	}
>>   };
>>
> 

[PATCH v2 09/10] btrfs-progs: tests: add testcase for undelete-subvol

[Lu Fengqi]

The testcase checks the functionality of "btrfs rescue undelete-subvol",
including recovering an intact subvolume, and handling correctly
incomplete subvolume.

Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
---
v2: add shell quoting to test script.

 .../030-undelete-subvol/deleted_subvolume.img      | Bin 0 -> 4096 bytes
 .../030-undelete-subvol/drop_progress.raw.xz       | Bin 0 -> 23452 bytes
 tests/misc-tests/030-undelete-subvol/test.sh       |  34 +++++++++++++++++++++
 3 files changed, 34 insertions(+)
 create mode 100644 tests/misc-tests/030-undelete-subvol/deleted_subvolume.img
 create mode 100644 tests/misc-tests/030-undelete-subvol/drop_progress.raw.xz
 create mode 100755 tests/misc-tests/030-undelete-subvol/test.sh

diff --git a/tests/misc-tests/030-undelete-subvol/deleted_subvolume.img b/tests/misc-tests/030-undelete-subvol/deleted_subvolume.img
new file mode 100644
index 0000000000000000000000000000000000000000..9168a8e33fbbb14f9ea78514721a52191b94d1a0
GIT binary patch
literal 4096
zcmeH|do+|=AIB%x8p^0djl0}JQb@$O%TY$;lEx)vlF+ymV;WO45{D?%IqnhB$S}Fi
zjN6EihKkC4NQhyKaT(V$2k-mO={@KF_pD{Fy}r-)`R-@^*504B_p@W+PlCQF!fF8j
zZGs!m9b0|F@NPJOvGJw?_&4=#{gw~i3;@(EFS(@+TiUf5-~Erz<=ODYja#|MmTnGi
zw`~I31pbc*g!Km3O1HeZD~KRV`{Pj~uzt<!(k&GKVR-=o%Mv|>3r2&j3Mv|UgTH}a
zWDkSP-%;<0K6oq%LMk0=5)g}2I{;`k(RwtnGuI_hB01WRSAongy!#b`_p*d$38#+;
zcYI;9gzxul-@Q0?=XZCRp1a_O(t&`~#!SFI<>VfKQp$%5D%q@qRO^IYiZQ11&z1Ee
z*LJ*O&zE@akHeln2DGHANbna7)texSTKS8w%9X2EBg2OP_&dC*;ZbHjccM=V#lO%}
zATc9#siZ*izH<@v{m4ksm!$X!TLEok)EiGD1DIiBhF{t3@g30ydV%CQ75thpDR_Sz
zkNpn7>SYJJu-@rDqHlw-Wb-Wdq8i)|MhOAQWLvnU+THRaIvD@>=47X$XKjKl#3c>c
z+gVL~u-1qB(5Zq5RlSf2!q|mGkUWzvayXPkcZ+}`@>&ZNwc>iAb(MsWDCeg9_g?z@
z+UzN}@~(J8&Vn&7SGm+x+QKwvX*meP^nxg%)4Y+tC!c%#Li1uw)!=uIZLYL%h({>6
zI?6W8VFdIKTctfwulDqceyAL)1|1Zhpij6oC^|RGxbla6`S|q7kxq3u8hQMPc;I2Y
zCWZ`c;*ciOjl@3Y^DqdF9BWsR_*&1bIYVLtMwmWM2N+tNJ-7j(O%B-L35XizYGSd1
zB~$y~k-c18T=w!oFaLH~OTx)ncn*m3#tpvpB({MHrvM%M=n2J!-i}LGsv{&tIzHE?
z<~5bwAY@lJ)i%#ao<x?SA3mL#^UhRfE(EYw`F~o7XMxB_rfBpg`?{m%Ci?}4NDL3Z
zFaVHTbX7=>Op^!R$6CJL^-x?kT(roP@)*Q8XnwB2j3pd-#ynd#jp7WCx#4WSfc|{y
z#5H*01m+|Z94ps6)UYtvKCre(ZF$dTAxE?$+j2>nxqYpZ_Rkl)A)IQAwvH~^(SzZi
z0p9u3gpBilS%_h>xu4tFT+N#=O{4QmFL|iRrJJjnU7WSMuKUyHtOvks7;`zGw0BVH
zJQ}jPdQ!xy(s!Us?OIYAl7nvL&Ww$7V~c{(G8ho_TID+~?_UeBFyGcNvvctBnsrrf
zR>%@OeHtPSWh}(@*~6Mo%wR1Xc{kY<0UPWj-p+;|;pI=V<}0+sd36pSJ9HL1<k)SY
zedyJJH`V;qs}eahO35L0A>GOmQHYWmh`5IF7C{XLOE6~i$Hfktz9xZp$KS@J7FBer
zU6XsQh2^|0*VWg^M;!+bUUhGGt|)c?CClbV{&HB)#jL~#x7@Hgi$!??u)-rbQ^;oY
zY!$-MsH;KQ%fzKtwafY8t$(=9mvBoO{a>gHD^*bc*g7T=1Gzb&olJjE*{j2yz>ok&
z7M6gGH4Q*~983Vsv#8?bD~SiDobt+CdWEqjraBk8ESoH7vV6%oI3Ejb*&G790PCr$
zuNVEyMF_j+Q>(9d(j#nI_W+HW?IqPn%^pKydSPdh>>Y3s5VIg;-RjrS*7CMI<pjgD
zy@k<|nP3VL@ndp8<K>7{u~HVW_{Y*$se=Z}$CDIdvvKrhnca(1M-26(9HuWE?Ohji
zuBpmHt(QTgQ-Yq{m73~S!#xX-w!hB|`k?xhx6>;m!B6zHz=2>h)6=4D_a-K3kJ6t;
zWMpX8X0U`fZB*msQ+^4vT8hol-6nS`h35;Ds!prmWNZ?5xhs3K9QX}xIY9op0XE<2
zd-c9BbluK;C2&UT-_kTap|WlT=|N;-RvR<kdv2}t6fKqh0`IM+Q{rb^!_>trenZQU
zPiVGB&=zwO#labtRhCASg?G_h$wLNDK8)U6ui@n~0zl=OVap45dP71-m<U4Q6{Fzs
zMM_GKki$jYRwe&LfwYaE^&@*Ja~QGj{(V%y{8|cwbYtaLwY@PFu7*0-Gi2}@`5%J{
zXI9Rmc-)7felm=~CiMi=s78&rYxK?Ut6J(^D8gq>(j?er4a{gMzE>X8BhF^HR&+yN
z``L&RC#iGzIqve?b}mDKJ3R?!Sj~hEkClMj*aQPHs-je`Hw)G=a^_UGlEFuf?-JGn
zrt3Oz*O1f$_uqL2)&V~`vD^hS#j3mRtrVnrHk6fM)Az8Q5M9FVc;SjPt^dWinLHEw
z(Yd%niJ8}8Uk@IFxs`OJKq)!4{LOe!xf<W3<4#vGp4<HE^uZfWJHLYEy_k`g&X>}C
zCCb1~=m9Lg$9ZHABR*mWv80B!dG3fdJeMV!XUBJIiz)c*3_^%D@4@C+zdY>o!34x-
zf=~s)a%m<7<=N$mEk3g{K0aS)5Q~o%4<x&sXGY_N+Hsb3S)-Yphz`_@A*S9yO-7{A
zYQBQoFFhvh0X-TkNY8+hCRNwn{k<HIh8#&^(dI|jPa{8~3N7DTQsk!GRra-)%F7T=
z20Fx*h7a@QQ97Te92F>(9R>QL7W2uspH*KopFWYyiKEB0WdA*o`T(uUGBdW_&Hk*E
zzTw9jeaVG?gtI%=*AAzOt@~-PogS@pC<cvNndN*@;PVNLs(gipI;+<=K4);D{ZsHy
ZeM(;pZJdD++rKZroo*A@Ch+ek@Fx_v;e7xA

literal 0
HcmV?d00001

diff --git a/tests/misc-tests/030-undelete-subvol/drop_progress.raw.xz b/tests/misc-tests/030-undelete-subvol/drop_progress.raw.xz
new file mode 100644
index 0000000000000000000000000000000000000000..3db2c80e61fa9bb99db374509dbb95910965d467
GIT binary patch
literal 23452
zcmeHPXH=63mktC7z4zV}r8hyO7ZF71J@gJj=p6y+(xijbKnNWvN|D}+^dbU^sPx_m
zAee9V?4CV)c4qg}`DSLFv-y{QZ{GXfd!PF}w_J((=^Fw77^^d{6wv`JX!HO8fOt%Q
z7>Rs>(P<0-IA0-=um<EkDJ2RwF>{T^q#LWS2+;ZNGGoXTU+*0WG?kjb!RY9Rd@zE!
zprWN%HWIB*2GqTt(O<mJ>QjR79XdXGC|eojjqRj@Ntje7V=&Dfp58RkEphE+!*iK3
ze=gkgvO#(Wb3wy1yH#b!yCt$!YIsJE^$1(}*0xh|G%dc{X8vwN4zHQ}bVmsR<s^;p
zo1FQ@#en!Cnsw{>RrWRKCJQwJZ4r;|fIzaNqWP$xtDSB^E`4R#+kNfF<kDzNG(HMk
z-Pu^-_90d6FamaI%>#ZlImmLpgm-*(wek`j)@7COOkP^)r&;tzG%wF$uJ>jkzy^dj
z*q1CnCUcRGf<3#$dF~&$F~=`u6%?zfjrjSgZ#=WTN%JruO&XD64@f4|;_Itq@hv``
znjt?-*%=Zcor6q8k0n~uoI_Sjmizo#?Qx)NGtnCjRYcZ6yKf~=j+^wl)tqQ;;UoIe
z6dYQm&UF5`k}lBct=xCfMBv`>_%x+lk+Tb7FuRhD^!2Cr(cdZ=3Hg+ski3Y-op&tg
zFEWTlcBk^eKNL|WnH>4v24}=}Oi<&5mq8M6PBQuKPF}pW3#$g6<m`@L=xf?G^==1)
zw`THQ3|^II!O9__?;6u-(mh`8tz0P_5#DB&|B{nd4&56jy~NjIe4+TdrJ{Psxs|Qp
zxvgM<kGh)>7be*!O|OI5jBm@GHp1l$d3pM0hxL^?iZNjay1ELxtIk(9q%;7k{=WOq
zVcqtGQXM9N6owoF$=%Ja6A{~D@{W^FvI{VZ9!L<G!uPCSV;P&gP8eJT1Yl=#GGK`W
z7}G-v+%xe(;&<|Oy@ZWe{cQ<yK|oEVFYZ>(Uc`zt6i4mv(BPmaC9yTV{>?PC1dsJG
z4AQ!27bo-Ofq5a~mLqp4hPsA?n;ise0$mkdvDwg^$n=EO81vd*1PIXPHN*)(Tyl%q
zOt)_gg-@hrL<e#v9&&ghXgaVm9%fw4R=rIi>ba(Mc9JL}fC;bOb?$U|cn^Ofk)*8B
zjG<|?hJ)J3AY#KtQ@?G)tHNM;1&oP(-alV?`BjsPpu6ye5LK2e``x~)%3>aI&vA{+
zHKu|WvTGm=s1F(VVIJuhE2#V@!?+<tGQ#h(lJdGOWRO`s1B;a7O^;jzlrgkx2cvIS
zzmLFe9cwt1fID}zcmK0f@G-N17w!sp?nER}GoP7F%#_ZjQfx0G&?quSKGkkxuV6m&
z=~rrE;?LHRB2kysZ^ev3Z&d=iZs%#+D|9hFq;N95EP^WnTa#J&B*%)nPTw6}_s<Bl
zqPIQ)@+;pOP;Roalb-?IIIMBrr^=KW8aC(3*7&v{A#gpu*&SGvdD2IJ2Op=Q5G}25
zklWFiMX&$Ckx8hwNBGTq7_rY^r{|DDEp$sGg6gm*DTAwZ_33uGcr5$jexgcEH^W;%
zuRK>sAk#x&uDZlU?3J9}e$^#srG{thpZ}!K;4#7oW|li&(2_j6X@w&mJjCg^7}UEq
zk%uHk;{t5EYZYJp>8|iw|DjHNzo(Vgw2Vkf#YQfHj1WnjeCFz7aE3R6OueB<RC*ck
z@oOeOuWrY)@T1Tt9oE0o3;&o$YY<7-<{^X8VF0PRcK@xOC{98H?Y#4fwv0$?zWR$?
zZCsY{^W3&wgq~;TD{Z3?;=1&Nn;<W12H7gP+g5$iv?f9U)zcg01tX9dZVIY<t7Zi<
z-TYHDsuFybkArJoL_+*lR?nrb39D5C8N7Jl?B?x?-V>X<t>+U)Z*SyD;@uq`ho*x+
z=m>N|fxM7WYKaPPD01+E->9Fg0IhvjdE_8fQk;p5x2fOm^#<{LYwG|LWicVi#9;{C
zx_vXnL{_+W&HbRz0iR(qoUeg0<Tg|I&}Gr^k=3exF=MJOqdqK$_@GP6komO1zyYmB
zwIe&wsO!QcS?nxZL|AA#DR|5RB+%Qf0sG(q*Y_k~u@N`W=w^2twPe-gVW&2}a8ad8
z%N_;v*wpSN*a<Lzk}2@WYh%37__qcM7wFrR4R`pb9QlErOMPzDmKGJwmxFi792*bB
zH@@jyKRpa+*|>*LN~>%A#XckA!l7J~c<4uGl)=avBiUMHks*>T=bqt8v&f?cYc$%8
z8{%;<Pu^4cFbg^JBj8^L)Yi!)8<v8b*h<c@V&+bQW(eq{AA6UFlCwZ2-ufL$k!xA}
z_uTZyc*XBZi+}E;U(}05pd$MRlFGkFHYyc<cNRt&><3{BWiXV%{-+xGj{yUehoL;|
zukf%}XnBCPpk-<fVdt&bB;TFc{J7~)SslYBk>w^~(#oQZj-fGkK6HclZ%SVq9lXdr
zcj_gxFG!iDhahN7wz4|U$~oGnuBJ3iwcTCTE}MLT(#wm))4-KOjqL99t=6iO>uKFS
zU%fRRpnm|H6xQ0g5{;2m0R19@rEeBKMtpiD==xBCGG4A#*{H{<HfaqyFbcxh&HD(K
z8;X)P0z4a6t-5|A#g$3&uO(AI4Uxg-nfFE2eyG~-x5O9}(EOl!^+y`)2u%<$)d=@c
z+NQ26whNL5{Y=uvPe@`%YuHRU$5E*amC8`5>?e~z-{GbL7O59HE$uP+2$^=<B4sRH
zyBp|9Byub8XGt=|7;&xiXeOmjYWuNTF!d=2N~7L8-GhoacHE~GYo|-elx7gg6S^R!
zuz91JubA_GvvLL(q~T#?_3bM*In(?CO@iR#MxAZ{+V-WDy9_kbu5LyVHkf{@_pg~n
z6eGO1WXq~Crp5X_)NnSv8nlXUzN_J~uQ5NX-2d{qr7%BZr;)uqP;(tDlSQL8wKP|^
zuI(V}M6&^<9A0I;LklU+7Rxg4dB+Y8#647dEuCf?F0!A*c7|~k^L`PufZisl9<r&K
z;%LK62=#x{inLuQoEA0A;wcrrI~lV^kZ2L#q23$ABmqb#=}B0^s{MipV#6M>+PPBR
zdUVX-eXI_HHwGB{<(PNY1zMhLZz^?2e=Knlj`ZRX+_moZ8b)L@=geZuwIhAtE6eug
zf*pbDgtqi`TWFE2Ph!Wlu1HmQ)D6!lz8Hs*BJ`v1CaQ!6RtQx-eGPw~uRyN&WlQhz
zk=>iIaVGd^>MC!U+Gr{Vg?Sps{vO7PX8-%UI~Fir1cl7U;1mpR`4ur;BIRqPO*skD
zUIwOT0y>u1V`7RS^xOA49LIz{kmZ6Y$#;jQGc!S1cY+><wZh-V*-l7$N~idhLlfb$
z1WWzOi(Zx#`7dxE^?7@(#tv^15o9{Prl5*e7=EM2Ik^ICZolshd+^}WKW|y25UDji
z{s1BOBr%(zjl?p~GeZ*J;ahl5k?aZ4>Dm3ML|pMYY#rlNIc4)++^#uf;bJN@TkK=f
z_Bw284rZOT4z&LYSUDY_pk~v<EbdG>Bv^f5Ha#3r<)TdmTiRJ5ckF}$zG|ngCk*n|
z4)BCs<YnR<XO=HMjCZXoA-pI@qu=(>g?`M^UVfjyL!iWS`vWjQ{jzB+BHG;Sr0@+h
zOWfN++PSCJXolgG#sbl;ej<e2-imGP;ASkAY_?3BcU3|5*X5*b)0HvyeP*W#C)I-=
z2t>0yp|fBG85#HtE#QTmE@pe%B_9UpmLzswnnT9tx0nwwjPvqeVm{ryPAKT6^Fe1`
zRaNW#eRf_qBAIG66H4}e{-}_~er-*09h!ierL|+n>V-={3mIO+h`_hcMPW@!Zj1^`
z;|61ftjw>)%lOl+6ky||l5PHN*CfL2FnA$09L^;91)uPMS=X#bgH^St&&d;>WhEYO
zyXW*tk)NfQqdiUxy?1zV3Y2IaG+}7>8I_ia(<|3c$V!r7Ke^%9R5tN6A2BCMHrFUG
zw@tbs+tXYjPqJ3rIG&T^;xq@0t=7Lf*IBuR+t3q#<Alm#uR+(>*!fj%Vy(k&n7Lrp
zKG8rAy|UYZVS2|E>(J9~sE={1Ls)O<hP`W(AX;zQXFp4IT2K8lsD};&>cs6VI5Ki%
zu=&>SNi#n$Zs3}Bnf%oJly1E_-Qg{ts&xSknv)Ceu#lKtcAaJRMo;16V;??*Y56zC
zRufN`5<4wZpm=nt8EDyNxJPm5aL<HWHTRU{;PYXor+F?L0rg!131L>j_devfbyrtj
z=8;mHp*HLboI8^q?iYbL(5ui+|6m~tcZihy+<~vNPWUCA+r4h{<+HvVyv{a!IWm_=
zutREfGyGEnck1&{m6oU|dl8RL)tc2~t=0GTEUP?}Mat=S;}+ysIdXaYn(WdnARhiw
z>t&0thm|MoUdF00Cc(SnRowcnE2I=Rn^=5a&C5>>j}SG4)G%Iqqq01>wK(d^CVOjJ
zFfQQP;PW&gqAstiuu}-8#it2KEESu2kD_RfL}|AQ#ZYv0V;fz>H?`2i7me|pn?;tJ
z#ro+iQKlx*?I(v7{^Bs05t)4ds#gEpGsCcv%h6|4=Fuy1x|+TqF5{5$j?67uJ;+cR
zqqx&H5#Mepe;t1V_Zcxbtq071U|rhWkJB-%wJ>FG8A5ls?{H5xir8s^Y!0_H(%ItZ
zfVwdu|JBXQ;?k}q&I-m%&W|#&l)jx%t8{1osL(r10H2;uewqjtA>lT3Aj93<&Fw^T
zcJQ}rxBS@aUri%z8Y@knI!tMKvAw9l#72MwltqDHb2>_lO0tRaAjz;N7lAoD_cweh
zNbR8ddsG<XcpI5x^)Q9dfwNRTT{1%t^$Y(*VV3*CntMa$P6h=6gh_G7Fg>?6U)BJv
z*|(*Ebk#5}*jELbciu_&0-wGfHYUT<=BwD3PI}CwJ)Y7+m_`Y7cb>pQi>i-&%%}vM
z7qaa>XSsP6DlPD(7uVc`x0D@wjX-TPE-Ik*HKO&yXPW9+o6KG_`e!g1r(F@!Onb!K
zopWdWhpL0kZ7sD?Z}>>!8iwR<)pC)m$<cfxX~|Pfed~6meavl0qQ6J&=FiNNTzXYW
zfyn6VTSkMJZq~|z;cA#Y8aTVq&dT6S=5ZQ&Pfjz97G?pvciZ?E5*}X};$)Rs<}<vm
znzL9spONHxg&B!@e<+x6oIFGa!*Co+7#R?l3oEx3+#bDdE;+)`QsSY?iS5(!>V|8n
zpGt&@Uvn&8X3t1sCE%XITe(x`V6}oIlT%rqvUe%hvGaJz?+udmG=tg}N*d2y(6%P(
zg!QYaNQn?{b_zAjB#Z{+u#h5SX2m)DicK*XF3vdABAe)7m4pT3D$W>1$Ad8~<PW+X
zywp8xsGrzZ%05Krwur}O0g{CZo{&d0<tjAKB#vX2IGV-@kv#TP$BGm5UX3vw?%cOP
zYom>ldE?0WY~+|5BMB*c)Z*b6cabH${WR-~=?MCq=sn_yX`A>fskIy3STB2BO&t9!
znLdr>#3hxP!d1rw?dZ_u2O{4;ca<5i_u)McAu5TNsVehTSJIeCi9ZF@<5)A2M?F&G
zyPH$A_9g)F(M(h%TG_VdBw4cxDHIv5C`!K6!pST2h=9mgPu(aQ!z6kBsb}Wr4|09}
z$fiO#BL4dG1x=lCy8PrWUp1ze`;!uDL*?ii4)qdAGnei`xsEHQv&w|t`-P9|Mbb|V
z)J5+w96!*`(i_j+&w3hxOD4V9Mq^v9M9scQAfsX|QCP}gWYTD+=re97f6$}i0f(gB
zVtM0uJ6L1(oeLXpce+l?RYI|asnMcS6KL)mvG^X&E4PxN#Df>+s(q32#MDuTxR#AJ
z=S9%l%(X*cM|{c7T7$Y%EEQ(Ii396!N#IuRe2E`62y~$vXVlxU-An{F(7f#O`rH*c
zWhZAC@G*?%ig>WTQEIkA(C-~Qa2v=_zre<+qd2f6JEwnGc|C77@58*ef5|N?tj>lW
z{uBCUk8=9t>R~~l^z2!a+~=MB=XQfE3fb+IB#KNq?E3@Atp^<)qN*!k^5@YyEQJ#u
zwUK}EJjfrzsDF#D7C+LAM6pE_TST$NpNuV{+Ulsb`mYBA{}>=ZH9b*H&%dJSNsPw*
zuT4n?;C%HR+WbErVEnDJ>7P#U{&1kmE`{2o3Nch6hAPBRh1h=?H~(oSB)(_L9938R
zuDap@niOEmLc|w=w0c>^k84}^@Xjkm(Y^KPaVE#-m1U9X)H*I^9xn`J0MF`Z40Xgl
zw-!k0S_di}k9+8bW@A8xTj$oJE7N(9yjz+7WU=IjVd-bvCipcN6Qy$*p@8#;fwT0N
z!HTWikEBa{6mY3)!9oMr<LaK={O`NewG&y>v4Ez7F5=`Jf}$C43N-jS42N^O3~6}1
z!VRA?sklw-|KpgL{*!Gll%xHyqoIThl#qcEGJdLMJXA=cLJ}2{sF3{6rDhb%{9l&w
z|Mr{-MazE|Ek~gw3MEk}`IDjKFAXJ+{@WD04FQ!#LEm{z<!_(Be=T}_#|;_xR@%yc
zlPmtk7TkB=tohqoIRdaIXbCJbn+%eEU-ust^SFBnD8Vv}{Egc@f7|!`6K=um!J4a3
zk{C)7Lp3c?O-oeM^80SP{q6AsMJG{o@~@d5L*3Z>&hvh#kVJ(fDkOixvzI6h5v3vi
znHplSXfpr=*mesVH2U-k5T7H4`*6nj769$I9xXUHSmu|z+cKE}>*Q48{J>v+127B>
F{{akY3)=ty

literal 0
HcmV?d00001

diff --git a/tests/misc-tests/030-undelete-subvol/test.sh b/tests/misc-tests/030-undelete-subvol/test.sh
new file mode 100755
index 000000000000..d3dfa5008601
--- /dev/null
+++ b/tests/misc-tests/030-undelete-subvol/test.sh
@@ -0,0 +1,34 @@
+#!/bin/bash
+# Test undelete-subvol can recover the intact subvolume, and can skip the
+# incomplete subvolume.
+
+source "$TOP/tests/common"
+
+check_prereq btrfs
+check_prereq btrfs-image
+
+setup_root_helper
+
+check_image()
+{
+	TEST_DEV="$1"
+
+	run_check_stdout "$TOP/btrfs" rescue undelete-subvol "$TEST_DEV" \
+		| grep -q "Recovered 1 subvols" \
+		|| _fail "failed to undelete subvol $image" >&2
+
+	run_check "$TOP/btrfs" check "$TEST_DEV"
+
+	# check whether the recovered image can be mounted normally
+	run_check_mount_test_dev
+
+	run_check_stdout $SUDO_HELPER "$TOP/btrfs" subvolume list "$TEST_MNT" \
+		| grep -q "lost+found" \
+		|| _fail "failed to find the recovered subvol $image" >&2
+
+	run_check_umount_test_dev "$TEST_MNT"
+
+	rm "$TEST_DEV"
+}
+
+check_all_images
-- 
2.16.2

Re: [PATCH v2 09/10] btrfs-progs: tests: add testcase for undelete-subvol

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 12669 bytes --]



On 2018年03月27日 15:06, Lu Fengqi wrote:
> The testcase checks the functionality of "btrfs rescue undelete-subvol",
> including recovering an intact subvolume, and handling correctly
> incomplete subvolume.
> 
> Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
> ---
> v2: add shell quoting to test script.
> 
>  .../030-undelete-subvol/deleted_subvolume.img      | Bin 0 -> 4096 bytes
>  .../030-undelete-subvol/drop_progress.raw.xz       | Bin 0 -> 23452 bytes

030 is already taken.

And just a nitpick, the file name is not obvious for its content.
In fact for "drop_progress", it have 2 orphan roots while only one can
be recovered.
For "deleted_subvolume" it has 1 orphan root and it can be recovered.

Despite that, it looks good to me.

Thanks,
Qu

>  tests/misc-tests/030-undelete-subvol/test.sh       |  34 +++++++++++++++++++++
>  3 files changed, 34 insertions(+)
>  create mode 100644 tests/misc-tests/030-undelete-subvol/deleted_subvolume.img
>  create mode 100644 tests/misc-tests/030-undelete-subvol/drop_progress.raw.xz
>  create mode 100755 tests/misc-tests/030-undelete-subvol/test.sh
> 
> diff --git a/tests/misc-tests/030-undelete-subvol/deleted_subvolume.img b/tests/misc-tests/030-undelete-subvol/deleted_subvolume.img
> new file mode 100644
> index 0000000000000000000000000000000000000000..9168a8e33fbbb14f9ea78514721a52191b94d1a0
> GIT binary patch
> literal 4096
> zcmeH|do+|=AIB%x8p^0djl0}JQb@$O%TY$;lEx)vlF+ymV;WO45{D?%IqnhB$S}Fi
> zjN6EihKkC4NQhyKaT(V$2k-mO={@KF_pD{Fy}r-)`R-@^*504B_p@W+PlCQF!fF8j
> zZGs!m9b0|F@NPJOvGJw?_&4=#{gw~i3;@(EFS(@+TiUf5-~Erz<=ODYja#|MmTnGi
> zw`~I31pbc*g!Km3O1HeZD~KRV`{Pj~uzt<!(k&GKVR-=o%Mv|>3r2&j3Mv|UgTH}a
> zWDkSP-%;<0K6oq%LMk0=5)g}2I{;`k(RwtnGuI_hB01WRSAongy!#b`_p*d$38#+;
> zcYI;9gzxul-@Q0?=XZCRp1a_O(t&`~#!SFI<>VfKQp$%5D%q@qRO^IYiZQ11&z1Ee
> z*LJ*O&zE@akHeln2DGHANbna7)texSTKS8w%9X2EBg2OP_&dC*;ZbHjccM=V#lO%}
> zATc9#siZ*izH<@v{m4ksm!$X!TLEok)EiGD1DIiBhF{t3@g30ydV%CQ75thpDR_Sz
> zkNpn7>SYJJu-@rDqHlw-Wb-Wdq8i)|MhOAQWLvnU+THRaIvD@>=47X$XKjKl#3c>c
> z+gVL~u-1qB(5Zq5RlSf2!q|mGkUWzvayXPkcZ+}`@>&ZNwc>iAb(MsWDCeg9_g?z@
> z+UzN}@~(J8&Vn&7SGm+x+QKwvX*meP^nxg%)4Y+tC!c%#Li1uw)!=uIZLYL%h({>6
> zI?6W8VFdIKTctfwulDqceyAL)1|1Zhpij6oC^|RGxbla6`S|q7kxq3u8hQMPc;I2Y
> zCWZ`c;*ciOjl@3Y^DqdF9BWsR_*&1bIYVLtMwmWM2N+tNJ-7j(O%B-L35XizYGSd1
> zB~$y~k-c18T=w!oFaLH~OTx)ncn*m3#tpvpB({MHrvM%M=n2J!-i}LGsv{&tIzHE?
> z<~5bwAY@lJ)i%#ao<x?SA3mL#^UhRfE(EYw`F~o7XMxB_rfBpg`?{m%Ci?}4NDL3Z
> zFaVHTbX7=>Op^!R$6CJL^-x?kT(roP@)*Q8XnwB2j3pd-#ynd#jp7WCx#4WSfc|{y
> z#5H*01m+|Z94ps6)UYtvKCre(ZF$dTAxE?$+j2>nxqYpZ_Rkl)A)IQAwvH~^(SzZi
> z0p9u3gpBilS%_h>xu4tFT+N#=O{4QmFL|iRrJJjnU7WSMuKUyHtOvks7;`zGw0BVH
> zJQ}jPdQ!xy(s!Us?OIYAl7nvL&Ww$7V~c{(G8ho_TID+~?_UeBFyGcNvvctBnsrrf
> zR>%@OeHtPSWh}(@*~6Mo%wR1Xc{kY<0UPWj-p+;|;pI=V<}0+sd36pSJ9HL1<k)SY
> zedyJJH`V;qs}eahO35L0A>GOmQHYWmh`5IF7C{XLOE6~i$Hfktz9xZp$KS@J7FBer
> zU6XsQh2^|0*VWg^M;!+bUUhGGt|)c?CClbV{&HB)#jL~#x7@Hgi$!??u)-rbQ^;oY
> zY!$-MsH;KQ%fzKtwafY8t$(=9mvBoO{a>gHD^*bc*g7T=1Gzb&olJjE*{j2yz>ok&
> z7M6gGH4Q*~983Vsv#8?bD~SiDobt+CdWEqjraBk8ESoH7vV6%oI3Ejb*&G790PCr$
> zuNVEyMF_j+Q>(9d(j#nI_W+HW?IqPn%^pKydSPdh>>Y3s5VIg;-RjrS*7CMI<pjgD
> zy@k<|nP3VL@ndp8<K>7{u~HVW_{Y*$se=Z}$CDIdvvKrhnca(1M-26(9HuWE?Ohji
> zuBpmHt(QTgQ-Yq{m73~S!#xX-w!hB|`k?xhx6>;m!B6zHz=2>h)6=4D_a-K3kJ6t;
> zWMpX8X0U`fZB*msQ+^4vT8hol-6nS`h35;Ds!prmWNZ?5xhs3K9QX}xIY9op0XE<2
> zd-c9BbluK;C2&UT-_kTap|WlT=|N;-RvR<kdv2}t6fKqh0`IM+Q{rb^!_>trenZQU
> zPiVGB&=zwO#labtRhCASg?G_h$wLNDK8)U6ui@n~0zl=OVap45dP71-m<U4Q6{Fzs
> zMM_GKki$jYRwe&LfwYaE^&@*Ja~QGj{(V%y{8|cwbYtaLwY@PFu7*0-Gi2}@`5%J{
> zXI9Rmc-)7felm=~CiMi=s78&rYxK?Ut6J(^D8gq>(j?er4a{gMzE>X8BhF^HR&+yN
> z``L&RC#iGzIqve?b}mDKJ3R?!Sj~hEkClMj*aQPHs-je`Hw)G=a^_UGlEFuf?-JGn
> zrt3Oz*O1f$_uqL2)&V~`vD^hS#j3mRtrVnrHk6fM)Az8Q5M9FVc;SjPt^dWinLHEw
> z(Yd%niJ8}8Uk@IFxs`OJKq)!4{LOe!xf<W3<4#vGp4<HE^uZfWJHLYEy_k`g&X>}C
> zCCb1~=m9Lg$9ZHABR*mWv80B!dG3fdJeMV!XUBJIiz)c*3_^%D@4@C+zdY>o!34x-
> zf=~s)a%m<7<=N$mEk3g{K0aS)5Q~o%4<x&sXGY_N+Hsb3S)-Yphz`_@A*S9yO-7{A
> zYQBQoFFhvh0X-TkNY8+hCRNwn{k<HIh8#&^(dI|jPa{8~3N7DTQsk!GRra-)%F7T=
> z20Fx*h7a@QQ97Te92F>(9R>QL7W2uspH*KopFWYyiKEB0WdA*o`T(uUGBdW_&Hk*E
> zzTw9jeaVG?gtI%=*AAzOt@~-PogS@pC<cvNndN*@;PVNLs(gipI;+<=K4);D{ZsHy
> ZeM(;pZJdD++rKZroo*A@Ch+ek@Fx_v;e7xA
> 
> literal 0
> HcmV?d00001
> 
> diff --git a/tests/misc-tests/030-undelete-subvol/drop_progress.raw.xz b/tests/misc-tests/030-undelete-subvol/drop_progress.raw.xz
> new file mode 100644
> index 0000000000000000000000000000000000000000..3db2c80e61fa9bb99db374509dbb95910965d467
> GIT binary patch
> literal 23452
> zcmeHPXH=63mktC7z4zV}r8hyO7ZF71J@gJj=p6y+(xijbKnNWvN|D}+^dbU^sPx_m
> zAee9V?4CV)c4qg}`DSLFv-y{QZ{GXfd!PF}w_J((=^Fw77^^d{6wv`JX!HO8fOt%Q
> z7>Rs>(P<0-IA0-=um<EkDJ2RwF>{T^q#LWS2+;ZNGGoXTU+*0WG?kjb!RY9Rd@zE!
> zprWN%HWIB*2GqTt(O<mJ>QjR79XdXGC|eojjqRj@Ntje7V=&Dfp58RkEphE+!*iK3
> ze=gkgvO#(Wb3wy1yH#b!yCt$!YIsJE^$1(}*0xh|G%dc{X8vwN4zHQ}bVmsR<s^;p
> zo1FQ@#en!Cnsw{>RrWRKCJQwJZ4r;|fIzaNqWP$xtDSB^E`4R#+kNfF<kDzNG(HMk
> z-Pu^-_90d6FamaI%>#ZlImmLpgm-*(wek`j)@7COOkP^)r&;tzG%wF$uJ>jkzy^dj
> z*q1CnCUcRGf<3#$dF~&$F~=`u6%?zfjrjSgZ#=WTN%JruO&XD64@f4|;_Itq@hv``
> znjt?-*%=Zcor6q8k0n~uoI_Sjmizo#?Qx)NGtnCjRYcZ6yKf~=j+^wl)tqQ;;UoIe
> z6dYQm&UF5`k}lBct=xCfMBv`>_%x+lk+Tb7FuRhD^!2Cr(cdZ=3Hg+ski3Y-op&tg
> zFEWTlcBk^eKNL|WnH>4v24}=}Oi<&5mq8M6PBQuKPF}pW3#$g6<m`@L=xf?G^==1)
> zw`THQ3|^II!O9__?;6u-(mh`8tz0P_5#DB&|B{nd4&56jy~NjIe4+TdrJ{Psxs|Qp
> zxvgM<kGh)>7be*!O|OI5jBm@GHp1l$d3pM0hxL^?iZNjay1ELxtIk(9q%;7k{=WOq
> zVcqtGQXM9N6owoF$=%Ja6A{~D@{W^FvI{VZ9!L<G!uPCSV;P&gP8eJT1Yl=#GGK`W
> z7}G-v+%xe(;&<|Oy@ZWe{cQ<yK|oEVFYZ>(Uc`zt6i4mv(BPmaC9yTV{>?PC1dsJG
> z4AQ!27bo-Ofq5a~mLqp4hPsA?n;ise0$mkdvDwg^$n=EO81vd*1PIXPHN*)(Tyl%q
> zOt)_gg-@hrL<e#v9&&ghXgaVm9%fw4R=rIi>ba(Mc9JL}fC;bOb?$U|cn^Ofk)*8B
> zjG<|?hJ)J3AY#KtQ@?G)tHNM;1&oP(-alV?`BjsPpu6ye5LK2e``x~)%3>aI&vA{+
> zHKu|WvTGm=s1F(VVIJuhE2#V@!?+<tGQ#h(lJdGOWRO`s1B;a7O^;jzlrgkx2cvIS
> zzmLFe9cwt1fID}zcmK0f@G-N17w!sp?nER}GoP7F%#_ZjQfx0G&?quSKGkkxuV6m&
> z=~rrE;?LHRB2kysZ^ev3Z&d=iZs%#+D|9hFq;N95EP^WnTa#J&B*%)nPTw6}_s<Bl
> zqPIQ)@+;pOP;Roalb-?IIIMBrr^=KW8aC(3*7&v{A#gpu*&SGvdD2IJ2Op=Q5G}25
> zklWFiMX&$Ckx8hwNBGTq7_rY^r{|DDEp$sGg6gm*DTAwZ_33uGcr5$jexgcEH^W;%
> zuRK>sAk#x&uDZlU?3J9}e$^#srG{thpZ}!K;4#7oW|li&(2_j6X@w&mJjCg^7}UEq
> zk%uHk;{t5EYZYJp>8|iw|DjHNzo(Vgw2Vkf#YQfHj1WnjeCFz7aE3R6OueB<RC*ck
> z@oOeOuWrY)@T1Tt9oE0o3;&o$YY<7-<{^X8VF0PRcK@xOC{98H?Y#4fwv0$?zWR$?
> zZCsY{^W3&wgq~;TD{Z3?;=1&Nn;<W12H7gP+g5$iv?f9U)zcg01tX9dZVIY<t7Zi<
> z-TYHDsuFybkArJoL_+*lR?nrb39D5C8N7Jl?B?x?-V>X<t>+U)Z*SyD;@uq`ho*x+
> z=m>N|fxM7WYKaPPD01+E->9Fg0IhvjdE_8fQk;p5x2fOm^#<{LYwG|LWicVi#9;{C
> zx_vXnL{_+W&HbRz0iR(qoUeg0<Tg|I&}Gr^k=3exF=MJOqdqK$_@GP6komO1zyYmB
> zwIe&wsO!QcS?nxZL|AA#DR|5RB+%Qf0sG(q*Y_k~u@N`W=w^2twPe-gVW&2}a8ad8
> z%N_;v*wpSN*a<Lzk}2@WYh%37__qcM7wFrR4R`pb9QlErOMPzDmKGJwmxFi792*bB
> zH@@jyKRpa+*|>*LN~>%A#XckA!l7J~c<4uGl)=avBiUMHks*>T=bqt8v&f?cYc$%8
> z8{%;<Pu^4cFbg^JBj8^L)Yi!)8<v8b*h<c@V&+bQW(eq{AA6UFlCwZ2-ufL$k!xA}
> z_uTZyc*XBZi+}E;U(}05pd$MRlFGkFHYyc<cNRt&><3{BWiXV%{-+xGj{yUehoL;|
> zukf%}XnBCPpk-<fVdt&bB;TFc{J7~)SslYBk>w^~(#oQZj-fGkK6HclZ%SVq9lXdr
> zcj_gxFG!iDhahN7wz4|U$~oGnuBJ3iwcTCTE}MLT(#wm))4-KOjqL99t=6iO>uKFS
> zU%fRRpnm|H6xQ0g5{;2m0R19@rEeBKMtpiD==xBCGG4A#*{H{<HfaqyFbcxh&HD(K
> z8;X)P0z4a6t-5|A#g$3&uO(AI4Uxg-nfFE2eyG~-x5O9}(EOl!^+y`)2u%<$)d=@c
> z+NQ26whNL5{Y=uvPe@`%YuHRU$5E*amC8`5>?e~z-{GbL7O59HE$uP+2$^=<B4sRH
> zyBp|9Byub8XGt=|7;&xiXeOmjYWuNTF!d=2N~7L8-GhoacHE~GYo|-elx7gg6S^R!
> zuz91JubA_GvvLL(q~T#?_3bM*In(?CO@iR#MxAZ{+V-WDy9_kbu5LyVHkf{@_pg~n
> z6eGO1WXq~Crp5X_)NnSv8nlXUzN_J~uQ5NX-2d{qr7%BZr;)uqP;(tDlSQL8wKP|^
> zuI(V}M6&^<9A0I;LklU+7Rxg4dB+Y8#647dEuCf?F0!A*c7|~k^L`PufZisl9<r&K
> z;%LK62=#x{inLuQoEA0A;wcrrI~lV^kZ2L#q23$ABmqb#=}B0^s{MipV#6M>+PPBR
> zdUVX-eXI_HHwGB{<(PNY1zMhLZz^?2e=Knlj`ZRX+_moZ8b)L@=geZuwIhAtE6eug
> zf*pbDgtqi`TWFE2Ph!Wlu1HmQ)D6!lz8Hs*BJ`v1CaQ!6RtQx-eGPw~uRyN&WlQhz
> zk=>iIaVGd^>MC!U+Gr{Vg?Sps{vO7PX8-%UI~Fir1cl7U;1mpR`4ur;BIRqPO*skD
> zUIwOT0y>u1V`7RS^xOA49LIz{kmZ6Y$#;jQGc!S1cY+><wZh-V*-l7$N~idhLlfb$
> z1WWzOi(Zx#`7dxE^?7@(#tv^15o9{Prl5*e7=EM2Ik^ICZolshd+^}WKW|y25UDji
> z{s1BOBr%(zjl?p~GeZ*J;ahl5k?aZ4>Dm3ML|pMYY#rlNIc4)++^#uf;bJN@TkK=f
> z_Bw284rZOT4z&LYSUDY_pk~v<EbdG>Bv^f5Ha#3r<)TdmTiRJ5ckF}$zG|ngCk*n|
> z4)BCs<YnR<XO=HMjCZXoA-pI@qu=(>g?`M^UVfjyL!iWS`vWjQ{jzB+BHG;Sr0@+h
> zOWfN++PSCJXolgG#sbl;ej<e2-imGP;ASkAY_?3BcU3|5*X5*b)0HvyeP*W#C)I-=
> z2t>0yp|fBG85#HtE#QTmE@pe%B_9UpmLzswnnT9tx0nwwjPvqeVm{ryPAKT6^Fe1`
> zRaNW#eRf_qBAIG66H4}e{-}_~er-*09h!ierL|+n>V-={3mIO+h`_hcMPW@!Zj1^`
> z;|61ftjw>)%lOl+6ky||l5PHN*CfL2FnA$09L^;91)uPMS=X#bgH^St&&d;>WhEYO
> zyXW*tk)NfQqdiUxy?1zV3Y2IaG+}7>8I_ia(<|3c$V!r7Ke^%9R5tN6A2BCMHrFUG
> zw@tbs+tXYjPqJ3rIG&T^;xq@0t=7Lf*IBuR+t3q#<Alm#uR+(>*!fj%Vy(k&n7Lrp
> zKG8rAy|UYZVS2|E>(J9~sE={1Ls)O<hP`W(AX;zQXFp4IT2K8lsD};&>cs6VI5Ki%
> zu=&>SNi#n$Zs3}Bnf%oJly1E_-Qg{ts&xSknv)Ceu#lKtcAaJRMo;16V;??*Y56zC
> zRufN`5<4wZpm=nt8EDyNxJPm5aL<HWHTRU{;PYXor+F?L0rg!131L>j_devfbyrtj
> z=8;mHp*HLboI8^q?iYbL(5ui+|6m~tcZihy+<~vNPWUCA+r4h{<+HvVyv{a!IWm_=
> zutREfGyGEnck1&{m6oU|dl8RL)tc2~t=0GTEUP?}Mat=S;}+ysIdXaYn(WdnARhiw
> z>t&0thm|MoUdF00Cc(SnRowcnE2I=Rn^=5a&C5>>j}SG4)G%Iqqq01>wK(d^CVOjJ
> zFfQQP;PW&gqAstiuu}-8#it2KEESu2kD_RfL}|AQ#ZYv0V;fz>H?`2i7me|pn?;tJ
> z#ro+iQKlx*?I(v7{^Bs05t)4ds#gEpGsCcv%h6|4=Fuy1x|+TqF5{5$j?67uJ;+cR
> zqqx&H5#Mepe;t1V_Zcxbtq071U|rhWkJB-%wJ>FG8A5ls?{H5xir8s^Y!0_H(%ItZ
> zfVwdu|JBXQ;?k}q&I-m%&W|#&l)jx%t8{1osL(r10H2;uewqjtA>lT3Aj93<&Fw^T
> zcJQ}rxBS@aUri%z8Y@knI!tMKvAw9l#72MwltqDHb2>_lO0tRaAjz;N7lAoD_cweh
> zNbR8ddsG<XcpI5x^)Q9dfwNRTT{1%t^$Y(*VV3*CntMa$P6h=6gh_G7Fg>?6U)BJv
> z*|(*Ebk#5}*jELbciu_&0-wGfHYUT<=BwD3PI}CwJ)Y7+m_`Y7cb>pQi>i-&%%}vM
> z7qaa>XSsP6DlPD(7uVc`x0D@wjX-TPE-Ik*HKO&yXPW9+o6KG_`e!g1r(F@!Onb!K
> zopWdWhpL0kZ7sD?Z}>>!8iwR<)pC)m$<cfxX~|Pfed~6meavl0qQ6J&=FiNNTzXYW
> zfyn6VTSkMJZq~|z;cA#Y8aTVq&dT6S=5ZQ&Pfjz97G?pvciZ?E5*}X};$)Rs<}<vm
> znzL9spONHxg&B!@e<+x6oIFGa!*Co+7#R?l3oEx3+#bDdE;+)`QsSY?iS5(!>V|8n
> zpGt&@Uvn&8X3t1sCE%XITe(x`V6}oIlT%rqvUe%hvGaJz?+udmG=tg}N*d2y(6%P(
> zg!QYaNQn?{b_zAjB#Z{+u#h5SX2m)DicK*XF3vdABAe)7m4pT3D$W>1$Ad8~<PW+X
> zywp8xsGrzZ%05Krwur}O0g{CZo{&d0<tjAKB#vX2IGV-@kv#TP$BGm5UX3vw?%cOP
> zYom>ldE?0WY~+|5BMB*c)Z*b6cabH${WR-~=?MCq=sn_yX`A>fskIy3STB2BO&t9!
> znLdr>#3hxP!d1rw?dZ_u2O{4;ca<5i_u)McAu5TNsVehTSJIeCi9ZF@<5)A2M?F&G
> zyPH$A_9g)F(M(h%TG_VdBw4cxDHIv5C`!K6!pST2h=9mgPu(aQ!z6kBsb}Wr4|09}
> z$fiO#BL4dG1x=lCy8PrWUp1ze`;!uDL*?ii4)qdAGnei`xsEHQv&w|t`-P9|Mbb|V
> z)J5+w96!*`(i_j+&w3hxOD4V9Mq^v9M9scQAfsX|QCP}gWYTD+=re97f6$}i0f(gB
> zVtM0uJ6L1(oeLXpce+l?RYI|asnMcS6KL)mvG^X&E4PxN#Df>+s(q32#MDuTxR#AJ
> z=S9%l%(X*cM|{c7T7$Y%EEQ(Ii396!N#IuRe2E`62y~$vXVlxU-An{F(7f#O`rH*c
> zWhZAC@G*?%ig>WTQEIkA(C-~Qa2v=_zre<+qd2f6JEwnGc|C77@58*ef5|N?tj>lW
> z{uBCUk8=9t>R~~l^z2!a+~=MB=XQfE3fb+IB#KNq?E3@Atp^<)qN*!k^5@YyEQJ#u
> zwUK}EJjfrzsDF#D7C+LAM6pE_TST$NpNuV{+Ulsb`mYBA{}>=ZH9b*H&%dJSNsPw*
> zuT4n?;C%HR+WbErVEnDJ>7P#U{&1kmE`{2o3Nch6hAPBRh1h=?H~(oSB)(_L9938R
> zuDap@niOEmLc|w=w0c>^k84}^@Xjkm(Y^KPaVE#-m1U9X)H*I^9xn`J0MF`Z40Xgl
> zw-!k0S_di}k9+8bW@A8xTj$oJE7N(9yjz+7WU=IjVd-bvCipcN6Qy$*p@8#;fwT0N
> z!HTWikEBa{6mY3)!9oMr<LaK={O`NewG&y>v4Ez7F5=`Jf}$C43N-jS42N^O3~6}1
> z!VRA?sklw-|KpgL{*!Gll%xHyqoIThl#qcEGJdLMJXA=cLJ}2{sF3{6rDhb%{9l&w
> z|Mr{-MazE|Ek~gw3MEk}`IDjKFAXJ+{@WD04FQ!#LEm{z<!_(Be=T}_#|;_xR@%yc
> zlPmtk7TkB=tohqoIRdaIXbCJbn+%eEU-ust^SFBnD8Vv}{Egc@f7|!`6K=um!J4a3
> zk{C)7Lp3c?O-oeM^80SP{q6AsMJG{o@~@d5L*3Z>&hvh#kVJ(fDkOixvzI6h5v3vi
> znHplSXfpr=*mesVH2U-k5T7H4`*6nj769$I9xXUHSmu|z+cKE}>*Q48{J>v+127B>
> F{{akY3)=ty
> 
> literal 0
> HcmV?d00001
> 
> diff --git a/tests/misc-tests/030-undelete-subvol/test.sh b/tests/misc-tests/030-undelete-subvol/test.sh
> new file mode 100755
> index 000000000000..d3dfa5008601
> --- /dev/null
> +++ b/tests/misc-tests/030-undelete-subvol/test.sh
> @@ -0,0 +1,34 @@
> +#!/bin/bash
> +# Test undelete-subvol can recover the intact subvolume, and can skip the
> +# incomplete subvolume.
> +
> +source "$TOP/tests/common"
> +
> +check_prereq btrfs
> +check_prereq btrfs-image
> +
> +setup_root_helper
> +
> +check_image()
> +{
> +	TEST_DEV="$1"
> +
> +	run_check_stdout "$TOP/btrfs" rescue undelete-subvol "$TEST_DEV" \
> +		| grep -q "Recovered 1 subvols" \
> +		|| _fail "failed to undelete subvol $image" >&2
> +
> +	run_check "$TOP/btrfs" check "$TEST_DEV"
> +
> +	# check whether the recovered image can be mounted normally
> +	run_check_mount_test_dev
> +
> +	run_check_stdout $SUDO_HELPER "$TOP/btrfs" subvolume list "$TEST_MNT" \
> +		| grep -q "lost+found" \
> +		|| _fail "failed to find the recovered subvol $image" >&2
> +
> +	run_check_umount_test_dev "$TEST_MNT"
> +
> +	rm "$TEST_DEV"
> +}
> +
> +check_all_images
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 09/10] btrfs-progs: tests: add testcase for undelete-subvol

[Lu Fengqi]

On 04/18/2018 01:42 PM, Qu Wenruo wrote:
> 
> 
> On 2018年03月27日 15:06, Lu Fengqi wrote:
>> The testcase checks the functionality of "btrfs rescue undelete-subvol",
>> including recovering an intact subvolume, and handling correctly
>> incomplete subvolume.
>>
>> Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
>> ---
>> v2: add shell quoting to test script.
>>
>>   .../030-undelete-subvol/deleted_subvolume.img      | Bin 0 -> 4096 bytes
>>   .../030-undelete-subvol/drop_progress.raw.xz       | Bin 0 -> 23452 bytes
> 
> 030 is already taken.
> 
> And just a nitpick, the file name is not obvious for its content.
> In fact for "drop_progress", it have 2 orphan roots while only one can
> be recovered.
> For "deleted_subvolume" it has 1 orphan root and it can be recovered.
> 
> Despite that, it looks good to me.

I am ashamed of the bad image naming. And I have renamed the images, add 
your above description to test.sh.

-- 
Thanks,
Lu

> 
> Thanks,
> Qu
> 
>>   tests/misc-tests/030-undelete-subvol/test.sh       |  34 +++++++++++++++++++++
>>   3 files changed, 34 insertions(+)
>>   create mode 100644 tests/misc-tests/030-undelete-subvol/deleted_subvolume.img
>>   create mode 100644 tests/misc-tests/030-undelete-subvol/drop_progress.raw.xz
>>   create mode 100755 tests/misc-tests/030-undelete-subvol/test.sh
>>
>> diff --git a/tests/misc-tests/030-undelete-subvol/deleted_subvolume.img b/tests/misc-tests/030-undelete-subvol/deleted_subvolume.img
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..9168a8e33fbbb14f9ea78514721a52191b94d1a0
>> GIT binary patch
>> literal 4096
>> zcmeH|do+|=AIB%x8p^0djl0}JQb@$O%TY$;lEx)vlF+ymV;WO45{D?%IqnhB$S}Fi
>> zjN6EihKkC4NQhyKaT(V$2k-mO={@KF_pD{Fy}r-)`R-@^*504B_p@W+PlCQF!fF8j
>> zZGs!m9b0|F@NPJOvGJw?_&4=#{gw~i3;@(EFS(@+TiUf5-~Erz<=ODYja#|MmTnGi
>> zw`~I31pbc*g!Km3O1HeZD~KRV`{Pj~uzt<!(k&GKVR-=o%Mv|>3r2&j3Mv|UgTH}a
>> zWDkSP-%;<0K6oq%LMk0=5)g}2I{;`k(RwtnGuI_hB01WRSAongy!#b`_p*d$38#+;
>> zcYI;9gzxul-@Q0?=XZCRp1a_O(t&`~#!SFI<>VfKQp$%5D%q@qRO^IYiZQ11&z1Ee
>> z*LJ*O&zE@akHeln2DGHANbna7)texSTKS8w%9X2EBg2OP_&dC*;ZbHjccM=V#lO%}
>> zATc9#siZ*izH<@v{m4ksm!$X!TLEok)EiGD1DIiBhF{t3@g30ydV%CQ75thpDR_Sz
>> zkNpn7>SYJJu-@rDqHlw-Wb-Wdq8i)|MhOAQWLvnU+THRaIvD@>=47X$XKjKl#3c>c
>> z+gVL~u-1qB(5Zq5RlSf2!q|mGkUWzvayXPkcZ+}`@>&ZNwc>iAb(MsWDCeg9_g?z@
>> z+UzN}@~(J8&Vn&7SGm+x+QKwvX*meP^nxg%)4Y+tC!c%#Li1uw)!=uIZLYL%h({>6
>> zI?6W8VFdIKTctfwulDqceyAL)1|1Zhpij6oC^|RGxbla6`S|q7kxq3u8hQMPc;I2Y
>> zCWZ`c;*ciOjl@3Y^DqdF9BWsR_*&1bIYVLtMwmWM2N+tNJ-7j(O%B-L35XizYGSd1
>> zB~$y~k-c18T=w!oFaLH~OTx)ncn*m3#tpvpB({MHrvM%M=n2J!-i}LGsv{&tIzHE?
>> z<~5bwAY@lJ)i%#ao<x?SA3mL#^UhRfE(EYw`F~o7XMxB_rfBpg`?{m%Ci?}4NDL3Z
>> zFaVHTbX7=>Op^!R$6CJL^-x?kT(roP@)*Q8XnwB2j3pd-#ynd#jp7WCx#4WSfc|{y
>> z#5H*01m+|Z94ps6)UYtvKCre(ZF$dTAxE?$+j2>nxqYpZ_Rkl)A)IQAwvH~^(SzZi
>> z0p9u3gpBilS%_h>xu4tFT+N#=O{4QmFL|iRrJJjnU7WSMuKUyHtOvks7;`zGw0BVH
>> zJQ}jPdQ!xy(s!Us?OIYAl7nvL&Ww$7V~c{(G8ho_TID+~?_UeBFyGcNvvctBnsrrf
>> zR>%@OeHtPSWh}(@*~6Mo%wR1Xc{kY<0UPWj-p+;|;pI=V<}0+sd36pSJ9HL1<k)SY
>> zedyJJH`V;qs}eahO35L0A>GOmQHYWmh`5IF7C{XLOE6~i$Hfktz9xZp$KS@J7FBer
>> zU6XsQh2^|0*VWg^M;!+bUUhGGt|)c?CClbV{&HB)#jL~#x7@Hgi$!??u)-rbQ^;oY
>> zY!$-MsH;KQ%fzKtwafY8t$(=9mvBoO{a>gHD^*bc*g7T=1Gzb&olJjE*{j2yz>ok&
>> z7M6gGH4Q*~983Vsv#8?bD~SiDobt+CdWEqjraBk8ESoH7vV6%oI3Ejb*&G790PCr$
>> zuNVEyMF_j+Q>(9d(j#nI_W+HW?IqPn%^pKydSPdh>>Y3s5VIg;-RjrS*7CMI<pjgD
>> zy@k<|nP3VL@ndp8<K>7{u~HVW_{Y*$se=Z}$CDIdvvKrhnca(1M-26(9HuWE?Ohji
>> zuBpmHt(QTgQ-Yq{m73~S!#xX-w!hB|`k?xhx6>;m!B6zHz=2>h)6=4D_a-K3kJ6t;
>> zWMpX8X0U`fZB*msQ+^4vT8hol-6nS`h35;Ds!prmWNZ?5xhs3K9QX}xIY9op0XE<2
>> zd-c9BbluK;C2&UT-_kTap|WlT=|N;-RvR<kdv2}t6fKqh0`IM+Q{rb^!_>trenZQU
>> zPiVGB&=zwO#labtRhCASg?G_h$wLNDK8)U6ui@n~0zl=OVap45dP71-m<U4Q6{Fzs
>> zMM_GKki$jYRwe&LfwYaE^&@*Ja~QGj{(V%y{8|cwbYtaLwY@PFu7*0-Gi2}@`5%J{
>> zXI9Rmc-)7felm=~CiMi=s78&rYxK?Ut6J(^D8gq>(j?er4a{gMzE>X8BhF^HR&+yN
>> z``L&RC#iGzIqve?b}mDKJ3R?!Sj~hEkClMj*aQPHs-je`Hw)G=a^_UGlEFuf?-JGn
>> zrt3Oz*O1f$_uqL2)&V~`vD^hS#j3mRtrVnrHk6fM)Az8Q5M9FVc;SjPt^dWinLHEw
>> z(Yd%niJ8}8Uk@IFxs`OJKq)!4{LOe!xf<W3<4#vGp4<HE^uZfWJHLYEy_k`g&X>}C
>> zCCb1~=m9Lg$9ZHABR*mWv80B!dG3fdJeMV!XUBJIiz)c*3_^%D@4@C+zdY>o!34x-
>> zf=~s)a%m<7<=N$mEk3g{K0aS)5Q~o%4<x&sXGY_N+Hsb3S)-Yphz`_@A*S9yO-7{A
>> zYQBQoFFhvh0X-TkNY8+hCRNwn{k<HIh8#&^(dI|jPa{8~3N7DTQsk!GRra-)%F7T=
>> z20Fx*h7a@QQ97Te92F>(9R>QL7W2uspH*KopFWYyiKEB0WdA*o`T(uUGBdW_&Hk*E
>> zzTw9jeaVG?gtI%=*AAzOt@~-PogS@pC<cvNndN*@;PVNLs(gipI;+<=K4);D{ZsHy
>> ZeM(;pZJdD++rKZroo*A@Ch+ek@Fx_v;e7xA
>>
>> literal 0
>> HcmV?d00001
>>
>> diff --git a/tests/misc-tests/030-undelete-subvol/drop_progress.raw.xz b/tests/misc-tests/030-undelete-subvol/drop_progress.raw.xz
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..3db2c80e61fa9bb99db374509dbb95910965d467
>> GIT binary patch
>> literal 23452
>> zcmeHPXH=63mktC7z4zV}r8hyO7ZF71J@gJj=p6y+(xijbKnNWvN|D}+^dbU^sPx_m
>> zAee9V?4CV)c4qg}`DSLFv-y{QZ{GXfd!PF}w_J((=^Fw77^^d{6wv`JX!HO8fOt%Q
>> z7>Rs>(P<0-IA0-=um<EkDJ2RwF>{T^q#LWS2+;ZNGGoXTU+*0WG?kjb!RY9Rd@zE!
>> zprWN%HWIB*2GqTt(O<mJ>QjR79XdXGC|eojjqRj@Ntje7V=&Dfp58RkEphE+!*iK3
>> ze=gkgvO#(Wb3wy1yH#b!yCt$!YIsJE^$1(}*0xh|G%dc{X8vwN4zHQ}bVmsR<s^;p
>> zo1FQ@#en!Cnsw{>RrWRKCJQwJZ4r;|fIzaNqWP$xtDSB^E`4R#+kNfF<kDzNG(HMk
>> z-Pu^-_90d6FamaI%>#ZlImmLpgm-*(wek`j)@7COOkP^)r&;tzG%wF$uJ>jkzy^dj
>> z*q1CnCUcRGf<3#$dF~&$F~=`u6%?zfjrjSgZ#=WTN%JruO&XD64@f4|;_Itq@hv``
>> znjt?-*%=Zcor6q8k0n~uoI_Sjmizo#?Qx)NGtnCjRYcZ6yKf~=j+^wl)tqQ;;UoIe
>> z6dYQm&UF5`k}lBct=xCfMBv`>_%x+lk+Tb7FuRhD^!2Cr(cdZ=3Hg+ski3Y-op&tg
>> zFEWTlcBk^eKNL|WnH>4v24}=}Oi<&5mq8M6PBQuKPF}pW3#$g6<m`@L=xf?G^==1)
>> zw`THQ3|^II!O9__?;6u-(mh`8tz0P_5#DB&|B{nd4&56jy~NjIe4+TdrJ{Psxs|Qp
>> zxvgM<kGh)>7be*!O|OI5jBm@GHp1l$d3pM0hxL^?iZNjay1ELxtIk(9q%;7k{=WOq
>> zVcqtGQXM9N6owoF$=%Ja6A{~D@{W^FvI{VZ9!L<G!uPCSV;P&gP8eJT1Yl=#GGK`W
>> z7}G-v+%xe(;&<|Oy@ZWe{cQ<yK|oEVFYZ>(Uc`zt6i4mv(BPmaC9yTV{>?PC1dsJG
>> z4AQ!27bo-Ofq5a~mLqp4hPsA?n;ise0$mkdvDwg^$n=EO81vd*1PIXPHN*)(Tyl%q
>> zOt)_gg-@hrL<e#v9&&ghXgaVm9%fw4R=rIi>ba(Mc9JL}fC;bOb?$U|cn^Ofk)*8B
>> zjG<|?hJ)J3AY#KtQ@?G)tHNM;1&oP(-alV?`BjsPpu6ye5LK2e``x~)%3>aI&vA{+
>> zHKu|WvTGm=s1F(VVIJuhE2#V@!?+<tGQ#h(lJdGOWRO`s1B;a7O^;jzlrgkx2cvIS
>> zzmLFe9cwt1fID}zcmK0f@G-N17w!sp?nER}GoP7F%#_ZjQfx0G&?quSKGkkxuV6m&
>> z=~rrE;?LHRB2kysZ^ev3Z&d=iZs%#+D|9hFq;N95EP^WnTa#J&B*%)nPTw6}_s<Bl
>> zqPIQ)@+;pOP;Roalb-?IIIMBrr^=KW8aC(3*7&v{A#gpu*&SGvdD2IJ2Op=Q5G}25
>> zklWFiMX&$Ckx8hwNBGTq7_rY^r{|DDEp$sGg6gm*DTAwZ_33uGcr5$jexgcEH^W;%
>> zuRK>sAk#x&uDZlU?3J9}e$^#srG{thpZ}!K;4#7oW|li&(2_j6X@w&mJjCg^7}UEq
>> zk%uHk;{t5EYZYJp>8|iw|DjHNzo(Vgw2Vkf#YQfHj1WnjeCFz7aE3R6OueB<RC*ck
>> z@oOeOuWrY)@T1Tt9oE0o3;&o$YY<7-<{^X8VF0PRcK@xOC{98H?Y#4fwv0$?zWR$?
>> zZCsY{^W3&wgq~;TD{Z3?;=1&Nn;<W12H7gP+g5$iv?f9U)zcg01tX9dZVIY<t7Zi<
>> z-TYHDsuFybkArJoL_+*lR?nrb39D5C8N7Jl?B?x?-V>X<t>+U)Z*SyD;@uq`ho*x+
>> z=m>N|fxM7WYKaPPD01+E->9Fg0IhvjdE_8fQk;p5x2fOm^#<{LYwG|LWicVi#9;{C
>> zx_vXnL{_+W&HbRz0iR(qoUeg0<Tg|I&}Gr^k=3exF=MJOqdqK$_@GP6komO1zyYmB
>> zwIe&wsO!QcS?nxZL|AA#DR|5RB+%Qf0sG(q*Y_k~u@N`W=w^2twPe-gVW&2}a8ad8
>> z%N_;v*wpSN*a<Lzk}2@WYh%37__qcM7wFrR4R`pb9QlErOMPzDmKGJwmxFi792*bB
>> zH@@jyKRpa+*|>*LN~>%A#XckA!l7J~c<4uGl)=avBiUMHks*>T=bqt8v&f?cYc$%8
>> z8{%;<Pu^4cFbg^JBj8^L)Yi!)8<v8b*h<c@V&+bQW(eq{AA6UFlCwZ2-ufL$k!xA}
>> z_uTZyc*XBZi+}E;U(}05pd$MRlFGkFHYyc<cNRt&><3{BWiXV%{-+xGj{yUehoL;|
>> zukf%}XnBCPpk-<fVdt&bB;TFc{J7~)SslYBk>w^~(#oQZj-fGkK6HclZ%SVq9lXdr
>> zcj_gxFG!iDhahN7wz4|U$~oGnuBJ3iwcTCTE}MLT(#wm))4-KOjqL99t=6iO>uKFS
>> zU%fRRpnm|H6xQ0g5{;2m0R19@rEeBKMtpiD==xBCGG4A#*{H{<HfaqyFbcxh&HD(K
>> z8;X)P0z4a6t-5|A#g$3&uO(AI4Uxg-nfFE2eyG~-x5O9}(EOl!^+y`)2u%<$)d=@c
>> z+NQ26whNL5{Y=uvPe@`%YuHRU$5E*amC8`5>?e~z-{GbL7O59HE$uP+2$^=<B4sRH
>> zyBp|9Byub8XGt=|7;&xiXeOmjYWuNTF!d=2N~7L8-GhoacHE~GYo|-elx7gg6S^R!
>> zuz91JubA_GvvLL(q~T#?_3bM*In(?CO@iR#MxAZ{+V-WDy9_kbu5LyVHkf{@_pg~n
>> z6eGO1WXq~Crp5X_)NnSv8nlXUzN_J~uQ5NX-2d{qr7%BZr;)uqP;(tDlSQL8wKP|^
>> zuI(V}M6&^<9A0I;LklU+7Rxg4dB+Y8#647dEuCf?F0!A*c7|~k^L`PufZisl9<r&K
>> z;%LK62=#x{inLuQoEA0A;wcrrI~lV^kZ2L#q23$ABmqb#=}B0^s{MipV#6M>+PPBR
>> zdUVX-eXI_HHwGB{<(PNY1zMhLZz^?2e=Knlj`ZRX+_moZ8b)L@=geZuwIhAtE6eug
>> zf*pbDgtqi`TWFE2Ph!Wlu1HmQ)D6!lz8Hs*BJ`v1CaQ!6RtQx-eGPw~uRyN&WlQhz
>> zk=>iIaVGd^>MC!U+Gr{Vg?Sps{vO7PX8-%UI~Fir1cl7U;1mpR`4ur;BIRqPO*skD
>> zUIwOT0y>u1V`7RS^xOA49LIz{kmZ6Y$#;jQGc!S1cY+><wZh-V*-l7$N~idhLlfb$
>> z1WWzOi(Zx#`7dxE^?7@(#tv^15o9{Prl5*e7=EM2Ik^ICZolshd+^}WKW|y25UDji
>> z{s1BOBr%(zjl?p~GeZ*J;ahl5k?aZ4>Dm3ML|pMYY#rlNIc4)++^#uf;bJN@TkK=f
>> z_Bw284rZOT4z&LYSUDY_pk~v<EbdG>Bv^f5Ha#3r<)TdmTiRJ5ckF}$zG|ngCk*n|
>> z4)BCs<YnR<XO=HMjCZXoA-pI@qu=(>g?`M^UVfjyL!iWS`vWjQ{jzB+BHG;Sr0@+h
>> zOWfN++PSCJXolgG#sbl;ej<e2-imGP;ASkAY_?3BcU3|5*X5*b)0HvyeP*W#C)I-=
>> z2t>0yp|fBG85#HtE#QTmE@pe%B_9UpmLzswnnT9tx0nwwjPvqeVm{ryPAKT6^Fe1`
>> zRaNW#eRf_qBAIG66H4}e{-}_~er-*09h!ierL|+n>V-={3mIO+h`_hcMPW@!Zj1^`
>> z;|61ftjw>)%lOl+6ky||l5PHN*CfL2FnA$09L^;91)uPMS=X#bgH^St&&d;>WhEYO
>> zyXW*tk)NfQqdiUxy?1zV3Y2IaG+}7>8I_ia(<|3c$V!r7Ke^%9R5tN6A2BCMHrFUG
>> zw@tbs+tXYjPqJ3rIG&T^;xq@0t=7Lf*IBuR+t3q#<Alm#uR+(>*!fj%Vy(k&n7Lrp
>> zKG8rAy|UYZVS2|E>(J9~sE={1Ls)O<hP`W(AX;zQXFp4IT2K8lsD};&>cs6VI5Ki%
>> zu=&>SNi#n$Zs3}Bnf%oJly1E_-Qg{ts&xSknv)Ceu#lKtcAaJRMo;16V;??*Y56zC
>> zRufN`5<4wZpm=nt8EDyNxJPm5aL<HWHTRU{;PYXor+F?L0rg!131L>j_devfbyrtj
>> z=8;mHp*HLboI8^q?iYbL(5ui+|6m~tcZihy+<~vNPWUCA+r4h{<+HvVyv{a!IWm_=
>> zutREfGyGEnck1&{m6oU|dl8RL)tc2~t=0GTEUP?}Mat=S;}+ysIdXaYn(WdnARhiw
>> z>t&0thm|MoUdF00Cc(SnRowcnE2I=Rn^=5a&C5>>j}SG4)G%Iqqq01>wK(d^CVOjJ
>> zFfQQP;PW&gqAstiuu}-8#it2KEESu2kD_RfL}|AQ#ZYv0V;fz>H?`2i7me|pn?;tJ
>> z#ro+iQKlx*?I(v7{^Bs05t)4ds#gEpGsCcv%h6|4=Fuy1x|+TqF5{5$j?67uJ;+cR
>> zqqx&H5#Mepe;t1V_Zcxbtq071U|rhWkJB-%wJ>FG8A5ls?{H5xir8s^Y!0_H(%ItZ
>> zfVwdu|JBXQ;?k}q&I-m%&W|#&l)jx%t8{1osL(r10H2;uewqjtA>lT3Aj93<&Fw^T
>> zcJQ}rxBS@aUri%z8Y@knI!tMKvAw9l#72MwltqDHb2>_lO0tRaAjz;N7lAoD_cweh
>> zNbR8ddsG<XcpI5x^)Q9dfwNRTT{1%t^$Y(*VV3*CntMa$P6h=6gh_G7Fg>?6U)BJv
>> z*|(*Ebk#5}*jELbciu_&0-wGfHYUT<=BwD3PI}CwJ)Y7+m_`Y7cb>pQi>i-&%%}vM
>> z7qaa>XSsP6DlPD(7uVc`x0D@wjX-TPE-Ik*HKO&yXPW9+o6KG_`e!g1r(F@!Onb!K
>> zopWdWhpL0kZ7sD?Z}>>!8iwR<)pC)m$<cfxX~|Pfed~6meavl0qQ6J&=FiNNTzXYW
>> zfyn6VTSkMJZq~|z;cA#Y8aTVq&dT6S=5ZQ&Pfjz97G?pvciZ?E5*}X};$)Rs<}<vm
>> znzL9spONHxg&B!@e<+x6oIFGa!*Co+7#R?l3oEx3+#bDdE;+)`QsSY?iS5(!>V|8n
>> zpGt&@Uvn&8X3t1sCE%XITe(x`V6}oIlT%rqvUe%hvGaJz?+udmG=tg}N*d2y(6%P(
>> zg!QYaNQn?{b_zAjB#Z{+u#h5SX2m)DicK*XF3vdABAe)7m4pT3D$W>1$Ad8~<PW+X
>> zywp8xsGrzZ%05Krwur}O0g{CZo{&d0<tjAKB#vX2IGV-@kv#TP$BGm5UX3vw?%cOP
>> zYom>ldE?0WY~+|5BMB*c)Z*b6cabH${WR-~=?MCq=sn_yX`A>fskIy3STB2BO&t9!
>> znLdr>#3hxP!d1rw?dZ_u2O{4;ca<5i_u)McAu5TNsVehTSJIeCi9ZF@<5)A2M?F&G
>> zyPH$A_9g)F(M(h%TG_VdBw4cxDHIv5C`!K6!pST2h=9mgPu(aQ!z6kBsb}Wr4|09}
>> z$fiO#BL4dG1x=lCy8PrWUp1ze`;!uDL*?ii4)qdAGnei`xsEHQv&w|t`-P9|Mbb|V
>> z)J5+w96!*`(i_j+&w3hxOD4V9Mq^v9M9scQAfsX|QCP}gWYTD+=re97f6$}i0f(gB
>> zVtM0uJ6L1(oeLXpce+l?RYI|asnMcS6KL)mvG^X&E4PxN#Df>+s(q32#MDuTxR#AJ
>> z=S9%l%(X*cM|{c7T7$Y%EEQ(Ii396!N#IuRe2E`62y~$vXVlxU-An{F(7f#O`rH*c
>> zWhZAC@G*?%ig>WTQEIkA(C-~Qa2v=_zre<+qd2f6JEwnGc|C77@58*ef5|N?tj>lW
>> z{uBCUk8=9t>R~~l^z2!a+~=MB=XQfE3fb+IB#KNq?E3@Atp^<)qN*!k^5@YyEQJ#u
>> zwUK}EJjfrzsDF#D7C+LAM6pE_TST$NpNuV{+Ulsb`mYBA{}>=ZH9b*H&%dJSNsPw*
>> zuT4n?;C%HR+WbErVEnDJ>7P#U{&1kmE`{2o3Nch6hAPBRh1h=?H~(oSB)(_L9938R
>> zuDap@niOEmLc|w=w0c>^k84}^@Xjkm(Y^KPaVE#-m1U9X)H*I^9xn`J0MF`Z40Xgl
>> zw-!k0S_di}k9+8bW@A8xTj$oJE7N(9yjz+7WU=IjVd-bvCipcN6Qy$*p@8#;fwT0N
>> z!HTWikEBa{6mY3)!9oMr<LaK={O`NewG&y>v4Ez7F5=`Jf}$C43N-jS42N^O3~6}1
>> z!VRA?sklw-|KpgL{*!Gll%xHyqoIThl#qcEGJdLMJXA=cLJ}2{sF3{6rDhb%{9l&w
>> z|Mr{-MazE|Ek~gw3MEk}`IDjKFAXJ+{@WD04FQ!#LEm{z<!_(Be=T}_#|;_xR@%yc
>> zlPmtk7TkB=tohqoIRdaIXbCJbn+%eEU-ust^SFBnD8Vv}{Egc@f7|!`6K=um!J4a3
>> zk{C)7Lp3c?O-oeM^80SP{q6AsMJG{o@~@d5L*3Z>&hvh#kVJ(fDkOixvzI6h5v3vi
>> znHplSXfpr=*mesVH2U-k5T7H4`*6nj769$I9xXUHSmu|z+cKE}>*Q48{J>v+127B>
>> F{{akY3)=ty
>>
>> literal 0
>> HcmV?d00001
>>
>> diff --git a/tests/misc-tests/030-undelete-subvol/test.sh b/tests/misc-tests/030-undelete-subvol/test.sh
>> new file mode 100755
>> index 000000000000..d3dfa5008601
>> --- /dev/null
>> +++ b/tests/misc-tests/030-undelete-subvol/test.sh
>> @@ -0,0 +1,34 @@
>> +#!/bin/bash
>> +# Test undelete-subvol can recover the intact subvolume, and can skip the
>> +# incomplete subvolume.
>> +
>> +source "$TOP/tests/common"
>> +
>> +check_prereq btrfs
>> +check_prereq btrfs-image
>> +
>> +setup_root_helper
>> +
>> +check_image()
>> +{
>> +	TEST_DEV="$1"
>> +
>> +	run_check_stdout "$TOP/btrfs" rescue undelete-subvol "$TEST_DEV" \
>> +		| grep -q "Recovered 1 subvols" \
>> +		|| _fail "failed to undelete subvol $image" >&2
>> +
>> +	run_check "$TOP/btrfs" check "$TEST_DEV"
>> +
>> +	# check whether the recovered image can be mounted normally
>> +	run_check_mount_test_dev
>> +
>> +	run_check_stdout $SUDO_HELPER "$TOP/btrfs" subvolume list "$TEST_MNT" \
>> +		| grep -q "lost+found" \
>> +		|| _fail "failed to find the recovered subvol $image" >&2
>> +
>> +	run_check_umount_test_dev "$TEST_MNT"
>> +
>> +	rm "$TEST_DEV"
>> +}
>> +
>> +check_all_images
>>
> 

[PATCH v2 10/10] btrfs-progs: undelete-subvol: update completion and documentation

[Lu Fengqi]

Add undelete-subvol to btrfs-completion, and update btrfs-rescue
documentation to introduce undelete-subvol.

Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
---
v2: just add -s option documentation.

 Documentation/btrfs-rescue.asciidoc | 12 ++++++++++++
 btrfs-completion                    |  2 +-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/Documentation/btrfs-rescue.asciidoc b/Documentation/btrfs-rescue.asciidoc
index f94a0ff2b45e..573a865b1158 100644
--- a/Documentation/btrfs-rescue.asciidoc
+++ b/Documentation/btrfs-rescue.asciidoc
@@ -60,6 +60,18 @@ assume an answer of 'yes' to all questions.
 -v::::
 verbose mode.
 
+*undelete-subvol* <device>::
+Undelete deleted subvolumes that still left intact on the device.
++
+This command will create the lost+found directory, and recover all the
+subvolumes to this directory with the name "sub<id>". If -s <subvolid> option
+is given, then just recover the subvolume which specified by <subvolid>.
++
+`Options`
++
+-s::::
+specify the subvolume which will be recovered.
+
 *zero-log* <device>::
 clear the filesystem log tree
 +
diff --git a/btrfs-completion b/btrfs-completion
index ae683f4ecf61..859595155a4b 100644
--- a/btrfs-completion
+++ b/btrfs-completion
@@ -35,7 +35,7 @@ _btrfs()
 	commands_balance='start pause cancel resume status'
 	commands_device='scan add delete remove ready stats usage'
 	commands_scrub='start cancel resume status'
-	commands_rescue='chunk-recover super-recover zero-log'
+	commands_rescue='chunk-recover super-recover undelete-subvol zero-log'
 	commands_inspect_internal='inode-resolve logical-resolve subvolid-resolve rootid min-dev-size dump-tree dump-super tree-stats'
 	commands_property='get set list'
 	commands_quota='enable disable rescan'
-- 
2.16.2

Re: [PATCH v2 00/10] undelete subvolume offline version

[Lu Fengqi]

On Tue, Mar 27, 2018 at 03:06:48PM +0800, Lu Fengqi wrote:
>This patchset will add undelete-subvol subcommand for btrfs rescue. This
>enhancement allows undeleting a subvolume on an unmounted filesystem.

Gentle ping.

Any advice about this patchset will be welcomed.

-- 
Thanks,
Lu

>
>Patchset can be fetched from github:
>https://github.com/littleroad/btrfs-progs.git undelete
>
>v1->v2: add -s option to allow user specify the subvolume which will be
>recovered.
>
>The first six patches are not modified.
>7th-8th add the -s option to specify subvol_id instead of recovering all
>subvolumes.
>9th add shell quoting to test script.
>10th just add the -s option documentation.
>
>Lu Fengqi (10):
>  btrfs-progs: copy btrfs_del_orphan_item from kernel
>  btrfs-progs: extract btrfs_link_subvol from btrfs_mksubvol
>  btrfs-progs: use btrfs_find_free_dir_index to find free inode index
>  btrfs-progs: undelete-subvol: introduce is_subvol_intact
>  btrfs-progs: undelete-subvol: introduce recover_dead_root
>  btrfs-progs: undelete-subvol: introduce link_subvol_to_lostfound
>  btrfs-progs: undelete-subvol: introduce btrfs_undelete_intact_subvols
>  btrfs-progs: undelete-subvol: add undelete-subvol subcommand
>  btrfs-progs: tests: add testcase for undelete-subvol
>  btrfs-progs: undelete-subvol: update completion and documentation
>
> Documentation/btrfs-rescue.asciidoc                |  12 +
> Makefile                                           |   3 +-
> btrfs-completion                                   |   2 +-
> cmds-rescue.c                                      |  70 ++++++
> convert/main.c                                     |  57 +++++
> ctree.h                                            |   8 +-
> inode.c                                            |  99 ++++----
> .../030-undelete-subvol/deleted_subvolume.img      | Bin 0 -> 4096 bytes
> .../030-undelete-subvol/drop_progress.raw.xz       | Bin 0 -> 23452 bytes
> tests/misc-tests/030-undelete-subvol/test.sh       |  34 +++
> undelete-subvol.c                                  | 254 +++++++++++++++++++++
> undelete-subvol.h                                  |  19 ++
> 12 files changed, 511 insertions(+), 47 deletions(-)
> create mode 100644 tests/misc-tests/030-undelete-subvol/deleted_subvolume.img
> create mode 100644 tests/misc-tests/030-undelete-subvol/drop_progress.raw.xz
> create mode 100755 tests/misc-tests/030-undelete-subvol/test.sh
> create mode 100644 undelete-subvol.c
> create mode 100644 undelete-subvol.h
>
>-- 
>2.16.2
>
>
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html