diff mbox series

[v2,4/4] btrfs: send: fix invalid commands for inodes in disconnected roots

Message ID 20210125194210.24071-5-roman.anasal@bdsu.de (mailing list archive)
State New, archived
Headers show
Series btrfs: send: correctly recreate changed inodes | expand

Commit Message

Roman Anasal | BDSU Jan. 25, 2021, 7:42 p.m. UTC
When doing an inceremental send using an independently created subvolume
as the send parent compare_refs will falsely report changed refs for
inodes with the same inode number, same generation and same ref name.

This is due to dir_changed returning true if the generations of the
parent directory differ in the subvolume and the send parent. Normally
this situation would cause the parent directory to be recreated and the
contained inodes would need to be moved (e.g. by link/unlink).

In the case of the root directory (ino BTRFS_FIRST_FREE_OBJECTID) though
changed_inode will not try to recreated it since this would produce a
stream trying to remove and recreate the subvolume itself.
For independently created subvolumes the generation will always be
different since create_subvol() commits the transaction after createing
a new subvolume.
By handling this case in dir_changed as well the produced commands are
correct again.

Summarizing the conditions for this:
- inode has same number, type/rdev, ref and generation in subvolume and
  send parent
- ref is a child of the subvolume root directory
- root directory has the same inode number - which is always
  BTRFS_FIRST_FREE_OBJECTID
- root directory has different generation in subvolume and send parent
  which is always true for independent subvolumes, i.e. if they're not
  snapshots (of snapshots) of one another

Example reproducer:
  btrfs subvolume create subvol1
  btrfs subvolume create subvol2
  touch subvol1/a subvol2/a
  btrfs property set subvol1 ro true
  btrfs property set subvol2 ro true
  btrfs send -p subvol1 subvol2 | btrfs receive --dump

The produced tree state here is (starting at gen 6):
  |-- subvol1       (ino 256, gen 6)
  |   `-- a         (ino 257, gen 8)
  |
  `-- subvol2       (ino 256, gen 7)
      `-- a         (ino 257, gen 8)

subvol1/a and subvol2/a are files with the same inode number, generation
and path name. The subvolume root directories have the same inode number
but different generations.

Example output of the receive command:
  At subvol subvol2
  snapshot        ./subvol2                       uuid=e783948d-4fd5-0d47-9777-43036e468170 transid=8 parent_uuid=7b0cefdb-738d-e342-a903-501df1877b01 parent_transid=8
  utimes          ./subvol2/                      atime=2021-01-25T18:07:42+0000 mtime=2021-01-25T18:07:42+0000 ctime=2021-01-25T18:07:42+0000
  link            ./subvol2/a                     dest=a
  unlink          ./subvol2/a
  utimes          ./subvol2/                      atime=2021-01-25T18:07:42+0000 mtime=2021-01-25T18:07:42+0000 ctime=2021-01-25T18:07:42+0000
  utimes          ./subvol2/a                     atime=2021-01-25T18:07:42+0000 mtime=2021-01-25T18:07:42+0000 ctime=2021-01-25T18:07:42+0000

=> the `link` command causes the receiver to fail with:
   ERROR: link a -> a failed: File exists

Signed-off-by: Roman Anasal <roman.anasal@bdsu.de>
---
v2:
  - add this patch based on feedback in
    https://lore.kernel.org/linux-btrfs/9e177865-0408-c321-951e-ce0f3ff33389@gmail.com/
---
 fs/btrfs/send.c | 9 +++++++++
 1 file changed, 9 insertions(+)
diff mbox series

Patch

diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index ef544525f..3114770be 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -6543,6 +6543,15 @@  static int dir_changed(struct send_ctx *sctx, u64 dir)
 	u64 orig_gen, new_gen;
 	int ret;
 
+	/*
+	 * Treat the root dir case special here: changed_inode will never
+	 * produce a stream that tries to delete/rmdir/rename the root dir.
+	 * So we must assume the root always as unchanged here to not produce
+	 * incorrect link/rename commands for contained refs.
+	 */
+	if (dir == BTRFS_FIRST_FREE_OBJECTID)
+		return 0;
+
 	ret = get_inode_info(sctx->send_root, dir, NULL, &new_gen, NULL, NULL,
 			     NULL, NULL);
 	if (ret)