Re: GRUB 2.06
On Tue, Apr 20, 2021 at 04:33:37PM -0400, Eli Schwartz wrote: > On 4/20/21 1:48 PM, John Paul Adrian Glaubitz wrote: > > I think it's reasonable to expect from a distribution that they > > backport upstream fixes, at least in Debian, openSUSE and Fedora, it > > isn't a problem. > > I think it is unreasonable -- backporting upstream fixes is a sign that > upstream software has failed to be usable out of the box, and needs to > release more often. > > If a distro is being *forced* to backport 200+ patches, we've officially > entered the Madness Place: > > https://build.opensuse.org/package/view_file/Base:System/grub2/grub2.spec > lines 174 - 395 (9 pages of source files, most named 0001?) > > https://src.fedoraproject.org/rpms/grub2/tree/rawhide > carefully numbered list of patches 0001 - 0198 AFAICT F34 dropped more than 100 custom patches after 2.06~rc1 release. And this did not happen by chance. It required a lot of work. I know the situation is still not perfect. Though other folks and I are still going to work on dropping most of these patches. So, please be patient... > https://salsa.debian.org/grub-team/grub/-/blob/master/debian/patches/series > patch series of 219 patches ...same for Debian and other distros... > ... > > I'm sorry? Claiming that distros "are capable of backporting, therefore > it's reasonable to expect it is their job to do so" is completely > missing the point. > > Claiming that for these distros "it isn't a problem" to roll a patchset > for elaborate backport lists, based on I guess the evidence that they've > done so, unjustly conflates "we like doing this" with "we were forced to > do this with much gnashing of teeth". > > I can't point to citations where either one has been said, but I somehow > doubt the former viewpoint is the one distro maintainers are holding. > (Well, I'm given to understand Debian maintainers seem to actively enjoy > having many patches. So I guess I shouldn't be *too* surprised that a > Debian Developer is insisting that people maintain downstream patches.) > > "Basically insulting except not outright" a person who would like to see > more frequent (read: any) maintenance releases because I dunno, clearly > they're just irresponsible at running a distro if they're afraid of > importing 200 patches in one go, is kind of really bad. > > This is a real problem that grub really has. Whyever that might be a > problem (the standard reason is lack of developer time) can be seen as > forgivable, because software development is hard and all too often > unrewarding, and demanding more releases may not actually be feasible in > practice. > > But I'd really, really, really like to NOT see victim blaming and > holding the current state of affairs as some kind of joyous ideal which > must be held sacred, because pooh-pooh anyone who dares post to the list > merely asking if such a thing might be possible -- it's your own darned > job, noob, stop being unreasonable and lern2patch, reasonable distros > will reasonably patch. > > (I think it bears mentioning again -- 200+ patches. *200*. At what point > is there more patches than source code?) > > ... > > Anyway, a grub 2.04.1 would have been fantastic 6 months or a year ago. > At this point, people should just use 2.06-rc1, there is not much point > in trying to stabilize a maintenance release in the middle of the > freeze/RC cycle for 2.06 as the same effort is better spent getting 2.06 > out the door. Sorry, I am not going to make maintenance releases until we are able to make stable releases in proper cadence. Then we can get back to this discussion... Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: GRUB 2.06
On Tue, Apr 20, 2021 at 03:58:18PM -0400, Eli Schwartz wrote: > On 4/8/21 3:28 PM, Didier Spaier wrote: > > I am fine with shipping GRUB up to some commit in Slint, but not all > > distributions accept to do that, or to carry a zillion patches as does > > Debian. > > > > As result, end users complain "GRUB is broken" whereas a patch that fix > the > > issue of which they suffer of has been committed upstream a long time ago. > > > > An example that comes to mind is: > > 6643507ce30f775008e093580f0c9499dfb2c485 > > > > Folks running Slackware-current also suffer of it: > > https://https://www.linuxquestions.org/questions/slackware-14/requests-for-current-14-2-15-0-a-4175620463/#post6236544www.linuxquestions.org/questions/slackware-14/requests-for-current-14-2-15-0-a-4175620463/#post6236544 > > Can you *please* not do this? > > References: > <20210408165349.otipx6gohm64b...@tomti.i.net-space.pl> > In-Reply-To: <20210408165349.otipx6gohm64b...@tomti.i.net-space.pl> > > And randomly top-posting on top of a completely unrelated thread that > you've hit "reply to" despite having no connection to your actual reply. > > There is zero benefit to this, and it's seriously inconvenient to people > who have seemingly disappearing emails due to threading. FYI, Didier spotted own mistake earlier and said sorry in separate email... Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCHv2] grub-install: Add backup and restore
Hi,
On Michael request I am considering this as 2.06 release material...
On Mon, Dec 07, 2020 at 12:37:28PM +, Dimitri John Ledkov wrote:
> Refactor clean_grub_dir to create a backup of all the files, instead
> of just irrevocably removing them as the first action. If available,
> register on_exit handle to restore the backup if any errors occur, or
> remove the backup if everything was successful. If on_exit is not
> available, the backup remains on disk for manual recovery.
>
> This allows safer upgrades of MBR & modules, such that
> modules/images/fonts/translations are consistent with MBR in case of
> errors. For example accidental grub-install /dev/non-existent-disk
> currently clobbers and upgrades modules in /boot/grub, despite not
> actually updating any MBR. This increases peak disk-usage slightly, by
> requiring temporarily twice the disk space to complete grub-install.
>
> Also add modinfo.sh to the cleanup/backup/restore codepath, to ensure
> it is also cleaned / backed up / restored.
>
> Signed-off-by: Dimitri John Ledkov
> ---
>
> Changes since v1: as reported on linux-ext4 mailing list and debugged
> by Colin Watson, the grub_util_path_concat_ext call was incorrect in
> the restore backup case as there was no extention needed. Instead the
> call is corrected to just grub_util_path_concat.
>
> This patch is now shipped in Ubuntu & Debian in multiple series. It
> would be nice to have this merged upstream, as it greatly improves
> grub upgrades and prevents missmatches of MBR & modules.
>
> configure.ac | 2 +-
> util/grub-install-common.c | 105 +++--
> 2 files changed, 91 insertions(+), 16 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 7c10a4db7..71cd414c3 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -419,7 +419,7 @@ else
> fi
>
> # Check for functions and headers.
> -AC_CHECK_FUNCS(posix_memalign memalign getextmntent)
> +AC_CHECK_FUNCS(posix_memalign memalign getextmntent on_exit)
I think we should use atexit() instead of on_exit(). The mans say former
is more portable...
> AC_CHECK_HEADERS(sys/param.h sys/mount.h sys/mnttab.h limits.h)
>
> # glibc 2.25 still includes sys/sysmacros.h in sys/types.h but emits
> deprecation
> diff --git a/util/grub-install-common.c b/util/grub-install-common.c
> index 277eaf4e2..74584b92b 100644
> --- a/util/grub-install-common.c
> +++ b/util/grub-install-common.c
> @@ -185,38 +185,113 @@ grub_install_mkdir_p (const char *dst)
>free (t);
> }
>
> +static int
> +strcmp_ext (const char *a, const char *b, const char *ext)
> +{
> + char *bsuffix = grub_util_path_concat_ext (1, b, ext);
> + int r = strcmp (a, bsuffix);
Please add empty line here.
> + free (bsuffix);
> + return r;
> +}
> +
> +enum clean_grub_dir_mode
> +{
> + CLEAN = 0,
> + CLEAN_BACKUP = 1,
> + CREATE_BACKUP = 2,
> + RESTORE_BACKUP = 3,
Do we need these assignments? I do not think so...
> +};
> +
> static void
> -clean_grub_dir (const char *di)
> +clean_grub_dir_real (const char *di, enum clean_grub_dir_mode mode)
> {
>grub_util_fd_dir_t d;
>grub_util_fd_dirent_t de;
> + char suffix[2] = "";
> +
> + if ((mode == CLEAN_BACKUP) || (mode == RESTORE_BACKUP))
> +{
> + strcpy (suffix, "-");
It seems to me "~" is more common as backup marker. I would use it
instead of "-"...
> +}
Please drop these curly brackets...
>d = grub_util_fd_opendir (di);
>if (!d)
> -grub_util_error (_("cannot open directory `%s': %s"),
> - di, grub_util_fd_strerror ());
> +{
> + if (mode == CLEAN_BACKUP)
> + return;
> + grub_util_error (_("cannot open directory `%s': %s"),
> +di, grub_util_fd_strerror ());
> +}
>
>while ((de = grub_util_fd_readdir (d)))
> {
>const char *ext = strrchr (de->d_name, '.');
Please add empty line here...
> - if ((ext && (strcmp (ext, ".mod") == 0
> -|| strcmp (ext, ".lst") == 0
> -|| strcmp (ext, ".img") == 0
> -|| strcmp (ext, ".mo") == 0)
> -&& strcmp (de->d_name, "menu.lst") != 0)
> - || strcmp (de->d_name, "efiemu32.o") == 0
> - || strcmp (de->d_name, "efiemu64.o") == 0)
> + if ((ext && (strcmp_ext (ext, ".mod", suffix) == 0
> +|| strcmp_ext (ext, ".lst", suffix) == 0
> +|| strcmp_ext (ext, ".img", suffix) == 0
> +|| strcmp_ext (ext, ".mo", suffix) == 0)
> +&& strcmp_ext (de->d_name, "menu.lst", suffix) != 0)
> + || strcmp_ext (de->d_name, "modinfo.sh", suffix) == 0
> + || strcmp_ext (de->d_name, "efiemu32.o", suffix) == 0
> + || strcmp_ext (de->d_name, "efiemu64.o", suffix) == 0)
> {
> - char *x = grub_util_path_concat (2, di, de->d_name);
> - if (grub_util_unlink (x) < 0)
> - grub_util_error (_("cannot delete `%s': %s"), x,
> - grub_util_fd_strerror ());
> - fr
Re: GRUB error: unknown filesystem on ia64
On 4/19/21 7:56 PM, Javier Martinez Canillas wrote: [snip] >> >> Could you send the patch to the mailing list separately with git send-email >> or so that it doesn't get overlooked in the longer discussion we had? >> > > Yes, I was waiting for someone else more familiar with the COFF/PE binary > format to chime in. But I'll post it as a proper patch in a couple of days > if no one complains. > Daniel pointed me out that my patch was wrong. Instead proper padding has to be added by grub-mkimage for the sections in the actual PE32+ binary file. That indeed makes more sense to me and I'll post a different RFC patch now. Best regards, Javier ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[RFC PATCH] util/mkimage: Fix wrong PE32+ section sizes for some arches
Commit f60ba9e5945 (util/mkimage: Refactor section setup to use a helper)
added a helper function to setup PE sections. But it also changed how the
raw data offsets were calculated since all the section sizes are aligned.
But for some platforms (i.e: ia64 and aa64) the kernel image size wasn't
aligned using the section alignment, which causes the PE section headers
to not match the actual section sizes in the PE32+ binary file.
This caused problems on ia64 EFI machines, since the .data section size
is bigger than the actual section in the PE32+ binary, overlapping with
part of the mods section. That leads to GRUB not being able to load any
built-in module.
Fix it by aligning the kernel_size to the section alignment, that makes
the sizes and offsets in the PE section headers to match the sections
in the PE32+ binary file.
Reported-by: John Paul Adrian Glaubitz
Signed-off-by: Javier Martinez Canillas
---
Hello, this is an RFC because I want someone else more familiar with this
to double check that this approach is sane.
It would be also useful if someone can test on an aarch64 machine, I have
compared the generated EFI binaries and are the same in both cases. But
still it seems to me that an explicit alignment is needed for EM_AARCH64.
Best regards,
Javier
util/grub-mkimagexx.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/util/grub-mkimagexx.c b/util/grub-mkimagexx.c
index 00f49ccaaaf..2d0c9a0cb6b 100644
--- a/util/grub-mkimagexx.c
+++ b/util/grub-mkimagexx.c
@@ -2375,6 +2375,10 @@ SUFFIX (grub_mkimage_load_image) (const char
*kernel_path,
layout->got_off = layout->kernel_size;
layout->kernel_size += ALIGN_UP (layout->got_size, 16);
+
+ if (image_target->id == IMAGE_EFI)
+layout->kernel_size = ALIGN_UP (layout->kernel_size,
+image_target->section_align);
}
if (image_target->elf_target == EM_AARCH64)
{
@@ -2386,6 +2390,11 @@ SUFFIX (grub_mkimage_load_image) (const char
*kernel_path,
layout->got_off = layout->kernel_size;
layout->kernel_size += ALIGN_UP (layout->got_size, 16);
+
+
+ if (image_target->id == IMAGE_EFI)
+layout->kernel_size = ALIGN_UP (layout->kernel_size,
+image_target->section_align);
}
#endif
}
--
2.31.1
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
