Re: [PATCH v4] xen/pygrub: grub2/grub.cfg from RHEL 7 has new commands in menuentry

[Andrew Cooper <andrew.cooper3@citrix.com>]

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 <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