#39 - RHEL 7 doesn't boot under pygrub

Owner: George Dunlap <George.Dunlap@eu.citrix.com>

Date: Mon Feb 10 16:45:01 2014

Last Update: Mon Feb 10 16:45:02 2014

Severity: normal

Affects:

State: Closed

[ Retrieve as mbox ]


From: Joby Poriyath <joby.poriyath@citrix.com>
To: Ian Campbell <Ian.Campbell@citrix.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>, George Dunlap <george.dunlap@eu.citrix.com>, xen-devel@lists.xen.org
Subject: [Xen-devel] [PATCH v4] xen/pygrub: grub2/grub.cfg from RHEL 7 has new commands in menuentry
Date: Tue, 4 Feb 2014 18:10:35 +0000
Message-ID: <20140204181023.GA5293@citrix.com>

[ Reply to this message; Retrieve Raw Message; Archives: marc.info, gmane ]

menuentry in grub2/grub.cfg uses linux16 and initrd16 commands
instead of linux and initrd. Due to this RHEL 7 (beta) guest failed to
boot after the installation.

In addition to this, RHEL 7 menu entries have two different single-quote
delimited strings on the same line, and the greedy grouping for menuentry
parsing gets both strings, and the options inbetween.

Signed-off-by: Joby Poriyath <joby.poriyath@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: george.dunlap@citrix.com
---
v2: Added RHEL 7 grub.cfg in pygrub/examples
v3 & v4: Tidied the commit message based on Andrew Cooper's feedback

Kindly consider this patch for xen-4.4 as RHEL 7 (beta) fails to boot
on Xen.

 tools/pygrub/examples/rhel-7-beta.grub2 |  118 +++++++++++++++++++++++++++++++
 tools/pygrub/src/GrubConf.py            |    4 +-
 2 files changed, 121 insertions(+), 1 deletion(-)
 create mode 100644 tools/pygrub/examples/rhel-7-beta.grub2

diff --git a/tools/pygrub/examples/rhel-7-beta.grub2 b/tools/pygrub/examples/rhel-7-beta.grub2
new file mode 100644
index 0000000..88f0f99
--- /dev/null
+++ b/tools/pygrub/examples/rhel-7-beta.grub2
@@ -0,0 +1,118 @@
+#
+# 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 [ -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
+set timeout=5
+### END /etc/grub.d/00_header ###
+
+### BEGIN /etc/grub.d/10_linux ###
+menuentry 'Red Hat Enterprise Linux Everything, with Linux 3.10.0-54.0.1.el7.x86_64' --class red --class gnu-linux --class gnu --class os $menuentry_id_option 'gnulinux-3.10.0-54.0.1.el7.x86_64-advanced-d23b8b49-4cfe-4900-8ef1-ec80bc633163' {
+	load_video
+	set gfxpayload=keep
+	insmod gzio
+	insmod part_msdos
+	insmod xfs
+	set root='hd0,msdos1'
+	if [ x$feature_platform_search_hint = xy ]; then
+	  search --no-floppy --fs-uuid --set=root --hint='hd0,msdos1'  89ffef78-82b3-457c-bc57-42cccc373851
+	else
+	  search --no-floppy --fs-uuid --set=root 89ffef78-82b3-457c-bc57-42cccc373851
+	fi
+	linux16 /vmlinuz-3.10.0-54.0.1.el7.x86_64 root=/dev/mapper/rhel-root ro rd.lvm.lv=rhel/swap vconsole.keymap=uk crashkernel=auto rd.lvm.lv=rhel/root vconsole.font=latarcyrheb-sun16 LANG=en_GB.UTF-8
+	initrd16 /initramfs-3.10.0-54.0.1.el7.x86_64.img
+}
+menuentry 'Red Hat Enterprise Linux Everything, with Linux 0-rescue-af34f0b8cf364cdbbe6d093f8228a37f' --class red --class gnu-linux --class gnu --class os $menuentry_id_option 'gnulinux-0-rescue-af34f0b8cf364cdbbe6d093f8228a37f-advanced-d23b8b49-4cfe-4900-8ef1-ec80bc633163' {
+	load_video
+	insmod gzio
+	insmod part_msdos
+	insmod xfs
+	set root='hd0,msdos1'
+	if [ x$feature_platform_search_hint = xy ]; then
+	  search --no-floppy --fs-uuid --set=root --hint='hd0,msdos1'  89ffef78-82b3-457c-bc57-42cccc373851
+	else
+	  search --no-floppy --fs-uuid --set=root 89ffef78-82b3-457c-bc57-42cccc373851
+	fi
+	linux16 /vmlinuz-0-rescue-af34f0b8cf364cdbbe6d093f8228a37f root=/dev/mapper/rhel-root ro rd.lvm.lv=rhel/swap vconsole.keymap=uk crashkernel=auto rd.lvm.lv=rhel/root vconsole.font=latarcyrheb-sun16
+	initrd16 /initramfs-0-rescue-af34f0b8cf364cdbbe6d093f8228a37f.img
+}
+
+### END /etc/grub.d/10_linux ###
+
+### 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/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/src/GrubConf.py b/tools/pygrub/src/GrubConf.py
index cb853c9..974cded 100644
--- a/tools/pygrub/src/GrubConf.py
+++ b/tools/pygrub/src/GrubConf.py
@@ -348,7 +348,9 @@ class Grub2Image(_GrubImage):
                 
     commands = {'set:root': 'root',
                 'linux': 'kernel',
+                'linux16': 'kernel',
                 'initrd': 'initrd',
+                'initrd16': 'initrd',
                 'echo': None,
                 'insmod': None,
                 'search': None}
