Re: [libvirt] PATCH: Move domain event helpers out of internal.h/libvirt.c

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

 



On Wed, Oct 29, 2008 at 12:06:11PM +0000, Daniel P. Berrange wrote:
> There are a bunch of helper functions relating to domain events, which
> are only used internally by hypervisor drivers. Following the principle
> that libvirt.c should only contain functions exported in the API, and
> internal.h should not define any function signatures, these helpers
> need to move.
> 
> So I'm inventing a domain_events.c, and domain_events.h file to contain
> the domain events helper code, in much same way as domain_conf.c and
> domain_conf.h contain the domain XML helper code.

  That sounds fine to me.

> Again no functional change here. With this patch applied, the cleanup
> of internal.h is basically complete - at least more than good enough
> for now. It'd be nice to move the struct definitions of virDomainPtr,
> etc elsewhere, perhaps to src/libvirt.h because they're public API,
> but not exported publically.

  Well precisely they are internal definitions which we can't put in
libvirt.h , that's why the header was named that way, and i think it's
fine to keep them there.

  I'm general the cleanup patches are trying to do the right thing,
but I'm just opposed to duplicate headers with similar names, and
adding to libvirt.c things which are not just the public entry point
of libvirt.h (sure there were some but at least the previous patch
cleans some of them).

  Adding new modules has no serious cost, while doing the cleanup
increasing the modularity is a good idea.

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel@xxxxxxxxxxxx  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

--
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]