From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH v4] xen/pygrub: grub2/grub.cfg from RHEL 7 has new commands in menuentry Date: Wed, 5 Feb 2014 12:21:00 +0000 Message-ID: <52F22CAC.3050003@citrix.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: Ian Campbell , Joby Poriyath , xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 05/02/14 11:49, 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? No - it is well specified regex syntax. Skimming the xend code, it gets moderate use. It is even already used in pygrub itself: "bootfsgroup = re.findall('zfs-bootfs=(.*?)[\s\,\"]', bootfsargs)" > 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. That is a matter of opinion, but I would disagree. I personally use lazy matching quite often, and encounter it moderately frequently in others code. ~Andrew > >>> >>> 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). > > 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