Re: [PATCH v2 grub] hurdhelper: Add helper for loading GNU/Hurd multiboot modules

[Samuel Thibault]

Hello,

Damien Zammit, le dim. 16 juil. 2023 10:40:29 +0000, a ecrit:
> This adds a new grub module called hurdhelper.

That's both simple to implement and interesting for use, thanks!

> This module simplifies the loading of hurd multiboot modules
> by allowing them to be loaded in one line (in order of loading).
> 
> A typical use case would be:
> 
>  grub> insmod hurdhelper
>  grub> multiboot /boot/gnumach-1.8-486-dbg.gz root=part:2:device:wd0 noide
>  grub> hurdmodules pci-arbiter acpi rumpdisk ext2fs exec

I'd rather see that as separate commands, so as to be able to pass
options to them as desired.

> +#define HURD_FS_PARAMS1              
> "--multiboot-command-line=${kernel-command-line}"
> +#define HURD_FS_PARAMS2              "--store-type=typed"
> +#define HURD_FS_PARAMS3              "${root}"

I'd rather make the user explicitly tell the module that they're loading
a FS server.

> +#define LIB_PREFIX           "/lib/"
> +#define EXEC_TASK            "exec"
> +#define LD_SO_1                      "ld.so.1"

We don't want to hardcode that. The user may want to use a static exec,
etc.

I.e. I'd rather see something like this:

hurdmodule /hurd/pci-arbiter.static
hurdmodule /hurd/acpi.static
hurdmodule /hurd/rumpdisk.static
hurdrootmodule /hurd/ext2fs.static
hurdexecmodule /lib/ld.so /hurd/exec

Or possibly, by hardcoding only "/hurd/" and ".static" and using lookups
to automatically discover what's available:

hurdmodule pci-arbiter
hurdmodule acpi
hurdmodule rumpdisk
hurdrootmodule ext2fs
hurdexecmodule /lib/ld.so /hurd/exec

The user can also just use

hurdexecmodule exec

if they want something simpler to type. That'll just not share libc etc.
with the rest of the system, but that's fine for booting something. For
grub-generated configurations, we can keep loading ld.so explicitly to
load the dynamically-linked version.

> +static int
> +is_fs_task (char *arg)
> +{
> +  if (!grub_strncmp(arg, FS_TASK, 6))

It's not that simple, we can have many different filesystems. Better
just make the user explicitly tell which module is the root FS module.

> +static int
> +is_exec_task (char *arg)
> +{
> +  if (!grub_strncmp(arg, EXEC_TASK, 4))

I'd also rather make this explicit. Also because (see my mail on the
hurd mailing list) the exec server is special to the root FS.

Samuel