On 6/1/20 6:35 PM, Dave Chinner wrote:
> 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?
since repair depends on it .... probably so.
(hm I haven't seen what happens if we update the primary under normal
circumstances, and then primary & secondaries have valid but different
geometries...?)
> (I have a vague recollection of being asked something about this on
> IRC and me muttering something about CXFS only trashing the primary
> superblock...)
Yeah I blathered about it, we have a function to call to do this for growfs,
so it'd be trivial and seems wise.
-Eric
> Cheers,
>
> Dave.
>