@@ -394,7 +396,7 @@ class Grub2ConfigFile(_GrubConfigFile):
                 continue
 
             # new image
-            title_match = re.match('^menuentry ["\'](.*)["\'] (.*){', l)
+            title_match = re.match('^menuentry ["\'](.*?)["\'] (.*){', l)
             if title_match:
                 if img is not None:
                     raise RuntimeError, "syntax error: cannot nest menuentry (%d %s)" % (len(img),img)
-- 
1.7.10.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

From: Ian Campbell <Ian.Campbell@citrix.com>
To: Joby Poriyath <joby.poriyath@citrix.com>
Cc: xen-devel@lists.xen.org, George Dunlap <george.dunlap@eu.citrix.com>, Andrew Cooper <andrew.cooper3@citrix.com>
Subject: Re: [Xen-devel] [PATCH v4] xen/pygrub: grub2/grub.cfg from RHEL 7 has new commands in menuentry
Date: Wed, 5 Feb 2014 09:22:59 +0000
Message-ID: <1391592179.6497.73.camel@kazak.uk.xensource.com>

[ Reply to this message; Retrieve Raw Message; Archives: marc.info, gmane ]

On Tue, 2014-02-04 at 18:10 +0000, Joby Poriyath wrote:
> menuentry in grub2/grub.cfg uses linux16 and initrd16 commands
> instead of linux and initrd. Due to this RHEL 7 (beta) guest failed to
> boot after the installation.
> 
> In addition to this, RHEL 7 menu entries have two different single-quote
> delimited strings on the same line, and the greedy grouping for menuentry
> parsing gets both strings, and the options inbetween.
> 
> Signed-off-by: Joby Poriyath <joby.poriyath@citrix.com>
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: george.dunlap@citrix.com

Acked-by: Ian Campbell <ian.campbell@citrix.com>

IMHO this can go into 4.4, unless George objects today I shall commit.

Ian,

