[Xen-devel] [XEN PATCH 0/3] read grubenv and set default from it

[Xen-devel] [XEN PATCH 0/3] read grubenv and set default from it

[YOUNG, MICHAEL A.]

This series of patches is to improve the parsing by pygrub of grub
configuration on Fedora. The current result of parsing is generally
that the second kernel listed is set as the default due to a
set default=1 line in grub.cfg which is only intended to be
reached after repeated boot failures.

The patches read the grubenv file (which consists of key=value lines
padded to 1024 characters by # characters) to get the values of
next_entry and saved_entry, which can be a kernel string or an
order number. Unfortunately, for Fedora 31 at least, this is
often a BLS-style string so it isn't necessarily useful. The patches
use the value of next_entry or of saved_entry to set the default
kernel or sets it to the first kernel listed if those values are set
but not used.


Michael Young (3):
  set default kernel from grubenv next_entry or saved_entry
  read a grubenv file if it is next to the grub.cfg file
  Example Fedora 31 grub.cfg and grubenv files

 tools/pygrub/examples/fedora-31.grub.cfg | 200 +++++++++++++++++++++++
 tools/pygrub/examples/fedora-31.grubenv  |   5 +
 tools/pygrub/src/GrubConf.py             |  31 +++-
 tools/pygrub/src/pygrub                  |  21 ++-
 4 files changed, 253 insertions(+), 4 deletions(-)
 create mode 100644 tools/pygrub/examples/fedora-31.grub.cfg
 create mode 100644 tools/pygrub/examples/fedora-31.grubenv

-- 
2.21.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [XEN PATCH 1/3] set default kernel from grubenv next_entry or saved_entry

[YOUNG, MICHAEL A.]

This patch reads the contents of a grubenv file if available, and
uses the value of next_entry (in preference) or of saved_entry to
set the default kernel if there is a matching title or if it is a
number.  If either next_entry or saved_entry is set and neither is
used then the default is set to 0.

Signed-off-by: Michael Young <m.a.young@durham.ac.uk>
---
 tools/pygrub/src/GrubConf.py | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/tools/pygrub/src/GrubConf.py b/tools/pygrub/src/GrubConf.py
index 73f1bbed2f..1fc68cadb2 100644
--- a/tools/pygrub/src/GrubConf.py
+++ b/tools/pygrub/src/GrubConf.py
@@ -368,7 +368,7 @@ class Grub2ConfigFile(_GrubConfigFile):
     def new_image(self, title, lines):
         return Grub2Image(title, lines)
  
-    def parse(self, buf = None):
+    def parse(self, buf = None, grubenv = None):
         if buf is None:
             if self.filename is None:
                 raise ValueError("No config file defined to parse!")
@@ -379,10 +379,17 @@ class Grub2ConfigFile(_GrubConfigFile):
         else:
             lines = buf.split("\n")
 
+        if grubenv is not None:
+            lines = grubenv.split("\n") + lines
+
         in_function = False
         img = None
         title = ""
         menu_level=0
+        img_count=0
+        saved_entry = ""
+        next_entry = ""
+        entry_matched = False
         for l in lines:
             l = l.strip()
             # skip blank lines
@@ -408,6 +415,13 @@ class Grub2ConfigFile(_GrubConfigFile):
                     raise RuntimeError("syntax error: cannot nest menuentry (%d %s)" % (len(img),img))
                 img = []
                 title = title_match.group(1)
+                if title == next_entry:
+                    setattr(self, 'default', img_count)
+                    entry_matched = True
+                if title == saved_entry and not entry_matched:
+                    setattr(self, 'default', img_count)
+                    entry_matched = True
+                img_count += 1
                 continue
 
             if l.startswith("submenu"):
@@ -432,6 +446,14 @@ class Grub2ConfigFile(_GrubConfigFile):
 
             (com, arg) = grub_exact_split(l, 2)
         
+            if com == "next_entry":
+                next_entry = arg
+                continue
+
+            if com == "saved_entry":
+                saved_entry = arg
+                continue
+
             if com == "set":
                 (com,arg) = grub2_handle_set(arg)
                 
@@ -448,6 +470,13 @@ class Grub2ConfigFile(_GrubConfigFile):
                 pass
             else:
                 logging.warning("Unknown directive %s" %(com,))
+
+        if next_entry.isdigit():
+            setattr(self, 'default', next_entry)
+        elif saved_entry.isdigit() and not entry_matched:
+            setattr(self, 'default', saved_entry)
+        elif (next_entry != "" or saved_entry != "") and not entry_matched:
+            setattr(self, 'default', 0)
             
         if img is not None:
             raise RuntimeError("syntax error: end of file with open menuentry(%d %s)" % (len(img),img))
-- 
2.21.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [XEN PATCH 2/3] read a grubenv file if it is next to the grub.cfg file

[YOUNG, MICHAEL A.]

When a grub.cfg file is found this patch checks if there is grubenv
file in the same directory as the grub.cfg file. If there is it
passes the contents to parse().

Signed-off-by: Michael Young <m.a.young@durham.ac.uk>
---
 tools/pygrub/src/pygrub | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/tools/pygrub/src/pygrub b/tools/pygrub/src/pygrub
index ce7ab0eb8c..53a0803817 100755
--- a/tools/pygrub/src/pygrub
+++ b/tools/pygrub/src/pygrub
@@ -457,10 +457,25 @@ class Grub:
         # limit read size to avoid pathological cases
         buf = f.read(FS_READ_MAX)
         del f
-        if sys.version_info[0] < 3:
-            self.cf.parse(buf)
+        # check for a grubenv file next to the grub.cfg file
+        (fdir, fsep, ffile) = self.cf.filename.rpartition("/")
+        if fdir != "" and ffile == "grub.cfg":
+            fenv = fdir + "/grubenv"
         else:
-            self.cf.parse(buf.decode())
+            fenv = ""
+        if fenv != "" and fs.file_exists(fenv):
+            fenvf = fs.open_file(fenv)
+            grubenv = fenvf.read(FS_READ_MAX)
+            del fenvf
+            if sys.version_info[0] < 3:
+                self.cf.parse(buf, grubenv)
+            else:
+                self.cf.parse(buf.decode(), grubenv.decode())
+        else:
+            if sys.version_info[0] < 3:
+                self.cf.parse(buf)
+            else:
+                self.cf.parse(buf.decode())
 
     def image_index(self):
         if isinstance(self.cf.default, int):
-- 
2.21.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [XEN PATCH 3/3] Example Fedora 31 grub.cfg and grubenv files

[YOUNG, MICHAEL A.]

This patch adds an example grub.cfg and grubenv file for reference

Signed-off-by: Michael Young <m.a.young@durham.ac.uk>
---
 tools/pygrub/examples/fedora-31.grub.cfg | 200 +++++++++++++++++++++++
 tools/pygrub/examples/fedora-31.grubenv  |   5 +
 2 files changed, 205 insertions(+)
 create mode 100644 tools/pygrub/examples/fedora-31.grub.cfg
 create mode 100644 tools/pygrub/examples/fedora-31.grubenv

diff --git a/tools/pygrub/examples/fedora-31.grub.cfg b/tools/pygrub/examples/fedora-31.grub.cfg
new file mode 100644
index 0000000000..9ce38d43b7
--- /dev/null
+++ b/tools/pygrub/examples/fedora-31.grub.cfg
@@ -0,0 +1,200 @@
+#
+# DO NOT EDIT THIS FILE
+#
+# It is automatically generated by grub2-mkconfig using templates
+# from /etc/grub.d and settings from /etc/default/grub
+#
+
+### BEGIN /etc/grub.d/00_header ###
+set pager=1
+
+if [ -f ${config_directory}/grubenv ]; then
+  load_env -f ${config_directory}/grubenv
+elif [ -s $prefix/grubenv ]; then
+  load_env
+fi
+if [ "${next_entry}" ] ; then
+   set default="${next_entry}"
+   set next_entry=
+   save_env next_entry
+   set boot_once=true
+else
+   set default="${saved_entry}"
+fi
+
+if [ x"${feature_menuentry_id}" = xy ]; then
+  menuentry_id_option="--id"
+else
+  menuentry_id_option=""
+fi
+
+export menuentry_id_option
+
+if [ "${prev_saved_entry}" ]; then
+  set saved_entry="${prev_saved_entry}"
+  save_env saved_entry
+  set prev_saved_entry=
+  save_env prev_saved_entry
+  set boot_once=true
+fi
+
+function savedefault {
+  if [ -z "${boot_once}" ]; then
+    saved_entry="${chosen}"
+    save_env saved_entry
+  fi
+}
+
+function load_video {
+  if [ x$feature_all_video_module = xy ]; then
+    insmod all_video
+  else
+    insmod efi_gop
+    insmod efi_uga
+    insmod ieee1275_fb
+    insmod vbe
+    insmod vga
+    insmod video_bochs
+    insmod video_cirrus
+  fi
+}
+
+terminal_output console
+if [ x$feature_timeout_style = xy ] ; then
+  set timeout_style=menu
+  set timeout=5
+# Fallback normal timeout code in case the timeout_style feature is
+# unavailable.
+else
+  set timeout=5
+fi
+### END /etc/grub.d/00_header ###
+
+### BEGIN /etc/grub.d/01_users ###
+if [ -f ${prefix}/user.cfg ]; then
+  source ${prefix}/user.cfg
+  if [ -n "${GRUB2_PASSWORD}" ]; then
+    set superusers="root"
+    export superusers
+    password_pbkdf2 root ${GRUB2_PASSWORD}
+  fi
+fi
+### END /etc/grub.d/01_users ###
+
+### BEGIN /etc/grub.d/08_fallback_counting ###
+insmod increment
+# Check if boot_counter exists and boot_success=0 to activate this behaviour.
+if [ -n "${boot_counter}" -a "${boot_success}" = "0" ]; then
+  # if countdown has ended, choose to boot rollback deployment,
+  # i.e. default=1 on OSTree-based systems.
+  if  [ "${boot_counter}" = "0" -o "${boot_counter}" = "-1" ]; then
+    set default=1
+    set boot_counter=-1
+  # otherwise decrement boot_counter
+  else
+    decrement boot_counter
+  fi
+  save_env boot_counter
+fi
+### END /etc/grub.d/08_fallback_counting ###
+
+### BEGIN /etc/grub.d/10_linux ###
+menuentry 'Fedora (5.3.6-300.fc31.x86_64) 31 (Thirty One)' --class fedora --class gnu-linux --class gnu --class os --unrestricted $menuentry_id_option 'gnulinux-5.3.6-300.fc31.x86_64-advanced-1dec723d-4bdf-461a-a09b-e71ba5974892' {
+	load_video
+	set gfxpayload=keep
+	insmod gzio
+	insmod part_msdos
+	insmod ext2
+	set root='hd0,msdos1'
+	if [ x$feature_platform_search_hint = xy ]; then
+	  search --no-floppy --fs-uuid --set=root --hint='hd0,msdos1'  00c6a44a-1624-4c8f-bd08-bc66ea59edbe
+	else
+	  search --no-floppy --fs-uuid --set=root 00c6a44a-1624-4c8f-bd08-bc66ea59edbe
+	fi
+	linux	/vmlinuz-5.3.6-300.fc31.x86_64 root=/dev/mapper/fedora_localhost--live-root ro resume=/dev/mapper/fedora_localhost--live-swap rd.lvm.lv=fedora_localhost-live/root rd.lvm.lv=fedora_localhost-live/swap rhgb quiet 
+	initrd	/initramfs-5.3.6-300.fc31.x86_64.img
+}
+menuentry 'Fedora (0-rescue-fbc80dcce05d4a1a9b9a7951232a4eeb) 31 (Thirty One)' --class fedora --class gnu-linux --class gnu --class os --unrestricted $menuentry_id_option 'gnulinux-0-rescue-fbc80dcce05d4a1a9b9a7951232a4eeb-advanced-1dec723d-4bdf-461a-a09b-e71ba5974892' {
+	load_video
+	insmod gzio
+	insmod part_msdos
+	insmod ext2
+	set root='hd0,msdos1'
+	if [ x$feature_platform_search_hint = xy ]; then
+	  search --no-floppy --fs-uuid --set=root --hint='hd0,msdos1'  00c6a44a-1624-4c8f-bd08-bc66ea59edbe
+	else
+	  search --no-floppy --fs-uuid --set=root 00c6a44a-1624-4c8f-bd08-bc66ea59edbe
+	fi
+	linux	/vmlinuz-0-rescue-fbc80dcce05d4a1a9b9a7951232a4eeb root=/dev/mapper/fedora_localhost--live-root ro resume=/dev/mapper/fedora_localhost--live-swap rd.lvm.lv=fedora_localhost-live/root rd.lvm.lv=fedora_localhost-live/swap rhgb quiet 
+	initrd	/initramfs-0-rescue-fbc80dcce05d4a1a9b9a7951232a4eeb.img
+}
+
+### END /etc/grub.d/10_linux ###
+
+### BEGIN /etc/grub.d/10_reset_boot_success ###
+insmod increment
+# Hiding the menu is ok if last boot was ok or if this is a first boot attempt to boot the entry
+if [ "${boot_success}" = "1" -o "${boot_indeterminate}" = "1" ]; then
+  set menu_hide_ok=1
+else
+  set menu_hide_ok=0 
+fi
+# Reset boot_indeterminate after a successful boot, increment otherwise
+if [ "${boot_success}" = "1" ] ; then
+  set boot_indeterminate=0
+else
+  increment boot_indeterminate
+fi
+# Reset boot_success for current boot 
+set boot_success=0
+save_env boot_success boot_indeterminate
+### END /etc/grub.d/10_reset_boot_success ###
+
+### BEGIN /etc/grub.d/12_menu_auto_hide ###
+if [ x$feature_timeout_style = xy ] ; then
+  if [ "${menu_show_once}" ]; then
+    unset menu_show_once
+    save_env menu_show_once
+    set timeout_style=menu
+    set timeout=60
+  elif [ "${menu_auto_hide}" -a "${menu_hide_ok}" = "1" ]; then
+    set orig_timeout_style=${timeout_style}
+    set orig_timeout=${timeout}
+    if [ "${fastboot}" = "1" ]; then
+      # timeout_style=menu + timeout=0 avoids the countdown code keypress check
+      set timeout_style=menu
+      set timeout=0
+    else
+      set timeout_style=hidden
+      set timeout=1
+    fi
+  fi
+fi
+### END /etc/grub.d/12_menu_auto_hide ###
+
+### BEGIN /etc/grub.d/20_linux_xen ###
+
+### END /etc/grub.d/20_linux_xen ###
+
+### BEGIN /etc/grub.d/20_ppc_terminfo ###
+### END /etc/grub.d/20_ppc_terminfo ###
+
+### BEGIN /etc/grub.d/30_os-prober ###
+### END /etc/grub.d/30_os-prober ###
+
+### BEGIN /etc/grub.d/30_uefi-firmware ###
+### END /etc/grub.d/30_uefi-firmware ###
+
+### BEGIN /etc/grub.d/40_custom ###
+# This file provides an easy way to add custom menu entries.  Simply type the
+# menu entries you want to add after this comment.  Be careful not to change
+# the 'exec tail' line above.
+### END /etc/grub.d/40_custom ###
+
+### BEGIN /etc/grub.d/41_custom ###
+if [ -f  ${config_directory}/custom.cfg ]; then
+  source ${config_directory}/custom.cfg
+elif [ -z "${config_directory}" -a -f  $prefix/custom.cfg ]; then
+  source $prefix/custom.cfg;
+fi
+### END /etc/grub.d/41_custom ###
diff --git a/tools/pygrub/examples/fedora-31.grubenv b/tools/pygrub/examples/fedora-31.grubenv
new file mode 100644
index 0000000000..6df61b0fe5
--- /dev/null
+++ b/tools/pygrub/examples/fedora-31.grubenv
@@ -0,0 +1,5 @@
+# GRUB Environment Block
+saved_entry=fbc80dcce05d4a1a9b9a7951232a4eeb-5.3.6-300.fc31.x86_64
+kernelopts=root=/dev/mapper/fedora_localhost--live-root ro resume=/dev/mapper/fedora_localhost--live-swap rd.lvm.lv=fedora_localhost-live/root rd.lvm.lv=fedora_localhost-live/swap rhgb quiet
+boot_success=1
+######################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################
\ No newline at end of file
-- 
2.21.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH 0/3] read grubenv and set default from it

