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