From owner-freebsd-hackers@FreeBSD.ORG Tue May 8 14:15:38 2012 Return-Path: Delivered-To: freebsd-hackers@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 17371106566B; Tue, 8 May 2012 14:15:38 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from bigwig.baldwin.cx (bigknife-pt.tunnel.tserv9.chi1.ipv6.he.net [IPv6:2001:470:1f10:75::2]) by mx1.freebsd.org (Postfix) with ESMTP id DD9FE8FC08; Tue, 8 May 2012 14:15:37 +0000 (UTC) Received: from John-Baldwins-MacBook-Air.local (c-68-39-198-164.hsd1.de.comcast.net [68.39.198.164]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 3BE00B93B; Tue, 8 May 2012 10:15:37 -0400 (EDT) Message-ID: <4FA92A88.2030000@FreeBSD.org> Date: Tue, 08 May 2012 10:15:36 -0400 From: John Baldwin User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:12.0) Gecko/20120428 Thunderbird/12.0.1 MIME-Version: 1.0 To: Andriy Gapon References: <4F8999D2.1080902@FreeBSD.org> <201205070953.04032.jhb@freebsd.org> <4FA7E069.8020208@FreeBSD.org> <201205071343.45955.jhb@freebsd.org> <4FA8C7E3.8070006@FreeBSD.org> In-Reply-To: <4FA8C7E3.8070006@FreeBSD.org> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Tue, 08 May 2012 10:15:37 -0400 (EDT) Cc: freebsd-fs@FreeBSD.org, freebsd-hackers@FreeBSD.org Subject: Re: [review request] zfsboot/zfsloader: support accessing filesystems within a pool 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: Tue, 08 May 2012 14:15:38 -0000 On 5/8/12 3:14 AM, Andriy Gapon wrote: > on 07/05/2012 20:43 John Baldwin said the following: >> On Monday, May 07, 2012 10:47:05 am Andriy Gapon wrote: >>> on 07/05/2012 16:53 John Baldwin said the following: > [snip] >>> What do you think about the -LOCORE- change that Bruce inspired? >> >> In general I think this looks good. I have only one suggestion. In other >> code (e.g. the genassym constants in the kernel) where we define constants >> for field offsets, we make the constant be the uppercase name of the field >> itself (e.g. TD_PCB for offsetof(struct thread, td_pcb)). I would rather >> do that here as well. In this case the field names do not have a prefix, >> but let's just use a BA_ prefix for members of 'bootargs'. BI_SIZE is >> already correct, but this would mean renaming HT_OFF to BA_HOWTO, BF_OFF to >> BA_BOOTFLAGS, and BI_OFF to BA_BOOTINFO. > > OK, doing this. > >> I think you can probably leave BA_SIZE as-is. > > I see that i386 genassym has a few different styles for sizeof constants: > ABBRSIZE > FULL_NAME_SIZE > ABBR_SIZEOF > > FULL_NAME_SIZE looked the most appealing to me (and seems to be the most > common), so I decided to change BA_SIZE to BOOTARGS_SIZE. > I hope that this makes sense and I am not starting a bikeshed :-) Yeah, given the inconsistency in sizeof() constants in genassym.c, just about anything is fine, which is why I hesitated to suggest any change. BOOTARGS_SIZE is fine. I probably slightly prefer that because it is less ambiguous (in case the structure has a foo_size member such as bi_size in bootinfo). Bruce might even suggest adding a ba_ prefix to all the members of struct bootargs btw. I would not be opposed, but you've already done a fair bit of work on this patch. -- John Baldwin