From owner-freebsd-hackers@FreeBSD.ORG Wed Dec 10 18:53:26 2008 Return-Path: Delivered-To: hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 385D31065673; Wed, 10 Dec 2008 18:53:26 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from server.baldwin.cx (bigknife-pt.tunnel.tserv9.chi1.ipv6.he.net [IPv6:2001:470:1f10:75::2]) by mx1.freebsd.org (Postfix) with ESMTP id A7AB38FC24; Wed, 10 Dec 2008 18:53:25 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from localhost.corp.yahoo.com (john@localhost [IPv6:::1]) (authenticated bits=0) by server.baldwin.cx (8.14.3/8.14.3) with ESMTP id mBAIqvkR024770; Wed, 10 Dec 2008 13:53:14 -0500 (EST) (envelope-from jhb@freebsd.org) From: John Baldwin To: freebsd-current@freebsd.org Date: Wed, 10 Dec 2008 13:31:23 -0500 User-Agent: KMail/1.9.7 References: <493DA269.2070805@FreeBSD.org> <20081208235119.GA46608@onelab2.iet.unipi.it> In-Reply-To: <20081208235119.GA46608@onelab2.iet.unipi.it> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200812101331.24182.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH authentication, not delayed by milter-greylist-2.0.2 (server.baldwin.cx [IPv6:::1]); Wed, 10 Dec 2008 13:53:17 -0500 (EST) X-Virus-Scanned: ClamAV 0.94.2/8743/Wed Dec 10 10:08:45 2008 on server.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-2.6 required=4.2 tests=AWL,BAYES_00,NO_RELAYS autolearn=ham version=3.1.3 X-Spam-Checker-Version: SpamAssassin 3.1.3 (2006-06-01) on server.baldwin.cx Cc: Maxim Sobolev , Luigi Rizzo , hackers@freebsd.org, Luigi Rizzo , "current@freebsd.org" Subject: Re: Enhancing cdboot [patch for review] X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 10 Dec 2008 18:53:26 -0000 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