linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Btrfs: set dead flag on the right root when destroying snapshot
@ 2014-05-25  2:44 Filipe David Borba Manana
  2014-05-25  2:55 ` [PATCH v2] " Filipe David Borba Manana
  0 siblings, 1 reply; 3+ messages in thread
From: Filipe David Borba Manana @ 2014-05-25  2:44 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Filipe David Borba Manana

We were setting the BTRFS_ROOT_SUBVOL_DEAD flag on the root of the
parent of our target snapshot, instead of setting it in the target
snapshot's root.

This is easy to observe by running the following scenario:

    mkfs.btrfs -f /dev/sdd
    mount /dev/sdd /mnt

    btrfs subvolume create /mnt/first_subvol
    btrfs subvolume snapshot -r /mnt /mnt/mysnap1

    btrfs subvolume delete /mnt/first_subvol
    btrfs subvolume snapshot -r /mnt /mnt/mysnap2

    btrfs send -p /mnt/mysnap1 /mnt/mysnap2 -f /tmp/send.data

The send command failed because the send ioctl returned -EPERM.
A test case for xfstests follows.

Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---
 fs/btrfs/ioctl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 362720a..482cad5 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -2314,7 +2314,7 @@ static noinline int btrfs_ioctl_snap_destroy(struct file *file,
 	spin_lock(&dest->root_item_lock);
 	root_flags = btrfs_root_flags(&root->root_item);
 	if (root->send_in_progress == 0) {
-		btrfs_set_root_flags(&root->root_item,
+		btrfs_set_root_flags(&dest->root_item,
 				root_flags | BTRFS_ROOT_SUBVOL_DEAD);
 		spin_unlock(&dest->root_item_lock);
 	} else {
@@ -2417,7 +2417,7 @@ out_unlock:
 	if (err) {
 		spin_lock(&dest->root_item_lock);
 		root_flags = btrfs_root_flags(&root->root_item);
-		btrfs_set_root_flags(&root->root_item,
+		btrfs_set_root_flags(&dest->root_item,
 				root_flags & ~BTRFS_ROOT_SUBVOL_DEAD);
 		spin_unlock(&dest->root_item_lock);
 	}
-- 
1.9.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH v2] Btrfs: set dead flag on the right root when destroying snapshot
  2014-05-25  2:44 [PATCH] Btrfs: set dead flag on the right root when destroying snapshot Filipe David Borba Manana
@ 2014-05-25  2:55 ` Filipe David Borba Manana
  2014-05-27 15:57   ` David Sterba
  0 siblings, 1 reply; 3+ messages in thread
From: Filipe David Borba Manana @ 2014-05-25  2:55 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Filipe David Borba Manana

We were setting the BTRFS_ROOT_SUBVOL_DEAD flag on the root of the
parent of our target snapshot, instead of setting it in the target
snapshot's root.

This is easy to observe by running the following scenario:

    mkfs.btrfs -f /dev/sdd
    mount /dev/sdd /mnt

    btrfs subvolume create /mnt/first_subvol
    btrfs subvolume snapshot -r /mnt /mnt/mysnap1

    btrfs subvolume delete /mnt/first_subvol
    btrfs subvolume snapshot -r /mnt /mnt/mysnap2

    btrfs send -p /mnt/mysnap1 /mnt/mysnap2 -f /tmp/send.data

The send command failed because the send ioctl returned -EPERM.
A test case for xfstests follows.

Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---

V2: Add missing replacements of 'root' with 'dest'.

 fs/btrfs/ioctl.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 362720a..38f2169 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -2312,16 +2312,16 @@ static noinline int btrfs_ioctl_snap_destroy(struct file *file,
 	 * again is not run concurrently.
 	 */
 	spin_lock(&dest->root_item_lock);
-	root_flags = btrfs_root_flags(&root->root_item);
-	if (root->send_in_progress == 0) {
-		btrfs_set_root_flags(&root->root_item,
+	root_flags = btrfs_root_flags(&dest->root_item);
+	if (dest->send_in_progress == 0) {
+		btrfs_set_root_flags(&dest->root_item,
 				root_flags | BTRFS_ROOT_SUBVOL_DEAD);
 		spin_unlock(&dest->root_item_lock);
 	} else {
 		spin_unlock(&dest->root_item_lock);
 		btrfs_warn(root->fs_info,
 			"Attempt to delete subvolume %llu during send",
-			root->root_key.objectid);
+			dest->root_key.objectid);
 		err = -EPERM;
 		goto out_dput;
 	}
@@ -2416,8 +2416,8 @@ out_up_write:
 out_unlock:
 	if (err) {
 		spin_lock(&dest->root_item_lock);
-		root_flags = btrfs_root_flags(&root->root_item);
-		btrfs_set_root_flags(&root->root_item,
+		root_flags = btrfs_root_flags(&dest->root_item);
+		btrfs_set_root_flags(&dest->root_item,
 				root_flags & ~BTRFS_ROOT_SUBVOL_DEAD);
 		spin_unlock(&dest->root_item_lock);
 	}
-- 
1.9.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] Btrfs: set dead flag on the right root when destroying snapshot
  2014-05-25  2:55 ` [PATCH v2] " Filipe David Borba Manana
@ 2014-05-27 15:57   ` David Sterba
  0 siblings, 0 replies; 3+ messages in thread
From: David Sterba @ 2014-05-27 15:57 UTC (permalink / raw)
  To: Filipe David Borba Manana; +Cc: linux-btrfs

On Sun, May 25, 2014 at 03:55:44AM +0100, Filipe David Borba Manana wrote:
> We were setting the BTRFS_ROOT_SUBVOL_DEAD flag on the root of the
> parent of our target snapshot, instead of setting it in the target
> snapshot's root.
> 
> This is easy to observe by running the following scenario:
> 
>     mkfs.btrfs -f /dev/sdd
>     mount /dev/sdd /mnt
> 
>     btrfs subvolume create /mnt/first_subvol
>     btrfs subvolume snapshot -r /mnt /mnt/mysnap1
> 
>     btrfs subvolume delete /mnt/first_subvol
>     btrfs subvolume snapshot -r /mnt /mnt/mysnap2
> 
>     btrfs send -p /mnt/mysnap1 /mnt/mysnap2 -f /tmp/send.data
> 
> The send command failed because the send ioctl returned -EPERM.
> A test case for xfstests follows.
> 
> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>

Reviewed-by: David Sterba <dsterba@suse.cz>

Thanks for catching it, I was so focused on verifying the locks to do
what they're supposed to do and missed the typo, not that the variable
names help to avoid confusion.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2014-05-27 15:57 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-25  2:44 [PATCH] Btrfs: set dead flag on the right root when destroying snapshot Filipe David Borba Manana
2014-05-25  2:55 ` [PATCH v2] " Filipe David Borba Manana
2014-05-27 15:57   ` David Sterba

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).