grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 1/2] Warn on small MBR gaps on complicated setups


From: Vladimir 'phcoder' Serbinenko
Subject: Re: [PATCH 1/2] Warn on small MBR gaps on complicated setups
Date: Fri, 13 Nov 2020 17:01:47 +0100



On Wed, 11 Nov 2020, 04:39 Michael Chang, <mchang@suse.com> wrote:
On Tue, Nov 10, 2020 at 08:49:52PM +0100, Vladimir 'phcoder' Serbinenko wrote:
> From: Vladimir Serbinenko <phcoder@gmail.com>
> Date: Tue, 10 Nov 2020 20:42:12 +0100
> Subject: [PATCH 1/2] Warn on small MBR gaps on complicated setups
>
> Signed-off-by: Vladimir Serbinenko <phcoder@gmail.com>
> ---
>  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         |  9 ++++++---
>  util/grub-setup.c           |  2 +-
>  util/setup.c                |  5 +++--
>  8 files changed, 55 insertions(+), 11 deletions(-)
>
> diff --git a/grub-core/partmap/gpt.c b/grub-core/partmap/gpt.c
> index 72a2e37cd..38e4eaef8 100644
> --- a/grub-core/partmap/gpt.c
> +++ b/grub-core/partmap/gpt.c
> @@ -25,6 +25,9 @@
>  #include <grub/msdos_partition.h>
>  #include <grub/gpt_partition.h>
>  #include <grub/i18n.h>
> +#ifdef GRUB_UTIL
> +#include <grub/emu/misc.h>
> +#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) {
> +    grub_util_warn("Your BIOS Boot Partition is under 1 MiB, please
> increase its size.");
> +  }

Why did you drop the GRUB_MIN_RECOMMENDED_MBRGAP introduced in v2 patch
? Also the limitation is differently put (450 vs 1900) compared with
previous version.
My fault. I have intended to resubmit v2+a small change with additional docs as v3 but have resubmitted v1+docs. Will resubmit proper version

>    if (ctx.len < *nsectors)
>      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 ee3f24982..d1b81cbd0 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");
> +  }
> +

Same above.

In addition, the short mbr warning seems to be emitted only if certain
embedding error happened while it looked to me that the warning should be
used to remind user the potential issue before any error encountered.

Thanks,
Michael


>    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
> +grub_install_is_short_mgrgap_supported(void);
> +
>  #endif
> diff --git a/util/grub-install-common.c b/util/grub-install-common.c
> index 277eaf4e2..5696507a9 100644
> --- a/util/grub-install-common.c
> +++ b/util/grub-install-common.c
> @@ -234,6 +234,30 @@ char *grub_install_source_directory = NULL;
>  char *grub_install_locale_directory = NULL;
>  char *grub_install_themes_directory = NULL;
>
> +int
> +grub_install_is_short_mgrgap_supported()
> +{
> +  int i, j;
> +  static const char *whitelist[] =
> +    {
> +     "part_msdos", "biosdisk", "affs", "afs", "bfs", "archelp",
> +     "cpio", "cpio_be", "newc", "odc", "ext2", "fat", "exfat",
> +     "f2fs", "fshelp", "hfs", "hfsplus",
> +     "iso9660", "jfs", "minix", "minix2", "minix3", "minix_be",
> +     "minix2_be", "minix2_be", "nilfs2", "ntfs", "ntfscomp",
> +     "reiserfs", "romfs", "sfs", "tar", "udf",
> +     "ufs1", "ufs1_be", "ufs2", "xfs"
> +    };
> +  for (i = 0; i < modules.n_entries; i++) {
> +    for (j = 0; j < ARRAY_SIZE (whitelist); j++)
> +      if (strcmp(modules.entries[i], whitelist[j]) == 0)
> + break;
> +    if (j == ARRAY_SIZE (whitelist))
> +      return 0;
> +  }
> +  return 1;
> +}
> +
>  void
>  grub_install_push_module (const char *val)
>  {
> diff --git a/util/grub-install.c b/util/grub-install.c
> index a35a2e2e8..6813f3144 100644
> --- a/util/grub-install.c
> +++ b/util/grub-install.c
> @@ -1718,10 +1718,13 @@ main (int argc, char *argv[])
>   install_device);
>
>   /*  Now perform the installation.  */
> - if (install_bootsector)
> + if (install_bootsector) {
> +   int support_short_mbr_gap = grub_install_is_short_mgrgap_supported();
>     grub_util_bios_setup (platdir, "boot.img", "core.img",
>   install_drive, force,
> - fs_probe, allow_floppy, add_rs_codes);
> + fs_probe, allow_floppy, add_rs_codes,
> + !support_short_mbr_gap);
> + }
>   break;
>        }
>      case GRUB_INSTALL_PLATFORM_SPARC64_IEEE1275:
> @@ -1748,7 +1751,7 @@ main (int argc, char *argv[])
>     grub_util_sparc_setup (platdir, "boot.img", "core.img",
>   install_drive, force,
>   fs_probe, allow_floppy,
> - 0 /* unused */ );
> + 0 /* unused */, 0 /* unused */ );
>   break;
>        }
>
> diff --git a/util/grub-setup.c b/util/grub-setup.c
> index 42b98ad3c..1783224dd 100644
> --- a/util/grub-setup.c
> +++ b/util/grub-setup.c
> @@ -315,7 +315,7 @@ main (int argc, char *argv[])
>      arguments.core_file ? : DEFAULT_CORE_FILE,
>      dest_dev, arguments.force,
>      arguments.fs_probe, arguments.allow_floppy,
> -    arguments.add_rs_codes);
> +    arguments.add_rs_codes, 0);
>
>    /* Free resources.  */
>    grub_fini_all ();
> diff --git a/util/setup.c b/util/setup.c
> index 3be88aae1..acba4eca1 100644
> --- a/util/setup.c
> +++ b/util/setup.c
> @@ -254,7 +254,8 @@ 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 __attribute__ ((unused))) /* unused on sparc64 */
> +       int add_rs_codes __attribute__ ((unused)), /* unused on sparc64 */
> +       int warn_small __attribute__ ((unused))) /* unused on sparc64 */
>  {
>    char *core_path;
>    char *boot_img, *core_img, *boot_path;
> @@ -530,7 +531,7 @@ SETUP (const char *dir,
>   GRUB_EMBED_PCBIOS, &sectors);
>      else if (ctx.dest_partmap)
>        err = ctx.dest_partmap->embed (dest_dev->disk, &nsec, maxsec,
> -      GRUB_EMBED_PCBIOS, &sectors);
> +      GRUB_EMBED_PCBIOS, &sectors, warn_small);
>      else
>        err = fs->fs_embed (dest_dev, &nsec, maxsec,
>     GRUB_EMBED_PCBIOS, &sectors);
> --
> 2.20.1
>
> --
> Regards
> Vladimir 'phcoder' Serbinenko
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel


_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

reply via email to

[Prev in Thread] Current Thread [Next in Thread]