Stefan Behrens
2013-Jul-04 14:14 UTC
[PATCH RESEND] Btrfs: fix wrong write offset when replacing a device
Miao Xie reported the following issue: The filesystem was corrupted after we did a device replace. Steps to reproduce: # mkfs.btrfs -f -m single -d raid10 <device0>..<device3> # mount <device0> <mnt> # btrfs replace start -rfB 1 <device4> <mnt> # umount <mnt> # btrfsck <device4> The reason for the issue is that we changed the write offset by mistake, introduced by commit 625f1c8dc. We read the data from the source device at first, and then write the data into the corresponding place of the new device. In order to implement the "-r" option, the source location is remapped using btrfs_map_block(). The read takes place on the mapped location, and the write needs to take place on the unmapped location. Currently the write is using the mapped location, and this commit changes it back by undoing the change to the write address that the aforementioned commit added by mistake. Reported-by: Miao Xie <miaox@cn.fujitsu.com> Cc: <stable@vger.kernel.org> # 3.10+ Signed-off-by: Stefan Behrens <sbehrens@giantdisaster.de> --- Resent to add stable@vger.kernel.org since the regression was added in 3.10 RC1. fs/btrfs/scrub.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c index 4ba2a69..64a157b 100644 --- a/fs/btrfs/scrub.c +++ b/fs/btrfs/scrub.c @@ -2495,7 +2495,7 @@ again: ret = scrub_extent(sctx, extent_logical, extent_len, extent_physical, extent_dev, flags, generation, extent_mirror_num, - extent_physical); + extent_logical - logical + physical); if (ret) goto out; -- 1.8.3.2 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Miao Xie
2013-Jul-05 08:57 UTC
Re: [PATCH RESEND] Btrfs: fix wrong write offset when replacing a device
On thu, 4 Jul 2013 16:14:23 +0200, Stefan Behrens wrote:> Miao Xie reported the following issue: > > The filesystem was corrupted after we did a device replace. > > Steps to reproduce: > # mkfs.btrfs -f -m single -d raid10 <device0>..<device3> > # mount <device0> <mnt> > # btrfs replace start -rfB 1 <device4> <mnt> > # umount <mnt> > # btrfsck <device4> > > The reason for the issue is that we changed the write offset by mistake, > introduced by commit 625f1c8dc. > > We read the data from the source device at first, and then write the > data into the corresponding place of the new device. In order to > implement the "-r" option, the source location is remapped using > btrfs_map_block(). The read takes place on the mapped location, and > the write needs to take place on the unmapped location. Currently > the write is using the mapped location, and this commit changes it > back by undoing the change to the write address that the aforementioned > commit added by mistake. > > Reported-by: Miao Xie <miaox@cn.fujitsu.com> > Cc: <stable@vger.kernel.org> # 3.10+ > Signed-off-by: Stefan Behrens <sbehrens@giantdisaster.de>Thanks! Reviewed-and-Tested-by: Miao Xie <miaox@cn.fujitsu.com>> --- > Resent to add stable@vger.kernel.org since the regression was added > in 3.10 RC1. > > fs/btrfs/scrub.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c > index 4ba2a69..64a157b 100644 > --- a/fs/btrfs/scrub.c > +++ b/fs/btrfs/scrub.c > @@ -2495,7 +2495,7 @@ again: > ret = scrub_extent(sctx, extent_logical, extent_len, > extent_physical, extent_dev, flags, > generation, extent_mirror_num, > - extent_physical); > + extent_logical - logical + physical); > if (ret) > goto out; > >-- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Josef Bacik
2013-Jul-05 12:32 UTC
Re: [PATCH RESEND] Btrfs: fix wrong write offset when replacing a device
On Thu, Jul 04, 2013 at 04:14:23PM +0200, Stefan Behrens wrote:> Miao Xie reported the following issue: > > The filesystem was corrupted after we did a device replace. > > Steps to reproduce: > # mkfs.btrfs -f -m single -d raid10 <device0>..<device3> > # mount <device0> <mnt> > # btrfs replace start -rfB 1 <device4> <mnt> > # umount <mnt> > # btrfsck <device4> > > The reason for the issue is that we changed the write offset by mistake, > introduced by commit 625f1c8dc. > > We read the data from the source device at first, and then write the > data into the corresponding place of the new device. In order to > implement the "-r" option, the source location is remapped using > btrfs_map_block(). The read takes place on the mapped location, and > the write needs to take place on the unmapped location. Currently > the write is using the mapped location, and this commit changes it > back by undoing the change to the write address that the aforementioned > commit added by mistake. >I''d like to see a xfstest for this please so we can make sure something like this doesn''t happen in the future. You can use SCRATCH_DEV_POOL to have multiple devices. Please cc linux-btrfs when you submit it so I can review it and hopefully get it in faster. Thanks, Josef -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html