[Steven Haigh]

Just for the record, the grub packages have been updated in Fedora 31 
to automatically disable BLS when installing / removing a kernel on Xen 
Dom0 / DomU installations.

As such, we should never come across a Fedora 31 install with BLS 
enabled from this point forwards.

There is currently ongoing work to disable BLS during the installation 
via anaconda - but this hasn't hit yet - and I believe it's already a 
freeze exception.

If / when pygrub is able to properly read and boot from BLS based 
configurations (I'm not sure if this patchset makes pygrub BLS 
compatible, or just fixes the existing issues) - but we can look at 
revisiting removing these workarounds from anaconda / grub2 packages in 
F30 / F31 / Rawhide.

Steven Haigh

📧 netwiz@crc.id.au     💻 https://www.crc.id.au
📞 +613 9001 6090       📱 +614 1293 5897


On Fri, Oct 25, 2019 at 22:52, "YOUNG, MICHAEL A." 
<m.a.young@durham.ac.uk> wrote:
> This series of patches is to improve the parsing by pygrub of grub
> configuration on Fedora. The current result of parsing is generally
> that the second kernel listed is set as the default due to a
> set default=1 line in grub.cfg which is only intended to be
> reached after repeated boot failures.
> 
> The patches read the grubenv file (which consists of key=value lines
> padded to 1024 characters by # characters) to get the values of
> next_entry and saved_entry, which can be a kernel string or an
> order number. Unfortunately, for Fedora 31 at least, this is
> often a BLS-style string so it isn't necessarily useful. The patches
> use the value of next_entry or of saved_entry to set the default
> kernel or sets it to the first kernel listed if those values are set
> but not used.
> 
> 
> Michael Young (3):
>   set default kernel from grubenv next_entry or saved_entry
>   read a grubenv file if it is next to the grub.cfg file
>   Example Fedora 31 grub.cfg and grubenv files
> 
>  tools/pygrub/examples/fedora-31.grub.cfg | 200 
> +++++++++++++++++++++++
>  tools/pygrub/examples/fedora-31.grubenv  |   5 +
>  tools/pygrub/src/GrubConf.py             |  31 +++-
>  tools/pygrub/src/pygrub                  |  21 ++-
>  4 files changed, 253 insertions(+), 4 deletions(-)
>  create mode 100644 tools/pygrub/examples/fedora-31.grub.cfg
>  create mode 100644 tools/pygrub/examples/fedora-31.grubenv
> 
> --
> 2.21.0
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH 0/3] read grubenv and set default from it

[YOUNG, MICHAEL A.]

On Sat, 26 Oct 2019, Steven Haigh wrote:

> If / when pygrub is able to properly read and boot from BLS based 
> configurations (I'm not sure if this patchset makes pygrub BLS compatible, or 
> just fixes the existing issues) - but we can look at revisiting removing 
> these workarounds from anaconda / grub2 packages in F30 / F31 / Rawhide.

The patchset doesn't add BLS compatibility but should be useful for what I 
expect BLS support to look like (I have a idea of what would be required 
though I haven't worked out the details yet).

 	Michael Young

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH 0/3] read grubenv and set default from it

[Steven Haigh]

Awesome - thanks Michael.

I'll try and test this out tomorrow.
Steven Haigh

📧 netwiz@crc.id.au     💻 https://www.crc.id.au
📞 +613 9001 6090       📱 +614 1293 5897


On Sat, Oct 26, 2019 at 16:00, "YOUNG, MICHAEL A." 
<m.a.young@durham.ac.uk> wrote:
> On Sat, 26 Oct 2019, Steven Haigh wrote:
> 
>>  If / when pygrub is able to properly read and boot from BLS based
>>  configurations (I'm not sure if this patchset makes pygrub BLS 
>> compatible, or
>>  just fixes the existing issues) - but we can look at revisiting 
>> removing
>>  these workarounds from anaconda / grub2 packages in F30 / F31 / 
>> Rawhide.
> 
> The patchset doesn't add BLS compatibility but should be useful for 
> what I
> expect BLS support to look like (I have a idea of what would be 
> required
> though I haven't worked out the details yet).
> 
>  	Michael Young
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH 0/3] read grubenv and set default from it

