[libvirt] [PATCH RFC 2/9] conf: Introduce virPoolObj and virPoolTableObj and PoolObj API's

[Daniel P. Berrange]

On Wed, Mar 01, 2017 at 10:27:32AM +0100, Martin Kletzander wrote:
> On Wed, Mar 01, 2017 at 09:06:13AM +0000, Daniel P. Berrange wrote:
> > On Wed, Mar 01, 2017 at 09:55:03AM +0100, Martin Kletzander wrote:
> > > On Tue, Feb 28, 2017 at 04:39:52PM +0000, Daniel P. Berrange wrote:
> > > > FWIW, the virObject framework as it exists today was just the bare
> > > > minimum we needed in order to get a basic inheritance system up and
> > > > running in libvirt. I rather expected that we would extend it in the
> > > > future to add further concepts, inspired/borrowed from glib (which
> > > > is what stimulated my creation of virObject in the first place). In
> > > > particular I could see
> > > >
> > > > - Interface support - the virObjectListable concept sounds like the
> > > >   kind of thing that would be suited to an interface, rather than a
> > > >   subclass, as it lets different objects support the API contract
> > > >   without forcing a common ancestor which might not be appropriate
> > > >
> > > 
> > > This is really interesting and bunch of ideas *how* this would be
> > > implemented pop into my mind.  I like the overall idea of using more
> > > virObject goodness and expanding it so that it can to more.
> > > 
> > > I thing I get why interfaces are more suitable in some cases.  Having
> > > 'lockable'  and 'poolable' interfaces, for example, make it possible to
> > > have classes that are lockable, poolable, both, or none of those things
> > > without having three intermediate classes to derive from.
> > > 
> > > I do not understand one tiny thing, though.  Why is interface good for
> > > this one particular occasion when every poolable object will always need
> > > to be lockable?  I think in this particular case having
> > > virObjectPoolable derive from virObjectLockable is pretty reasonable.
> > 
> > I didn't think about that specific case too much. Considering the
> > integration with the access control code, however, the access control
> > driver needs to obtain 1 or more identifying attributes from an object
> > to serve as an "identity" against which access rules are written.
> > Rather than having the access control code know about specific objects,
> > you could invent a "virObjectIdentity" struct which contains a generic
> > list of key+value pairs and then define an virOobjectIdentifier
> > interface which contained a single method that returned a virObjectIdentity
> > instance.  This would decouple the access control code from the specific
> > config classes we use.
> > 
> 
> Os, so it's about the ACL code.  That makes sense, yes.
> 
> > > One more thing.  Can we do something about the static type-safety of
> > > virObject APIs in the future?  The only idea I could come up with is
> > > optional GCC plugin tailored to our usage, but that seems cumbersome.
> > 
> > Any particular parts you are thinking about ?
> > 
> 
> I'm talking about the fact that if you want to have a function (say
> virObjectLock() for example) for a class, it must accept void * if it's
> supposed to be called on an object of a subclass as well.  We are
> checking whether it's the right class and if that function can be called
> on that object, yada yada, but that is done in runtime and not
> compile-time.  My question was whether there are any ideas how ti make
> it more safe during compile-time.

It doesn't have to accept void *. That is just a convenience to avoid need
for type casting - I'd suggest such usage of 'void *' be only done for the
top most classes virObject & virObjectLockable. Once we get more specialized
we should require explicit casts, which will give us a greater degree of
compile time checking - of course it doesn't prevent people doing incorrect
casts, but it does at least force the programmer to think about what object
they are passing in.

Also note, when I say "use casts", I don't mean use a plain C language
cast - I mean use something that actually validates the object type
dynamically.

In GLib (and QEMU's object model), for each class you define, you add
a macro for doing casts to objects of that class.

eg if you had a  virObjectPoolable, you would define a macro

  #define  VIR_OBJECT_POOLABLE(obj) \
        VIR_OBJECT_CHECK(obj, virObjectPoolableClass)

and the VIR_OBJECT_CHECK macro would call virObjectIsClass() and
return NULL if it fails. Every API against the object would have
to have a check for NULL present so you could blindly do

     virObjectPoolableSomeMethod(VIR_OBJECT_POOLABLE(dom))

With GLib & QENMU, VIR_OBJECT_CHECK would in fact assert() if the
wrong class was passed, avoidig the need for NULL checks in every
method, but we avoid assert in libvirt.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|