Re: [Qemu-devel] [Patch] set boot sequence from command line

[J. Mayer]

On Wed, 2007-10-24 at 23:59 +0200, andrzej zaborowski wrote:
> On 24/10/2007, Dan Kenigsberg <address@hidden> wrote:
> > Real PCs try to boot from a CD, then try the hard drive, and finally go
> > to the network. And they let their owner change that order.
> 
> With the difference that on real PCs this is controlled by the BIOS
> menu rather than a hardware switch, but the latter seems more
> convenient for qemu.
> 
> > This patch lets Qemu do the same with "-boot dcn".
> >
> > I'll be happy to hear comments about it,
> >
> > Dan.
> >
> > diff --git a/hw/an5206.c b/hw/an5206.c
> > index 94ecccb..2134184 100644
> > --- a/hw/an5206.c
> > +++ b/hw/an5206.c
> > @@ -27,7 +27,7 @@ void DMA_run (void)
> >
> >  /* Board init.  */
> >
> > -static void an5206_init(int ram_size, int vga_ram_size, int boot_device,
> > +static void an5206_init(int ram_size, int vga_ram_size, char *boot_device,
> >                       DisplayState *ds, const char **fd_filename, int 
> > snapshot,
> >                       const char *kernel_filename, const char 
> > *kernel_cmdline,
> >                       const char *initrd_filename, const char *cpu_model)
> 
> BTW, it may be a good idea to pass all these values (maybe except ds)
> as a single struct, for purely practical reasons.
> Regards

Maybe the use of several structure may be better:
- one could be used to describe the kernel boot (with kernel_filename,
kernel_cmdline, initrd_filename) and could be NULL
- one for the hardware emulation parameters (ram_size, vga_ram_sze,
cpu_model)
- one for the emulation parameters if needed (snapshot, ds...)
...
It may be more consistent than use a single structure melting misc stuff
that are not directly related.

One remark about the submited patch:
why are the boot order table limited to 3 elements ? There are at least
4 choices available today (floppy, disk, CDROM, network) and maybe more
in the future for some architecture (refering to the curently emulated
hardware: 2 floppies, 4 IDE devices, <n> network devices, SCSI
storages, ...).

I guess it's not a so good idea to override the boot_device table in the
machine init routines. Imho, it would better be passed as a const char *
argument. For the PowerPC part, there could be a local int boot_device
variable that would be initialized to the first argument of the table.
And this would not change the NVRAM initialisation API: if this API need
to support more than one boot device in the future, it will have to be
completelly reworked (for other reasons too), then I would suggest to be
conservative and do not change this API at all.

-- 
J. Mayer <address@hidden>
Never organized