(dropping anaconda-devel and pjones from the cc... not generally
applicable for anaconda-devel and I'm fielding this for Peter)
On Wed, 2006-06-07 at 17:26 -0400, Aron Griffis wrote:
> The following patches give grubby the ability to manipulate
> multiboot-capable elilo.conf. Fedora's elilo already contains the
> support, these patches simply teach grubby to take advantage of it.
> These patches are an enabler for using Xen on ia64 with Fedora.
>
> I have tested these changes with both grubby and elilo, using both
> multiboot and non-multiboot configurations.
Aron --
Overall, these look pretty good. A few stylistic-ish things
* The patches don't work individually -- applying them in sequence ends
up with some not compiling bits. Was pretty easy to fix up, but worth
noting
* dprintf is already included in the standard library as a GNU
extension, but with a different functionality -- something like
dbgprintf would be better just to avoid problems there
* Instead of having the getLineByType2 thing, it would probably be a
little cleaner to just change the types to be a normal int instead of an
enum and then be able to determine matches via bitwise operators
In the bigger realm, there seems to be a bug or two lingering. Running
the test suite ('make test' from the toplevel) seems to fail for
x86/x86_64 with the patches applied -- it looks like we're gaining an
initrd in cases where it's unexpected (copy-default shouldn't be copying
the initrd, but it looks like it might be). Also, it looks like kernel
arguments might be getting lost in the multiboot case. I'll try to look
at this a little more later in the afternoon, but given how the past two
weeks have been "later in the afternoon" could end up being Friday :/
Also, it'd be nice to have some test cases to add to the test suite just
so that we can more easily ensure that minor bugfixes don't cause
regressions.
Jeremy
|
