kinda headsup..

[Julian Elischer]

Bjoern A. Zeeb wrote:
> On Sun, 8 Jun 2008, Julian Elischer wrote:
> 
>> At the BSDCAn devsummit we discussed how to proceed with committing 
>> Vimage to -current.
>>
>> the Milestones included something like:
>>
>> June 8 (today) Headsup....
>>
>> June 15        commit changes that add macros for vnet
>>               (network module) and vinet(inet virtualisation)
>>               with macros defined in such a way to make 0 actual
>>               differences. provable by md5 etc.
>>               Documentat
>>                 s/hostname/g//V_hostname/
>>                 #define V_hostname hostname
>>               2 weeks settle time, next step prepared, tested
>>               and reviewed.
> 
> For which part were you talking about a sed/awk script to use?
> Can we have a diff for just this part (once it is avail?)
> 
> 
> [schedule]
> 
> * I am missing the BIG HEADS UP somewhere for all the people with
> outstanding work so that they will not re-do any integration multiple
> times.
> 
> * I am missing the developers and users documentation in the schedule.
> 
> 
> 
>> diffs can be found at:
>> http://www.freebsd.org/~julian/vimage.diff and it are usually
>> fairly up to date.
> 
> I am just starting to skip through the patch, not doing a close review
> atm (not checking functional changes, etc. at all), and even this is
> hard at the end of the day...
> 
> Are sys/ddb/db_command.c related in any way to this?
> 
> sys/kern/init_main.c has an extra whitespace before the LIST_FIRST.
> 
> sys/kern/kern_linker.c Isizeof(lookup) should be 4 space indent not 2
> tabs.
> 
> Do we need those changes like sys/kern/kern_switch.c ?

This diff there includes experimental changes to virtualise things
like load average, and they will not be part of the commit.
so ignore anything that smells like "scheduler"

> 
> sys/kern/kern_sysctl.c has indentation problems in the
> @@ -1322,7 +1421,17 @@ junk
> 
> sys/kern/kern_timeout.c has an extra whitespace

yes we (I) will be trying to clean that sort of thing..

> 
> sys/kern/kern_vimage.c says "XXX RCS tag goes here" so add it.

SNV tag?

> sys/kern/kern_vimage.c has // comments no-no
>   - " -  s,#define NAME,#define\tNAME,g
>   - " -  vnet_mod_register,vnet_mod_register_multi,(more)... declarations
>   - " -  adds a new suser() call.
>   - " -  in vi_symlookup() 2nsd line of for, remove a space
>   - " -  thinks like this scare me:
>     /* A brute force check whether there's enough mem for a new vimage */
>          especially if its freed again instantly
>   - " -  near  Detach / free per-module state instances remove whitespace
>   - " -  vi_free() remove a \t before the break;
>   - " -  db_show_vnets should probably check for db_pager_quit
> 
> sys/kern/kern_xxx.c the printf looks like debugging?
> 
> sys/kern/sys_socket.c has an unrelated whitespace change
> 
> sys/kern/uipc_domain.c removes a comment I am entirely sure it can be 
> removed.
>   - " -  why do we need to change net_init_domain(?here?) just to cast 
> again?
> 
> sys/kern/uipc_socket.c  junk @@ -1284,13 +1314,17 @@  s,\t,    ,
>   - " -  if (how != SHUT_RD) { int error;  add \n
> 
> sys/kern/vfs_lookup.c adds something called IMUNES_SYMLINK_HACK which
> should either be renamed or removed.
> 
> sys/modules/Makefile does not look like it belongs there
> 
> sys/modules/netgraph/Makefile   looks really strange, can we fix that?
> 
> sys/modules/netgraph/pipe/Makefile has an extra space
> 
> sys/modules/netgraph/wormhole/Makefile has an extra space
> 
> sys/net/bpf.c adds an IMUNES_BPF_HACK, and defines it - either
>     rename or remove; also has whitespace issues and debugging
>     printfs in there (that should not compile).
> 
> sys/net/if.c @@ -292,31 +317,73 @@ junk if (IS_DEFAULT_VNET(curvnet)) {
>     ... needs an extra \t, no? doesn't look nice; there are more
>     of those in this file; maybe not yet; not before the #ifdefs go.
>   - " - SYSINT .. if_attachdomain was a wrong ws change
>   - " - junk @@ -1842,6 +1971,24 @@ adds another suser()
>   - " - at the end there are two unrelated/wrong ws changes
> 
> sys/net/if_ethersubr.c    ether_reassign() has whitespace issues
>   - " - SYSCTL_V_INT for ether_ipfw  2nd line indent looks wrong
> 
> sys/net/if_gif.c SYSCTL_V_INT 2nd line, parallel_tunnels indent
>   - " -  gifmodevent() empty line wrongly removed
> 
> sys/net/if_gif.h #define\tNAME
> 
> sys/net/if_gre.c  is there a reason to rename the local variables?
> 
> sys/net/if_loop.c  I cannot see a difference for vnet_loif_iattach
>     w/ or w/o the #ifdef. Should the outer one go?
>   - " -  is there a need to move the loif check up in lo_clone_destroy?
>   - " - junk @@ -190,7 +266,7 @@ use 4 spaces
> 
> sys/net/if_mib.c SYSCTL_V_INT  fix ws
> 
> sys/net/if_var.h do we need to move if_index?
> 
> sys/net/route.c    static uma_zone_t rtzone; has an uneeded ws change
>   - " -  rtable_init() ws wrong
>   - " -  is that realted to more MRT changes or why are functions split
>          and shuffled?
>   - " -  there were and still are more ws problems around V_rt_tables
>   - " -  return 0; ws problem
>   - " -  rtable_idetach() ws problem and more and the return
> 
> sys/net/rtsock.c  rnh =\n ... whitespace next line
> 
> sys/net/vnet.h    XXX RCS tag goes here   do so
>   - " -  struct vnet_net has ws issues with the _ether_ipfw line
>   - " -  #define\tNAME
> 
> 
> 
> I am running out of battery, so I am going to continue with the
> next ~20%+- in sys/net80211/**, l 6556 tomorrow.
> 
> 
> General: values in return statements should be enclosed in parentheses.
> 
> General: function declarations K&R vs. ANSI vs ...
> 
> General: you are adding 92 lines with XXX, 18 say "locking", 2 say
> WRONG, 10 say RCS, (other), ... can we get (most of) them fixed before
> committing? (fixed, not removed)

thanks max

Our hope is to generate a set of patches derived from that we have now
rather than commit what we have exactly, so we hope we can get your 
cleanups included as we create those diffs.

feel free to use p4 to fix things yourself if you want to..


> 
> 
> /bz
>