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

Re: [libvirt] [PATCH v2] Link libvirt_util with datatypes



On Wed, Apr 15, 2015 at 04:31:01PM +0200, Peter Krempa wrote:
> On Wed, Apr 15, 2015 at 16:02:45 +0200, Martin Kletzander wrote:
> > On Wed, Apr 15, 2015 at 02:19:04PM +0100, Daniel P. Berrange wrote:
> > >On Wed, Apr 15, 2015 at 02:29:52PM +0200, Martin Kletzander wrote:
> > >> Most of the types in datatypes.[hc] depend on virObject and virClass,
> > >> but they were specified separatedly from that.  We were lucky enough for
> > >> this to work because wherever the datatypes files were used, that
> > >> file (binary/shared object) was linked to libvirt_util as well.
> > >>
> > >> Fixing this comes up as a pretty nice cleanup.
> > >
> > >I'm not really seeing what problem needs fixing here. The libvirt_util
> > >library doesn't depend on the datatypes file, and everything that we
> > >build links to libvirt_util, so AFAIK there's nothing that needs fixing
> > >
> > >libvirt_util.la doesn't depend on datatypes.h - at least it shouldn't
> > >unless we regressed somewhere.
> > >
> > 
> > Yes, it doesn't, but datatypes depend on libvirt_util.  And since we
> > don't have any place in the whole code base where we would use
> > libvirt_util and not datatypes, it makes sense to put them together,
> > IMHO.  It's just a matter of point of view, but if you want, I can
> > make DATATYPES as the library and it would have libvirt_util (or
> > virobject, virerror etc. at least) as part of it.  But now we just
> 
> I'd rather see this as making datatypes part of the utils library.
> libtool should then handle ti just right.

The idea behind datatypes.{c,h} being separate is that they are just
representing the public API objects, and as a general rule our internal
code shouldn't be using those objects. The drivers (eg QEMU, LXC, etc)
should convert from the public API object into the appropriate
internal object types. This is certainly true of the virDomainPtr
virNetworkPtr, etc types. Sometimes we have virConnectPtr leaking
into certain internal APIs, but we've got rid of alot of that
pollution over the years.

So I think putting datatypes.c into the libvirt_util would be a step
in the wrong direction.

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


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