[systemd-devel] [PATCH v2] Add reboot to EFI support

[Jan Janssen]

Hi,

On 2015-04-02 11:34, Lennart Poettering wrote:
> On Thu, 26.03.15 16:09, Jan Janssen (medhefgo at web.de) wrote:
>
> Heya,
>
> Hmm, so we already support passing special reboot() parameters, and
> this is done by manipulating a file in /run, without introducing any
> new targets. To me it appears that boot-into-firmware-setup is
> something hat should be handled the same way, i.e. as a special
> parameter for the *normal* poweroff path, instead of introducing a new
> poweroff path for it. Of course, instead of manipulating /run for this
> we should directly manipulate the respective EFI variable.
>
> I hence think this should be a new switch --firmware-setup or so to
> systemctl. Of course, that sounds awfully specific and I don't really
> like too much adding a new switch just for this flag, but it's the
> least best option I see.

That was my original approach. Kay said "--firmware" sounded weird.

> The existing boot argument is passed as-is to the kernel, hence
> giving the argument "efi" a special meaning would mean once couldn't
> pass that parameter anymore to the kernel.

I had the same reservation, but it was suggested to ignore this and just 
piggyback on this instead.

> I would strongly prefer naming the switch something like "firmware"
> instead of "EFI", since we shouldn't encode the technology here, but
> the generic term. Also, this should mention that this is about the
> setup tool of the firmware, since EFI is available all the time, and
> this is really about the *setup* tool of the firmware...

Someone suggested "firmware" is too generic, so I switched to EFI. Would 
be nice if people made up their mind on that one...

> I think ultimately we need to expose this even in GNOME, similar to
> the way Window exposes this. To cover that we should probably add a
> bus API to logind in some form to manipulate the EFI var in question,
> and "systemctl reboot --firmware-setup" would use that. (And yes, a
> similar bus API for specifying the generic reboot parameter probably
> should exist alongside it).

Äääähm... this is exactly what this patch does, adding CanRebootToEfi()
and a RebootToEfi() functions. What did I miss?

Unless you mean changing those into a pair of properties to just set the
indication and then the bus client would have to manually trigger Reboot()?
In fact, that's what I kind of got in my mind after sending this patch. 
It would also work nicely with a separate RebootArguments property 
without the hassle of introducing more complex logic into the target 
related functions in logind. My original approach was adding a 
RebootWithArguments function, but my brain cannot get the code to look 
nicely. But making them into properties and requiring the client to 
issue a Reboot themselves would be a neat way around that.

> Of course the bus API should also support a CanFirmwareSetup() call or
> so, that reports whether the logic is available at all.

As I said, this patch adds this. Though, it would be nice if some 
consensus would come about whether to call this firmware or EFI. I think 
being specific is probably nicer. Unless this were to return a string 
indicating what kind of firmware setup is supported (if ever any others 
would come about in the future), returning "efi" for EFI systems.

> Does this make sense?
>
> Lennart
>

Jan