Message ID | 1400986544-22440-1-git-send-email-fdmanana@gmail.com (mailing list archive) |
---|---|
State | Accepted |
Headers | show |
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. -- 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
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); }
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(-)