Marcel J.E. Mol
2013-Jun-24 16:21 UTC
[PATCH] pygrub/GrubConf: fix boot problem for fedora 19 grub.cfg (2nd attempt)
Booting a fedora 19 domU failed because a it could not properly parse the grub.cfg file. This was cased by set default="${next_entry}" This statement actually is within an ''if'' statement, so maybe it would be better to skip code within if/fi blocks... But this patch seems to work fine. Signed-off-by: Marcel Mol <marcel@mesa.nl> --- tools/pygrub/src/GrubConf.py | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/tools/pygrub/src/GrubConf.py b/tools/pygrub/src/GrubConf.py index 629951f..6324c62 100644 --- a/tools/pygrub/src/GrubConf.py +++ b/tools/pygrub/src/GrubConf.py @@ -427,6 +427,8 @@ class Grub2ConfigFile(_GrubConfigFile): if self.commands[com] is not None: if arg.strip() == "${saved_entry}": arg = "0" + elif arg.strip() == "${next_entry}": + arg = "0" setattr(self, self.commands[com], arg.strip()) else: logging.info("Ignored directive %s" %(com,)) -- 1.7.7.6 -- ======-------- Marcel J.E. Mol MESA Consulting B.V. =======--------- ph. +31-(0)6-54724868 P.O. Box 112 =======--------- marcel@mesa.nl 2630 AC Nootdorp __==== www.mesa.nl ---____U_n_i_x______I_n_t_e_r_n_e_t____ The Netherlands ____ They couldn''t think of a number, Linux user 1148 -- counter.li.org so they gave me a name! -- Rupert Hine -- www.ruperthine.com
Ian Campbell
2013-Jun-26 13:50 UTC
Re: [PATCH] pygrub/GrubConf: fix boot problem for fedora 19 grub.cfg (2nd attempt)
On Mon, 2013-06-24 at 18:21 +0200, Marcel J.E. Mol wrote:> Booting a fedora 19 domU failed because a it could not properly > parse the grub.cfg file. This was cased by > > set default="${next_entry}" >Any chance you could send us an example of the actual problematic configuration file, ideally as a patch adding a new file to tools/pygrub/examples/. That "test suite" is rather rudimentary (to the extent I''m a bit embarrassed to call it a suite...) but collecting them when a bug is fixed is a good way to grow it.> This statement actually is within an ''if'' statement, so maybe it would > be better to skip code within if/fi blocks... > But this patch seems to work fine. > > Signed-off-by: Marcel Mol <marcel@mesa.nl>This change looks good to me: Acked-by: Ian Campbell <ian.campbell@citix.com> George, WRT 4.3 what do you think? I think the change is low risk for any grub.conf without "${next_entry}" in it and a clear improvement to any which does.> --- > tools/pygrub/src/GrubConf.py | 2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/tools/pygrub/src/GrubConf.py b/tools/pygrub/src/GrubConf.py > index 629951f..6324c62 100644 > --- a/tools/pygrub/src/GrubConf.py > +++ b/tools/pygrub/src/GrubConf.py > @@ -427,6 +427,8 @@ class Grub2ConfigFile(_GrubConfigFile): > if self.commands[com] is not None: > if arg.strip() == "${saved_entry}": > arg = "0" > + elif arg.strip() == "${next_entry}": > + arg = "0" > setattr(self, self.commands[com], arg.strip()) > else: > logging.info("Ignored directive %s" %(com,)) > -- > 1.7.7.6 >
George Dunlap
2013-Jun-26 13:52 UTC
Re: [PATCH] pygrub/GrubConf: fix boot problem for fedora 19 grub.cfg (2nd attempt)
On Mon, Jun 24, 2013 at 5:21 PM, Marcel J.E. Mol <marcel@mesa.nl> wrote:> Booting a fedora 19 domU failed because a it could not properly > parse the grub.cfg file. This was cased by > > set default="${next_entry}" > > This statement actually is within an ''if'' statement, so maybe it would > be better to skip code within if/fi blocks... > But this patch seems to work fine. > > Signed-off-by: Marcel Mol <marcel@mesa.nl>Thanks Marcel -- is this in a default and/or auto-generated Fedora grub.cfg, or was this something you put in manually? In any case, I think it''s a bit too late for 4.3 -- there''s too much risk that this might break something else; and there has been over two months of RCs where Fedora users could have reported this bug. It will have to wait until 4.4 / 4.3.1. If this is part of a default instal, however, we should probably add a release note to that effect. -George
Wei Liu
2013-Jun-26 13:57 UTC
Re: [PATCH] pygrub/GrubConf: fix boot problem for fedora 19 grub.cfg (2nd attempt)
On Wed, Jun 26, 2013 at 02:52:05PM +0100, George Dunlap wrote:> On Mon, Jun 24, 2013 at 5:21 PM, Marcel J.E. Mol <marcel@mesa.nl> wrote: > > Booting a fedora 19 domU failed because a it could not properly > > parse the grub.cfg file. This was cased by > > > > set default="${next_entry}" > > > > This statement actually is within an ''if'' statement, so maybe it would > > be better to skip code within if/fi blocks... > > But this patch seems to work fine. > > > > Signed-off-by: Marcel Mol <marcel@mesa.nl> > > Thanks Marcel -- is this in a default and/or auto-generated Fedora > grub.cfg, or was this something you put in manually? >I think this is auto-generated. A patch back in May 3 changed grub.d/00_header.in. revno: 4969 committer: Andrey Borzenkov <arvidjaar@gmail.com> branch nick: trunk timestamp: Mon 2013-05-06 22:13:34 +0400 message: Reimplement grub-reboot to not depend on saved_entry. Use next_entry variable for one time boot menu entry. What surprises me is that Fedora 19 picked up the change so quickly... Wei.> In any case, I think it''s a bit too late for 4.3 -- there''s too much > risk that this might break something else; and there has been over two > months of RCs where Fedora users could have reported this bug. It > will have to wait until 4.4 / 4.3.1. >> If this is part of a default instal, however, we should probably add a > release note to that effect. > > -George > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
Konrad Rzeszutek Wilk
2013-Jun-26 15:01 UTC
Re: [PATCH] pygrub/GrubConf: fix boot problem for fedora 19 grub.cfg (2nd attempt)
On Mon, Jun 24, 2013 at 06:21:32PM +0200, Marcel J.E. Mol wrote:> Booting a fedora 19 domU failed because a it could not properly > parse the grub.cfg file. This was cased by > > set default="${next_entry}" > > This statement actually is within an ''if'' statement, so maybe it would > be better to skip code within if/fi blocks... > But this patch seems to work fine. > > Signed-off-by: Marcel Mol <marcel@mesa.nl>Excellent, and also Tested-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> and it also fixes Red Hat bug: 978036 George, would you be OK putting this in 4.3?> --- > tools/pygrub/src/GrubConf.py | 2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/tools/pygrub/src/GrubConf.py b/tools/pygrub/src/GrubConf.py > index 629951f..6324c62 100644 > --- a/tools/pygrub/src/GrubConf.py > +++ b/tools/pygrub/src/GrubConf.py > @@ -427,6 +427,8 @@ class Grub2ConfigFile(_GrubConfigFile): > if self.commands[com] is not None: > if arg.strip() == "${saved_entry}": > arg = "0" > + elif arg.strip() == "${next_entry}": > + arg = "0" > setattr(self, self.commands[com], arg.strip()) > else: > logging.info("Ignored directive %s" %(com,)) > -- > 1.7.7.6 > > -- > ======-------- Marcel J.E. Mol MESA Consulting B.V. > =======--------- ph. +31-(0)6-54724868 P.O. Box 112 > =======--------- marcel@mesa.nl 2630 AC Nootdorp > __==== www.mesa.nl ---____U_n_i_x______I_n_t_e_r_n_e_t____ The Netherlands ____ > They couldn''t think of a number, Linux user 1148 -- counter.li.org > so they gave me a name! -- Rupert Hine -- www.ruperthine.com > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel >
George Dunlap
2013-Jun-26 16:05 UTC
Re: [PATCH] pygrub/GrubConf: fix boot problem for fedora 19 grub.cfg (2nd attempt)
On Wed, Jun 26, 2013 at 4:01 PM, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:> On Mon, Jun 24, 2013 at 06:21:32PM +0200, Marcel J.E. Mol wrote: >> Booting a fedora 19 domU failed because a it could not properly >> parse the grub.cfg file. This was cased by >> >> set default="${next_entry}" >> >> This statement actually is within an ''if'' statement, so maybe it would >> be better to skip code within if/fi blocks... >> But this patch seems to work fine. >> >> Signed-off-by: Marcel Mol <marcel@mesa.nl> > > Excellent, and also > > Tested-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > > and it also fixes Red Hat bug: 978036 > > George, would you be OK putting this in 4.3?Hrm. On further review, since this affects basically all Fedora installations, and it mirrors a grub changeset that directlys witches ${saved_entry} to ${next_entry}, I guess on the whole it''s worth the risk. Re the release: Not-very-happilly-Acked-by: George Dunlap <george.dunlap@eu.citrix.com>
Marcel J.E. Mol
2013-Jun-26 18:14 UTC
Re: [PATCH] pygrub/GrubConf: fix boot problem for fedora 19 grub.cfg (2nd attempt)
On Wed, Jun 26, 2013 at 02:50:46PM +0100, Ian Campbell wrote:> On Mon, 2013-06-24 at 18:21 +0200, Marcel J.E. Mol wrote: > > Booting a fedora 19 domU failed because a it could not properly > > parse the grub.cfg file. This was cased by > > > > set default="${next_entry}" > > > > Any chance you could send us an example of the actual problematic > configuration file, ideally as a patch adding a new file to > tools/pygrub/examples/. That "test suite" is rather rudimentary (to the > extent I''m a bit embarrassed to call it a suite...) but collecting them > when a bug is fixed is a good way to grow it.The grub.cfg is autogenerated in Fedora 19. I''ll add a patch with the grub.cfg that failed on me. -- ======-------- Marcel J.E. Mol MESA Consulting B.V. =======--------- ph. +31-(0)6-54724868 P.O. Box 112 =======--------- marcel@mesa.nl 2630 AC Nootdorp __==== www.mesa.nl ---____U_n_i_x______I_n_t_e_r_n_e_t____ The Netherlands ____ They couldn''t think of a number, Linux user 1148 -- counter.li.org so they gave me a name! -- Rupert Hine -- www.ruperthine.com
Marcel J.E. Mol
2013-Jun-26 18:17 UTC
Re: [PATCH] pygrub/GrubConf: fix boot problem for fedora 19 grub.cfg (2nd attempt)
On Wed, Jun 26, 2013 at 02:52:05PM +0100, George Dunlap wrote:> On Mon, Jun 24, 2013 at 5:21 PM, Marcel J.E. Mol <marcel@mesa.nl> wrote: > > Booting a fedora 19 domU failed because a it could not properly > > parse the grub.cfg file. This was cased by > > > > set default="${next_entry}" > > > > This statement actually is within an ''if'' statement, so maybe it would > > be better to skip code within if/fi blocks... > > But this patch seems to work fine. > > > > Signed-off-by: Marcel Mol <marcel@mesa.nl> > > Thanks Marcel -- is this in a default and/or auto-generated Fedora > grub.cfg, or was this something you put in manually?Yes, it is auto generated, with all the defaults installing a new kernel will recreate the grub.cfg file.> In any case, I think it''s a bit too late for 4.3 -- there''s too much > risk that this might break something else; and there has been over two > months of RCs where Fedora users could have reported this bug. It > will have to wait until 4.4 / 4.3.1. > > If this is part of a default instal, however, we should probably add a > release note to that effect.It is quite a harmless patch. I wonder if it could break anything as pygrub does not seem to evaluate $variable in a grub.cfg file (correct me if I''m wrong...) -Marcel -- ======-------- Marcel J.E. Mol MESA Consulting B.V. =======--------- ph. +31-(0)6-54724868 P.O. Box 112 =======--------- marcel@mesa.nl 2630 AC Nootdorp __==== www.mesa.nl ---____U_n_i_x______I_n_t_e_r_n_e_t____ The Netherlands ____ They couldn''t think of a number, Linux user 1148 -- counter.li.org so they gave me a name! -- Rupert Hine -- www.ruperthine.com
Ian Campbell
2013-Jun-27 11:31 UTC
Re: [PATCH] pygrub/GrubConf: fix boot problem for fedora 19 grub.cfg (2nd attempt)
On Wed, 2013-06-26 at 17:05 +0100, George Dunlap wrote:> On Wed, Jun 26, 2013 at 4:01 PM, Konrad Rzeszutek Wilk > <konrad.wilk@oracle.com> wrote: > > On Mon, Jun 24, 2013 at 06:21:32PM +0200, Marcel J.E. Mol wrote: > >> Booting a fedora 19 domU failed because a it could not properly > >> parse the grub.cfg file. This was cased by > >> > >> set default="${next_entry}" > >> > >> This statement actually is within an ''if'' statement, so maybe it would > >> be better to skip code within if/fi blocks... > >> But this patch seems to work fine. > >> > >> Signed-off-by: Marcel Mol <marcel@mesa.nl> > > > > Excellent, and also > > > > Tested-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > > > > and it also fixes Red Hat bug: 978036 > > > > George, would you be OK putting this in 4.3? > > Hrm. On further review, since this affects basically all Fedora > installations, and it mirrors a grub changeset that directlys witches > ${saved_entry} to ${next_entry}, I guess on the whole it''s worth the > risk. > > Re the release: > > Not-very-happilly-Acked-by: George Dunlap <george.dunlap@eu.citrix.com>Applied.
Konrad Rzeszutek Wilk
2013-Jul-02 15:01 UTC
Re: [PATCH] pygrub/GrubConf: fix boot problem for fedora 19 grub.cfg (2nd attempt)
On Mon, Jun 24, 2013 at 06:21:32PM +0200, Marcel J.E. Mol wrote:> Booting a fedora 19 domU failed because a it could not properly > parse the grub.cfg file. This was cased by > > set default="${next_entry}" > > This statement actually is within an ''if'' statement, so maybe it would > be better to skip code within if/fi blocks... > But this patch seems to work fine. > > Signed-off-by: Marcel Mol <marcel@mesa.nl>Hey Marcel, Thank you for finding the patch. As you have seen it is both in Xen 4.4 and it is part of the Fedora 19 Xen 4.2 candidate patches. I was wondering if you have the time to leave feedback at https://admin.fedoraproject.org/updates/FEDORA-2013-11837/xen-4.2.2-10.fc19 for the updated Xen RPMs which have this patch (and also some other ones)? And again - thank you for taking the time to dig in PyGrub and to submit the patch!> --- > tools/pygrub/src/GrubConf.py | 2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/tools/pygrub/src/GrubConf.py b/tools/pygrub/src/GrubConf.py > index 629951f..6324c62 100644 > --- a/tools/pygrub/src/GrubConf.py > +++ b/tools/pygrub/src/GrubConf.py > @@ -427,6 +427,8 @@ class Grub2ConfigFile(_GrubConfigFile): > if self.commands[com] is not None: > if arg.strip() == "${saved_entry}": > arg = "0" > + elif arg.strip() == "${next_entry}": > + arg = "0" > setattr(self, self.commands[com], arg.strip()) > else: > logging.info("Ignored directive %s" %(com,)) > -- > 1.7.7.6 > > -- > ======-------- Marcel J.E. Mol MESA Consulting B.V. > =======--------- ph. +31-(0)6-54724868 P.O. Box 112 > =======--------- marcel@mesa.nl 2630 AC Nootdorp > __==== www.mesa.nl ---____U_n_i_x______I_n_t_e_r_n_e_t____ The Netherlands ____ > They couldn''t think of a number, Linux user 1148 -- counter.li.org > so they gave me a name! -- Rupert Hine -- www.ruperthine.com > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel >
Marcel J.E. Mol
2013-Jul-02 23:07 UTC
Re: [PATCH] pygrub/GrubConf: fix boot problem for fedora 19 grub.cfg (2nd attempt)
On Tue, Jul 02, 2013 at 11:01:18AM -0400, Konrad Rzeszutek Wilk wrote:> On Mon, Jun 24, 2013 at 06:21:32PM +0200, Marcel J.E. Mol wrote: > > Booting a fedora 19 domU failed because a it could not properly > > parse the grub.cfg file. This was cased by > > > > set default="${next_entry}" > > > > This statement actually is within an ''if'' statement, so maybe it would > > be better to skip code within if/fi blocks... > > But this patch seems to work fine. > > > > Signed-off-by: Marcel Mol <marcel@mesa.nl> > > Hey Marcel, > > Thank you for finding the patch. As you have seen it is both in Xen 4.4 and > it is part of the Fedora 19 Xen 4.2 candidate patches. I was wondering if you > have the time to leave feedback at https://admin.fedoraproject.org/updates/FEDORA-2013-11837/xen-4.2.2-10.fc19 > for the updated Xen RPMs which have this patch (and also some other ones)?Done> > And again - thank you for taking the time to dig in PyGrub and to submit the > patch!Glad to be of help. My server was still running the rawhide from fedora core 8, being the latest dom0 fedora for a long time. I just thought it was about time to update it :) Fedora 19 was almost there so I just gave it a try on a test machine. Running the old domU cliens on the new xen server went fine and as I planned to also update the domU clients, I first tried installing a fresh fedora 19 client. And we know what happened then... It was a bit difficult to debug the code as I''m not that fluent in python and somehow the error message was vague as it does not indicate the real code location where the problem occured. Thanks, -Marcel -- ======-------- Marcel J.E. Mol MESA Consulting B.V. =======--------- ph. +31-(0)6-54724868 P.O. Box 112 =======--------- marcel@mesa.nl 2630 AC Nootdorp __==== www.mesa.nl ---____U_n_i_x______I_n_t_e_r_n_e_t____ The Netherlands ____ They couldn''t think of a number, Linux user 1148 -- counter.li.org so they gave me a name! -- Rupert Hine -- www.ruperthine.com