From: Roman Anasal <roman.anasal@bdsu.de>
To: linux-btrfs@vger.kernel.org
Cc: Roman Anasal <roman.anasal@bdsu.de>
Subject: [PATCH 0/2] btrfs: send: correctly recreate changed inodes
Date: Mon, 11 Jan 2021 20:02:41 +0100 [thread overview]
Message-ID: <20210111190243.4152-1-roman.anasal@bdsu.de> (raw)
Trying to incremental send and receive some subvolumes I repeatedly ran into
errors with btrfs-receive failing. Taking a deeper look into the issue showed
that btrfs-send was producing (semantically) invalid command streams. I could
pin down the conditions of this happening to having inodes with the same number
and the same generation but different inode type in the parent and the sent
subvolume.
The cause of this was that the kernel code for btrfs-send did not check if the
type of the inode changed but only recreated the inode when the generation
changed, too. Having the two inodes originate in the same generation though
would produce a command stream that causes the receiver to fail.
This small patch set adds the check for the same type and causes a deleted and
create of the inode if necessary.
For this the first patch renames send_ctx.cur_inode_new_gen to
cur_inode_recreated because that is what this currently _actually_ is used for:
detecting whether an inode was/should be recreated.
I deemed this refactoring/renaming to be appropriate because the second patch
then adds another condition that will set cur_inode_recreated - i.e. if the
inode types differ.
Looking through the code and developing this patch I also identified an
assumption that seems to be hard coded into many places and that probably caused
other bugs, too, e.g. [1]:
That people would only ever incremental send read-only snapshots of the provided
parent snapshot that they're based off.
But this assumption fails/causes bugs when send/receive is used with:
1. snapshots that were modified after their creation
2. subvolumes that were created independently and are not descendants of each
other
(Although 2. may look like total nonsense it actually makes sense for
independent subvolumes that share some extents e.g. by cp --reflink or some
other means of deduplication.)
Thus I suspect there to be further hidden bugs for very rare edge cases
especially where the generation number is used to check for differences -
because taking 1./2. into account the generations between the subvolume and
parent may arbitrarily be smaller, equal or greater and as such does not qualify
as a good indicator for change detection.
The same is true for the inode numbers.
This makes it also connected to [1]:
"btrfs: send: fix wrong file path when there is an inode with a pending rmdir"
[1] https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git/commit/?id=0b3f407e6728d990ae1630a02c7b952c21c288d3
Roman Anasal (2):
btrfs: send: rename send_ctx.cur_inode_new_gen to cur_inode_recreated
btrfs: send: fix invalid commands for inodes with changed type but
same gen
fs/btrfs/send.c | 47 +++++++++++++++++++++++++++--------------------
1 file changed, 27 insertions(+), 20 deletions(-)
--
2.26.2
next reply other threads:[~2021-01-11 19:04 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-11 19:02 Roman Anasal [this message]
2021-01-11 19:02 ` [PATCH 1/2] btrfs: send: rename send_ctx.cur_inode_new_gen to cur_inode_recreated Roman Anasal
2021-01-11 19:02 ` [PATCH 2/2] btrfs: send: fix invalid commands for inodes with changed type but same gen Roman Anasal
2021-01-11 19:30 ` Andrei Borzenkov
2021-01-11 20:53 ` Roman Anasal | BDSU
2021-01-12 11:27 ` Filipe Manana
2021-01-12 12:07 ` Graham Cobb
2021-01-12 12:30 ` Filipe Manana
2021-01-12 13:10 ` Roman Anasal | BDSU
2021-01-12 13:54 ` Filipe Manana
2021-01-12 14:37 ` Roman Anasal | BDSU
2021-01-12 15:08 ` Filipe Manana
2021-01-11 21:15 ` David Sterba
2021-01-11 21:31 ` Roman Anasal | BDSU
2021-01-11 21:19 ` kernel test robot
2021-01-11 21:19 ` kernel test robot
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=20210111190243.4152-1-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.