Re: [libvirt] [PATCH 07/13] Add libvirt-admin library

On Tue, Jun 16, 2015 at 01:16:56PM +0200, Martin Kletzander wrote: > On Tue, Jun 16, 2015 at 11:42:52AM +0100, Daniel P. Berrange wrote: > >On Tue, Jun 16, 2015 at 11:58:24AM +0200, Martin Kletzander wrote: > >>Initial scratch of the admin library. It has its own virAdmConnectPtr > >>that inherits from virAbstractConnectPtr and thus trivially supports > >>error reporting. > >> > >>There's pkg-config file added and spec-file adjusted as well. > >> > >>Since the library should be "minimalistic" and not depend on any other > >>library, the list of files is especially crafted for it. Most of them > >>could've been put to it's own sub-libraries that would be LIBADD'd to > >>libvirt_util, libvirt_net_rpc and libvirt_setuid_rpc_client to minimize > >>the number of object files being built, but that's a refactoring that > >>isn't the orginal aim of this commit. > >> > >>Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; > > > > > >>+/** > >>+ * virAdmInitialize: > >>+ * > >>+ * Initialize the library. > >>+ * > >>+ * Returns 0 in case of success, -1 in case of error > >>+ */ > >>+static int > >>+virAdmInitialize(void) > >>+{ > >>+ if (virOnce(&virAdmGlobalOnce, virAdmGlobalInit) < 0) > >>+ return -1; > >>+ > >>+ if (virAdmGlobalError) > >>+ return -1; > >>+ > >>+ return 0; > >>+} > > > >This is declared static > > > > This should not be static, > > > > >>+LIBVIRT_ADMIN_1.3.0 { > >>+ global: > >>+ virAdmInitialize; > >>+ virAdmConnectOpen; > >>+ virAdmConnectClose; > >>+ virAdmConnectRef; > > > >So does not need to be listed here > > > > so it can be listed here. This is similarly to virInitialize(). We have virInitialize() as an explicitly callable function in libvirt.so because it was needed for thread safety before we introduced proper thread support. Now that we're using virOnce to do atomic initialization, AFAIK, there is no compelling reason for a virInitialize function to be called explicitly. So I'm not convinced we need to expose virAdmInitialize, unless I'm forgetting something

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 :| -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list