Re: [PATCH] efivars: Allow disabling use as a pstore backend

[Seth Forshee <seth.forshee@canonical.com>]

On Wed, Mar 13, 2013 at 03:49:30PM +0000, Matt Fleming wrote:
> > diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> > index 9b00072..35f7d16 100644
> > --- a/drivers/firmware/Kconfig
> > +++ b/drivers/firmware/Kconfig
> > @@ -53,6 +53,15 @@ config EFI_VARS
> >  	  Subsequent efibootmgr releases may be found at:
> >  	  <http://linux.dell.com/efibootmgr>
> >  
> > +config EFI_VARS_PSTORE
> > +	bool "Use efivars as a pstore backend"
> > +	depends on EFI_VARS
> > +	default y
> > +	help
> > +	  Say Y here to enable the use of efivars as a storage backend
> > +	  for pstore. This setting can be overridden using the
> > +	  efivars.pstore_enable parameter.
> > +
> 
> The depends here should probably include PSTORE, which I think it did on
> previous iterations. I can't think of a situation where you'd want to
> disable CONFIG_PSTORE but enable CONFIG_EFI_VARS_PSTORE. Also, we don't
> need to use CONFIG_EFI_VARS_PSTORE solely for the purpose of setting the
> initial value of efivars_pstore_enabled - there's merit in also using it
> to leave out the pstore code in efivars.c during compilation.
> 
> Granted, this change does mean that your decision at compile time may be
> final, and you can't enable the pstore code at runtime if you built your
> kernel/efivars module with CONFIG_EFI_VARS_PSTORE=n. Arguably, I think
> that's a feature that some people will want, e.g. "Don't allow enabling
> the efivars pstore backend at all... no, srsly!".

Hmm. Since my original patch was to compile pstore support out
completely, I interpreted Peter's suggestion to be allowing the user
override the default even if the default is disabled. That's the
assumption my implementation was based off of, but maybe I
misunderstood.

But if Peter agrees with your changes then I'm fine with them too.

Thanks,
Seth