RE: [PATCH] btrfs-progs: mkfs: Enable -d dup for single device

[Zhao Lei <zhaolei@cn.fujitsu.com>]

Hi, David Sterba

> -----Original Message-----
> From: David Sterba [mailto:dsterba@suse.cz]
> Sent: Tuesday, October 13, 2015 8:36 PM
> To: Zhao Lei <zhaolei@cn.fujitsu.com>
> Cc: dsterba@suse.cz; linux-btrfs@vger.kernel.org; clm@fb.com
> Subject: Re: [PATCH] btrfs-progs: mkfs: Enable -d dup for single device
> 
> On Tue, Oct 13, 2015 at 07:40:30PM +0800, Zhao Lei wrote:
> > > What I remember from the comment is that "it's slightly offset that
> > > would lead to corruption".
> >
> > Before this patch, I had done git blame to search why the condition
> > was added, and hadn't found the exact reason.
> 
> found it: commit bc3f116fec194f1d7329b160c266fe16b9266a1e and it was not
> aobut data/dup but mixed bgs with sectorisze != nodesize:
> 
>  26 +       nodesize = btrfs_super_nodesize(disk_super);
>  27 +       leafsize = btrfs_super_leafsize(disk_super);
>  28 +       sectorsize = btrfs_super_sectorsize(disk_super);
>  29 +       stripesize = btrfs_super_stripesize(disk_super);
>  30 +
>  31 +       /*
>  32 +        * mixed block groups end up with duplicate but slightly offset
>  33 +        * extent buffers for the same range.  It leads to corruptions
>  34 +        */
>  35 +       if ((features & BTRFS_FEATURE_INCOMPAT_MIXED_GROUPS)
> &&
>  36 +           (sectorsize != leafsize)) {
>  37 +               printk(KERN_WARNING "btrfs: unequal
> leaf/node/sector sizes "
>  38 +                               "are not allowed for mixed block
> groups on %s\n",
>  39 +                               sb->s_id);
>  40 +               goto fail_alloc;
>  41 +       }
>  42 +
> 
Thanks for this information, I'll investigate is similar problem in non-mixed
with dup.

> > I will queue xfstests(btrfs/generic) at this profile with all mount
> > option for multi-times, to check is something wrong with this.
> 
> Thanks. We need to cover more: the balance conversion forbids data/dup
> profile, I'm not sure if scrub handles that properly, and the ususal suspects in
> the rescue tools (fsck, restore, chunk-recover).
>
Agree, a new profile may be have potential problem because existing code
haven't check the support status.

IMHO, it is still necessary except we can prove this function should not exist.
But we'll need to do more works to confirm above potential problem.

Thanks
Zhaolei