[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