Re: [PATCH] reboot: Fix variable assignments in type_store

From: Petr Mladek
Date: Fri Nov 13 2020 - 15:06:36 EST


On Fri 2020-11-13 03:58:49, Matteo Croce wrote:
> On Fri, Nov 13, 2020 at 3:46 AM Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
> >
> > On Fri, 13 Nov 2020 02:38:18 +0100 Matteo Croce <mcroce@xxxxxxxxxxxxxxxxxxx> wrote:
> >
> > > At this point, since 'pci' enables BOOT_CF9_FORCE type and
> > > BOOT_CF9_SAFE is not user selectable, should I simply leave only
> > > 'pci'?
> > >
> > > This way, we'll have the same set of options for both sysfs and kernel cmdline.
> >
> > Well, you're the reboot expert ;)
> >
>
> So honored! :)
>
> > But my $0.02 is yes, let's keep the command-line and sysfs interfaces
> > in sync and cover it all in documentation. It would of course be
> > problematic to change the existing reboot= interface.
> >
> > I assume that means doing this?
> >
> > - #define BOOT_CF9_FORCE_STR "cf9_force"
> > + #define BOOT_CF9_FORCE_STR "pci"
> > - #define BOOT_CF9_SAFE_STR "cf9_safe"
>
> Either BOOT_PCI_STR or BOOT_CF9_FORCE_STR, I have no strong preference.
>
> The syntax is 'pci' while the enum BOOT_CF9_FORCE, so we can't please both.

The question is whether we should modify/allow to set these values at
all.

Anyway, we must prevent them on non-x86 architectures because
the reboot behavior would be undefined there. They could probably
make a mess even on many x86-architectures.

I have to admit it has become much more complicated than I thought.
It brings back Andrew's original question whether this interface is
really needed. Are you going to use in the real life?

The interface might do more harm then good when it allows to set
reboot_type that is not normally accessible or disable it when
it is strictly needed.

Anyway, we should get input from some x86-experts about the BOOT_CF9
values.

Best Regards,
Petr