| Message ID | 20190103110727.23558-2-po-hsu.lin@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | [T,SRU,1/1] Btrfs: send, don't send rmdir for same target multiple times | expand |
On 1/3/19 12:07 PM, Po-Hsu Lin wrote: > From: Filipe Manana <fdmanana@gmail.com> > > BugLink: https://bugs.launchpad.net/bugs/1809868 > > When doing an incremental send, if we delete a directory that has N > 1 > hardlinks for the same file and that file has the highest inode number > inside the directory contents, an incremental send would send N times an > rmdir operation against the directory. This made the btrfs receive command > fail on the second rmdir instruction, as the target directory didn't exist > anymore. > > Steps to reproduce the issue: > > $ mkfs.btrfs -f /dev/sdb3 > $ mount /dev/sdb3 /mnt/btrfs > $ mkdir -p /mnt/btrfs/a/b/c > $ echo 'ola mundo' > /mnt/btrfs/a/b/c/foo.txt > $ ln /mnt/btrfs/a/b/c/foo.txt /mnt/btrfs/a/b/c/bar.txt > $ btrfs subvolume snapshot -r /mnt/btrfs /mnt/btrfs/snap1 > $ btrfs send /mnt/btrfs/snap1 -f /tmp/base.send > $ rm -f /mnt/btrfs/a/b/c/foo.txt > $ rm -f /mnt/btrfs/a/b/c/bar.txt > $ rmdir /mnt/btrfs/a/b/c > $ btrfs subvolume snapshot -r /mnt/btrfs /mnt/btrfs/snap2 > $ btrfs send -p /mnt/btrfs/snap1 /mnt/btrfs/snap2 -f /tmp/incremental.send > > $ umount /mnt/btrfs > $ mkfs.btrfs -f /dev/sdb3 > $ mount /dev/sdb3 /mnt/btrfs > $ btrfs receive /mnt/btrfs -f /tmp/base.send > $ btrfs receive /mnt/btrfs -f /tmp/incremental.send > > The second btrfs receive command failed with: > > ERROR: rmdir o259-6-0 failed. No such file or directory > > A test case for xfstests follows. > > Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com> > Signed-off-by: Josef Bacik <jbacik@fb.com> > (backported from commit 29d6d30f5c8aa58b04f40a58442df3bcaae5a1d5) Simple backport, regression potential seems to be low. Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com> > Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com> > --- > fs/btrfs/send.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c > index d553752..9cc2ef0 100644 > --- a/fs/btrfs/send.c > +++ b/fs/btrfs/send.c > @@ -2686,6 +2686,7 @@ static int process_recorded_refs(struct send_ctx *sctx) > u64 ow_gen; > int did_overwrite = 0; > int is_orphan = 0; > + u64 last_dir_ino_rm = 0; > > verbose_printk("btrfs: process_recorded_refs %llu\n", sctx->cur_ino); > > @@ -2941,7 +2942,8 @@ verbose_printk("btrfs: process_recorded_refs %llu\n", sctx->cur_ino); > ret = send_utimes(sctx, cur->dir, cur->dir_gen); > if (ret < 0) > goto out; > - } else if (ret == inode_state_did_delete) { > + } else if (ret == inode_state_did_delete && > + cur->dir != last_dir_ino_rm) { > ret = can_rmdir(sctx, cur->dir, sctx->cur_ino); > if (ret < 0) > goto out; > @@ -2953,6 +2955,7 @@ verbose_printk("btrfs: process_recorded_refs %llu\n", sctx->cur_ino); > ret = send_rmdir(sctx, valid_path); > if (ret < 0) > goto out; > + last_dir_ino_rm = cur->dir; > } > } > }
On 03.01.19 12:07, Po-Hsu Lin wrote: > From: Filipe Manana <fdmanana@gmail.com> > > BugLink: https://bugs.launchpad.net/bugs/1809868 > > When doing an incremental send, if we delete a directory that has N > 1 > hardlinks for the same file and that file has the highest inode number > inside the directory contents, an incremental send would send N times an > rmdir operation against the directory. This made the btrfs receive command > fail on the second rmdir instruction, as the target directory didn't exist > anymore. > > Steps to reproduce the issue: > > $ mkfs.btrfs -f /dev/sdb3 > $ mount /dev/sdb3 /mnt/btrfs > $ mkdir -p /mnt/btrfs/a/b/c > $ echo 'ola mundo' > /mnt/btrfs/a/b/c/foo.txt > $ ln /mnt/btrfs/a/b/c/foo.txt /mnt/btrfs/a/b/c/bar.txt > $ btrfs subvolume snapshot -r /mnt/btrfs /mnt/btrfs/snap1 > $ btrfs send /mnt/btrfs/snap1 -f /tmp/base.send > $ rm -f /mnt/btrfs/a/b/c/foo.txt > $ rm -f /mnt/btrfs/a/b/c/bar.txt > $ rmdir /mnt/btrfs/a/b/c > $ btrfs subvolume snapshot -r /mnt/btrfs /mnt/btrfs/snap2 > $ btrfs send -p /mnt/btrfs/snap1 /mnt/btrfs/snap2 -f /tmp/incremental.send > > $ umount /mnt/btrfs > $ mkfs.btrfs -f /dev/sdb3 > $ mount /dev/sdb3 /mnt/btrfs > $ btrfs receive /mnt/btrfs -f /tmp/base.send > $ btrfs receive /mnt/btrfs -f /tmp/incremental.send > > The second btrfs receive command failed with: > > ERROR: rmdir o259-6-0 failed. No such file or directory > > A test case for xfstests follows. > > Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com> > Signed-off-by: Josef Bacik <jbacik@fb.com> > (backported from commit 29d6d30f5c8aa58b04f40a58442df3bcaae5a1d5) > Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com> Acked-by: Stefan Bader <stefan.bader@canonical.com> > --- > fs/btrfs/send.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c > index d553752..9cc2ef0 100644 > --- a/fs/btrfs/send.c > +++ b/fs/btrfs/send.c > @@ -2686,6 +2686,7 @@ static int process_recorded_refs(struct send_ctx *sctx) > u64 ow_gen; > int did_overwrite = 0; > int is_orphan = 0; > + u64 last_dir_ino_rm = 0; > > verbose_printk("btrfs: process_recorded_refs %llu\n", sctx->cur_ino); > > @@ -2941,7 +2942,8 @@ verbose_printk("btrfs: process_recorded_refs %llu\n", sctx->cur_ino); > ret = send_utimes(sctx, cur->dir, cur->dir_gen); > if (ret < 0) > goto out; > - } else if (ret == inode_state_did_delete) { > + } else if (ret == inode_state_did_delete && > + cur->dir != last_dir_ino_rm) { > ret = can_rmdir(sctx, cur->dir, sctx->cur_ino); > if (ret < 0) > goto out; > @@ -2953,6 +2955,7 @@ verbose_printk("btrfs: process_recorded_refs %llu\n", sctx->cur_ino); > ret = send_rmdir(sctx, valid_path); > if (ret < 0) > goto out; > + last_dir_ino_rm = cur->dir; > } > } > } >
diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c index d553752..9cc2ef0 100644 --- a/fs/btrfs/send.c +++ b/fs/btrfs/send.c @@ -2686,6 +2686,7 @@ static int process_recorded_refs(struct send_ctx *sctx) u64 ow_gen; int did_overwrite = 0; int is_orphan = 0; + u64 last_dir_ino_rm = 0; verbose_printk("btrfs: process_recorded_refs %llu\n", sctx->cur_ino); @@ -2941,7 +2942,8 @@ verbose_printk("btrfs: process_recorded_refs %llu\n", sctx->cur_ino); ret = send_utimes(sctx, cur->dir, cur->dir_gen); if (ret < 0) goto out; - } else if (ret == inode_state_did_delete) { + } else if (ret == inode_state_did_delete && + cur->dir != last_dir_ino_rm) { ret = can_rmdir(sctx, cur->dir, sctx->cur_ino); if (ret < 0) goto out; @@ -2953,6 +2955,7 @@ verbose_printk("btrfs: process_recorded_refs %llu\n", sctx->cur_ino); ret = send_rmdir(sctx, valid_path); if (ret < 0) goto out; + last_dir_ino_rm = cur->dir; } } }