Date: Wed, 10 Dec 2008 13:31:23 -0500 From: John Baldwin <jhb@freebsd.org> To: freebsd-current@freebsd.org Cc: Maxim Sobolev <sobomax@freebsd.org>, Luigi Rizzo <luigi@freebsd.org>, hackers@freebsd.org, Luigi Rizzo <rizzo@iet.unipi.it>, "current@freebsd.org" <current@freebsd.org> Subject: Re: Enhancing cdboot [patch for review] Message-ID: <200812101331.24182.jhb@freebsd.org> In-Reply-To: <20081208235119.GA46608@onelab2.iet.unipi.it> References: <493DA269.2070805@FreeBSD.org> <20081208235119.GA46608@onelab2.iet.unipi.it>
next in thread | previous in thread | raw e-mail | index | archive | help
On Monday 08 December 2008 06:51:19 pm Luigi Rizzo wrote: > On Mon, Dec 08, 2008 at 02:40:41PM -0800, Maxim Sobolev wrote: > > Hi, > > > > Below please find patch that enhances cdboot with two compile-time options: > ... > > Any comments/suggestions are appreciated. If there are no objections I > > would like to commit the change. The long-term goal is to make > > CDBOOT_PROMPT default mode for installation CD. > > > > http://sobomax.sippysoft.com/~sobomax/cdboot.diff > > Looks good. Some comments: > 1. since there is plenty of space in the cdboot sector, why don't you > make the two option always compiled in, controlling which one to > activate through flags in the bootsector itself, to be set > patching the binary sector itself using a mechanism similar to > boot0cfg. > Of course you cannot alter a cdrom after you burn it, > but it makes it easier to build CDs with one or the other defaults, > patching cdboot or the iso image itself before creating/burning it. I don't think this is very useful because CDs are read-only. You can just as easily build a different cdboot rather than having to write some custom cdbootcfg util to patch the binary. > 2. in fact, the 'silent' option could be disabled at runtime by > pressing some key (e.g. adding a short wait loop before proceeding; > if this is meant for custom, unattended CDs the extra delay should not > matter much); I don't imagine anyone will know to press a key to get verbose messages, and the CD boot process is quick enough you would have to add an artificial delay to it to allow for the keypress. > 3. one nitpick -- in one of the first chunks you replace $start > with $LOAD, but if i am not mistaken operation depends on $LOAD = $start, > so why don't you always use the same ? No, because he relocates it, $start is now the relocated address, but the BIOS loads it at LOAD which is now != $start. > Also in terms of relocation size, wouldn't it be the case of > hardwiring the size of the cd boot sector: > > - mov $((end_init - start)/2),%cx > + mov 1024,%cx I prefer the existing code to make sure and copy the full boot loader, whatever it's size is. Maxim, My only comment is to please make the new block comment match the style of the existing block comments by having '#\n' lines before and after. -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200812101331.24182.jhb>