Btrfs: update commit root on snapshot creation after orphan cleanup

On snapshot creation (either writable or read-only), we do orphan cleanup
against the root of the snapshot. If the cleanup did remove any orphans,
then the current root node will be different from the commit root node
until the next transaction commit happens.

A send operation always uses the commit root of a snapshot - this means
it will see the orphans if it starts computing the send stream before the
next transaction commit happens (triggered by a timer or sync() for .e.g),
which is when the commit root gets assigned a reference to current root,
where the orphans are not visible anymore. The consequence of send seeing
the orphans is explained below.

For example:

    mkfs.btrfs -f /dev/sdd
    mount -o commit=999 /dev/sdd /mnt

    # open a file with O_TMPFILE and leave it open
    # write some data to the file
    btrfs subvolume snapshot -r /mnt /mnt/snap1

    btrfs send /mnt/snap1 -f /tmp/send.data

The send operation will fail with the following error:

    ERROR: send ioctl failed with -116: Stale file handle

What happens here is that our snapshot has an orphan inode still visible
through the commit root, that corresponds to the tmpfile. However send
will attempt to call inode.c:btrfs_iget(), with the goal of reading the
file's data, which will return -ESTALE because it will use the current
root (and not the commit root) of the snapshot.

Of course, there are other cases where we can get orphans, but this
example using a tmpfile makes it much easier to reproduce the issue.

Therefore on snapshot creation, after calling btrfs_orphan_cleanup, if
the commit root is different from the current root, just commit the
transaction associated with the snapshot's root (if it exists), so that
a send will not see any orphans that don't exist anymore. This also
guarantees a send will always see the same content regardless of whether
a transaction commit happened already before the send was requested and
after the orphan cleanup (meaning the commit root and current roots are
the same) or it hasn't happened yet (commit and current roots are
different).

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

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 04ece8f..219e26f 100644 (file)
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -712,6 +712,35 @@ static int create_snapshot(struct btrfs_root *root, struct inode *dir,
        if (ret)
                goto fail;
 
+       /*
+        * If orphan cleanup did remove any orphans, it means the tree was
+        * modified and therefore the commit root is not the same as the
+        * current root anymore. This is a problem, because send uses the
+        * commit root and therefore can see inode items that don't exist
+        * in the current root anymore, and for example make calls to
+        * btrfs_iget, which will do tree lookups based on the current root
+        * and not on the commit root. Those lookups will fail, returning a
+        * -ESTALE error, and making send fail with that error. So make sure
+        * a send does not see any orphans we have just removed, and that it
+        * will see the same inodes regardless of whether a transaction
+        * commit happened before it started (meaning that the commit root
+        * will be the same as the current root) or not.
+        */
+       if (readonly && pending_snapshot->snap->node !=
+           pending_snapshot->snap->commit_root) {
+               trans = btrfs_join_transaction(pending_snapshot->snap);
+               if (IS_ERR(trans) && PTR_ERR(trans) != -ENOENT) {
+                       ret = PTR_ERR(trans);
+                       goto fail;
+               }
+               if (!IS_ERR(trans)) {
+                       ret = btrfs_commit_transaction(trans,
+                                                      pending_snapshot->snap);
+                       if (ret)
+                               goto fail;
+               }
+       }
+
        inode = btrfs_lookup_dentry(dentry->d_parent->d_inode, dentry);
        if (IS_ERR(inode)) {
                ret = PTR_ERR(inode);