All of lore.kernel.org
 help / color / mirror / Atom feed
From: Roman Anasal <roman.anasal@bdsu.de>
To: linux-btrfs@vger.kernel.org
Cc: Roman Anasal <roman.anasal@bdsu.de>
Subject: [PATCH v2 4/4] btrfs: send: fix invalid commands for inodes in disconnected roots
Date: Mon, 25 Jan 2021 20:42:10 +0100	[thread overview]
Message-ID: <20210125194210.24071-5-roman.anasal@bdsu.de> (raw)
In-Reply-To: <20210125194210.24071-1-roman.anasal@bdsu.de>

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 --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)
-- 
2.26.2


      parent reply	other threads:[~2021-01-26  0:08 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-25 19:42 [PATCH v2 0/4] btrfs: send: correctly recreate changed inodes Roman Anasal
2021-01-25 19:42 ` [PATCH v2 1/4] btrfs: send: rename send_ctx.cur_inode_new_gen to cur_inode_recreated Roman Anasal
2021-01-25 19:42 ` [PATCH v2 2/4] btrfs: send: fix invalid commands for inodes with changed type but same gen Roman Anasal
2021-01-25 19:42 ` [PATCH v2 3/4] btrfs: send: fix invalid commands for inodes with changed rdev " Roman Anasal
2021-01-25 20:51   ` Filipe Manana
2021-01-26 19:19     ` Roman Anasal | BDSU
2021-01-27 10:53       ` Filipe Manana
2021-01-31 15:52     ` Roman Anasal | BDSU
2021-02-02 11:56       ` Filipe Manana
2021-02-03 16:20         ` Roman Anasal | BDSU
2021-01-25 19:42 ` Roman Anasal [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210125194210.24071-5-roman.anasal@bdsu.de \
    --to=roman.anasal@bdsu.de \
    --cc=linux-btrfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.