On Fri, May 22, 2015 at 12:59:44PM +0100, Daniel P. Berrange wrote:
On Thu, May 21, 2015 at 10:22:26PM -0700, Martin Kletzander wrote:On Thu, May 21, 2015 at 05:46:59PM +0200, Michal Privoznik wrote: >On 20.05.2015 07:19, 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.> >>+ datatypes.c \ >>+ util/viralloc.c \ >>+ util/viratomic.c \ >>+ util/virauth.c \ >>+ util/virauthconfig.c \ >>+ util/virbitmap.c \ >>+ util/virbuffer.c \ >>+ util/vircommand.c \ >>+ util/virerror.c \ >>+ util/virevent.c \ >>+ util/vireventpoll.c \ >>+ util/virfile.c \ >>+ util/virhash.c \ >>+ util/virhashcode.c \ >>+ util/virjson.c \ >>+ util/virkeyfile.c \ >>+ util/virlog.c \ >>+ util/virobject.c \ >>+ util/virpidfile.c \ >>+ util/virprocess.c \ >>+ util/virrandom.c \ >>+ util/virseclabel.c \ >>+ util/virsocketaddr.c \ >>+ util/virstorageencryption.c \ >>+ util/virstoragefile.c \ >>+ util/virstring.c \ >>+ util/virthread.c \ >>+ util/virtime.c \ >>+ util/virtypedparam.c \ >>+ util/viruri.c \ >>+ util/virutil.c \ >>+ util/viruuid.c \ >>+ util/virxml.c \ >>+ remote/remote_protocol.c \ > >This drags in (de)serializers for all the public APIs we have. I guess >you have it here just becase of serializers for some basic types (e.g. >string). Well, if we introduce a separate libvirt_admin.x file, rpcgen >will generate the serializers again, and just for the types we need. So >I think it's safe to drop this line (and libvirt-admin.so will lose >some weight). > Yes, but I have to rename that to something else than remote_string because that would collide in the libvirt daemon. That would mean I have to add each of the new names to gendispatch.pl. And so on and so on, just to get rid of some (de)serializers in the client library. I'll do that for remote_string for now, but if there are more than that, we should probably put those into another file. Well, you'll see how much bigger the diff will be even now. >Then, I wonder if we need to recompile nearly all utils/ again. Can't >we just link libvirt_utils.so in? > Well, I wanted to make it lightweight even when it increases compilation time as it looks like we are constantly OK with that (setuid_rpc_client, vbox libs, etc.), but as I said in the commit message, I'd like to see a commit that minimizes the files being compiled.I don't think you are actually making this lightweight as you intend. In reality anyone who is using libvirt-admin.so will likely already have libvirt.so in memory. So by recompiling the files you are causing two copies of the same code to be kept in RAM. So if libvirt-admin.so just linked to libvirt.so, then we would in fact be more lightweight than this, and avoid the recompilation.
OK, I though that shouldn't be the case, so it is installable separably, but since we allow only local socket for now, you're right that there's libvirt.so already loaded.
The recompilation for the setuid binary is a special case because we have security requirements that trump the extra overhead that the duplication implies both at compile time and runtime. So I don't think the recompilation is really justified in the case of libvirt-admin.so One day though, I do think we should perhaps turn things like libvirt_util.la, libvirt_rpc.la, etc into a formally installed .so's that we can dynamically link to from other libvirt parts. We'd /not/ include any header files, as these .so's would be solely for libvirt usage, not application usage. I think this would ultimately better achieve what you're trying todo here. 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 :|
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
