Re: [PATCH] btrfs: disable zstd support for i386-pc

[Mike Gilbert]

On Sun, Jun 21, 2020 at 2:56 PM Eli Schwartz <eschwartz@archlinux.org> wrote:
>
> On 6/21/20 2:26 PM, Mike Gilbert wrote:
> > On Thu, Jun 11, 2020 at 6:58 PM Eli Schwartz <eschwartz@archlinux.org> 
> > wrote:
> >>
> >> On 11/7/19 12:08 AM, Vladimir 'phcoder' Serbinenko wrote:
> >>> On Wed, 6 Nov 2019, 20:55 Michael Chang, <MChang@suse.com> wrote:
> >>>
> >>>> On Wed, Nov 06, 2019 at 11:15:04AM -0800, Vladimir 'phcoder' Serbinenko
> >>>> wrote:
> >>>>> Please don't do it this way. The right solution is to move it to 
> >>>>> separate
> >>>>> module and include zstd module when needed.
> >>>>
> >>>> I fully agree to work out a proper solution, but before that I think it
> >>>> is necessary to stop it from spread out going forward, as the running
> >>>> system upgrading to new version could be affected and the consequence is
> >>>> fatal - an unbootable system, qualified to be show stopper imho.
> >>>>
> >>> We don't do a release right now, so I don't think it's justified. Also
> >>> increase in size can easily come from a compiler. If an increase in size
> >>> can make system unbootable on upgrade, I'd rather be fixing this than just
> >>> pepper over it.
> >>
> >> Given grub 2.06 is on the horizon, is it time to re-evaluate this and
> >> disable something known to be badly broken?
> >
> > If I have read the thread correctly, this only breaks if you try to
> > install grub in an undersized embedding area. This would really only
> > affect new grub installs; existing ones can keep using the currently
> > installed code.
>
> "changing grub only affects people who newly run the grub-install
> command" therefore it's not worth considering whether to make sure
> grub-install completes without errors? I don't comprehend this logic.
> Who ever suggested we care about people who aren't the target audience
> of new grub versions to begin with?
>
> But anyway this isn't true. There are valid reasons to reinstall grub on
> old systems, in which case you are most likely not benefiting from zstd
> support one way or another, but in this case, rerunning grub-install
> destroys the working bootloader code and fails to replace.
>
> https://bugs.archlinux.org/task/63656 our infrastructure apparently
> somehow ended up mismatching core.img and /boot modules, and had to
> rerun grub-install (it's not clear what happened exactly, possibly
> grub-install accidentally got rerun in a broken manner?) and then this
> remote server would not boot, and could not be fixed without hunting for
> the right old version of grub via the rescue system, reinstalling it,
> and using it to restore the boring (working) old pre-zstd bootloader
> code, after a lot of painful debugging.
>
> > I think it would be better to hold out for a real fix than to disable
> > a feature for an entire platform where only a subset of configurations
> > would not work.
>
> I'm astonished that "it's better to hold out for a real fix than to
> ensure users' systems actually work".
>
> Why is the benefit of enabling zstd support in situations where it very
> often breaks everything, worth more than the disadvantage of, in fact,
> breaking everything? The feature was broken out of the gate, no proper
> fix seems imminent, let's bandage the wound before more people get hurt.

If the feature is "broken", it should be reverted fully, not disabled
on a single platform. That might encourage the original contributor to
implement it properly next time.