linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Roman Anasal | BDSU <roman.anasal@bdsu.de>
To: "linux-btrfs@vger.kernel.org" <linux-btrfs@vger.kernel.org>,
	"arvidjaar@gmail.com" <arvidjaar@gmail.com>
Subject: Re: [PATCH 2/2] btrfs: send: fix invalid commands for inodes with changed type but same gen
Date: Mon, 11 Jan 2021 20:53:14 +0000	[thread overview]
Message-ID: <424d62853024d8b0bc5ca03206eeca35be6014a2.camel@bdsu.de> (raw)
In-Reply-To: <9e177865-0408-c321-951e-ce0f3ff33389@gmail.com>

On Mon, 2021-01-11 at 22:30 +0300, Andrei Borzenkov wrote:
> 11.01.2021 22:02, Roman Anasal пишет:
> > 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.


> >   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
> >       `-- 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 <roman.anasal@bdsu.de>

  reply	other threads:[~2021-01-11 20:54 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-11 19:02 [PATCH 0/2] btrfs: send: correctly recreate changed inodes Roman Anasal
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 [this message]
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:
  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=424d62853024d8b0bc5ca03206eeca35be6014a2.camel@bdsu.de \
    --to=roman.anasal@bdsu.de \
    --cc=arvidjaar@gmail.com \
    --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 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).