[Steven Haigh]

On 2019-10-26 09:52, YOUNG, MICHAEL A. wrote:
> This series of patches is to improve the parsing by pygrub of grub
> configuration on Fedora. The current result of parsing is generally
> that the second kernel listed is set as the default due to a
> set default=1 line in grub.cfg which is only intended to be
> reached after repeated boot failures.
> 
> The patches read the grubenv file (which consists of key=value lines
> padded to 1024 characters by # characters) to get the values of
> next_entry and saved_entry, which can be a kernel string or an
> order number. Unfortunately, for Fedora 31 at least, this is
> often a BLS-style string so it isn't necessarily useful. The patches
> use the value of next_entry or of saved_entry to set the default
> kernel or sets it to the first kernel listed if those values are set
> but not used.
> 
> 
> Michael Young (3):
>   set default kernel from grubenv next_entry or saved_entry
>   read a grubenv file if it is next to the grub.cfg file
>   Example Fedora 31 grub.cfg and grubenv files
> 
>  tools/pygrub/examples/fedora-31.grub.cfg | 200 +++++++++++++++++++++++
>  tools/pygrub/examples/fedora-31.grubenv  |   5 +
>  tools/pygrub/src/GrubConf.py             |  31 +++-
>  tools/pygrub/src/pygrub                  |  21 ++-
>  4 files changed, 253 insertions(+), 4 deletions(-)
>  create mode 100644 tools/pygrub/examples/fedora-31.grub.cfg
>  create mode 100644 tools/pygrub/examples/fedora-31.grubenv

Tested-by: Steven Haigh <netwiz@crc.id.au>

No issues located, seems to work with F31 guests as advertised.

I believe these would be candidates for backports into other supported 
Xen versions as well.

-- 
Steven Haigh

? netwiz@crc.id.au     ? http://www.crc.id.au
? +61 (3) 9001 6090    ? 0412 935 897

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH 1/3] set default kernel from grubenv next_entry or saved_entry

