[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: [PATCHES] 686-bigmem/Xen enabled netboot images



On Thursday 10 July 2008, Ian Campbell wrote:
> First patch is to kernel wedge and adds the Xen block and net devices
> (optional since they won't appear in the 486 images) as well as making
> generic_serial optional in order to allow 686-bigmem kernel udebs to be
> built. [kernel-wedge.patch]

! +++ packages/kernel/kernel-wedge/modules/nic-modules	(working copy)
! +xen-netfront ?
! +++ packages/kernel/kernel-wedge/modules/scsi-modules	(working copy)
! +xen-blkfront ?

As these modules are only going to be used with the i386 -bigmem kernel 
and even only exist there, I wonder if we want them in kernel-wedge.

In this case I think adding them only where they are actually used is 
preferable. That would mean adding them in relevant files in 
linux-kernel-di-i386-2.6/modules/i386/ instead.

> Second patch is to linux-kernel-di-i386-2.6 and simply adds the
> 686-bigmem flavour kernels. [linux-kernel-di-i386-2.6.patch]

I think a bit longer explanation in the changelog would be good: that the 
new variant is intended to be used for Xen installations.

> Third patch is to base-installer and causes the 686-bigmem kernel to be
> installed into the new system iff the installer is also running a
> bigmem kernel. This has been filed as #480054 and I'll send an update
> there too. [base-installer.patch]

First of all I'm afraid that even your latest patch is going to be 
invalidated by a new change in i386 kernel selection: #490542 :-P

But more importantly, the last hunk of the patch is not going to do what 
you want. Did you run the testsuite ('cd kernel/tests; ./runtests i386')?
I think you'll see failures. Unless I'm very much mistaken your code does 
not produce the desired fallback to first the "plain" -686 flavor.

As #490542 is a further simplification I think it's easiest to have your 
change on top of that one (I expect to commit tomorrow). And with those 
changes I don't think a separate testcase for AMD is still needed.

> Final patch is the the installer itself to cause a 686-bigmem netboot
> image to be built. [installer.patch]

! +++ installer/build/config/i386/netboot-bigmem.cfg	(revision 0)

AFAICT the following is missing to ensure the result lands in a subdir:
EXTRANAME = netboot/bigmem/

Besides that I still wonder if just "bigmem" is the best name for this. I 
would think that users are going to be looking for Xen images.

My proposal would be to use "netboot-xen" as internal name (the fact that 
we need the bigmem kernel for Xen is secondary to the purpose for which 
we build it IMO) and maybe "netboot/xen-bigmem" for the directory 
(EXTRANAME).

> New from last time is a patch to finish-install to add a getty on hvc0
> if Xen is detected. It's unfortunate that the test for Xen has to be
> present but this is the only way which I could find that work reliably.
> [finish-install.patch]

No real problems except the indentation: please use tabs as in the rest of 
the file.


BTW, I propose that you (if you're interested) are given commit access to 
the D-I repository on alioth so you can commit this yourself and maintain 
Xen (and maybe virtualization in general) support directly. I see you 
already have an account.

Cheers,
FJP

Attachment: signature.asc
Description: This is a digitally signed message part.


Reply to: