On Tue, Mar 25, 2025 at 8:26 AM, Vladimir 'phcoder' Serbinenko
<phco...@gmail.com> wrote:
> Le mar. 25 mars 2025, 10:15, Alec Brown <alec.r.br...@oracle.com> a écrit :
>
>> Irritatingly, BLS defines paths relatives to the mountpoint of the
>> filesystem which contains its snippets, not / or any other fixed
>> location. So grub2-emu needs to know whether /boot is a separate
>> filesystem from / and conditionally prepend a path.
>>
>> Signed-off-by: Robbie Harwood <rharw...@redhat.com>
>> Signed-off-by: Alec Brown <alec.r.br...@oracle.com>
>> ---
>> grub-core/Makefile.core.def | 4 +++
>> grub-core/commands/blsuki.c | 54 ++++++++++++++++++++++++++++-----
>> grub-core/osdep/linux/getroot.c | 8 +++++
>> grub-core/osdep/unix/getroot.c | 10 ++++++
>> include/grub/emu/misc.h | 2 +-
>> 5 files changed, 70 insertions(+), 8 deletions(-)
>>
>> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
>> index f3b776c0a..9a0e7bc55 100644
>> --- a/grub-core/Makefile.core.def
>> +++ b/grub-core/Makefile.core.def
>> @@ -367,6 +367,10 @@ kernel = {
>> emu = kern/emu/cache_s.S;
>> emu = kern/emu/hostdisk.c;
>> emu = osdep/unix/hostdisk.c;
>> + emu = osdep/relpath.c;
>> + emu = osdep/getroot.c;
>> + emu = osdep/unix/getroot.c;
>> + emu = osdep/devmapper/getroot.c;
>> emu = osdep/exec.c;
>> extra_dist = osdep/unix/exec.c;
>> emu = osdep/devmapper/hostdisk.c;
>> diff --git a/grub-core/commands/blsuki.c b/grub-core/commands/blsuki.c
>> index 0f77fb568..12a9a1ed1 100644
>> --- a/grub-core/commands/blsuki.c
>> +++ b/grub-core/commands/blsuki.c
>> @@ -32,6 +32,13 @@
>> #include <grub/lib/envblk.h>
>> #include <grub/lib/vercmp.h>
>>
>> +#ifdef GRUB_MACHINE_EMU
>> +#include <grub/emu/misc.h>
>> +#define GRUB_BOOT_DEVICE "/boot"
>> +#else
>> +#define GRUB_BOOT_DEVICE ""
>> +#endif
>> +
>> GRUB_MOD_LICENSE ("GPLv3+");
>>
>> #define GRUB_BLS_CONFIG_PATH "/loader/entries/"
>> @@ -53,8 +60,40 @@ struct keyval
>>
>> static grub_blsuki_entry_t *entries = NULL;
>>
>> +/*
>> + * Cache probing in blsuki_update_boot_device(). Used for linux entry
>> also.
>> + */
>> +static int separate_boot = -1;
>>
>
> On non-emu it's an unused static. It might trigger a warning. I'd prefer
> this to be inside ifdef.
There's one instance where this value gets used to check if we add the
GRUB_BOOT_DEVICE to the start of the Linux kernel path, but this could be
changed to use the blsuki_update_boot_device() instead.
>
>> +
>> #define FOR_BLSUKI_ENTRIES(var) FOR_LIST_ELEMENTS (var, entries)
>>
>> +/*
>> + * BLS appears to make paths relative to the filesystem that snippets are
>> + * on, not /. Attempt to cope.
>> + */
>> +static char *blsuki_update_boot_device (char *tmp)
>> +{
>> +#ifdef GRUB_MACHINE_EMU
>> + char *ret;
>> +
>> + if (separate_boot != -1)
>> + goto probed;
>> +
>> + separate_boot = 0;
>> +
>> + ret = grub_make_system_path_relative_to_its_root (GRUB_BOOT_DEVICE);
>> +
>> + if (ret != NULL)
>> + separate_boot = 1;
>>
> Are you sure that != NULL is the right check? It looks like it should be
> strcmp with "/". How do you get a NULL here? Shouldn't happen if the path
> exists.
Ah you are correct. I think I confused this with the output of
grub_make_system_path_relative_to_its_root_os(). Upon review of the function,
if the path given is the mount point, it will return an empty string since it
won't return a trailing "/". I'll fix my code to reflect this.
>
>> +
>> + probed:
>> + if (!separate_boot)
>> + return grub_stpcpy (tmp, " ");
>> +#endif
>> +
>> + return grub_stpcpy (tmp, " " GRUB_BOOT_DEVICE);
>> +}
>> +
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
