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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Mar 01, 2017 at 09:42:37AM +0000, Daniel P. Berrange wrote:
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.


As I understand it, that's still done during runtime.  And since every
single call to virObjectPoolable* would require you to call it with the
macro, it's not that different to putting the functionality of the macro
inside that function.

Anyway, that's for a later day.

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/ :|

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux