compile error in master, restrict is missing
I understand that grub.git#master does (most likely) compile for everyone else: [ 135s] + env CFLAGS=-Wall CXXFLAGS=-Wall FFLAGS=-Wall ../grub2-src/configure --target=i386-suse-linux --with-platform=pc --prefix=/usr/lib64/grub2-chainloader --disable-boot-time --disable-cache-stats --disable-device-mapper --disable-efiemu --disable-grub-emu-pci --disable-grub-emu-sdl --disable-grub-mkfont --disable-grub-mount --disable-grub-themes --disable-liblzma --disable-libzfs --enable-mm-debug --disable-nls --disable-werror --enable-rpath --with-bootdir=/chainloader/grub2-chainloader --enable-grub-mkfont --enable-grub-themes --with-grubdir=grub2 ... [ 207s] In file included from ../grub2-src/util/grub-mkfont.c:21:0: [ 207s] ../grub2-src/include/grub/misc.h:264:64: error: expected ';', ',' or ')' before 'str' [ 207s] unsigned long EXPORT_FUNC(grub_strtoul) (const char * restrict str, const char ** const restrict end, int base); [ 207s] ^ Where is 'restrict' supposed to come from? Olaf pgpLe2AwYIyBC.pgp Description: Digitale Signatur von OpenPGP ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: compile error in master, restrict is missing
Hi Olaf, CC-ing Peter... On Wed, Mar 11, 2020 at 08:44:33AM +0100, Olaf Hering wrote: > I understand that grub.git#master does (most likely) compile for everyone > else: > > [ 135s] + env CFLAGS=-Wall CXXFLAGS=-Wall FFLAGS=-Wall > ../grub2-src/configure --target=i386-suse-linux --with-platform=pc > --prefix=/usr/lib64/grub2-chainloader --disable-boot-time > --disable-cache-stats --disable-device-mapper --disable-efiemu > --disable-grub-emu-pci --disable-grub-emu-sdl --disable-grub-mkfont > --disable-grub-mount --disable-grub-themes --disable-liblzma --disable-libzfs > --enable-mm-debug --disable-nls --disable-werror --enable-rpath > --with-bootdir=/chainloader/grub2-chainloader --enable-grub-mkfont > --enable-grub-themes --with-grubdir=grub2 > ... > [ 207s] In file included from ../grub2-src/util/grub-mkfont.c:21:0: > [ 207s] ../grub2-src/include/grub/misc.h:264:64: error: expected ';', ',' or > ')' before 'str' > [ 207s] unsigned long EXPORT_FUNC(grub_strtoul) (const char * restrict str, > const char ** const restrict end, int base); > [ 207s] ^ Yeah, I added tons of patches yesterday evening and build tested. Nothing suspicious pooped out. I expect that the issues you are hitting are related to set of configure flags which you are using. Anyway, it has to be fixed... > Where is 'restrict' supposed to come from? Peter, could you take a look at this? Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: GRUB 2.06 release
On Tue, Mar 03, 2020 at 06:26:03PM +0100, Daniel Kiper wrote: > On Wed, Feb 19, 2020 at 04:01:38PM +0100, Daniel Kiper wrote: > > Hi all, > > > > As I told during my FOSDEM 2020 presentation we are preparing for > > GRUB 2.06 release. Tentative schedule is below: > > - code freeze: 15th of March, 23:59:59 UTC; everything posted after > > that date will not be considered as a release material, > > Just a kind reminder... Less than two weeks left... Less than a week left... This is final call... I will review all patches which have not get my RB by the end of this week. Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: compile error in master, restrict is missing
On 3/11/20 11:39 AM, Daniel Kiper wrote: >> [ 135s] + env CFLAGS=-Wall CXXFLAGS=-Wall FFLAGS=-Wall >> ../grub2-src/configure --target=i386-suse-linux Which version of SUSE Linux is that, in case I want to reproduce that? FWIW, I work at SUSE, so I have access to all versions :). Adrian -- .''`. John Paul Adrian Glaubitz : :' : Debian Developer - glaub...@debian.org `. `' Freie Universitaet Berlin - glaub...@physik.fu-berlin.de `-GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: compile error in master, restrict is missing
Am Wed, 11 Mar 2020 11:39:08 +0100 schrieb Daniel Kiper : > Nothing suspicious pooped out. I expect that the issues you are > hitting are related to set of configure flags which you are using. It turned out --disable-mm-debug does for some reason fix the build with gcc7. But building with gcc48 does still trigger the failure. Olaf pgpw7RrBNxqz_.pgp Description: Digitale Signatur von OpenPGP ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: compile error in master, restrict is missing
Am Wed, 11 Mar 2020 11:56:27 +0100 schrieb John Paul Adrian Glaubitz : > Which version of SUSE Linux is that, in case I want to reproduce that? SLE_12 and Leap 42.3, everything with gcc-4.8. I may be able to work around it by using a newer compiler. Olaf pgpyMboyE0v9G.pgp Description: Digitale Signatur von OpenPGP ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: compile error in master, restrict is missing
On 3/11/20 12:01 PM, Olaf Hering wrote: > Am Wed, 11 Mar 2020 11:56:27 +0100 > schrieb John Paul Adrian Glaubitz : > >> Which version of SUSE Linux is that, in case I want to reproduce that? > > SLE_12 and Leap 42.3, everything with gcc-4.8. I may be able to work around > it by using a newer compiler. Ok, thanks. FWIW, Leap 42.3 is no longer supported by us. I happen to have a SLE-12 cloud instance running anyway now so I can help debug if necessary. Adrian -- .''`. John Paul Adrian Glaubitz : :' : Debian Developer - glaub...@debian.org `. `' Freie Universitaet Berlin - glaub...@physik.fu-berlin.de `-GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: compile error in master, restrict is missing
Hello Olaf, On 3/11/20 11:59 AM, Olaf Hering wrote: > Am Wed, 11 Mar 2020 11:39:08 +0100 > schrieb Daniel Kiper : > >> Nothing suspicious pooped out. I expect that the issues you are >> hitting are related to set of configure flags which you are using. > > It turned out --disable-mm-debug does for some reason fix the build with > gcc7. But building with gcc48 does still trigger the failure. > That's expected because unless specified, the default for GCC 4.8 is -std=gnu90 (C90 with GNU extensions) but the restrict type qualifier was introduced in C99. Does the following patch solve the issue? From 2e9879df9ddd4744f7b6c55bac9cf335e5832243 Mon Sep 17 00:00:00 2001 From: Javier Martinez Canillas Date: Wed, 11 Mar 2020 12:03:54 +0100 Subject: [PATCH] misc: Disable the restrict keyword when not building with C99 The restrict type qualifier was introduced in the C99 standard, so it will lead to a compile error if GRUB is not built with -std=c99 or newer. This check comes from grub-core/lib/zstd/xxhash.h file that does the same. Signed-off-by: Javier Martinez Canillas --- include/grub/misc.h | 4 1 file changed, 4 insertions(+) diff --git a/include/grub/misc.h b/include/grub/misc.h index cd5a8b4ae82..878ac18da7e 100644 --- a/include/grub/misc.h +++ b/include/grub/misc.h @@ -27,6 +27,10 @@ #include #include +#if !(defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 199901L)) /* ! C99 */ +# define restrict /* disable restrict */ +#endif + #define ALIGN_UP(addr, align) \ ((addr + (typeof (addr)) align - 1) & ~((typeof (addr)) align - 1)) #define ALIGN_UP_OVERHEAD(addr, align) ((-(addr)) & ((typeof (addr)) (align) - 1)) -- 2.24.1 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Warn if MBR gap is small and user uses advanced modules
Adding Michael, Mihai, Javier and Peter...
Below you can find what more or less Vladimir and I agreed WRT small MBR
gap. In general Vladimir convinced me to phase out small MBR gaps
support gradually. This is first step in this journey. We think that we
have to build some warnings into the code and extend documentation.
Please chime in what you think about that...
On Tue, Mar 10, 2020 at 01:23:10PM +0100, Vladimir 'phcoder' Serbinenko wrote:
> Daniel, do you want to adjust the whitelist?
>
> We don't want to support small MBR gap in pair with anything but
> the simplest config of biosdisk+part_msdos+simple filesystem. In this
> path "simple filesystems" are all current filesystems except zfs and
> btrfs
Missing SOB...
> ---
> grub-core/partmap/gpt.c | 9 -
> grub-core/partmap/msdos.c | 7 ++-
> include/grub/partition.h| 3 ++-
> include/grub/util/install.h | 7 +--
> util/grub-install-common.c | 24
> util/grub-install.c | 10 +++---
> util/grub-setup.c | 2 +-
> util/setup.c| 5 +++--
> 8 files changed, 56 insertions(+), 11 deletions(-)
>
> diff --git a/grub-core/partmap/gpt.c b/grub-core/partmap/gpt.c
> index 103f6796f..25a5a1934 100644
> --- a/grub-core/partmap/gpt.c
> +++ b/grub-core/partmap/gpt.c
> @@ -25,6 +25,9 @@
> #include
> #include
> #include
> +#ifdef GRUB_UTIL
> +#include
> +#endif
>
> GRUB_MOD_LICENSE ("GPLv3+");
>
> @@ -169,7 +172,8 @@ static grub_err_t
> gpt_partition_map_embed (struct grub_disk *disk, unsigned int *nsectors,
> unsigned int max_nsectors,
> grub_embed_type_t embed_type,
> - grub_disk_addr_t **sectors)
> + grub_disk_addr_t **sectors,
> + int warn_short)
> {
>struct gpt_partition_map_embed_ctx ctx = {
> .start = 0,
> @@ -191,6 +195,9 @@ gpt_partition_map_embed (struct grub_disk *disk,
> unsigned int *nsectors,
> N_("this GPT partition label contains no BIOS Boot Partition;"
> " embedding won't be possible"));
>
> + if (ctx.len < 450) {
Could you define constant somewhere?
Is it in sectors? Why 450? Should not it be 2048 if 1 MiB below?
...and missing "&& warn_short" check...
> +grub_util_warn("Your BIOS Boot Partition is under 1 MiB, please
> increase its size.");
> + }
>if (ctx.len < *nsectors)
Could not we use this check somehow instead of adding new one?
> return grub_error (GRUB_ERR_OUT_OF_RANGE,
> N_("your BIOS Boot Partition is too small;"
> diff --git a/grub-core/partmap/msdos.c b/grub-core/partmap/msdos.c
> index 7b8e45076..402bce050 100644
> --- a/grub-core/partmap/msdos.c
> +++ b/grub-core/partmap/msdos.c
> @@ -236,7 +236,8 @@ static grub_err_t
> pc_partition_map_embed (struct grub_disk *disk, unsigned int *nsectors,
> unsigned int max_nsectors,
> grub_embed_type_t embed_type,
> - grub_disk_addr_t **sectors)
> + grub_disk_addr_t **sectors,
> + int warn_short)
> {
>grub_disk_addr_t end = ~0ULL;
>struct grub_msdos_partition_mbr mbr;
> @@ -390,6 +391,10 @@ pc_partition_map_embed (struct grub_disk *disk,
> unsigned int *nsectors,
>return GRUB_ERR_NONE;
> }
>
> + if (end < 450 && warn_short) {
> +grub_util_warn("You have a short MBR gap and use advanced config.
> Please increase post-MBR gap");
Ditto?
> + }
> +
>if (end <= 1)
> return grub_error (GRUB_ERR_FILE_NOT_FOUND,
> N_("this msdos-style partition label has no "
> diff --git a/include/grub/partition.h b/include/grub/partition.h
> index 7adb7ec6e..5697e28d5 100644
> --- a/include/grub/partition.h
> +++ b/include/grub/partition.h
> @@ -55,7 +55,8 @@ struct grub_partition_map
>grub_err_t (*embed) (struct grub_disk *disk, unsigned int *nsectors,
> unsigned int max_nsectors,
> grub_embed_type_t embed_type,
> -grub_disk_addr_t **sectors);
> +grub_disk_addr_t **sectors,
> +int warn_short);
> #endif
> };
> typedef struct grub_partition_map *grub_partition_map_t;
> diff --git a/include/grub/util/install.h b/include/grub/util/install.h
> index 2631b1074..982115f57 100644
> --- a/include/grub/util/install.h
> +++ b/include/grub/util/install.h
> @@ -193,13 +193,13 @@ grub_util_bios_setup (const char *dir,
> const char *boot_file, const char *core_file,
> const char *dest, int force,
> int fs_probe, int allow_floppy,
> - int add_rs_codes);
> + int add_rs_codes, int warn_short_mbr_gap);
> void
> grub_util_sparc_setup (const char *dir,
> const char *boot_file, const char *core_file,
> const char *dest, int force,
> int fs_probe, int allow_floppy,
> -int add_rs_codes);
> +int add_rs_codes, int warn_short_mbr_gap);
>
> char *
> grub_install_get_image_targets_string (void);
> @@ -265,4 +265,7 @@ grub_util_get_target_name (const struct
> grub_install_image_target_desc *t);
> extern char *grub_install_copy_buffer;
> #define GRUB_INSTALL_COPY_BUFFER_SIZE 1048576
>
> +int
extern int
> +gr
Re: [PATCH] Warn if MBR gap is small and user uses advanced modules
Hello Daniel, On 3/11/20 1:43 PM, Daniel Kiper wrote: > Adding Michael, Mihai, Javier and Peter... > > Below you can find what more or less Vladimir and I agreed WRT small MBR > gap. In general Vladimir convinced me to phase out small MBR gaps > support gradually. This is first step in this journey. We think that we > have to build some warnings into the code and extend documentation. > Please chime in what you think about that... > I agree that is a good approach. And I'm happy to keep carrying the patch that adds file/line to grub_error() in Fedora until we don't have the gap size restriction anymore. For Vladimir's patch, I see that you pointed out things so I'll wait for v2 to do a review. Best regards, -- Javier Martinez Canillas Software Engineer - Desktop Hardware Enablement Red Hat ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Warn if MBR gap is small and user uses advanced modules
On Wed, Mar 11, 2020 at 01:43:14PM +0100, Daniel Kiper wrote:
> Adding Michael, Mihai, Javier and Peter...
>
> Below you can find what more or less Vladimir and I agreed WRT small MBR
> gap. In general Vladimir convinced me to phase out small MBR gaps
> support gradually. This is first step in this journey. We think that we
> have to build some warnings into the code and extend documentation.
> Please chime in what you think about that...
Usually I am also not in favor of making radical changes, so I could
share the opinions to take gradual change here. Even though I just
composed the patch to stop small mbr gap support, the diversity of
viewing the problem was still expected. In the end if that helps to
foster a better approach to improve the overall situation that's still a
worthwhile effort ...
>
> On Tue, Mar 10, 2020 at 01:23:10PM +0100, Vladimir 'phcoder' Serbinenko wrote:
> > Daniel, do you want to adjust the whitelist?
> >
> > We don't want to support small MBR gap in pair with anything but
> > the simplest config of biosdisk+part_msdos+simple filesystem. In this
> > path "simple filesystems" are all current filesystems except zfs and
> > btrfs
What about lvm, md and cryto disks here ? They could have simple file
system formatted on top, but the size of the core image can go much
bigger.
In addition, a parallel problem I want to address here is that grub
modules get copied to the platform directory way too early regardless of
the image setup result being successful or not. We'd better fix that
problem as well or the "warning" would actually be "critical" in case of
running into 'undefined symbol error' that renders system unbootable.
Thanks,
Michael
>
> Missing SOB...
>
> > ---
> > grub-core/partmap/gpt.c | 9 -
> > grub-core/partmap/msdos.c | 7 ++-
> > include/grub/partition.h| 3 ++-
> > include/grub/util/install.h | 7 +--
> > util/grub-install-common.c | 24
> > util/grub-install.c | 10 +++---
> > util/grub-setup.c | 2 +-
> > util/setup.c| 5 +++--
> > 8 files changed, 56 insertions(+), 11 deletions(-)
> >
> > diff --git a/grub-core/partmap/gpt.c b/grub-core/partmap/gpt.c
> > index 103f6796f..25a5a1934 100644
> > --- a/grub-core/partmap/gpt.c
> > +++ b/grub-core/partmap/gpt.c
> > @@ -25,6 +25,9 @@
> > #include
> > #include
> > #include
> > +#ifdef GRUB_UTIL
> > +#include
> > +#endif
> >
> > GRUB_MOD_LICENSE ("GPLv3+");
> >
> > @@ -169,7 +172,8 @@ static grub_err_t
> > gpt_partition_map_embed (struct grub_disk *disk, unsigned int *nsectors,
> > unsigned int max_nsectors,
> > grub_embed_type_t embed_type,
> > - grub_disk_addr_t **sectors)
> > + grub_disk_addr_t **sectors,
> > + int warn_short)
> > {
> >struct gpt_partition_map_embed_ctx ctx = {
> > .start = 0,
> > @@ -191,6 +195,9 @@ gpt_partition_map_embed (struct grub_disk *disk,
> > unsigned int *nsectors,
> > N_("this GPT partition label contains no BIOS Boot Partition;"
> > " embedding won't be possible"));
> >
> > + if (ctx.len < 450) {
>
> Could you define constant somewhere?
>
> Is it in sectors? Why 450? Should not it be 2048 if 1 MiB below?
>
> ...and missing "&& warn_short" check...
>
> > +grub_util_warn("Your BIOS Boot Partition is under 1 MiB, please
> > increase its size.");
> > + }
> >if (ctx.len < *nsectors)
>
> Could not we use this check somehow instead of adding new one?
>
> > return grub_error (GRUB_ERR_OUT_OF_RANGE,
> > N_("your BIOS Boot Partition is too small;"
> > diff --git a/grub-core/partmap/msdos.c b/grub-core/partmap/msdos.c
> > index 7b8e45076..402bce050 100644
> > --- a/grub-core/partmap/msdos.c
> > +++ b/grub-core/partmap/msdos.c
> > @@ -236,7 +236,8 @@ static grub_err_t
> > pc_partition_map_embed (struct grub_disk *disk, unsigned int *nsectors,
> > unsigned int max_nsectors,
> > grub_embed_type_t embed_type,
> > - grub_disk_addr_t **sectors)
> > + grub_disk_addr_t **sectors,
> > + int warn_short)
> > {
> >grub_disk_addr_t end = ~0ULL;
> >struct grub_msdos_partition_mbr mbr;
> > @@ -390,6 +391,10 @@ pc_partition_map_embed (struct grub_disk *disk,
> > unsigned int *nsectors,
> >return GRUB_ERR_NONE;
> > }
> >
> > + if (end < 450 && warn_short) {
> > +grub_util_warn("You have a short MBR gap and use advanced config.
> > Please increase post-MBR gap");
>
> Ditto?
>
> > + }
> > +
> >if (end <= 1)
> > return grub_error (GRUB_ERR_FILE_NOT_FOUND,
> > N_("this msdos-style partition label has no "
> > diff --git a/include/grub/partition.h b/include/grub/partition.h
> > index 7adb7ec6e..5697e28d5 100644
> > --- a/include/grub/partition.h
> > +++ b/include/grub/partition.h
> > @@ -55,7 +55,8 @@ struct grub_partition_map
> >grub_err_t (*embed) (struct grub_disk *disk, unsigned int *nsectors,
> > unsigned int max_nsectors,
> > grub_embed_type_t embed_type,
> > -grub_disk_a
Re: compile error in master, restrict is missing
Am Wed, 11 Mar 2020 12:30:01 +0100 schrieb Javier Martinez Canillas : > +#if !(defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 199901L)) This does not help. I have seen similar code in one of the zstd headers. Whatever the outcome will be, it should get a 'Fixes commit d5a32255d' tag. Olaf pgpiEZ3rgnXnK.pgp Description: Digitale Signatur von OpenPGP ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: compile error in master, restrict is missing
On 3/11/20 4:41 PM, Olaf Hering wrote: > Am Wed, 11 Mar 2020 12:30:01 +0100 > schrieb Javier Martinez Canillas : > >> +#if !(defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 199901L)) > > This does not help. I have seen similar code in one of the zstd headers. > Yes, I mentioned in the commit message that it was taken from there... I would had expected that change to avoid the build failure. I wonder why it didn't help. > Whatever the outcome will be, it should get a 'Fixes commit d5a32255d' tag. > Agreed. Daniel said that instead he will add a -std=gnu99 option so the compiler uses C99 even if is not its default. > Olaf > Best regards, -- Javier Martinez Canillas Software Engineer - Desktop Hardware Enablement Red Hat ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Warn if MBR gap is small and user uses advanced modules
Le 11/03/2020 à 13:43, Daniel Kiper a écrit :
> Adding Michael, Mihai, Javier and Peter...
>
> Below you can find what more or less Vladimir and I agreed WRT small MBR
> gap. In general Vladimir convinced me to phase out small MBR gaps
> support gradually. This is first step in this journey. We think that we
> have to build some warnings into the code and extend documentation.
> Please chime in what you think about that...
>
> On Tue, Mar 10, 2020 at 01:23:10PM +0100, Vladimir 'phcoder' Serbinenko wrote:
>> Daniel, do you want to adjust the whitelist?
>>
>> We don't want to support small MBR gap in pair with anything but
>> the simplest config of biosdisk+part_msdos+simple filesystem. In this
>> path "simple filesystems" are all current filesystems except zfs and
>> btrfs
>
> Missing SOB...
>
>> ---
>> grub-core/partmap/gpt.c | 9 -
>> grub-core/partmap/msdos.c | 7 ++-
>> include/grub/partition.h| 3 ++-
>> include/grub/util/install.h | 7 +--
>> util/grub-install-common.c | 24
>> util/grub-install.c | 10 +++---
>> util/grub-setup.c | 2 +-
>> util/setup.c| 5 +++--
>> 8 files changed, 56 insertions(+), 11 deletions(-)
>>
>> diff --git a/grub-core/partmap/gpt.c b/grub-core/partmap/gpt.c
>> index 103f6796f..25a5a1934 100644
>> --- a/grub-core/partmap/gpt.c
>> +++ b/grub-core/partmap/gpt.c
>> @@ -25,6 +25,9 @@
>> #include
>> #include
>> #include
>> +#ifdef GRUB_UTIL
>> +#include
>> +#endif
>>
>> GRUB_MOD_LICENSE ("GPLv3+");
>>
>> @@ -169,7 +172,8 @@ static grub_err_t
>> gpt_partition_map_embed (struct grub_disk *disk, unsigned int *nsectors,
>> unsigned int max_nsectors,
>> grub_embed_type_t embed_type,
>> - grub_disk_addr_t **sectors)
>> + grub_disk_addr_t **sectors,
>> + int warn_short)
>> {
>>struct gpt_partition_map_embed_ctx ctx = {
>> .start = 0,
>> @@ -191,6 +195,9 @@ gpt_partition_map_embed (struct grub_disk *disk,
>> unsigned int *nsectors,
>> N_("this GPT partition label contains no BIOS Boot Partition;"
>> " embedding won't be possible"));
>>
>> + if (ctx.len < 450) {
>
> Could you define constant somewhere?
>
> Is it in sectors? Why 450? Should not it be 2048 if 1 MiB below?
Whichs lead to a question: would the slot between 24K (or maybe
32K or 64K if it is wise to round it?) and 1M still a good fit for
a Bios boot partition in case of a gpt? if not in which cases?
> ...and missing "&& warn_short" check...
>
>> +grub_util_warn("Your BIOS Boot Partition is under 1 MiB, please
>> increase its size.");
>> + }
>>if (ctx.len < *nsectors)
>
> Could not we use this check somehow instead of adding new one?
>
>> return grub_error (GRUB_ERR_OUT_OF_RANGE,
>> N_("your BIOS Boot Partition is too small;"
>> diff --git a/grub-core/partmap/msdos.c b/grub-core/partmap/msdos.c
>> index 7b8e45076..402bce050 100644
>> --- a/grub-core/partmap/msdos.c
>> +++ b/grub-core/partmap/msdos.c
>> @@ -236,7 +236,8 @@ static grub_err_t
>> pc_partition_map_embed (struct grub_disk *disk, unsigned int *nsectors,
>> unsigned int max_nsectors,
>> grub_embed_type_t embed_type,
>> - grub_disk_addr_t **sectors)
>> + grub_disk_addr_t **sectors,
>> + int warn_short)
>> {
>>grub_disk_addr_t end = ~0ULL;
>>struct grub_msdos_partition_mbr mbr;
>> @@ -390,6 +391,10 @@ pc_partition_map_embed (struct grub_disk *disk,
>> unsigned int *nsectors,
>>return GRUB_ERR_NONE;
>> }
>>
>> + if (end < 450 && warn_short) {
>> +grub_util_warn("You have a short MBR gap and use advanced config.
>> Please increase post-MBR gap");
>
> Ditto?
>
>> + }
>> +
>>if (end <= 1)
>> return grub_error (GRUB_ERR_FILE_NOT_FOUND,
>> N_("this msdos-style partition label has no "
>> diff --git a/include/grub/partition.h b/include/grub/partition.h
>> index 7adb7ec6e..5697e28d5 100644
>> --- a/include/grub/partition.h
>> +++ b/include/grub/partition.h
>> @@ -55,7 +55,8 @@ struct grub_partition_map
>>grub_err_t (*embed) (struct grub_disk *disk, unsigned int *nsectors,
>> unsigned int max_nsectors,
>> grub_embed_type_t embed_type,
>> -grub_disk_addr_t **sectors);
>> +grub_disk_addr_t **sectors,
>> +int warn_short);
>> #endif
>> };
>> typedef struct grub_partition_map *grub_partition_map_t;
>> diff --git a/include/grub/util/install.h b/include/grub/util/install.h
>> index 2631b1074..982115f57 100644
>> --- a/include/grub/util/install.h
>> +++ b/include/grub/util/install.h
>> @@ -193,13 +193,13 @@ grub_util_bios_setup (const char *dir,
>> const char *boot_file, const char *core_file,
>> const char *dest, int force,
>> int fs_probe, int allow_floppy,
>> - int add_rs_codes);
>> + int add_rs_codes, int warn_short_mbr_gap);
>> void
>> grub_util_sparc_setup (const char *dir,
>> const char *boot_file, const char *core_file,
>> const char *dest, int force,
>>
[PATCH 0/7] ZFS/other CoW FS save_env support
Hey all, I previously discussed my concept for this patch in my email https://lists.gnu.org/archive/html/grub-devel/2020-01/msg4.html . I'm pleased to announce that I've gotten it into a working state and it is ready to review. There are a number of changes here, which I will break down below. First, the concept of "special files" is introduced into grub. These are files that are manipulated using different functions from the remainder of the filesystem. A filesystem advertises its support for these files by filling in new entries in the grub_fs struct. Second, the loadenv command was modified to use the special file functions of the root filesystem if they are detected. If that happens, these functions are used to open, read, and write to the loadenv file instead of the normal grub file functions. A variable was also added that allows the user to force a file to be used instead of the special files, or vice versa. Third, the grub-editenv command was modified to teach it to probe the root filesystem and then check in an array of structures that contain functions that will read and modify the filesystem's special file in userland. These functions are very similar to normal read and write functions, and so don't result in a very different code flow. Finally, the GRUB ZFS logic was modified to have new special_file functions. These functions manipulate the pad2 area of the ZFS label, which was previously unused. It now stores a version number, the raw contents of the grubenv file, and an embedded checksum for integrity purposes. GRUB was taught to read and verify these areas, and also to modify them, computing the embeddded checksum appropriately. In addition, if GRUB is build with libzfs support, an entry is added to the grub-editenv table that points GRUB to the appropriate libzfs functions, and init and fini functions are built into grub-editenv itself. Future work: * In the ZFS code could store a packed nvlist instead of a raw file, but this would involve further changes to the grub environment file code and was deferred for when it would be more useful and important. * For the special files code, there is only one special file allowed per filesystem, but a specification interface (using either an enum or a set of names) could be added in the future. * Other filesystem support was not built into this change, but extensibility was a primary goal, so adding support for btrfs or other filesystems should be relatively straightforward. * I did not implement the proposed UEFI variable support, but I did develop this with that future in mind, so adding it should be a relatively simple extension of these changes. This patchset has been tested on systems with and without ZFS as the boot filesystem, and built with and without ZFS libraries installed. It worked in each case I tried, including with manual corruption applied to the ZFS labels to force GRUB to read from the other label. This was tested in conjunction with https://github.com/zfsonlinux/zfs/pull/10009 , the ZoL patch that enables ZFS to read from and write to the padding areas we reserved for the data. Paul Dagnelie (7): Factor out file filter function for reuse Add support for special envblk functions in GRUB Factor out envblk buffer creation for reuse Use envblk file and fs functions to implement reading the grubenv file from special FS regions Add ZFS envblock functions Refactor default envblk size out for reuse Update editenv to support editing zfs envblock, fix whitespace, and autodetect bootenv support in libzfs configure.ac | 8 + grub-core/commands/loadenv.c | 122 +-- grub-core/fs/zfs/zfs.c | 134 +++-- grub-core/kern/file.c| 76 +++--- include/grub/file.h | 10 ++ include/grub/fs.h| 12 ++ include/grub/lib/envblk.h| 3 + include/grub/util/install.h | 3 + include/grub/util/libzfs.h | 7 + include/grub/zfs/vdev_impl.h | 33 ++--- util/editenv.c | 31 ++-- util/grub-editenv.c | 278 +-- 12 files changed, 632 insertions(+), 85 deletions(-) -- 2.19.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 4/7] Use envblk file and fs functions to implement reading the grubenv file from special FS regions
This patch modifies the loadenv command to take advantage of the new envblock
functions provided by a previous patch. These functions will be used instead
of the normal file-based functions if the boot filesystem is detected to
support them. There is also a config variable, grubenv_src, that can be used
to force the file or envblock logic to be used.
Signed-off-by: Paul Dagnelie
---
grub-core/commands/loadenv.c | 122 ++-
include/grub/lib/envblk.h| 2 +
2 files changed, 109 insertions(+), 15 deletions(-)
diff --git a/grub-core/commands/loadenv.c b/grub-core/commands/loadenv.c
index 3fd664aac..66f5345ec 100644
--- a/grub-core/commands/loadenv.c
+++ b/grub-core/commands/loadenv.c
@@ -79,6 +79,94 @@ open_envblk_file (char *filename,
return file;
}
+static grub_file_t
+open_envblk_block (grub_fs_t fs, grub_device_t dev, enum grub_file_type type)
+{
+ if (fs->fs_envblk_open)
+return grub_file_envblk_open(fs, dev, type);
+ return NULL;
+}
+
+static grub_file_t
+open_envblk (grub_extcmd_context_t ctxt, enum grub_file_type type)
+{
+ struct grub_arg_list *state = ctxt->state;
+ grub_file_t file = NULL;
+ grub_device_t device = NULL;
+ const char *source;
+ grub_fs_t fs = NULL;
+ char *filename = state[0].set ? state[0].arg : NULL;
+
+ source = grub_env_get ("grubenv_src");
+ if (! source || ! grub_strcmp (source, GRUB_ENVBLK_SRC_BLK))
+{
+ char *device_name;
+ const char *prefix;
+ grub_dprintf ("loadenv", "checking for envblk\n");
+
+ prefix = grub_env_get ("prefix");
+ if (! prefix)
+{
+ grub_error (GRUB_ERR_FILE_NOT_FOUND, N_("variable `%s' isn't set"),
"prefix");
+ return NULL;
+}
+
+ device_name = grub_file_get_device_name (prefix);
+ if (grub_errno)
+ return NULL;
+
+ device = grub_device_open (device_name);
+ if (! device)
+ return NULL;
+
+ fs = grub_fs_probe (device);
+ if (! fs) {
+grub_device_close (device);
+ return NULL;
+ }
+
+ /* We have to reopen the device here because it was closed in
grub_fs_probe. */
+ device = grub_device_open (device_name);
+ grub_free (device_name);
+ if (! device)
+ return NULL;
+
+}
+ if (! source)
+{
+ if (fs->fs_envblk_open)
+ {
+ source = GRUB_ENVBLK_SRC_BLK;
+ grub_dprintf ("loadenv", "envblk support detected\n");
+ }
+ else
+ {
+ source = GRUB_ENVBLK_SRC_FILE;
+ grub_dprintf ("loadenv", "envblk support not detected\n");
+ }
+}
+
+ if (! grub_strcmp (source, GRUB_ENVBLK_SRC_FILE))
+{
+ if (device)
+grub_device_close (device);
+ file = open_envblk_file (filename, type);
+
+ if (! file)
+ return NULL;
+}
+ else if (! grub_strcmp (source, GRUB_ENVBLK_SRC_BLK))
+{
+ file = open_envblk_block (fs, device, type);
+ if (! file)
+ {
+ grub_device_close (device);
+ return NULL;
+ }
+}
+ return file;
+}
+
static grub_envblk_t
read_envblk_file (grub_file_t file)
{
@@ -165,11 +253,10 @@ grub_cmd_load_env (grub_extcmd_context_t ctxt, int argc,
char **args)
whitelist.len = argc;
whitelist.list = args;
- /* state[0] is the -f flag; state[1] is the --skip-sig flag */
- file = open_envblk_file ((state[0].set) ? state[0].arg : 0,
- GRUB_FILE_TYPE_LOADENV
- | (state[1].set
- ? GRUB_FILE_TYPE_SKIP_SIGNATURE :
GRUB_FILE_TYPE_NONE));
+ file = open_envblk (ctxt, GRUB_FILE_TYPE_LOADENV
+| (state[1].set
+ ? GRUB_FILE_TYPE_SKIP_SIGNATURE :
+ GRUB_FILE_TYPE_NONE));
if (! file)
return grub_errno;
@@ -204,10 +291,9 @@ grub_cmd_list_env (grub_extcmd_context_t ctxt,
grub_file_t file;
grub_envblk_t envblk;
- file = open_envblk_file ((state[0].set) ? state[0].arg : 0,
- GRUB_FILE_TYPE_LOADENV
- | (state[1].set
- ? GRUB_FILE_TYPE_SKIP_SIGNATURE :
GRUB_FILE_TYPE_NONE));
+ file = open_envblk (ctxt, GRUB_FILE_TYPE_LOADENV
+ | (state[1].set
+ ? GRUB_FILE_TYPE_SKIP_SIGNATURE :
GRUB_FILE_TYPE_NONE));
if (! file)
return grub_errno;
@@ -379,7 +465,6 @@ save_env_read_hook (grub_disk_addr_t sector, unsigned
offset, unsigned length,
static grub_err_t
grub_cmd_save_env (grub_extcmd_context_t ctxt, int argc, char **args)
{
- struct grub_arg_list *state = ctxt->state;
grub_file_t file;
grub_envblk_t envblk;
struct grub_cmd_save_env_ctx ctx = {
@@ -390,9 +475,8 @@ grub_cmd_save_env (grub_extcmd_context_t ctxt, int argc,
char **args)
if (! argc)
return grub_error (GRUB_ERR_BAD_ARGUMENT, "no variable is specified");
- file = open_envblk_file ((state[0].set) ? state[0].arg : 0,
-
[PATCH 2/7] Add support for special envblk functions in GRUB
This patch introducces the idea of envblk functions to GRUB. These functions
are used to interact with a grubenv file stored not in the usual grub tree,
but instead in a special filesystem-managed region that can more easily be
accessed at boot time. These functions interface between calling code and the
per-fs logic, an interface for which is also added as part of this patch.
Signed-off-by: Paul Dagnelie
---
grub-core/kern/file.c | 30 --
include/grub/file.h | 10 ++
include/grub/fs.h | 12
3 files changed, 50 insertions(+), 2 deletions(-)
diff --git a/grub-core/kern/file.c b/grub-core/kern/file.c
index 75eb5e2fa..f929f61a0 100644
--- a/grub-core/kern/file.c
+++ b/grub-core/kern/file.c
@@ -149,6 +149,27 @@ grub_file_open (const char *name, enum grub_file_type type)
return 0;
}
+grub_file_t
+grub_file_envblk_open (grub_fs_t fs, grub_device_t device, enum grub_file_type
type)
+{
+ grub_file_t file = NULL;
+ file = (grub_file_t) grub_zalloc (sizeof (*file));
+ if (! file)
+return NULL;
+ file->device = device;
+ file->special = 1;
+
+ if ((fs->fs_envblk_open) (file) != GRUB_ERR_NONE) {
+grub_free(file);
+return NULL;
+ }
+
+ file->fs = fs;
+ grub_errno = GRUB_ERR_NONE;
+
+ return grub_apply_file_filters(file, type, NULL);
+}
+
grub_disk_read_hook_t grub_file_progress_hook;
grub_ssize_t
@@ -185,7 +206,10 @@ grub_file_read (grub_file_t file, void *buf, grub_size_t
len)
file->read_hook_data = file;
file->progress_offset = file->offset;
}
- res = (file->fs->fs_read) (file, buf, len);
+ if (grub_file_envblk (file))
+res = (file->fs->fs_envblk_read) (file, buf, len);
+ else
+res = (file->fs->fs_read) (file, buf, len);
file->read_hook = read_hook;
file->read_hook_data = read_hook_data;
if (res > 0)
@@ -197,7 +221,9 @@ grub_file_read (grub_file_t file, void *buf, grub_size_t
len)
grub_err_t
grub_file_close (grub_file_t file)
{
- if (file->fs->fs_close)
+ if (grub_file_envblk (file) && file->fs->fs_envblk_close)
+(file->fs->fs_envblk_close) (file);
+ else if (file->fs->fs_close)
(file->fs->fs_close) (file);
if (file->device)
diff --git a/include/grub/file.h b/include/grub/file.h
index 31567483c..1cc688f55 100644
--- a/include/grub/file.h
+++ b/include/grub/file.h
@@ -170,6 +170,9 @@ struct grub_file
/* Caller-specific data passed to the read hook. */
void *read_hook_data;
+
+ /* If the file is an FS's special file, which uses separate functions for
interaction. */
+ int special;
};
typedef struct grub_file *grub_file_t;
@@ -211,6 +214,7 @@ grub_ssize_t EXPORT_FUNC(grub_file_read) (grub_file_t file,
void *buf,
grub_size_t len);
grub_off_t EXPORT_FUNC(grub_file_seek) (grub_file_t file, grub_off_t offset);
grub_err_t EXPORT_FUNC(grub_file_close) (grub_file_t file);
+grub_file_t EXPORT_FUNC(grub_file_envblk_open) (grub_fs_t fs, grub_device_t
device, enum grub_file_type type);
/* Return value of grub_file_size() in case file size is unknown. */
#define GRUB_FILE_SIZE_UNKNOWN 0xULL
@@ -233,6 +237,12 @@ grub_file_seekable (const grub_file_t file)
return !file->not_easily_seekable;
}
+static inline int
+grub_file_envblk (const grub_file_t file)
+{
+ return file->special;
+}
+
grub_file_t
grub_file_offset_open (grub_file_t parent, enum grub_file_type type,
grub_off_t start, grub_off_t size);
diff --git a/include/grub/fs.h b/include/grub/fs.h
index 302e48d4b..ec1a7a50e 100644
--- a/include/grub/fs.h
+++ b/include/grub/fs.h
@@ -96,6 +96,18 @@ struct grub_fs
/* Whether blocklist installs have a chance to work. */
int blocklist_install;
#endif
+
+ /*
+ * The envblk functions are defined on filesystems that need to handle
+ * grub-writable files in a special way. This is most commonly the case for
+ * CoW filesystems like btrfs and ZFS. The normal read and close functions
+ * should detect that they are being called on a special file and act
+ * appropriately.
+ */
+ grub_err_t (*fs_envblk_open) (struct grub_file *file);
+ grub_ssize_t (*fs_envblk_read) (struct grub_file *file, char *buf,
grub_size_t len);
+ grub_err_t (*fs_envblk_write) (struct grub_file *file, char *buf,
grub_size_t len);
+ grub_err_t (*fs_envblk_close) (struct grub_file *file);
};
typedef struct grub_fs *grub_fs_t;
--
2.19.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 6/7] Refactor default envblk size out for reuse
This patch refactors the DEFAULT_ENVBLK_SIZE definition into a header file for
reuse in other code, and renames it to match other definitions in that header.
Signed-off-by: Paul Dagnelie
---
include/grub/lib/envblk.h | 1 +
util/editenv.c| 9 -
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/grub/lib/envblk.h b/include/grub/lib/envblk.h
index 287c87105..b9067dc37 100644
--- a/include/grub/lib/envblk.h
+++ b/include/grub/lib/envblk.h
@@ -23,6 +23,7 @@
#define GRUB_ENVBLK_DEFCFG "grubenv"
#define GRUB_ENVBLK_SRC_BLK"block"
#define GRUB_ENVBLK_SRC_FILE "file"
+#define GRUB_ENVBLK_DEFAULT_SIZE 1024
#ifndef ASM_FILE
diff --git a/util/editenv.c b/util/editenv.c
index 45aeba259..ef1aba25b 100644
--- a/util/editenv.c
+++ b/util/editenv.c
@@ -29,13 +29,12 @@
#include
#include
-#define DEFAULT_ENVBLK_SIZE1024
#define GRUB_ENVBLK_MESSAGE"# WARNING: Do not edit this file by tools
other than "PACKAGE"-editenv!!!\n"
void
grub_util_create_envblk_buffer (char *buf, size_t size)
{
- if (size < DEFAULT_ENVBLK_SIZE)
+ if (size < GRUB_ENVBLK_DEFAULT_SIZE)
grub_util_error (_("Envblock buffer too small"));
char *pbuf;
pbuf = buf;
@@ -53,8 +52,8 @@ grub_util_create_envblk_file (const char *name)
FILE *fp;
char *buf, *namenew;
- buf = xmalloc (DEFAULT_ENVBLK_SIZE);
- grub_util_create_envblk_buffer(buf, DEFAULT_ENVBLK_SIZE);
+ buf = xmalloc (GRUB_ENVBLK_DEFAULT_SIZE);
+ grub_util_create_envblk_buffer(buf, GRUB_ENVBLK_DEFAULT_SIZE);
namenew = xasprintf ("%s.new", name);
fp = grub_util_fopen (namenew, "wb");
@@ -62,7 +61,7 @@ grub_util_create_envblk_file (const char *name)
grub_util_error (_("cannot open `%s': %s"), namenew,
strerror (errno));
- if (fwrite (buf, 1, DEFAULT_ENVBLK_SIZE, fp) != DEFAULT_ENVBLK_SIZE)
+ if (fwrite (buf, 1, GRUB_ENVBLK_DEFAULT_SIZE, fp) !=
GRUB_ENVBLK_DEFAULT_SIZE)
grub_util_error (_("cannot write to `%s': %s"), namenew,
strerror (errno));
--
2.19.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 7/7] Update editenv to support editing zfs envblock, fix whitespace, and autodetect bootenv support in libzfs
This patch updates the grub-editenv command to support editing the zfs
envblock by using specific libzfs functions. In order to ensure that GRUB will
continue to build against both old and new versions of libzfs, logic was also
added to the configure script to detect if the bootenv functions are present
in the version of libzfs present at build time. There is also a small
whitespace fix in grub-editenv unrelated to the content of these changes.
Signed-off-by: Paul Dagnelie
---
configure.ac | 8 ++
include/grub/util/libzfs.h | 7 +
util/grub-editenv.c| 278 +++--
3 files changed, 284 insertions(+), 9 deletions(-)
diff --git a/configure.ac b/configure.ac
index 7d74eba66..7107cc3b8 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1853,6 +1853,14 @@ if test x"$libzfs_excuse" = x ; then
LIBNVPAIR="-lnvpair"
AC_DEFINE([HAVE_LIBNVPAIR], [1],
[Define to 1 if you have the NVPAIR library.])
+
+ AC_CHECK_LIB([zfs], [zpool_get_bootenv],
+ [libzfs_have_bootenv="yes"],
+ [])
+ if test x"$libzfs_have_bootenv" = xyes ; then
+AC_DEFINE([HAVE_LIBZFS_BOOTENV], [1],
+ [Define to 1 if you have a version of the ZFS library with
bootenv support.])
+ fi
fi
AC_SUBST([LIBZFS])
diff --git a/include/grub/util/libzfs.h b/include/grub/util/libzfs.h
index a02caa335..5ebc085d8 100644
--- a/include/grub/util/libzfs.h
+++ b/include/grub/util/libzfs.h
@@ -40,6 +40,13 @@ extern int zpool_get_physpath (zpool_handle_t *, const char
*);
extern nvlist_t *zpool_get_config (zpool_handle_t *, nvlist_t **);
+extern libzfs_handle_t *zpool_get_handle (zpool_handle_t *);
+
+extern int zpool_set_bootenv (zpool_handle_t *, const char *);
+extern int zpool_get_bootenv (zpool_handle_t *, char *, size_t, off_t);
+
+extern int libzfs_errno (libzfs_handle_t *);
+
#endif /* ! HAVE_LIBZFS_H */
libzfs_handle_t *grub_get_libzfs_handle (void);
diff --git a/util/grub-editenv.c b/util/grub-editenv.c
index f3662c95b..a282f45ee 100644
--- a/util/grub-editenv.c
+++ b/util/grub-editenv.c
@@ -24,7 +24,12 @@
#include
#include
#include
+#include
#include
+#include
+#include
+#include
+#include
#include
#include
@@ -36,7 +41,6 @@
#pragma GCC diagnostic error "-Wmissing-prototypes"
#pragma GCC diagnostic error "-Wmissing-declarations"
-
#include "progname.h"
#define DEFAULT_ENVBLK_PATH DEFAULT_DIRECTORY "/" GRUB_ENVBLK_DEFCFG
@@ -120,6 +124,79 @@ block, use `rm %s'."),
NULL, help_filter, NULL
};
+struct fs_envblk_spec {
+ const char *fs_name;
+ int (*fs_read) (void *, char *, size_t, off_t);
+ int (*fs_write) (void *, const char *);
+ void *(*fs_init) (grub_device_t);
+ void (*fs_fini) (void *);
+};
+typedef struct fs_envblk_spec fs_envblk_spec_t;
+
+struct fs_envblk {
+ struct fs_envblk_spec *spec;
+ grub_fs_t fs;
+ void *data;
+};
+typedef struct fs_envblk fs_envblk_t;
+
+fs_envblk_t *fs_envblk = NULL;
+
+#if defined(HAVE_LIBZFS) && defined(HAVE_LIBNVPAIR) &&
defined(HAVE_LIBZFS_BOOTENV)
+static void *
+grub_zfs_init (grub_device_t dev)
+{
+ libzfs_handle_t *g_zfs = libzfs_init();
+ int err;
+ char *name;
+ zpool_handle_t *zhp;
+
+ if (g_zfs == NULL)
+return NULL;
+
+ err = fs_envblk->fs->fs_label(dev, &name);
+ if (err != GRUB_ERR_NONE) {
+libzfs_fini(g_zfs);
+return NULL;
+ }
+ zhp = zpool_open(g_zfs, name);
+ if (zhp == NULL)
+{
+ libzfs_fini(g_zfs);
+ return NULL;
+}
+ return zhp;
+}
+
+static void
+grub_zfs_fini (void *arg)
+{
+ zpool_handle_t *zhp = arg;
+ libzfs_handle_t *g_zfs = zpool_get_handle(zhp);
+ zpool_close(zhp);
+ libzfs_fini(g_zfs);
+}
+
+/* We need to convert ZFS's error returning pattern to the one we expect */
+static int
+grub_zfs_get_bootenv (void *arg, char *buf, size_t size, off_t offset)
+{
+ zpool_handle_t *zhp = arg;
+ int error = zpool_get_bootenv (zhp, buf, size, offset);
+ if (error != -1)
+return error;
+ error = libzfs_errno(zpool_get_handle(zhp));
+ return error;
+}
+#endif
+
+fs_envblk_spec_t fs_envblk_table[] = {
+#if defined(HAVE_LIBZFS) && defined(HAVE_LIBNVPAIR) &&
defined(HAVE_LIBZFS_BOOTENV)
+ { "zfs", grub_zfs_get_bootenv, zpool_set_bootenv, grub_zfs_init,
grub_zfs_fini},
+#endif
+ { NULL, NULL, NULL, NULL, NULL }
+};
+
static grub_envblk_t
open_envblk_file (const char *name)
{
@@ -164,6 +241,58 @@ open_envblk_file (const char *name)
return envblk;
}
+static grub_envblk_t
+open_envblk (const char *name)
+{
+ char *buf = NULL;
+ off_t off = 0;
+ size_t size = GRUB_ENVBLK_DEFAULT_SIZE;
+ grub_envblk_t envblk;
+
+ if (fs_envblk == NULL)
+return open_envblk_file(name);
+
+ /*
+ * For normal grubenv files, we can just use the size of the file to
+ * allocate our buffer, but envblks don't necessarily advertise their size
+ * directly. We need to instead read and realloc a larger buffer until we
+ * have read in the entire file.
+ */
+ while (1)
+{
+ int rc;
+
[PATCH 1/7] Factor out file filter function for reuse
This patch refactors out the logic that applies filters to files for reuse in
other code.
Signed-off-by: Paul Dagnelie
---
grub-core/kern/file.c | 46 +--
1 file changed, 27 insertions(+), 19 deletions(-)
diff --git a/grub-core/kern/file.c b/grub-core/kern/file.c
index 58454458c..75eb5e2fa 100644
--- a/grub-core/kern/file.c
+++ b/grub-core/kern/file.c
@@ -57,14 +57,37 @@ grub_file_get_device_name (const char *name)
return 0;
}
+static grub_file_t
+grub_apply_file_filters (grub_file_t file, enum grub_file_type type, const
char *name)
+{
+ grub_file_filter_id_t filter;
+ grub_file_t last_file = NULL;
+
+ for (filter = 0; file && filter < ARRAY_SIZE (grub_file_filters);
+ filter++)
+if (grub_file_filters[filter])
+ {
+ last_file = file;
+ file = grub_file_filters[filter] (file, type);
+ if (file && file != last_file)
+ {
+ file->name = grub_strdup (name);
+ grub_errno = GRUB_ERR_NONE;
+ }
+ }
+ if (!file)
+grub_file_close (last_file);
+
+ return file;
+}
+
grub_file_t
grub_file_open (const char *name, enum grub_file_type type)
{
- grub_device_t device = 0;
- grub_file_t file = 0, last_file = 0;
+ grub_device_t device = NULL;
+ grub_file_t file = NULL;
char *device_name;
const char *file_name;
- grub_file_filter_id_t filter;
device_name = grub_file_get_device_name (name);
if (grub_errno)
@@ -113,22 +136,7 @@ grub_file_open (const char *name, enum grub_file_type type)
file->name = grub_strdup (name);
grub_errno = GRUB_ERR_NONE;
- for (filter = 0; file && filter < ARRAY_SIZE (grub_file_filters);
- filter++)
-if (grub_file_filters[filter])
- {
- last_file = file;
- file = grub_file_filters[filter] (file, type);
- if (file && file != last_file)
- {
- file->name = grub_strdup (name);
- grub_errno = GRUB_ERR_NONE;
- }
- }
- if (!file)
-grub_file_close (last_file);
-
- return file;
+ return grub_apply_file_filters(file, type, name);
fail:
if (device)
--
2.19.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 3/7] Factor out envblk buffer creation for reuse
This patch factors out the filling of the grubenv buffer into a separate
function for reuse.
Signed-off-by: Paul Dagnelie
---
include/grub/util/install.h | 3 +++
util/editenv.c | 26 +-
2 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/include/grub/util/install.h b/include/grub/util/install.h
index 2631b1074..2ca349503 100644
--- a/include/grub/util/install.h
+++ b/include/grub/util/install.h
@@ -246,6 +246,9 @@ grub_install_get_blocklist (grub_device_t root_dev,
void *data),
void *hook_data);
+void
+grub_util_create_envblk_buffer (char *, size_t);
+
void
grub_util_create_envblk_file (const char *name);
diff --git a/util/editenv.c b/util/editenv.c
index 81f68bd10..45aeba259 100644
--- a/util/editenv.c
+++ b/util/editenv.c
@@ -32,13 +32,29 @@
#define DEFAULT_ENVBLK_SIZE1024
#define GRUB_ENVBLK_MESSAGE"# WARNING: Do not edit this file by tools
other than "PACKAGE"-editenv!!!\n"
+void
+grub_util_create_envblk_buffer (char *buf, size_t size)
+{
+ if (size < DEFAULT_ENVBLK_SIZE)
+grub_util_error (_("Envblock buffer too small"));
+ char *pbuf;
+ pbuf = buf;
+ memcpy (pbuf, GRUB_ENVBLK_SIGNATURE, sizeof (GRUB_ENVBLK_SIGNATURE) - 1);
+ pbuf += sizeof (GRUB_ENVBLK_SIGNATURE) - 1;
+ memcpy (pbuf, GRUB_ENVBLK_MESSAGE, sizeof (GRUB_ENVBLK_MESSAGE) - 1);
+ pbuf += sizeof (GRUB_ENVBLK_MESSAGE) - 1;
+ memset (pbuf , '#',
+ size - sizeof (GRUB_ENVBLK_SIGNATURE) - sizeof (GRUB_ENVBLK_MESSAGE)
+ 2);
+}
+
void
grub_util_create_envblk_file (const char *name)
{
FILE *fp;
- char *buf, *pbuf, *namenew;
+ char *buf, *namenew;
buf = xmalloc (DEFAULT_ENVBLK_SIZE);
+ grub_util_create_envblk_buffer(buf, DEFAULT_ENVBLK_SIZE);
namenew = xasprintf ("%s.new", name);
fp = grub_util_fopen (namenew, "wb");
@@ -46,14 +62,6 @@ grub_util_create_envblk_file (const char *name)
grub_util_error (_("cannot open `%s': %s"), namenew,
strerror (errno));
- pbuf = buf;
- memcpy (pbuf, GRUB_ENVBLK_SIGNATURE, sizeof (GRUB_ENVBLK_SIGNATURE) - 1);
- pbuf += sizeof (GRUB_ENVBLK_SIGNATURE) - 1;
- memcpy (pbuf, GRUB_ENVBLK_MESSAGE, sizeof (GRUB_ENVBLK_MESSAGE) - 1);
- pbuf += sizeof (GRUB_ENVBLK_MESSAGE) - 1;
- memset (pbuf , '#',
- DEFAULT_ENVBLK_SIZE - sizeof (GRUB_ENVBLK_SIGNATURE) - sizeof
(GRUB_ENVBLK_MESSAGE) + 2);
-
if (fwrite (buf, 1, DEFAULT_ENVBLK_SIZE, fp) != DEFAULT_ENVBLK_SIZE)
grub_util_error (_("cannot write to `%s': %s"), namenew,
strerror (errno));
--
2.19.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 5/7] Add ZFS envblock functions
This patch adds a ZFS implementation of the fs_envblk_* functions. These
functions will be used to load the grubenv block from a padding area in the
label of ZFS. This padding area is protected by an embedded checksum, and
multiple copies are stored on each disk. This should provide sufficient
reliability for most use cases. Since this area is not part of the block tree,
it can be easily modified at boot time with a simple recalculation of the
embedded checksum, enabling save_env to work for ZFS boot filesystems.
Note that the open() function is doing the read; this is because the ZFS
envblk logic doesn't store the file size, so the only way to retrieve it is to
determine the length of the file using strlen. If this is considered too poor
of an approach to be acceptable, format changes can still be made to the
structure of the padding area.
Signed-off-by: Paul Dagnelie
---
grub-core/fs/zfs/zfs.c | 134 ---
include/grub/zfs/vdev_impl.h | 33 +
2 files changed, 139 insertions(+), 28 deletions(-)
diff --git a/grub-core/fs/zfs/zfs.c b/grub-core/fs/zfs/zfs.c
index 2f72e42bf..d741426cd 100644
--- a/grub-core/fs/zfs/zfs.c
+++ b/grub-core/fs/zfs/zfs.c
@@ -30,6 +30,7 @@
*
*/
+#include
#include
#include
#include
@@ -1146,7 +1147,6 @@ scan_disk (grub_device_t dev, struct grub_zfs_data *data,
{
int label = 0;
uberblock_phys_t *ub_array, *ubbest = NULL;
- vdev_boot_header_t *bh;
grub_err_t err;
int vdevnum;
struct grub_zfs_device_desc desc;
@@ -1155,13 +1155,6 @@ scan_disk (grub_device_t dev, struct grub_zfs_data *data,
if (!ub_array)
return grub_errno;
- bh = grub_malloc (VDEV_BOOT_HEADER_SIZE);
- if (!bh)
-{
- grub_free (ub_array);
- return grub_errno;
-}
-
vdevnum = VDEV_LABELS;
desc.dev = dev;
@@ -1175,7 +1168,7 @@ scan_disk (grub_device_t dev, struct grub_zfs_data *data,
{
desc.vdev_phys_sector
= label * (sizeof (vdev_label_t) >> SPA_MINBLOCKSHIFT)
- + ((VDEV_SKIP_SIZE + VDEV_BOOT_HEADER_SIZE) >> SPA_MINBLOCKSHIFT)
+ + ((VDEV_PAD_SIZE * 2) >> SPA_MINBLOCKSHIFT)
+ (label < VDEV_LABELS / 2 ? 0 :
ALIGN_DOWN (grub_disk_get_size (dev->disk), sizeof (vdev_label_t))
- VDEV_LABELS * (sizeof (vdev_label_t) >> SPA_MINBLOCKSHIFT));
@@ -1184,6 +1177,7 @@ scan_disk (grub_device_t dev, struct grub_zfs_data *data,
err = grub_disk_read (dev->disk, desc.vdev_phys_sector
+ (VDEV_PHYS_SIZE >> SPA_MINBLOCKSHIFT),
0, VDEV_UBERBLOCK_RING, (char *) ub_array);
+
if (err)
{
grub_errno = GRUB_ERR_NONE;
@@ -1219,12 +1213,10 @@ scan_disk (grub_device_t dev, struct grub_zfs_data
*data,
continue;
#endif
grub_free (ub_array);
- grub_free (bh);
return GRUB_ERR_NONE;
}
grub_free (ub_array);
- grub_free (bh);
return grub_error (GRUB_ERR_BAD_FS, "couldn't find a valid label");
}
@@ -3765,6 +3757,60 @@ zfs_mtime (grub_device_t device, grub_int32_t *mt)
return GRUB_ERR_NONE;
}
+static grub_err_t
+zfs_devs_read_zbt (struct grub_zfs_data *data, grub_uint64_t offset, char
*buf, grub_size_t len)
+{
+ grub_err_t err = GRUB_ERR_NONE;
+ zio_cksum_t zc;
+ unsigned int i;
+ ZIO_SET_CHECKSUM(&zc, offset, 0, 0, 0);
+
+ for (i = 0; i < data->n_devices_attached; i++)
+{
+ err = grub_disk_read (data->devices_attached[i].dev->disk,
+ offset >> SPA_MINBLOCKSHIFT,
+ offset & ((1 << SPA_MINBLOCKSHIFT) - 1),
+ len, buf);
+ if (err)
+ continue;
+ ZIO_SET_CHECKSUM(&zc, offset, 0, 0, 0);
+ err = zio_checksum_verify (zc, ZIO_CHECKSUM_LABEL,
GRUB_ZFS_LITTLE_ENDIAN,
+buf, len);
+ if (!err)
+ return GRUB_ERR_NONE;
+}
+ return err;
+}
+
+static grub_err_t
+grub_zfs_envblk_open (struct grub_file *file)
+{
+ grub_err_t err;
+ struct grub_zfs_data *data;
+ vdev_boot_envblock_t *vbe;
+ int l;
+
+ file->offset = 0;
+ data = zfs_mount (file->device);
+ file->data = data;
+ data->file_buf = grub_malloc (sizeof (vdev_boot_envblock_t));
+ for (l = 0; l < VDEV_LABELS / 2; l++)
+{
+ grub_uint64_t offset = l * sizeof (vdev_label_t) + offsetof
(vdev_label_t, vl_be);
+
+ err = zfs_devs_read_zbt (data, offset, data->file_buf,
+ sizeof (vdev_boot_envblock_t));
+ if (err == GRUB_ERR_NONE)
+ {
+ vbe = (vdev_boot_envblock_t *)data->file_buf;
+ file->size = grub_strlen (vbe->vbe_bootenv);
+ return err;
+ }
+}
+ zfs_unmount (data);
+ return err;
+}
+
/*
* zfs_open() locates a file in the rootpool by following the
* MOS and places the dnode of the file in the memory address DNODE.
@@ -3850,6 +3896,19 @@ grub_zfs_open (struct grub_file *file, const char
*fsfilename)
return GRUB_ERR_NONE;
}
+static grub_ssize_t
Re: [PATCH] Warn if MBR gap is small and user uses advanced modules
On Wed, Mar 11, 2020, 18:08 Didier Spaier wrote:
>
>
> Le 11/03/2020 à 13:43, Daniel Kiper a écrit :
> > Adding Michael, Mihai, Javier and Peter...
> >
> > Below you can find what more or less Vladimir and I agreed WRT small MBR
> > gap. In general Vladimir convinced me to phase out small MBR gaps
> > support gradually. This is first step in this journey. We think that we
> > have to build some warnings into the code and extend documentation.
> > Please chime in what you think about that...
> >
> > On Tue, Mar 10, 2020 at 01:23:10PM +0100, Vladimir 'phcoder' Serbinenko
> wrote:
> >> Daniel, do you want to adjust the whitelist?
> >>
> >> We don't want to support small MBR gap in pair with anything but
> >> the simplest config of biosdisk+part_msdos+simple filesystem. In this
> >> path "simple filesystems" are all current filesystems except zfs and
> >> btrfs
> >
> > Missing SOB...
> >
> >> ---
> >> grub-core/partmap/gpt.c | 9 -
> >> grub-core/partmap/msdos.c | 7 ++-
> >> include/grub/partition.h| 3 ++-
> >> include/grub/util/install.h | 7 +--
> >> util/grub-install-common.c | 24
> >> util/grub-install.c | 10 +++---
> >> util/grub-setup.c | 2 +-
> >> util/setup.c| 5 +++--
> >> 8 files changed, 56 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/grub-core/partmap/gpt.c b/grub-core/partmap/gpt.c
> >> index 103f6796f..25a5a1934 100644
> >> --- a/grub-core/partmap/gpt.c
> >> +++ b/grub-core/partmap/gpt.c
> >> @@ -25,6 +25,9 @@
> >> #include
> >> #include
> >> #include
> >> +#ifdef GRUB_UTIL
> >> +#include
> >> +#endif
> >>
> >> GRUB_MOD_LICENSE ("GPLv3+");
> >>
> >> @@ -169,7 +172,8 @@ static grub_err_t
> >> gpt_partition_map_embed (struct grub_disk *disk, unsigned int
> *nsectors,
> >> unsigned int max_nsectors,
> >> grub_embed_type_t embed_type,
> >> - grub_disk_addr_t **sectors)
> >> + grub_disk_addr_t **sectors,
> >> + int warn_short)
> >> {
> >>struct gpt_partition_map_embed_ctx ctx = {
> >> .start = 0,
> >> @@ -191,6 +195,9 @@ gpt_partition_map_embed (struct grub_disk *disk,
> >> unsigned int *nsectors,
> >> N_("this GPT partition label contains no BIOS Boot Partition;"
> >> " embedding won't be possible"));
> >>
> >> + if (ctx.len < 450) {
> >
> > Could you define constant somewhere?
> >
> > Is it in sectors? Why 450? Should not it be 2048 if 1 MiB below?
>
> Whichs lead to a question: would the slot between 24K (or maybe
> 32K or 64K if it is wise to round it?) and 1M still a good fit for
> a Bios boot partition in case of a gpt? if not in which cases?
>
BIOS boot partition should never be under 960K. Just it was never a problem
to begin with.
>
> > ...and missing "&& warn_short" check...
> >
> >> +grub_util_warn("Your BIOS Boot Partition is under 1 MiB, please
> >> increase its size.");
> >> + }
> >>if (ctx.len < *nsectors)
> >
> > Could not we use this check somehow instead of adding new one?
> >
> >> return grub_error (GRUB_ERR_OUT_OF_RANGE,
> >> N_("your BIOS Boot Partition is too small;"
> >> diff --git a/grub-core/partmap/msdos.c b/grub-core/partmap/msdos.c
> >> index 7b8e45076..402bce050 100644
> >> --- a/grub-core/partmap/msdos.c
> >> +++ b/grub-core/partmap/msdos.c
> >> @@ -236,7 +236,8 @@ static grub_err_t
> >> pc_partition_map_embed (struct grub_disk *disk, unsigned int *nsectors,
> >> unsigned int max_nsectors,
> >> grub_embed_type_t embed_type,
> >> - grub_disk_addr_t **sectors)
> >> + grub_disk_addr_t **sectors,
> >> + int warn_short)
> >> {
> >>grub_disk_addr_t end = ~0ULL;
> >>struct grub_msdos_partition_mbr mbr;
> >> @@ -390,6 +391,10 @@ pc_partition_map_embed (struct grub_disk *disk,
> >> unsigned int *nsectors,
> >>return GRUB_ERR_NONE;
> >> }
> >>
> >> + if (end < 450 && warn_short) {
> >> +grub_util_warn("You have a short MBR gap and use advanced config.
> >> Please increase post-MBR gap");
> >
> > Ditto?
> >
> >> + }
> >> +
> >>if (end <= 1)
> >> return grub_error (GRUB_ERR_FILE_NOT_FOUND,
> >> N_("this msdos-style partition label has no "
> >> diff --git a/include/grub/partition.h b/include/grub/partition.h
> >> index 7adb7ec6e..5697e28d5 100644
> >> --- a/include/grub/partition.h
> >> +++ b/include/grub/partition.h
> >> @@ -55,7 +55,8 @@ struct grub_partition_map
> >>grub_err_t (*embed) (struct grub_disk *disk, unsigned int *nsectors,
> >> unsigned int max_nsectors,
> >> grub_embed_type_t embed_type,
> >> -grub_disk_addr_t **sectors);
> >> +grub_disk_addr_t **sectors,
> >> +int warn_short);
> >> #endif
> >> };
> >> typedef struct grub_partition_map *grub_partition_map_t;
> >> diff --git a/include/grub/util/install.h b/include/grub/util/install.h
> >> index 2631b1074..982115f57 100644
> >> --- a/include/grub/util/install.h
> >> +++ b/include/grub/util/install.h
> >> @@ -193,13 +193,13 @@ grub_util_bios_setup (con
