[review request] zfsboot/zfsloader: support accessing
filesystems within a pool
John Baldwin
jhb at freebsd.org
Fri May 4 15:35:00 UTC 2012
On Thursday, May 03, 2012 11:23:51 am Andriy Gapon wrote:
> on 03/05/2012 18:02 Andriy Gapon said the following:
> >
> > Here's the latest version of the patches:
> > http://people.freebsd.org/~avg/zfsboot.patches.4.diff
>
> I've found a couple of problems in the previous version, so here's another one:
> http://people.freebsd.org/~avg/zfsboot.patches.5.diff
> The important change is in the first patch (__exec args).
A few comments/suggestions on the args bits:
- Please move the type of the 'kargs' struct into the new header and
give it a type name. Don't add the LOADER_ZFS_SUPPORT #ifdef's
however or the new size field (see below).
- Add #ifndef LOCORE guards to the new header around the structure so
it can be used in assembly as well as C.
- Move BI_SIZE and ARGOFF into the header as constants.
- Add a CTASSERT() in loader/main.c that BI_SIZE == sizeof(struct bootinfo)
- Add a KA_SIZE for sizeof(struct kargs) with a CTASSERT. Use it in this
instruction:
+ addl 0x1c(%esp),%ecx # Add size of variable args
in place of the 0x1c
- Don't add the new 'size' field to the end of the 'struct kargs'. Instead
add a comment saying that if KARGS_FLAGS_EXT is set, then a structure will
be present at (kargs + 1) that has a int size as its first member. Maybe
create a 'struct kargs_ext' that looks like this:
struct kargs_ext {
uint32_t size;
char data[0];
};
- Use 'zargs = (struct zfs_boot_args)(kargs + 1)' in loader/main.c.
- Change the ARGADJ line in btxcsu to be this:
.set ARGADJ,0x1000 - ARGOFF
- If you are adventurous, you could even add a new constant ARGSPACE or some
such with an appropriate comment in the new header that you use to replace
the 0x1000 in btx.S and in the definition of ARGADJ.
Hope that isn't too much, but I do think this will help make things even more
understandable.
--
John Baldwin
More information about the freebsd-hackers
mailing list