> ---
> v2: Added RHEL 7 grub.cfg in pygrub/examples
> v3 & v4: Tidied the commit message based on Andrew Cooper's feedback
> 
> Kindly consider this patch for xen-4.4 as RHEL 7 (beta) fails to boot
> on Xen.
> 
>  tools/pygrub/examples/rhel-7-beta.grub2 |  118 +++++++++++++++++++++++++++++++
>  tools/pygrub/src/GrubConf.py            |    4 +-
>  2 files changed, 121 insertions(+), 1 deletion(-)
>  create mode 100644 tools/pygrub/examples/rhel-7-beta.grub2
> 
> diff --git a/tools/pygrub/examples/rhel-7-beta.grub2 b/tools/pygrub/examples/rhel-7-beta.grub2
> new file mode 100644
> index 0000000..88f0f99
> --- /dev/null
> +++ b/tools/pygrub/examples/rhel-7-beta.grub2
> @@ -0,0 +1,118 @@
> +#
> +# 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 [ -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
> +set timeout=5
> +### END /etc/grub.d/00_header ###
> +
> +### BEGIN /etc/grub.d/10_linux ###
> +menuentry 'Red Hat Enterprise Linux Everything, with Linux 3.10.0-54.0.1.el7.x86_64' --class red --class gnu-linux --class gnu --class os $menuentry_id_option 'gnulinux-3.10.0-54.0.1.el7.x86_64-advanced-d23b8b49-4cfe-4900-8ef1-ec80bc633163' {
> +	load_video
> +	set gfxpayload=keep
> +	insmod gzio
> +	insmod part_msdos
> +	insmod xfs
> +	set root='hd0,msdos1'
> +	if [ x$feature_platform_search_hint = xy ]; then
> +	  search --no-floppy --fs-uuid --set=root --hint='hd0,msdos1'  89ffef78-82b3-457c-bc57-42cccc373851
> +	else
> +	  search --no-floppy --fs-uuid --set=root 89ffef78-82b3-457c-bc57-42cccc373851
> +	fi
> +	linux16 /vmlinuz-3.10.0-54.0.1.el7.x86_64 root=/dev/mapper/rhel-root ro rd.lvm.lv=rhel/swap vconsole.keymap=uk crashkernel=auto rd.lvm.lv=rhel/root vconsole.font=latarcyrheb-sun16 LANG=en_GB.UTF-8
> +	initrd16 /initramfs-3.10.0-54.0.1.el7.x86_64.img
> +}
> +menuentry 'Red Hat Enterprise Linux Everything, with Linux 0-rescue-af34f0b8cf364cdbbe6d093f8228a37f' --class red --class gnu-linux --class gnu --class os $menuentry_id_option 'gnulinux-0-rescue-af34f0b8cf364cdbbe6d093f8228a37f-advanced-d23b8b49-4cfe-4900-8ef1-ec80bc633163' {
> +	load_video
> +	insmod gzio
> +	insmod part_msdos
> +	insmod xfs
> +	set root='hd0,msdos1'
> +	if [ x$feature_platform_search_hint = xy ]; then
> +	  search --no-floppy --fs-uuid --set=root --hint='hd0,msdos1'  89ffef78-82b3-457c-bc57-42cccc373851
> +	else
> +	  search --no-floppy --fs-uuid --set=root 89ffef78-82b3-457c-bc57-42cccc373851
> +	fi
> +	linux16 /vmlinuz-0-rescue-af34f0b8cf364cdbbe6d093f8228a37f root=/dev/mapper/rhel-root ro rd.lvm.lv=rhel/swap vconsole.keymap=uk crashkernel=auto rd.lvm.lv=rhel/root vconsole.font=latarcyrheb-sun16
> +	initrd16 /initramfs-0-rescue-af34f0b8cf364cdbbe6d093f8228a37f.img
> +}
> +
> +### END /etc/grub.d/10_linux ###
> +
> +### 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/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/src/GrubConf.py b/tools/pygrub/src/GrubConf.py
> index cb853c9..974cded 100644
> --- a/tools/pygrub/src/GrubConf.py
> +++ b/tools/pygrub/src/GrubConf.py
> @@ -348,7 +348,9 @@ class Grub2Image(_GrubImage):
>                  
>      commands = {'set:root': 'root',
>                  'linux': 'kernel',
> +                'linux16': 'kernel',
>                  'initrd': 'initrd',
> +                'initrd16': 'initrd',
>                  'echo': None,
>                  'insmod': None,
>                  'search': None}
> @@ -394,7 +396,7 @@ class Grub2ConfigFile(_GrubConfigFile):
>                  continue
>  
>              # new image
> -            title_match = re.match('^menuentry ["\'](.*)["\'] (.*){', l)
> +            title_match = re.match('^menuentry ["\'](.*?)["\'] (.*){', l)
>              if title_match:
>                  if img is not None:
>                      raise RuntimeError, "syntax error: cannot nest menuentry (%d %s)" % (len(img),img)



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

From: George Dunlap <george.dunlap@eu.citrix.com>
To: Joby Poriyath <joby.poriyath@citrix.com>, Ian Campbell <Ian.Campbell@citrix.com>
Cc: xen-devel@lists.xen.org, Andrew Cooper <andrew.cooper3@citrix.com>
Subject: Re: [Xen-devel] [PATCH v4] xen/pygrub: grub2/grub.cfg from RHEL 7 has new commands in menuentry
Date: Wed, 5 Feb 2014 11:49:15 +0000
Message-ID: <52F2253B.9000000@eu.citrix.com>

[ Reply to this message; Retrieve Raw Message; Archives: marc.info, gmane ]

On 02/05/2014 09:22 AM, Ian Campbell wrote:
> On Tue, 2014-02-04 at 18:10 +0000, Joby Poriyath wrote:
>> menuentry in grub2/grub.cfg uses linux16 and initrd16 commands
>> instead of linux and initrd. Due to this RHEL 7 (beta) guest failed to
>> boot after the installation.
>>
>> In addition to this, RHEL 7 menu entries have two different single-quote
>> delimited strings on the same line, and the greedy grouping for menuentry
>> parsing gets both strings, and the options inbetween.

So you're saying that adding the '?' just happens to change the match 
because of a quirk in the algorithms in the python library? That seems 
more like a hack than a proper fix; there may be other versions of 
python (future versions, for instance) where the new regexp will have 
the same effect as the old one, and we'll have another regression.

Even if the behavior described is part of the defined interface, I'd be 
wary of using this because future developers may not realize what it's 
for, or how to modify it properly to retain the properties it has now.

>>
>> Signed-off-by: Joby Poriyath <joby.poriyath@citrix.com>
>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> Cc: george.dunlap@citrix.com
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
>
> IMHO this can go into 4.4, unless George objects today I shall commit.