[Ian Jackson]

YOUNG, MICHAEL A. writes ("[XEN PATCH 1/3] set default kernel from grubenv next_entry or saved_entry"):
> This patch reads the contents of a grubenv file if available, and
> uses the value of next_entry (in preference) or of saved_entry to
> set the default kernel if there is a matching title or if it is a
> number.  If either next_entry or saved_entry is set and neither is
> used then the default is set to 0.

Are you sure the grubenv file has a compatible enough syntax with the
grub.cfg file ?  I had a look at grub_split and AFIACT from the
confusing way it is expressed, it splits on the earliest of = ' ' '\t'
which I guess is right ...

Is it deliberate that your implementation strategy would honour
`next_entry' and `saved_entry' commands in grub.cfg ?

Are you sure it is correct that your implementation strategy would
honour `title' etc. if it occurred in grubenv ?  My reading of the
documentation is that grubenv may be less trusted.

Despite these misgivings, I think this patch is probably better than
nothing.

So:
  Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

But I think depending on the answer to my questions above we may want
a health warning of some kind in the release notes, or a followup.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH 2/3] read a grubenv file if it is next to the grub.cfg file

[Ian Jackson]

YOUNG, MICHAEL A. writes ("[XEN PATCH 2/3] read a grubenv file if it is next to the grub.cfg file"):
> When a grub.cfg file is found this patch checks if there is grubenv
> file in the same directory as the grub.cfg file. If there is it
> passes the contents to parse().

I am happy with the semantics of this patch.  But I am not really
happy with the duplication of the code to call self.cf.parse, so that
it is now quadriplicated.

> +        if fenv != "" and fs.file_exists(fenv):
> +            fenvf = fs.open_file(fenv)
> +            grubenv = fenvf.read(FS_READ_MAX)
> +            del fenvf
> +            if sys.version_info[0] < 3:
> +                self.cf.parse(buf, grubenv)
> +            else:
> +                self.cf.parse(buf.decode(), grubenv.decode())
> +        else:
> +            if sys.version_info[0] < 3:
> +                self.cf.parse(buf)
> +            else:
> +                self.cf.parse(buf.decode())

Can you please do something like

   grubenv = None
   if fenv ...
       ...

   if grubenv is not None and sys.version_info[0] < 3:
       grubenv = grubenv.decode()

and then you could at least avoid further multiplications of the call
to .parse...

Thanks,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH 3/3] Example Fedora 31 grub.cfg and grubenv files

[Ian Jackson]

YOUNG, MICHAEL A. writes ("[XEN PATCH 3/3] Example Fedora 31 grub.cfg and grubenv files"):
> This patch adds an example grub.cfg and grubenv file for reference

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Actual test cases of some kind would be most welcome.

Thanks,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel