Date: Mon, 7 May 2012 13:43:45 -0400 From: John Baldwin <jhb@freebsd.org> To: Andriy Gapon <avg@freebsd.org> Cc: freebsd-fs@freebsd.org, freebsd-hackers@freebsd.org, Bruce Evans <brde@optusnet.com.au> Subject: Re: [review request] zfsboot/zfsloader: support accessing filesystems within a pool Message-ID: <201205071343.45955.jhb@freebsd.org> In-Reply-To: <4FA7E069.8020208@FreeBSD.org> References: <4F8999D2.1080902@FreeBSD.org> <201205070953.04032.jhb@freebsd.org> <4FA7E069.8020208@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201205071343.45955.jhb>