Re: [PATCH] OpenSolaris/ZFS: C++ compatibility

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?  As I mentioned in the
>> 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.  The 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.  Since we don't have
>> BSD licensed nvpair code, we've thought of using Google protocol buffers
>> to allow extensible encoding of fault data.  The GP implementation is
>> MIT licensed and looks like it might be less cumbersome to use than
>> nvpairs.  For 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.  If the decision is made to use nvpairs, I would advocate for
emulating the Solaris API.  There'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.  Zfsd, 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.  Could I rewrite all
>> that I have in C?  Sure, 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.  The 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.  For the policies
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!).  That's too bad.  My hope is that, by submitting another
example of (dare I hope well written) C++ use in FreeBSD, that this will
change.  At least review the code (when I release it in a few weeks) before
begging me to rewrite it! :-)

--
Justin