Step 1.5 needs review

Brooks Davis brooks at freebsd.org
Tue Sep 2 00:24:00 UTC 2008


On Thu, Aug 28, 2008 at 07:03:30PM +0000, Bjoern A. Zeeb wrote:
> Hi,
> 
> in case you are interested or have volunteered before to review Step 1.5
> as described on   http://wiki.freebsd.org/Image/Notes200808DevSummit
> there are few things to do:
> 
> - review the diff (Julian posted an initial one).
> - make sure all (relevant) sysctl were caught.
> - make sure the INIT_VNET_* macro is there whereever it is needed.
> - do builds according to "HOWTO verify that the pure style changes are
>   all right" on the above mentioned page and verify that it is all
>   style changes. In case there are others we shoudl decide to either
>   commit them either upfront or afterwards if possible.
> - the 'include headers' one way or the other (as we have discussed at
>   the devsummit and that Julian has told me again) needs resolving.
>   As this has bikeshed potential, I'd prefer that the 'singed up'
>   reviewers decide that.
> - possibly more...
> 
> The plan would be to have a final patch by Monday morning UTC to be
> comitted by a volunteer.

I've gone over the patch and fixed some white space issues.  I've also
found some things I'm not sure what to do with.  Comments:

- GENERIC_NODEBUG should not be committed
- VNET_ITERLOOP_BEGIN/END is evil.  It would be really nice to find a
  way to do this that preserves {} pairs and isn't too magic.
- sys/kern/tty.c:
  - There's some #if 0 code that presumably should stay in the vimage
    branch for now and be fixed before the final commit.
  - TIOCDRAIN is being removed.  Is this a merge issue or something
    else?
- sys/net/if.h:
  - shouldn't net/vnet.h be included in if_var.h instead?  *_var.h is
    supposed to be the internals and I think this qualifies.  If so,
    there will be a number of files that added if.h includes that
    should add if_var.h includes instead.
- sys/netinet/ip_fw.h:
  - It's not clear to me that all the changes in this file should
    actually be here.  It looks like there may be some merge issues or
    WIP code.
- sys/netinet6/icmp6.c:
  - This (and IIRC a couple other files in ipsec) contain comments like
    "XXX this below is WRONG".  It would be nice if the issue could be
    fixed or if that's not feasible, a more detailed comment should
    be added.
- sys/netipsec/ipsec.c:
  - This and a few other files change the style of SYSCTL*() macro
    usage.  We're not at all consistent across the kernel, but my
    feeling is that we should either entirely preserve the local style
    or we should figure out what the style should be and do the churn
    now while we have to mangle the wrapping anyway.

Also, we're due for another integrate round soon.

-- Brooks
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 187 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-virtualization/attachments/20080902/ad30adfa/attachment.pgp


More information about the freebsd-virtualization mailing list