Date: Tue, 9 Dec 2008 00:51:19 +0100 From: Luigi Rizzo <rizzo@iet.unipi.it> To: Maxim Sobolev <sobomax@freebsd.org> Cc: Luigi Rizzo <luigi@freebsd.org>, hackers@freebsd.org, "current@freebsd.org" <current@freebsd.org> Subject: Re: Enhancing cdboot [patch for review] Message-ID: <20081208235119.GA46608@onelab2.iet.unipi.it> In-Reply-To: <493DA269.2070805@FreeBSD.org> References: <493DA269.2070805@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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. 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); 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 ? 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 4. another nitpick -- the value you pass in %si to the MBR does not seem to point to anything useful. As discussed about boot0.S and the followup in the mailing lists, there seems to be no standard but at least some MBR expect %si to point to a partition entry, so you should probably initialize one in a way similar way to that used by boot0.S cheers luigi
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20081208235119.GA46608>