[PATCH] Btrfs-progs: fix btrfs-convert rollback to check ROOT_BACKREF

[PATCH] Btrfs-progs: fix btrfs-convert rollback to check ROOT_BACKREF

[Liu Bo]

Btrfs has changed to delete subvolume/snapshot asynchronously, which means that
after umount itself, if we've already deleted 'ext2_saved', rollback can still
be completed.

So this adds a check for ROOT_BACKREF before checking ROOT_ITEM since
ROOT_BACKREF is immediately not in the btree after ioctl(BTRFS_IOC_SNAP_DESTROY)
returns.

Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
---
 btrfs-convert.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/btrfs-convert.c b/btrfs-convert.c
index 802930c..f8a6c16 100644
--- a/btrfs-convert.c
+++ b/btrfs-convert.c
@@ -2591,6 +2591,22 @@ static int do_rollback(const char *devname)
 	btrfs_init_path(&path);
 
 	key.objectid = CONV_IMAGE_SUBVOL_OBJECTID;
+	key.type = BTRFS_ROOT_BACKREF_KEY;
+	key.offset = BTRFS_FS_TREE_OBJECTID;
+	ret = btrfs_search_slot(NULL, root->fs_info->tree_root, &key, &path, 0,
+				0);
+	btrfs_release_path(&path);
+	if (ret > 0) {
+		fprintf(stderr, "unable to open subvol %llu\n",
+			(unsigned long long)key.objectid);
+		goto fail;
+	} else if (ret < 0) {
+		fprintf(stderr, "unable to open subvol %llu ret %d\n",
+			(unsigned long long)key.objectid, ret);
+		goto fail;
+	}
+
+	key.objectid = CONV_IMAGE_SUBVOL_OBJECTID;
 	key.type = BTRFS_ROOT_ITEM_KEY;
 	key.offset = (u64)-1;
 	image_root = btrfs_read_fs_root(root->fs_info, &key);
-- 
1.8.2.1

Re: [PATCH] Btrfs-progs: fix btrfs-convert rollback to check ROOT_BACKREF

[Qu Wenruo]

在 2015年10月18日 13:44, Liu Bo 写道:
> Btrfs has changed to delete subvolume/snapshot asynchronously, which means that
> after umount itself, if we've already deleted 'ext2_saved', rollback can still
> be completed.
>
> So this adds a check for ROOT_BACKREF before checking ROOT_ITEM since
> ROOT_BACKREF is immediately not in the btree after ioctl(BTRFS_IOC_SNAP_DESTROY)
> returns.
>
> Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
Reviewed-by: Qu Wenruo <quwenruo@cn.fujitsu.com>

Looks good to me.

Although the error message for ret > 0 case can be improved a little, like:
"unable to find convert image subvolume, maybe it's already deleted?\n".


BTW, would you please submit a test case for fstests? It won't be a hard 
one though.

Thanks,
Qu

> ---
>   btrfs-convert.c | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
>
> diff --git a/btrfs-convert.c b/btrfs-convert.c
> index 802930c..f8a6c16 100644
> --- a/btrfs-convert.c
> +++ b/btrfs-convert.c
> @@ -2591,6 +2591,22 @@ static int do_rollback(const char *devname)
>   	btrfs_init_path(&path);
>
>   	key.objectid = CONV_IMAGE_SUBVOL_OBJECTID;
> +	key.type = BTRFS_ROOT_BACKREF_KEY;
> +	key.offset = BTRFS_FS_TREE_OBJECTID;
> +	ret = btrfs_search_slot(NULL, root->fs_info->tree_root, &key, &path, 0,
> +				0);
> +	btrfs_release_path(&path);
> +	if (ret > 0) {
> +		fprintf(stderr, "unable to open subvol %llu\n",
> +			(unsigned long long)key.objectid);
> +		goto fail;
> +	} else if (ret < 0) {
> +		fprintf(stderr, "unable to open subvol %llu ret %d\n",
> +			(unsigned long long)key.objectid, ret);
> +		goto fail;
> +	}
> +
> +	key.objectid = CONV_IMAGE_SUBVOL_OBJECTID;
>   	key.type = BTRFS_ROOT_ITEM_KEY;
>   	key.offset = (u64)-1;
>   	image_root = btrfs_read_fs_root(root->fs_info, &key);
>

Re: [PATCH] Btrfs-progs: fix btrfs-convert rollback to check ROOT_BACKREF

[David Sterba]

On Sun, Oct 18, 2015 at 07:41:27PM +0800, Qu Wenruo wrote:
> 在 2015年10月18日 13:44, Liu Bo 写道:
> > Btrfs has changed to delete subvolume/snapshot asynchronously, which means that
> > after umount itself, if we've already deleted 'ext2_saved', rollback can still
> > be completed.
> >
> > So this adds a check for ROOT_BACKREF before checking ROOT_ITEM since
> > ROOT_BACKREF is immediately not in the btree after ioctl(BTRFS_IOC_SNAP_DESTROY)
> > returns.
> >
> > Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
> Reviewed-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
> 
> Looks good to me.
> 
> Although the error message for ret > 0 case can be improved a little, like:
> "unable to find convert image subvolume, maybe it's already deleted?\n".

I've adjusted the error messages.

> BTW, would you please submit a test case for fstests? It won't be a hard 
> one though.

Test added.