From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH v4] xen/pygrub: grub2/grub.cfg from RHEL 7 has new commands in menuentry Date: Wed, 5 Feb 2014 12:07:14 +0000 Message-ID: <1391602034.6497.128.camel@kazak.uk.xensource.com> References: <20140204181023.GA5293@citrix.com> <1391592179.6497.73.camel@kazak.uk.xensource.com> <52F2253B.9000000@eu.citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <52F2253B.9000000@eu.citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: George Dunlap Cc: Andrew Cooper , Joby Poriyath , xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On Wed, 2014-02-05 at 11:49 +0000, George Dunlap wrote: > 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 believe it is. Joby posted a link earlier. It also seems to be part of the Perl re syntax -- and lots of things use Perl's regex syntax so I think it is pretty "standard" (although I was not previously aware of it either). Wikipedia's regex page talks about it too. > 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. Hypothetical developer ignorance might call for a comment, but I think avoiding language features which provide the semantics we need just because they are a bit obscure would be a mistake. > >> Signed-off-by: Joby Poriyath > >> Reviewed-by: Andrew Cooper > >> Cc: george.dunlap@citrix.com > > Acked-by: Ian Campbell > > > > 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). FWIW I intended to run it over the (admittedly small) set of test cases in the tree as part of the commit process. I believe Joby has already done so anyway. > 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.