On Tue, Jan 12, 2021 at 9:42 AM Roman Anasal | BDSU
[off-list ref] wrote:

On Mon, 2021-01-11 at 22:30 +0300, Andrei Borzenkov wrote:

quoted

11.01.2021 22:02, Roman Anasal пишет:

quoted

When doing a send, if a new inode has the same number as an inode
in the
parent subvolume it will only be detected as to be recreated when
the
genrations differ. But with inodes in the same generation the
emitted
commands will cause the receiver to fail.

This case does not happen when doing incremental sends with
snapshots
that are kept read-only by the user all the time, but it may happen
if
- a snapshot was modified after it was created
- the subvol used as parent was created independently from the sent
subvol

Example reproducers:

  # case 1: same ino at same path
  btrfs subvolume create subvol1
  btrfs subvolume create subvol2
  mkdir subvol1/a
  touch subvol2/a

What happens in case of

echo foo > subvol1/a
echo bar > subvol2/a

?

`echo foo > subvol1/a` wouldn't work since subvol1/a is a directory.
However, replacing both preceding lines with:

  mkdir subvol1/a
  echo bar > subvol2/a

=> same as before with/without the patch, plus an additional write
command for the content

And with both lines as

  echo foo > subvol1/a
  echo bar > subvol2/a

=> produces an invalid stream (with and without this patch) with a
`link a -> a` followed by `unlink a` as seen in the example output
quoted further below.

So there is another bug here. The conditions for this seem to roughly
be: changed/new inode with same number, type and generation, since
reordering the commands so the inodes have different generations
produces a valid stream.

Changing the name to subvol2/b like in the second case below produces a
valid stream with a `link b -> a` followed by `unlink a`.

I'll take a look into this, too, and if possible provide another patch
for that case.

quoted

quoted

  btrfs property set subvol1 ro true
  btrfs property set subvol2 ro true
  btrfs send -p subvol1 subvol2 | btrfs receive --dump

You get all these issues because you are using an incremental send in
a way it's not supposed to.
You are using two subvolumes that are completely unrelated.

My surprise here is that we actually allow a user to try that, instead
of giving an error complaining that subvol1 and subvol2 aren't
snapshots of the same subvolume.

An incremental is supposed to work on snapshots of the same subvolume
(hence, have the same parent uuid).
That's what the entire code relies on to work correctly, and that's
what makes sense - to compute and send the differences between two
points in time of a subvolume.
That's why the code base assumes that inodes with the same number and
same generation must refer to the same inode in both the parent and
the send root.

What I think that needs to be answered is:

1) Are there actually people using incremental sends in that way?
(It's the first time I see such use case)

2) If so, why? That is completely unreliable, not only it can lead to
failure to apply the streams, but can result in all sorts of weirdness
(logical inconsistencies, etc) if applying such streams doesn't cause
an error.

3) Making sure such use cases work reliably would require many, many
changes to the send implementation, as it goes against what it
currently expects.
    Snapshot a subvolume, change the subvolume, snapshot it again,
then use both snapshots for the incremental send, that's the expected
scenario.

In other words, what I think we should have is a check that forbids
using two roots for an incremental send that are not snapshots of the
same subvolume (have different parent uuids).

Thanks.

quoted

quoted

The produced tree state here is:
  |-- subvol1
  |   `-- a/        (ino 257)
  |
  `-- subvol2
      `-- a         (ino 257)

  Where subvol1/a/ is a directory and subvol2/a is a file with the
same
  inode number and same generation.

Example output of the receive command:
  At subvol subvol2
  snapshot        ./subvol2                       uuid=19d2be0a-
5af1-fa44-9b3f-f21815178d00 transid=9 parent_uuid=1bac8b12-ddb2-
6441-8551-700456991785 parent_transid=9
  utimes          ./subvol2/                      atime=2021-01-
11T13:41:36+0000 mtime=2021-01-11T13:41:36+0000 ctime=2021-01-
11T13:41:36+0000
  link            ./subvol2/a                     dest=a
  unlink          ./subvol2/a
  utimes          ./subvol2/                      atime=2021-01-
11T13:41:36+0000 mtime=2021-01-11T13:41:36+0000 ctime=2021-01-
11T13:41:36+0000
  chmod           ./subvol2/a                     mode=644
  utimes          ./subvol2/a                     atime=2021-01-
11T13:41:36+0000 mtime=2021-01-11T13:41:36+0000 ctime=2021-01-
11T13:41:36+0000

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

Second example:
  # case 2: same ino at different path
  btrfs subvolume create subvol1
  btrfs subvolume create subvol2
  mkdir subvol1/a
  touch subvol2/b
  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:
  |-- subvol1
  |   `-- a/        (ino 257)
  |
  `-- subvol2
      `-- b         (ino 257)

  Where subvol1/a/ is a directory and subvol2/b is a file with the
same
  inode number and same generation.

Example output of the receive command:
  At subvol subvol2
  snapshot        ./subvol2                       uuid=ea93c47a-
5f47-724f-8a43-e15ce745aef0 transid=20 parent_uuid=f03578ef-5bca-
1445-a480-3df63677fddf parent_transid=20
  utimes          ./subvol2/                      atime=2021-01-
11T13:58:00+0000 mtime=2021-01-11T13:58:00+0000 ctime=2021-01-
11T13:58:00+0000
  link            ./subvol2/b                     dest=a
  unlink          ./subvol2/a
  utimes          ./subvol2/                      atime=2021-01-
11T13:58:00+0000 mtime=2021-01-11T13:58:00+0000 ctime=2021-01-
11T13:58:00+0000
  chmod           ./subvol2/b                     mode=644
  utimes          ./subvol2/b                     atime=2021-01-
11T13:58:00+0000 mtime=2021-01-11T13:58:00+0000 ctime=2021-01-
11T13:58:00+0000

=> the `link` command causes the receiver to fail with:
   ERROR: link b -> a failed: Operation not permitted

Signed-off-by: Roman Anasal <redacted>

-- 
Filipe David Manana,

“Whether you think you can, or you think you can't — you're right.”