archive mirror
 help / color / mirror / Atom feed
From: Roman Anasal <>
Cc: Roman Anasal <>
Subject: [PATCH 0/2] btrfs: send: correctly recreate changed inodes
Date: Mon, 11 Jan 2021 20:02:41 +0100	[thread overview]
Message-ID: <> (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

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

(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"


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


             reply	other threads:[~2021-01-11 19:04 UTC|newest]

Thread overview: 15+ 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

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:

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

  git send-email \ \ \ \

* 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).