I'm a bit on the fence about this one.  If this had been sent a month 
ago, it would be a no-brainer.  It certainly looks like it should work 
just fine.  On the other hand, pygrub is an important bit of 
functionality, and I'm not sure how much testing it gets. But of course 
the XenServer XenRT tests probably exercise it fairly well (or else they 
wouldn't be submitting this patch).

The Register seems to think that RHEL will be released "in the first 
half of 2014", which would certainly be before 4.5.  But we should have 
another point release before then, with enough time to do better testing 
and (possibly) come up with a better solution to the regexp problem 
above (assuming my interpretation is correct).

I'm wondering though whether it would make more sense to save this for 
4.4.1.

  -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

From: Ian Campbell <Ian.Campbell@citrix.com>
To: George Dunlap <george.dunlap@eu.citrix.com>
Cc: xen-devel@lists.xen.org, Andrew Cooper <andrew.cooper3@citrix.com>, Joby Poriyath <joby.poriyath@citrix.com>
Subject: Re: [Xen-devel] [PATCH v4] xen/pygrub: grub2/grub.cfg from RHEL 7 has new commands in menuentry
Date: Wed, 5 Feb 2014 12:07:14 +0000
Message-ID: <1391602034.6497.128.camel@kazak.uk.xensource.com>

[ Reply to this message; Retrieve Raw Message; Archives: marc.info, gmane ]

On Wed, 2014-02-05 at 11:49 +0000, George Dunlap wrote:
> On 02/05/2014 09:22 AM, Ian Campbell wrote:
> > On Tue, 2014-02-04 at 18:10 +0000, Joby Poriyath wrote:
> >> menuentry in grub2/grub.cfg uses linux16 and initrd16 commands
> >> instead of linux and initrd. Due to this RHEL 7 (beta) guest failed to
> >> boot after the installation.
> >>
> >> In addition to this, RHEL 7 menu entries have two different single-quote
> >> delimited strings on the same line, and the greedy grouping for menuentry
> >> parsing gets both strings, and the options inbetween.
> 
> So you're saying that adding the '?' just happens to change the match 
> because of a quirk in the algorithms in the python library? That seems 
> more like a hack than a proper fix; there may be other versions of 
> python (future versions, for instance) where the new regexp will have 
> the same effect as the old one, and we'll have another regression.
> 
> Even if the behavior described is part of the defined interface,

I believe it is. Joby posted a link earlier. It also seems to be part of
the Perl re syntax -- and lots of things use Perl's regex syntax so I
think it is pretty "standard" (although I was not previously aware of it
either). Wikipedia's regex page talks about it too.

>  I'd be 
> wary of using this because future developers may not realize what it's 
> for, or how to modify it properly to retain the properties it has now.

Hypothetical developer ignorance might call for a comment, but I think
avoiding language features which provide the semantics we need just
because they are a bit obscure would be a mistake.

> >> Signed-off-by: Joby Poriyath <joby.poriyath@citrix.com>
> >> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> >> Cc: george.dunlap@citrix.com
> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> >
> > IMHO this can go into 4.4, unless George objects today I shall commit.
> 
> I'm a bit on the fence about this one.  If this had been sent a month 
> ago, it would be a no-brainer.  It certainly looks like it should work 
> just fine.  On the other hand, pygrub is an important bit of 
> functionality, and I'm not sure how much testing it gets. But of course 
> the XenServer XenRT tests probably exercise it fairly well (or else they 
> wouldn't be submitting this patch).

FWIW I intended to run it over the (admittedly small) set of test cases
in the tree as part of the commit process. I believe Joby has already
done so anyway.

> The Register seems to think that RHEL will be released "in the first 
> half of 2014", which would certainly be before 4.5.  But we should have 
> another point release before then, with enough time to do better testing 
> and (possibly) come up with a better solution to the regexp problem 
> above (assuming my interpretation is correct).
> 
> I'm wondering though whether it would make more sense to save this for 
> 4.4.1.



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

From: Andrew Cooper <andrew.cooper3@citrix.com>
To: George Dunlap <george.dunlap@eu.citrix.com>
Cc: xen-devel@lists.xen.org, Ian Campbell <Ian.Campbell@citrix.com>, Joby Poriyath <joby.poriyath@citrix.com>
Subject: Re: [Xen-devel] [PATCH v4] xen/pygrub: grub2/grub.cfg from RHEL 7 has new commands in menuentry
Date: Wed, 5 Feb 2014 12:21:00 +0000
Message-ID: <52F22CAC.3050003@citrix.com>

[ Reply to this message; Retrieve Raw Message; Archives: marc.info, gmane ]

On 05/02/14 11:49, George Dunlap wrote:
> On 02/05/2014 09:22 AM, Ian Campbell wrote:
>> On Tue, 2014-02-04 at 18:10 +0000, Joby Poriyath wrote:
>>> menuentry in grub2/grub.cfg uses linux16 and initrd16 commands
>>> instead of linux and initrd. Due to this RHEL 7 (beta) guest failed to
>>> boot after the installation.
>>>
>>> In addition to this, RHEL 7 menu entries have two different
>>> single-quote
>>> delimited strings on the same line, and the greedy grouping for
>>> menuentry
>>> parsing gets both strings, and the options inbetween.
>
> So you're saying that adding the '?' just happens to change the match
> because of a quirk in the algorithms in the python library?

No - it is well specified regex syntax.

Skimming the xend code, it gets moderate use.

It is even already used in pygrub itself: "bootfsgroup =
re.findall('zfs-bootfs=(.*?)[\s\,\"]', bootfsargs)"


> That seems more like a hack than a proper fix; there may be other
> versions of python (future versions, for instance) where the new
> regexp will have the same effect as the old one, and we'll have
> another regression.
>
> Even if the behavior described is part of the defined interface, I'd
> be wary of using this because future developers may not realize what
> it's for, or how to modify it properly to retain the properties it has
> now.

That is a matter of opinion, but I would disagree.  I personally use
lazy matching quite often, and encounter it moderately frequently in
others code.

~Andrew

>
>>>
>>> Signed-off-by: Joby Poriyath <joby.poriyath@citrix.com>
>>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>> Cc: george.dunlap@citrix.com
>> Acked-by: Ian Campbell <ian.campbell@citrix.com>
>>
>> IMHO this can go into 4.4, unless George objects today I shall commit.
>
> I'm a bit on the fence about this one.  If this had been sent a month
> ago, it would be a no-brainer.  It certainly looks like it should work
> just fine.  On the other hand, pygrub is an important bit of
> functionality, and I'm not sure how much testing it gets. But of
> course the XenServer XenRT tests probably exercise it fairly well (or
> else they wouldn't be submitting this patch).
>
> The Register seems to think that RHEL will be released "in the first
> half of 2014", which would certainly be before 4.5.  But we should
> have another point release before then, with enough time to do better
> testing and (possibly) come up with a better solution to the regexp
> problem above (assuming my interpretation is correct).
>
> I'm wondering though whether it would make more sense to save this for
> 4.4.1.
>
>  -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

From: George Dunlap <george.dunlap@eu.citrix.com>
To: Ian Campbell <Ian.Campbell@citrix.com>
Cc: xen-devel@lists.xen.org, Joby Poriyath <joby.poriyath@citrix.com>, Andrew Cooper <andrew.cooper3@citrix.com>
Subject: Re: [Xen-devel] [PATCH v4] xen/pygrub: grub2/grub.cfg from RHEL 7 has new commands in menuentry
Date: Wed, 5 Feb 2014 12:33:07 +0000
Message-ID: <52F22F83.50607@eu.citrix.com>

[ Reply to this message; Retrieve Raw Message; Archives: marc.info, gmane ]

On 02/05/2014 12:07 PM, Ian Campbell wrote:
> On Wed, 2014-02-05 at 11:49 +0000, George Dunlap wrote:
>> On 02/05/2014 09:22 AM, Ian Campbell wrote:
>>> On Tue, 2014-02-04 at 18:10 +0000, Joby Poriyath wrote:
>>>> menuentry in grub2/grub.cfg uses linux16 and initrd16 commands
>>>> instead of linux and initrd. Due to this RHEL 7 (beta) guest failed to
>>>> boot after the installation.
>>>>
>>>> In addition to this, RHEL 7 menu entries have two different single-quote
>>>> delimited strings on the same line, and the greedy grouping for menuentry
>>>> parsing gets both strings, and the options inbetween.
>> So you're saying that adding the '?' just happens to change the match
>> because of a quirk in the algorithms in the python library? That seems
>> more like a hack than a proper fix; there may be other versions of
>> python (future versions, for instance) where the new regexp will have
>> the same effect as the old one, and we'll have another regression.
>>
>> Even if the behavior described is part of the defined interface,
> I believe it is. Joby posted a link earlier. It also seems to be part of
> the Perl re syntax -- and lots of things use Perl's regex syntax so I
> think it is pretty "standard" (although I was not previously aware of it
> either). Wikipedia's regex page talks about it too.
>
>>   I'd be
>> wary of using this because future developers may not realize what it's
>> for, or how to modify it properly to retain the properties it has now.
> Hypothetical developer ignorance might call for a comment, but I think
> avoiding language features which provide the semantics we need just
> because they are a bit obscure would be a mistake.

Well of course having something fixed in an obscure fashion is better 
than not having it fixed at all.  But having it fixed in a way which is 
more obvious and doesn't rely on quirks of internal algorithms is even 
better yet, if it can be done without being too ugly.  Wouldn't 
something like the following work, and be more robust? (I haven't tested 
this, I'm just going on Joby's description of the problem.)

title_match = re.match('^menuentry ["\']([^"\']*)["\'] (.*){', l)


OTOH, if as Andrew claims (in another e-mail in this thread), ".*?" is a 
common idiom for exactly that (and since it's so common, I can well 
believe there is an idiom) then it's even better to go with the idiom.

>
>>>> Signed-off-by: Joby Poriyath <joby.poriyath@citrix.com>
>>>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>> Cc: george.dunlap@citrix.com
>>> Acked-by: Ian Campbell <ian.campbell@citrix.com>
>>>
>>> IMHO this can go into 4.4, unless George objects today I shall commit.
>> I'm a bit on the fence about this one.  If this had been sent a month
>> ago, it would be a no-brainer.  It certainly looks like it should work
>> just fine.  On the other hand, pygrub is an important bit of
>> functionality, and I'm not sure how much testing it gets. But of course
>> the XenServer XenRT tests probably exercise it fairly well (or else they
>> wouldn't be submitting this patch).
> FWIW I intended to run it over the (admittedly small) set of test cases
> in the tree as part of the commit process. I believe Joby has already
> done so anyway.

Sure; I'm worried about the near infinite number of other grub configs 
not in our tree. :-)  (Although the vast majority are likely to be 
represented by those generated by a distro's update-grub with the 
default parameters.)

On the whole, given that RHEL 7 is not yet out, and the timing of the 
patch, I'm inclined to say this should wait until 4.4.1, unless there's 
a reason that wouldn't be suitable.

  -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

From: Ian Campbell <Ian.Campbell@citrix.com>
To: George Dunlap <george.dunlap@eu.citrix.com>
Cc: Joby Poriyath <joby.poriyath@citrix.com>, Andrew Cooper <andrew.cooper3@citrix.com>, xen-devel@lists.xen.org
Subject: Re: [Xen-devel] [PATCH v4] xen/pygrub: grub2/grub.cfg from RHEL 7 has new commands in menuentry
Date: Wed, 5 Feb 2014 12:45:32 +0000
Message-ID: <1391604332.6497.147.camel@kazak.uk.xensource.com>

[ Reply to this message; Retrieve Raw Message; Archives: marc.info, gmane ]

On Wed, 2014-02-05 at 12:33 +0000, George Dunlap wrote:
> On 02/05/2014 12:07 PM, Ian Campbell wrote:
> > On Wed, 2014-02-05 at 11:49 +0000, George Dunlap wrote:
> >> On 02/05/2014 09:22 AM, Ian Campbell wrote:
> >>> On Tue, 2014-02-04 at 18:10 +0000, Joby Poriyath wrote:
> >>>> menuentry in grub2/grub.cfg uses linux16 and initrd16 commands
> >>>> instead of linux and initrd. Due to this RHEL 7 (beta) guest failed to
> >>>> boot after the installation.
> >>>>
> >>>> In addition to this, RHEL 7 menu entries have two different single-quote
> >>>> delimited strings on the same line, and the greedy grouping for menuentry
> >>>> parsing gets both strings, and the options inbetween.
> >> So you're saying that adding the '?' just happens to change the match
> >> because of a quirk in the algorithms in the python library? That seems
> >> more like a hack than a proper fix; there may be other versions of
> >> python (future versions, for instance) where the new regexp will have
> >> the same effect as the old one, and we'll have another regression.
> >>
> >> Even if the behavior described is part of the defined interface,
> > I believe it is. Joby posted a link earlier. It also seems to be part of
> > the Perl re syntax -- and lots of things use Perl's regex syntax so I
> > think it is pretty "standard" (although I was not previously aware of it
> > either). Wikipedia's regex page talks about it too.
> >
> >>   I'd be
> >> wary of using this because future developers may not realize what it's
> >> for, or how to modify it properly to retain the properties it has now.
> > Hypothetical developer ignorance might call for a comment, but I think
> > avoiding language features which provide the semantics we need just
> > because they are a bit obscure would be a mistake.
> 
> Well of course having something fixed in an obscure fashion is better 
> than not having it fixed at all.  But having it fixed in a way which is 
> more obvious and doesn't rely on quirks of internal algorithms is even 
> better yet, if it can be done without being too ugly.  Wouldn't 
> something like the following work, and be more robust? (I haven't tested 
> this, I'm just going on Joby's description of the problem.)
> 
> title_match = re.match('^menuentry ["\']([^"\']*)["\'] (.*){', l)
> 
> 
> OTOH, if as Andrew claims 

As did I, right above.

> (in another e-mail in this thread), ".*?" is a 
> common idiom for exactly that (and since it's so common, I can well 
> believe there is an idiom) then it's even better to go with the idiom.

Yes, that was my point, this is apparently a common idiom.

> >>>> Signed-off-by: Joby Poriyath <joby.poriyath@citrix.com>
> >>>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> >>>> Cc: george.dunlap@citrix.com
> >>> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> >>>
> >>> IMHO this can go into 4.4, unless George objects today I shall commit.
> >> I'm a bit on the fence about this one.  If this had been sent a month
> >> ago, it would be a no-brainer.  It certainly looks like it should work
> >> just fine.  On the other hand, pygrub is an important bit of
> >> functionality, and I'm not sure how much testing it gets. But of course
> >> the XenServer XenRT tests probably exercise it fairly well (or else they
> >> wouldn't be submitting this patch).
> > FWIW I intended to run it over the (admittedly small) set of test cases
> > in the tree as part of the commit process. I believe Joby has already
> > done so anyway.
> 
> Sure; I'm worried about the near infinite number of other grub configs 
> not in our tree. :-)  (Although the vast majority are likely to be 
> represented by those generated by a distro's update-grub with the 
> default parameters.)
> 
> On the whole, given that RHEL 7 is not yet out, and the timing of the 
> patch, I'm inclined to say this should wait until 4.4.1, unless there's 
> a reason that wouldn't be suitable.
> 
>   -George



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

From: George Dunlap <George.Dunlap@eu.citrix.com>
To: Joby Poriyath <joby.poriyath@citrix.com>
Cc: "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>, Ian Campbell <Ian.Campbell@citrix.com>, Andrew Cooper <andrew.cooper3@citrix.com>
Subject: Re: [Xen-devel] [PATCH v4] xen/pygrub: grub2/grub.cfg from RHEL 7 has new commands in menuentry
Date: Mon, 10 Feb 2014 16:40:25 +0000
Message-ID: <CAFLBxZZOxFGRsHdDdz+LzMBsuSBkOMmXtKKLX1bpHRW4H+zU6A@mail.gmail.com>

[ Reply to this message; Retrieve Raw Message; Archives: marc.info, gmane ]

create ^
title it RHEL 7 doesn't boot under pygrub
thanks

On Tue, Feb 4, 2014 at 6:10 PM, Joby Poriyath <joby.poriyath@citrix.com> wrote:
> menuentry in grub2/grub.cfg uses linux16 and initrd16 commands
> instead of linux and initrd. Due to this RHEL 7 (beta) guest failed to
> boot after the installation.
>
> In addition to this, RHEL 7 menu entries have two different single-quote
> delimited strings on the same line, and the greedy grouping for menuentry
> parsing gets both strings, and the options inbetween.
>
> Signed-off-by: Joby Poriyath <joby.poriyath@citrix.com>
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: george.dunlap@citrix.com
> ---
> v2: Added RHEL 7 grub.cfg in pygrub/examples
> v3 & v4: Tidied the commit message based on Andrew Cooper's feedback
>
> Kindly consider this patch for xen-4.4 as RHEL 7 (beta) fails to boot
> on Xen.
>
>  tools/pygrub/examples/rhel-7-beta.grub2 |  118 +++++++++++++++++++++++++++++++
>  tools/pygrub/src/GrubConf.py            |    4 +-
>  2 files changed, 121 insertions(+), 1 deletion(-)
>  create mode 100644 tools/pygrub/examples/rhel-7-beta.grub2
>
> diff --git a/tools/pygrub/examples/rhel-7-beta.grub2 b/tools/pygrub/examples/rhel-7-beta.grub2
> new file mode 100644
> index 0000000..88f0f99
> --- /dev/null
> +++ b/tools/pygrub/examples/rhel-7-beta.grub2
> @@ -0,0 +1,118 @@
> +#
> +# 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 [ -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
> +set timeout=5
> +### END /etc/grub.d/00_header ###
> +
> +### BEGIN /etc/grub.d/10_linux ###
> +menuentry 'Red Hat Enterprise Linux Everything, with Linux 3.10.0-54.0.1.el7.x86_64' --class red --class gnu-linux --class gnu --class os $menuentry_id_option 'gnulinux-3.10.0-54.0.1.el7.x86_64-advanced-d23b8b49-4cfe-4900-8ef1-ec80bc633163' {
> +       load_video
> +       set gfxpayload=keep
> +       insmod gzio
> +       insmod part_msdos
> +       insmod xfs
> +       set root='hd0,msdos1'
> +       if [ x$feature_platform_search_hint = xy ]; then
> +         search --no-floppy --fs-uuid --set=root --hint='hd0,msdos1'  89ffef78-82b3-457c-bc57-42cccc373851
> +       else
> +         search --no-floppy --fs-uuid --set=root 89ffef78-82b3-457c-bc57-42cccc373851
> +       fi
> +       linux16 /vmlinuz-3.10.0-54.0.1.el7.x86_64 root=/dev/mapper/rhel-root ro rd.lvm.lv=rhel/swap vconsole.keymap=uk crashkernel=auto rd.lvm.lv=rhel/root vconsole.font=latarcyrheb-sun16 LANG=en_GB.UTF-8
> +       initrd16 /initramfs-3.10.0-54.0.1.el7.x86_64.img
> +}
> +menuentry 'Red Hat Enterprise Linux Everything, with Linux 0-rescue-af34f0b8cf364cdbbe6d093f8228a37f' --class red --class gnu-linux --class gnu --class os $menuentry_id_option 'gnulinux-0-rescue-af34f0b8cf364cdbbe6d093f8228a37f-advanced-d23b8b49-4cfe-4900-8ef1-ec80bc633163' {
> +       load_video
> +       insmod gzio
> +       insmod part_msdos
> +       insmod xfs
> +       set root='hd0,msdos1'
> +       if [ x$feature_platform_search_hint = xy ]; then
> +         search --no-floppy --fs-uuid --set=root --hint='hd0,msdos1'  89ffef78-82b3-457c-bc57-42cccc373851
> +       else
> +         search --no-floppy --fs-uuid --set=root 89ffef78-82b3-457c-bc57-42cccc373851
> +       fi
> +       linux16 /vmlinuz-0-rescue-af34f0b8cf364cdbbe6d093f8228a37f root=/dev/mapper/rhel-root ro rd.lvm.lv=rhel/swap vconsole.keymap=uk crashkernel=auto rd.lvm.lv=rhel/root vconsole.font=latarcyrheb-sun16
> +       initrd16 /initramfs-0-rescue-af34f0b8cf364cdbbe6d093f8228a37f.img
> +}
> +
> +### END /etc/grub.d/10_linux ###
> +
> +### 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/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/src/GrubConf.py b/tools/pygrub/src/GrubConf.py
> index cb853c9..974cded 100644
> --- a/tools/pygrub/src/GrubConf.py
> +++ b/tools/pygrub/src/GrubConf.py
> @@ -348,7 +348,9 @@ class Grub2Image(_GrubImage):
>
>      commands = {'set:root': 'root',
>                  'linux': 'kernel',
> +                'linux16': 'kernel',
>                  'initrd': 'initrd',
> +                'initrd16': 'initrd',
>                  'echo': None,
>                  'insmod': None,
>                  'search': None}
> @@ -394,7 +396,7 @@ class Grub2ConfigFile(_GrubConfigFile):
>                  continue
>
>              # new image
> -            title_match = re.match('^menuentry ["\'](.*)["\'] (.*){', l)
> +            title_match = re.match('^menuentry ["\'](.*?)["\'] (.*){', l)
>              if title_match:
>                  if img is not None:
>                      raise RuntimeError, "syntax error: cannot nest menuentry (%d %s)" % (len(img),img)
> --
> 1.7.10.4
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


From: Ian Campbell <Ian.Campbell@citrix.com>
To: Joby Poriyath <joby.poriyath@citrix.com>
Cc: xen-devel@lists.xen.org, George Dunlap <george.dunlap@eu.citrix.com>, Andrew Cooper <andrew.cooper3@citrix.com>
Subject: Re: [Xen-devel] [PATCH v4] xen/pygrub: grub2/grub.cfg from RHEL 7 has new commands in menuentry
Date: Wed, 12 Mar 2014 14:49:48 +0000
Message-ID: <1394635788.21145.101.camel@kazak.uk.xensource.com>

[ Reply to this message; Retrieve Raw Message; Archives: marc.info, gmane ]

On Tue, 2014-02-04 at 18:10 +0000, Joby Poriyath wrote:
> menuentry in grub2/grub.cfg uses linux16 and initrd16 commands
> instead of linux and initrd. Due to this RHEL 7 (beta) guest failed to
> boot after the installation.
> 
> In addition to this, RHEL 7 menu entries have two different single-quote
> delimited strings on the same line, and the greedy grouping for menuentry
> parsing gets both strings, and the options inbetween.
> 
> Signed-off-by: Joby Poriyath <joby.poriyath@citrix.com>
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

Already acked during 4.4 freeze, now applied to staging. Thanks.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

From: Ian Campbell <Ian.Campbell@citrix.com>
To: xen-devel <xen-devel@lists.xen.org>
Subject: [Xen-devel] closing various bugs
Date: Thu, 27 Mar 2014 12:44:54 +0000
Message-ID: <1395924294.22909.113.camel@kazak.uk.xensource.com>

[ Reply to this message; Retrieve Raw Message; Archives: marc.info, gmane ]

#15 - qemu-traditional mis-parses host bus 8 as 0
close 15
#21 - credit2: init_pcpu doesn't grab runqueue lock before pointing
close 21
#31 - xl: global keymap= option not recognised
close 31
#33 - Re: xl errors if /var/log/xen doesn't exist
close 33
#35 - stubdom build failures with gcc 4.8
close 35
#37 - Disable IOMMU if no soutbridge
close 37
#39 - RHEL 7 doesn't boot under pygrub
close 39
thanks



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel