[systemd-devel] [v1] shutdown: add kexec loading, avoid calling `kexec` binary unnessecarily

[Jan Janssen]

On 2015-02-28 02:02, Shawn Landden wrote:
> On Thu, Feb 26, 2015 at 12:04 AM, Jan Janssen <medhefgo at web.de
> <mailto:medhefgo at web.de>> wrote:
>
>     Shawn Landden <shawn <at> churchofgit.com <http://churchofgit.com>>
>     writes:
>
>     >  void strv_free(char **l) {
>     > -        strv_clear(l);
>     > +        char **k;
>     > +
>     > +        if (!l)
>     > +                return;
>     > +
>     > +        for (k = l; *k; k++)
>     > +                free(*k);
>     > +
>     >          free(l);
>     >  }
>     What are you trying to achieve here? I see no point in optimizing
>     out the *l
>     = NULL from strv_clear.
>
>     > +                                entry->linux_loc  = l + strspn(l,
>     WHITESPACE);
>     > +                        else if ((l = startswith(m, "initrd ")))
>     > +                                entry->initrd     = l + strspn(l,
>     WHITESPACE);
>     You need to support more than one initrd per kernel, see
>     https://wiki.archlinux.org/index.php/Microcode for why. Also, I am
>     pretty
>     sure you can have a initrd=/path/to/initrd in the kernel options entry.
>     Since the efi bootloader just appends each given initrd to the kernel
>     command line.
>
> I can't support more than one initrd per kernel with the
> kexec_file_load() syscall, and if initrd on the commandline works, then
> it will still work with this patch, so i don't need to change anything.
I doubt that it works. Afaik, the initrd is handled by the EFI stub (if 
that is used. Grub only uses it if linuxefi command is used). And I 
would assume that kexec doesn't use the stub. You should ask the kernel 
people how this API should be used with more than one initrd. My best 
guess is that you can just cat them into a tmp file and use that as target.

>
>     All in all I am wondering why you need a rbtree for all this in the
>     first
>     place? A simple hashmap should do just fine.
>
>     Also, you're not taking multi-boot into account (the machine-id field).
>     You're just discriminating based on the kernel version, but different
>     installations could have the same version field.
>
> fixed by testing that the machine-id is the same (I forgot this part of
> the spec thanks). Is there anyway I should save defaults? Is there
> anything in the spec that is missing? Perhaps it should specify how to
> save last-boot.

I think that's it. I'll shout when I see something missing, though :P

Jan