xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: George Dunlap <george.dunlap@eu.citrix.com>
To: Ian Campbell <Ian.Campbell@citrix.com>,
	Joby Poriyath <joby.poriyath@citrix.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>, xen-devel@lists.xen.org
Subject: Re: [PATCH v4] xen/pygrub: grub2/grub.cfg from RHEL 7 has new commands in menuentry
Date: Wed, 5 Feb 2014 11:49:15 +0000	[thread overview]
Message-ID: <52F2253B.9000000@eu.citrix.com> (raw)
In-Reply-To: <1391592179.6497.73.camel@kazak.uk.xensource.com>

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

  reply	other threads:[~2014-02-05 11:49 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-04 18:10 [PATCH v4] xen/pygrub: grub2/grub.cfg from RHEL 7 has new commands in menuentry Joby Poriyath
2014-02-05  9:22 ` Ian Campbell
2014-02-05 11:49   ` George Dunlap [this message]
2014-02-05 12:07     ` Ian Campbell
2014-02-05 12:33       ` George Dunlap
2014-02-05 12:45         ` Ian Campbell
2014-02-05 12:21     ` Andrew Cooper
2014-02-10 16:40 ` George Dunlap
2014-02-10 16:45   ` Processed: " xen
2014-03-12 14:49 ` Ian Campbell

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=52F2253B.9000000@eu.citrix.com \
    --to=george.dunlap@eu.citrix.com \
    --cc=Ian.Campbell@citrix.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=joby.poriyath@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).