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

John Baldwin jhb at FreeBSD.org
Tue May 8 14:15:38 UTC 2012


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


More information about the freebsd-fs mailing list