Re: [PATCH 2/2] xfs: Bypass sb alignment checks when custom values are used

[Dave Chinner <david@fromorbit.com>]

On Mon, Jun 01, 2020 at 05:06:36PM -0500, Eric Sandeen wrote:
> On 6/1/20 4:21 PM, Dave Chinner wrote:
> > The user was not responsible for this mess (combination of missing
> > validation in XFS code and bad storage firmware providing garbage)
> > so we should not put them on the hook for fixing it. We can do it
> > easily and without needing user intervention and so that's what we
> > should do.
> 
> FWIW, I have a working xfs_repair that fixes bad geometry as well,
> I ... guess that's probably still useful?

Yes, repair should definitely be proactive on this.

> It was doing similar things to what you suggested, though I wasn't
> rounding swidth up, I was setting it equal to sunit.  *shrug* rounding
> up is maybe better; the problematic devices usually have width < unit
> so rounding up will be the same as setting equal  :)

Yup, that's exactly why I suggested rounding up :)

> phase1()
> 
> +       /*
> +        * Now see if there's a problem with the stripe width; if it's bad,
> +        * we just set it equal to the stripe unit as a harmless alternative.
> +        */
> +        if (xfs_sb_version_hasdalign(sb)) {
> +                if ((sb->sb_unit && !sb->sb_width) ||
> +                    (sb->sb_width && sb->sb_unit && sb->sb_width % sb->sb_unit)) {
> +                       sb->sb_width = sb->sb_unit;
> +                       primary_sb_modified = 1;
> +                       geometry_modified = 1;
> +                       do_warn(
> +_("superblock has a bad stripe width, resetting to %d\n"),
> +                               sb->sb_width);
> +               }
> +       }
> 
> I also had to more or less ignore bad swidths throughout repair (and in
> xfs_validate_sb_common IIRC) to be able to get this far.  If repair thinks
> a superblock is bad, it goes looking for otheres to replace it and if the
> bad geometry came from the device, they are all equally "bad..."

Yeah. Which leads me to ask: should the kernel be updating the
secondary superblocks when it finds bad geometry in the primary, or
overwrites the geometry with user supplied info?

(I have a vague recollection of being asked something about this on
IRC and me muttering something about CXFS only trashing the primary
superblock...)

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com