[PATCH v2] tools/pygrub: Make pygrub understand default entry in string format

[PATCH v2] tools/pygrub: Make pygrub understand default entry in string format

[Boris Ostrovsky]

Currently pygrub can only correctly parse grub2's default attribute when
it is specified as a number. If it is set to ${saved_entry} or
${next_entry} then the first image (i.e. entry number 0) is selected.
If any other value is specified (typically this would be the string in
menuentry) pygrub will crash.

This patch will allow pygrub to interpret default attribute if it is
specified as a string (note that in case of submenus only the leaf
string will be considered).

Also issue a warning if default is set to ${saved_entry} or ${next_entry}.

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---

v2: log.warning() -> logging.warning()

 tools/pygrub/src/GrubConf.py |   10 +++++-----
 tools/pygrub/src/pygrub      |   21 ++++++++++++++++++++-
 2 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/tools/pygrub/src/GrubConf.py b/tools/pygrub/src/GrubConf.py
index 974cded..1d8644d 100644
--- a/tools/pygrub/src/GrubConf.py
+++ b/tools/pygrub/src/GrubConf.py
@@ -231,7 +231,7 @@ class _GrubConfigFile(object):
         if val == "saved":
             self._default = 0
         else:
-            self._default = int(val)
+            self._default = val
 
         if self._default < 0:
             raise ValueError, "default must be positive number"
@@ -431,11 +431,11 @@ class Grub2ConfigFile(_GrubConfigFile):
                 
             if self.commands.has_key(com):
                 if self.commands[com] is not None:
-                    if arg.strip() == "${saved_entry}":
+                    arg_strip = arg.strip()
+                    if arg_strip() == "${saved_entry}" or arg_strip == "${next_entry}":
+                        logging.warning("grub2's saved_entry/next_entry not supported")
                         arg = "0"
-                    elif arg.strip() == "${next_entry}":
-                        arg = "0"
-                    setattr(self, self.commands[com], arg.strip())
+                    setattr(self, self.commands[com], arg_strip)
                 else:
                     logging.info("Ignored directive %s" %(com,))
             elif com.startswith('set:'):
diff --git a/tools/pygrub/src/pygrub b/tools/pygrub/src/pygrub
index a4a2423..f46e54f 100644
--- a/tools/pygrub/src/pygrub
+++ b/tools/pygrub/src/pygrub
@@ -450,9 +450,28 @@ class Grub:
     def run(self):
         timeout = int(self.cf.timeout)
 
-        self.selected_image = self.cf.default
+        if self.cf.default.isdigit():
+            self.selected_image = int(self.cf.default)
+        else:
+            # We don't fully support submenus. Look for the leaf value in
+            # "submenu0>submenu1>...>menuentry" and hope that it's unique.
+            title = self.cf.default
+            while (1):
+                try:
+                    title = re.search('(\S)>(\S.+$)',title).group(2)
+                except AttributeError:
+                    break
+
+            # Map string to index in images array
+            self.selected_image = 0
+            for i in range(len(self.cf.images)):
+                if self.cf.images[i].title == title:
+                    self.selected_image = i
+                    break
+
         # If the selected (default) image doesn't exist we select the first entry
         if self.selected_image > len(self.cf.images):
+            logging.warning("Default image not found")
             self.selected_image = 0
         self.isdone = False
         while not self.isdone:
-- 
1.7.10.4

Re: [PATCH v2] tools/pygrub: Make pygrub understand default entry in string format

[Ian Campbell]

On Thu, 2014-06-26 at 15:34 -0400, Boris Ostrovsky wrote:
>                  if self.commands[com] is not None:
> -                    if arg.strip() == "${saved_entry}":
> +                    arg_strip = arg.strip()
> +                    if arg_strip() == "${saved_entry}" or arg_strip == "${next_entry}":

Is arg_strip (being the result of arg.strip()) really a callable object
here?

> diff --git a/tools/pygrub/src/pygrub b/tools/pygrub/src/pygrub
> index a4a2423..f46e54f 100644
> --- a/tools/pygrub/src/pygrub
> +++ b/tools/pygrub/src/pygrub
> +            # We don't fully support submenus. Look for the leaf value in
> +            # "submenu0>submenu1>...>menuentry" and hope that it's unique.
> +            title = self.cf.default
> +            while (1):

I'm not much of a Python-head, but "while True:" surely?

Ian.

Re: [PATCH v2] tools/pygrub: Make pygrub understand default entry in string format

[Boris Ostrovsky]

On 06/27/2014 07:58 AM, Ian Campbell wrote:
> On Thu, 2014-06-26 at 15:34 -0400, Boris Ostrovsky wrote:
>>                   if self.commands[com] is not None:
>> -                    if arg.strip() == "${saved_entry}":
>> +                    arg_strip = arg.strip()
>> +                    if arg_strip() == "${saved_entry}" or arg_strip == "${next_entry}":
> Is arg_strip (being the result of arg.strip()) really a callable object
> here?

No, that's a typo.


>> diff --git a/tools/pygrub/src/pygrub b/tools/pygrub/src/pygrub
>> index a4a2423..f46e54f 100644
>> --- a/tools/pygrub/src/pygrub
>> +++ b/tools/pygrub/src/pygrub
>> +            # We don't fully support submenus. Look for the leaf value in
>> +            # "submenu0>submenu1>...>menuentry" and hope that it's unique.
>> +            title = self.cf.default
>> +            while (1):
> I'm not much of a Python-head, but "while True:" surely?

They are both valid constructs (although possibly without parentheses). 
In fact, pygrub uses both:

ostr@workbase> egrep "while 1|while True" tools/pygrub/src/pygrub
         while 1:
         while 1:
         while True:
ostr@workbase>

Moreover, apparently, using '1' is very slightly faster. Not that I knew 
about it when I wrote this code ;-).

-boris