From owner-freebsd-hackers@FreeBSD.ORG Mon May 7 17:46:12 2012 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id ED4A5106566B; Mon, 7 May 2012 17:46:12 +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 C18788FC15; Mon, 7 May 2012 17:46:12 +0000 (UTC) Received: from jhbbsd.localnet (unknown [209.249.190.124]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 27BB1B978; Mon, 7 May 2012 13:46:12 -0400 (EDT) From: John Baldwin To: Andriy Gapon Date: Mon, 7 May 2012 13:43:45 -0400 User-Agent: KMail/1.13.5 (FreeBSD/8.2-CBSD-20110714-p13; KDE/4.5.5; amd64; ; ) References: <4F8999D2.1080902@FreeBSD.org> <201205070953.04032.jhb@freebsd.org> <4FA7E069.8020208@FreeBSD.org> In-Reply-To: <4FA7E069.8020208@FreeBSD.org> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201205071343.45955.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Mon, 07 May 2012 13:46:12 -0400 (EDT) Cc: freebsd-fs@freebsd.org, freebsd-hackers@freebsd.org, Bruce Evans 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: Mon, 07 May 2012 17:46:13 -0000 On Monday, May 07, 2012 10:47:05 am Andriy Gapon wrote: > on 07/05/2012 16:53 John Baldwin said the following: > > On Saturday, May 05, 2012 5:53:07 am Andriy Gapon wrote: > [snip] > >> The new patchset: http://people.freebsd.org/~avg/zfsboot.patches.7.diff > > > > Looks great, thanks! A few replies below: > > Here's a followup patch for the suggestions: > http://people.freebsd.org/~avg/bootargs.followup.diff > I will merge it into the main patch. > > 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. I think you can probably leave BA_SIZE as-is. > > Also, at some point we could use a genassym.c file ala the kernel builds to generate > > some of the constants in bootargs.h instead (e.g. the offsets of fields within > > structures, and BA_SIZE, though we probably want to ensure that BA_SIZE never > > changes). > > The genassym approach sounds good, but, indeed - later :) Yes, that can wait. I think it would not be very hard to do however. All you really need is access to sys/kern/genassym.sh and nm. -- John Baldwin