From owner-freebsd-current@FreeBSD.ORG Sun Feb 6 04:43:54 2011 Return-Path: Delivered-To: current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 15E801065673; Sun, 6 Feb 2011 04:43:54 +0000 (UTC) (envelope-from yanegomi@gmail.com) Received: from mail-wy0-f182.google.com (mail-wy0-f182.google.com [74.125.82.182]) by mx1.freebsd.org (Postfix) with ESMTP id 1C3958FC15; Sun, 6 Feb 2011 04:43:52 +0000 (UTC) Received: by wyf19 with SMTP id 19so3666749wyf.13 for ; Sat, 05 Feb 2011 20:43:52 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=yDdIWTblBX2fj77vtCF+LTLN7XPrGQudgZIg0YQtSow=; b=H8Xh047YMlHI3sei3k1VBEeLIZam1xQTEVtXJoTEGVaC3bTJIVCDLpg3TrulrA7AGc Ta+gK53YKVrYgYOUalOLwiMrhDWZFKlgZLfvdupP7ufJPV47T32YZ4rvutqQEKv+dM5c xdM+ve8eRg78ssJkw+GaMykptlmQz+fCYGW5E= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; b=BGZ/fsBSqRSWbouAnK0JArN1nmfseeWy07HDBKyhKHAeWrJEHc7RROouARsDJNYq0s 9rYozVWoo8V1vwQU9WGaIQPmqxRomRnF+lTH/mXGPlakoaiQYPIdYZ02hlgABelPfi7L LtgO+bY5pvAOpeFYsqNMsj/nNGB8CPX3/zhX0= MIME-Version: 1.0 Received: by 10.216.150.134 with SMTP id z6mr12773062wej.27.1296967432014; Sat, 05 Feb 2011 20:43:52 -0800 (PST) Sender: yanegomi@gmail.com Received: by 10.216.71.200 with HTTP; Sat, 5 Feb 2011 20:43:51 -0800 (PST) In-Reply-To: <4D4E1585.3030709@scsiguy.com> References: <4D4C3F89.50700@scsiguy.com> <20110205153920.GS2035@garage.freebsd.pl> <4D4DC2E8.60901@scsiguy.com> <20110205220646.GZ2035@garage.freebsd.pl> <4D4E1585.3030709@scsiguy.com> Date: Sat, 5 Feb 2011 20:43:51 -0800 X-Google-Sender-Auth: vSdOanQIqAaxHMZavvOrEXWx8QM Message-ID: From: Garrett Cooper To: gibbs@scsiguy.com Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: Pawel Jakub Dawidek , current@freebsd.org Subject: Re: [PATCH] OpenSolaris/ZFS: C++ compatibility X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 06 Feb 2011 04:43:54 -0000 On Sat, Feb 5, 2011 at 7:29 PM, Justin T. Gibbs wrote: > On 2/5/2011 3:06 PM, Pawel Jakub Dawidek wrote: >> On Sat, Feb 05, 2011 at 02:36:40PM -0700, Justin T. Gibbs wrote: >>> >>> Perhaps IllumOS will accept these changes back? =A0As I mentioned in th= e >>> change descriptions included with the patch, the header files already >>> show the intention of providing C++ support (extern "C" blocks), they >>> just don't quite deliver. =A0The changes shouldn't be controversial. >> >> Sure. To be clear: I'm not against those changes, I think they are worth >> it. And getting IllumOS to accept them back is definitely a good idea. > > Ok. > >>> We have talked internally about this at Spectra too. =A0Since we don't = have >>> BSD licensed nvpair code, we've thought of using Google protocol buffer= s >>> to allow extensible encoding of fault data. =A0The GP implementation is >>> MIT licensed and looks like it might be less cumbersome to use than >>> nvpairs. =A0For the first release of our product, however, we are just >>> making due with the string data that devctl provides. >> >> I've developed similar API during HAST work, maybe it is a good starting >> point? src/sbin/hastd/nv.{c,h}. > > Sure. =A0If the decision is made to use nvpairs, I would advocate for > emulating the Solaris API. =A0There's no reason to be slightly different > from an established implementation. > >>> The reason I chose C++ for this task is that devd, the source of the >>> events I process, already requires C++ so using C++ in zfsd doesn't >>> impose any new requirements on the system. =A0Zfsd, like even the C >>> kernel of FreeBSD is coded in an object oriented fashion, but its >>> much cleaner to implement this type of design in a language that >>> inherently supports object oriented concepts. =A0Could I rewrite all >>> that I have in C? =A0Sure, but there would have to be some compelling >>> reasons to offset the reduction in clarity and maintainability such >>> a change would cause. >> >> Hmm, so zfsd will receive events from devd? I'm in opinion that we >> should let devd alone. In my initial port I used devd, because it was >> closest match, but if we want to clean it up, we shouldn't go through >> devd. For example ZFS v28 can report whole binary blocks where checksum >> doesn't match and passing those through devd would be cumbersome. > > zfsd parses devctl event data (via devd's unix domain socket) into an > internal event class representation. =A0The data representation for the > event class as well as the source for the event data can be easily > changed out once a more complete solution is available. =A0For the polici= es > SpectraLogic needs for its first product launch, the data coming through > devctl is sufficient even if it is not ideal. > >>> Is your inability to help on a C++ version of this code due to distaste >>> for C++ or just a lack of experience with it? >> >> The latter. I'm sure there are many committers that are fluent in C++, >> but all of them know C. I was under impression that Warner implemented >> devd in C++ also as a kind of experiment, which nobody really followed. > > FreeBSD has lots of examples of really well written C code and shell code= , > but almost no examples written in more modern languages (C++ isn't even > that modern!). =A0That's too bad. =A0My hope is that, by submitting anoth= er > example of (dare I hope well written) C++ use in FreeBSD, that this will > change. =A0At least review the code (when I release it in a few weeks) be= fore > begging me to rewrite it! :-) The patch looks reasonable. It's kind of funny that the OpenSolaris folks use variable names that conflict with reserved keywords in C++ (class, private). Thanks! -Garrett