From owner-freebsd-virtualization@FreeBSD.ORG  Tue Sep  2 14:33:29 2008
Return-Path: <owner-freebsd-virtualization@FreeBSD.ORG>
Delivered-To: freebsd-virtualization@freebsd.org
Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34])
	by hub.freebsd.org (Postfix) with ESMTP id 40AB61065681
	for <freebsd-virtualization@freebsd.org>;
	Tue,  2 Sep 2008 14:33:29 +0000 (UTC)
	(envelope-from julian@elischer.org)
Received: from outU.internet-mail-service.net (outu.internet-mail-service.net
	[216.240.47.244])
	by mx1.freebsd.org (Postfix) with ESMTP id 259718FC1F
	for <freebsd-virtualization@freebsd.org>;
	Tue,  2 Sep 2008 14:33:29 +0000 (UTC)
	(envelope-from julian@elischer.org)
Received: from idiom.com (mx0.idiom.com [216.240.32.160])
	by out.internet-mail-service.net (Postfix) with ESMTP id E4A8B23DC;
	Tue,  2 Sep 2008 07:33:28 -0700 (PDT)
Received: from julian-mac.elischer.org (localhost [127.0.0.1])
	by idiom.com (Postfix) with ESMTP id 310FA2D6045;
	Tue,  2 Sep 2008 07:33:27 -0700 (PDT)
Message-ID: <48BD4EBE.4010005@elischer.org>
Date: Tue, 02 Sep 2008 07:33:34 -0700
From: Julian Elischer <julian@elischer.org>
User-Agent: Thunderbird 2.0.0.16 (Macintosh/20080707)
MIME-Version: 1.0
To: Brooks Davis <brooks@freebsd.org>
References: <20080828185639.P66593@maildrop.int.zabbadoz.net>
	<20080902000516.GA48622@lor.one-eyed-alien.net>
	<48BCED8E.5030109@elischer.org>
	<20080902141245.GB48622@lor.one-eyed-alien.net>
In-Reply-To: <20080902141245.GB48622@lor.one-eyed-alien.net>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Cc: "Bjoern A. Zeeb" <bz@freebsd.org>,
	FreeBSD virtualization mailing list <freebsd-virtualization@freebsd.org>
Subject: Re: Step 1.5 needs review
X-BeenThere: freebsd-virtualization@freebsd.org
X-Mailman-Version: 2.1.5
Precedence: list
List-Id: "Discussion of various virtualization techniques FreeBSD supports."
	<freebsd-virtualization.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-virtualization>,
	<mailto:freebsd-virtualization-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-virtualization>
List-Post: <mailto:freebsd-virtualization@freebsd.org>
List-Help: <mailto:freebsd-virtualization-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-virtualization>,
	<mailto:freebsd-virtualization-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Tue, 02 Sep 2008 14:33:29 -0000

Brooks Davis wrote:
> On Tue, Sep 02, 2008 at 12:38:54AM -0700, Julian Elischer wrote:
>> Brooks Davis wrote:
>>> 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.
>> The requirement is to take soem code that doesn something once.
>> and do it once for each vimage. There are of course many ways to do this..
>>
>> Once we have the code in, I think we should expand this out
>> and correctly indent the code, but for reasons of "minimum diff size"
>> teh current way seems ok to me though it doens't look pretty..
>>
>> I suggest that we eventually replace:
>>
>> 	VNET_ITERLOOP_BEGIN
>> 	stuff
>> 	VNET_ITERLOOP_END
>>
>>
>> with (eventually)
>>
>> 	FOREACH_VNET(vnet) {
>> 		stuff
>> 	}
>>
>> but that would require that the entire contents of "stuff"
>> would appear in the diff.
> 
> Thinking about it more, at a minimum, I think we should do:
> 	VNET_ITERLOOP_BEGIN
> 		stuff
> 	VNET_ITERLOOP_END

when? (i.e. at which stage)?


and doing FOREACH_VNET() {}
will allow brace matching...

> 
>>> - 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?
>>
>> not sure myself.. I've been only following the tty mashup from a distance.
>>
>>> - 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.
>> I actually looked around to find a good place to icnlude vnet.h from
>> and decided on if.h because it seemt o be included almist everywhere
>> that vnet.h needed to be included, but I'm not religious on it.
>>
>> teh original code actually includes vnet.h directly in about 50 source 
>> files.
>>
>> my attempt to include it from if.h cut that down to 3.
>>
>> I'm not sure I want to actually include the contents directly into
>> if.h  or any other place.. I think keeping a separate vnet.h and
>> vinet.h seems ok to me.
> 
> The #ifdef _KERNEL is a strong hint that it belongs in if_var.h if it's
> going to be included in another header (IMO, the vnet/vinet.h files
> aren't a good idea in the long term).
> 
> -- Brooks