From: Ian Campbell <Ian.Campbell@citrix.com>
To: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>,
Boris Ostrovsky <boris.ostrovsky@oracle.com>,
Ian Jackson <Ian.Jackson@eu.citrix.com>,
Xen-devel <xen-devel@lists.xen.org>
Subject: Re: [PATCH for-4.5 RFC] pygrub: Fix regression from c/s d1b93ea, attempt 2
Date: Thu, 20 Nov 2014 16:00:38 +0000 [thread overview]
Message-ID: <1416499238.14429.39.camel@citrix.com> (raw)
In-Reply-To: <1416237586-17785-1-git-send-email-andrew.cooper3@citrix.com>
On Mon, 2014-11-17 at 15:19 +0000, Andrew Cooper wrote:
> 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.
Would it be less invasive at this point in the release to have the place
where this stuff is used do isinstance(s, str) and isinstance(s, int)?
Also, in run_grub sel can be set to g.cf.default and then:
if sel == -1:
print "No kernel image selected!"
sys.exit(1)
I can't see where the -1 comes from though, and you aren't changing any
-1 into "-1", so maybe something more subtle is going on there.
Ian.
next prev parent reply other threads:[~2014-11-20 16:00 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-17 15:19 [PATCH for-4.5 RFC] pygrub: Fix regression from c/s d1b93ea, attempt 2 Andrew Cooper
2014-11-17 16:41 ` Boris Ostrovsky
2014-11-17 16:50 ` Andrew Cooper
2014-11-17 16:58 ` Ian Campbell
2014-11-17 16:59 ` Andrew Cooper
2014-11-17 17:15 ` Boris Ostrovsky
2014-11-20 16:00 ` Ian Campbell [this message]
2014-11-20 16:08 ` Andrew Cooper
2014-11-20 16:15 ` Ian Campbell
2014-11-20 16:45 ` Boris Ostrovsky
2014-11-21 13:32 ` Andrew Cooper
2014-11-21 17:09 ` Konrad Rzeszutek Wilk
2014-11-21 17:25 ` Boris Ostrovsky
2014-11-25 14:14 ` Ian Campbell
2014-11-25 16:03 ` Andrew Cooper
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1416499238.14429.39.camel@citrix.com \
--to=ian.campbell@citrix.com \
--cc=Ian.Jackson@eu.citrix.com \
--cc=andrew.cooper3@citrix.com \
--cc=boris.ostrovsky@oracle.com \
--cc=wei.liu2@citrix.com \
--cc=xen-devel@lists.xen.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.