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

[Xen-devel] [PATCH for-4.5 RFC] pygrub: Fix regression from c/s d1b93ea, attempt 2



c/s d1b93ea causes substantial functional regressions in pygrub's ability to
parse bootloader configuration files.

c/s d1b93ea itself changed an an interface which previously used exclusively
integers, to using strings in the case of a grub configuration with explicit
default set, along with changing the code calling the interface to require a
string.  The default value for "default" remained as an integer.

As a result, any Extlinux or Lilo configuration (which drives this interface
exclusively with integers), or Grub configuration which doesn't explicitly
declare a default will die with an AttributeError when attempting to call
"self.cf.default.isdigit()" where "default" is an integer.

Sadly, this AttributeError gets swallowed by the blanket ignore in the loop
which searches partitions for valid bootloader configurations, causing the
issue to be reported as "Unable to find partition containing kernel"

This patch attempts to fix the issue by altering all parts of this interface
to use strings, as opposed to integers.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CC: Ian Campbell <Ian.Campbell@xxxxxxxxxx>
CC: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>
CC: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
CC: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>

---

This patch is RFC because, while I have dev tested and proved that it unwedges
the specific senario I encountered, I have not yet tested that it contines to
boot all other PV guests.

As for 4.5-ness, this is a must-fix as far as I am concerned

Either:
 1) Revert d1b93ea (original bad changeset) and 4ee393f (attempt 1 to fix)
 2) Take this patch in addition which hopefully fixes the regressions
---
 tools/pygrub/src/ExtLinuxConf.py |    6 +++---
 tools/pygrub/src/GrubConf.py     |    7 ++-----
 tools/pygrub/src/LiloConf.py     |    6 +++---
 3 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/tools/pygrub/src/ExtLinuxConf.py b/tools/pygrub/src/ExtLinuxConf.py
index 510099b..e70fca6 100644
--- a/tools/pygrub/src/ExtLinuxConf.py
+++ b/tools/pygrub/src/ExtLinuxConf.py
@@ -123,7 +123,7 @@ class ExtLinuxConfigFile(object):
         self.filename = fn
         self.images = []
         self.timeout = -1
-        self._default = 0
+        self._default = "0"
 
         if fn is not None:
             self.parse()
@@ -191,8 +191,8 @@ class ExtLinuxConfigFile(object):
     def _get_default(self):
         for i in range(len(self.images)):
             if self.images[i].title == self._default:
-                return i
-        return 0
+                return str(i)
+        return "0"
     def _set_default(self, val):
         self._default = val
     default = property(_get_default, _set_default)
diff --git a/tools/pygrub/src/GrubConf.py b/tools/pygrub/src/GrubConf.py
index dea7044..645b6e2 100644
--- a/tools/pygrub/src/GrubConf.py
+++ b/tools/pygrub/src/GrubConf.py
@@ -170,7 +170,7 @@ class _GrubConfigFile(object):
         self.filename = fn
         self.images = []
         self.timeout = -1
-        self._default = 0
+        self._default = "0"
         self.passwordAccess = True
         self.passExc = None
 
@@ -229,12 +229,9 @@ class _GrubConfigFile(object):
         return self._default
     def _set_default(self, val):
         if val == "saved":
-            self._default = 0
+            self._default = "0"
         else:
             self._default = val
-
-        if self._default < 0:
-            raise ValueError, "default must be positive number"
     default = property(_get_default, _set_default)
 
     def set_splash(self, val):
diff --git a/tools/pygrub/src/LiloConf.py b/tools/pygrub/src/LiloConf.py
index 2cb649f..53411e6 100644
--- a/tools/pygrub/src/LiloConf.py
+++ b/tools/pygrub/src/LiloConf.py
@@ -89,7 +89,7 @@ class LiloConfigFile(object):
         self.filename = fn
         self.images = []
         self.timeout = -1
-        self._default = 0
+        self._default = "0"
 
         if fn is not None:
             self.parse()
@@ -156,8 +156,8 @@ class LiloConfigFile(object):
     def _get_default(self):
         for i in range(len(self.images)):
             if self.images[i].title == self._default:
-                return i
-        return 0
+                return str(i)
+        return "0"
     def _set_default(self, val):
         self._default = val
     default = property(_get_default, _set_default)
-- 
1.7.10.4


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.