[review request] zfsboot/zfsloader: support accessing filesystems within a pool

[John Baldwin]

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