On Thu, Oct 30, 2008 at 01:42:19PM +0000, Daniel P. Berrange wrote:
> This patches changes the way some of the drivers are built. Specifically
> the stateful drivers (QEMU, LXC, Network and Storage) are no longer
> compiled into libvirt.so Instead they are linked directly to the libvirt
> binary. They could only ever be executed as part of the daemon, and we had
> code checks to ensure this, so this just makes it sane at build time.
Hum .... if you force the QEmu driver in the libvirtd daemon,
hor do you implement qemu:///session ? I think I'm missing something
there...
> More importantly this is required for David Lively's host device support.
> That support uses HAL, which uses DBus, which is GPL/AFL licensed. As
> such we cannot ever link HAL/DBus against libvirt.so without loosing
> our LGPL status. With this patch, HAL/DBus bits will only ever be linked
> into the libvirtd daemon binary, so there is no LGPL compat problems in
> libvirt.so
Yup, understood and agreed. But all we need is force just the
hardware discovery to be linkeable only by the daemon, not everything
has to be linked that way.
> The changes this patch does are fairly simple
>
> - qemud/Makefile.am - add the libtool convenience libraries for qemu,
> lxc, storage and network drivers to libvirtd
> - src/Makefile.am - remove the libtool convenience libraries for
> qemu, lxc, storage & network drivers from libvirt.la
> - src/libvirt.c: Do not invoke the register methods for qemu, lxc,
> storage or network drivers in virInitialize()
> - qemud/qemud.c: Invoke the register methods for qemu, lxc, storage
> and network drivers after first calling virInitialize()
> - libvirt_sym.version.in: add all the internal symbols needed to be
> exported for qemu, lxc, storage & network drivers
>
> The end result here is functionally identical to before, with one
> annoying exception. If you passed a 'NULL' uri to virConnectOpen
> it would manually probe each driver to find a suitable URI. Since
> QEMU, LXC drivers are not in libvirt.so anymore this probing
> ceases to work.
Hum, we should find a way to have the do_open(NULL) probe done into
the server located on localhost. Maybe this means adding an extra remote
operation querying the node for a default hypervisor.
LIBVIRT_DEFAULT_URI is still being checked in the library code, so
that default behaviour can still be overriden by the user if this leads
to picking the wrong driver.
> I've not figured out best way to deal with this yet. One option is
> to not actually move the drivers out of libvirt.so at all. Just do
> it for the forthcoming node devices driver. ANother option is to just
> have a tiny QEMU/LXC stub driver in libvirt.so solely containing the
> probe() code. A further option is to make the remote driver implement
> the probe() method, so it actally talks to libvirtd and asks that
> for a probed URI.
IMHO the 3rd solution is the one making most sense from an
architecture POV. Maybe we can even get this to work for remote and
be able to ask a remote node if it supports Xen or KVM by default
(or something else ...)
> --- a/src/libvirt_sym.version.in Thu Oct 30 10:46:21 2008 +0000
> +++ b/src/libvirt_sym.version.in Thu Oct 30 11:04:27 2008 +0000
> @@ -257,6 +257,19 @@
> LIBVIRT_PRIVATE_@VERSION@ {
>
> global:
> + /* bridge.h */
> + brAddBridge;
> + brAddInterface;
> + brAddTap;
> + brDeleteBridge;
> + brInit;
> + brSetEnableSTP;
> + brSetForwardDelay;
> + brSetInetAddress;
> + brSetInetNetmask;
> + brSetInterfaceUp;
> + brShutdown;
> +
>
> /* buf.h */
> virBufferVSprintf;
> @@ -264,6 +277,18 @@
> virBufferAddChar;
> virBufferContentAndReset;
> virBufferError;
> +
> +
> + /* caps.h */
> + virCapabilitiesAddGuest;
> + virCapabilitiesAddGuestDomain;
> + virCapabilitiesAddGuestFeature;
> + virCapabilitiesAddHostNUMACell;
> + virCapabilitiesDefaultGuestEmulator;
> + virCapabilitiesFormatXML;
> + virCapabilitiesFree;
> + virCapabilitiesNew;
> + virCapabilitiesSetMacPrefix;
>
>
> /* conf.h */
> @@ -284,7 +309,62 @@
> virGetStorageVol;
>
>
> + /* domain_conf.h */
> + virDiskNameToBusDeviceIndex;
> + virDiskNameToIndex;
> + virDomainAssignDef;
> + virDomainConfigFile;
> + virDomainDefDefaultEmulator;
> + virDomainDefFormat;
> + virDomainDefFree;
> + virDomainDefParseFile;
> + virDomainDefParseString;
> + virDomainDeleteConfig;
> + virDomainDeviceDefParse;
> + virDomainDiskBusTypeToString;
> + virDomainDiskDeviceTypeToString;
> + virDomainDiskQSort;
> + virDomainEventCallbackListAdd;
> + virDomainEventCallbackListFree;
> + virDomainEventCallbackListRemove;
> + virDomainFindByID;
> + virDomainFindByName;
> + virDomainFindByUUID;
> + virDomainLoadAllConfigs;
> + virDomainObjListFree;
> + virDomainRemoveInactive;
> + virDomainSaveConfig;
> + virDomainSoundModelTypeToString;
> + virDomainVirtTypeToString;
> +
> +
> + /* iptables.h */
> + iptablesAddForwardAllowCross;
> + iptablesAddForwardAllowIn;
> + iptablesAddForwardAllowOut;
> + iptablesAddForwardAllowRelatedIn;
> + iptablesAddForwardMasquerade;
> + iptablesAddForwardRejectIn;
> + iptablesAddForwardRejectOut;
> + iptablesAddTcpInput;
> + iptablesAddUdpInput;
> + iptablesContextFree;
> + iptablesContextNew;
> + iptablesReloadRules;
> + iptablesRemoveForwardAllowCross;
> + iptablesRemoveForwardAllowIn;
> + iptablesRemoveForwardAllowOut;
> + iptablesRemoveForwardAllowRelatedIn;
> + iptablesRemoveForwardMasquerade;
> + iptablesRemoveForwardRejectIn;
> + iptablesRemoveForwardRejectOut;
> + iptablesRemoveTcpInput;
> + iptablesRemoveUdpInput;
> + iptablesSaveRules;
> +
> +
> /* libvirt_internal.h */
> + debugFlag;
> virStateInitialize;
> virStateCleanup;
> virStateReload;
> @@ -294,6 +374,10 @@
> virDomainMigratePrepare;
> virDomainMigratePerform;
> virDomainMigrateFinish;
> + virRegisterDriver;
> + virRegisterNetworkDriver;
> + virRegisterStateDriver;
> + virRegisterStorageDriver;
>
>
> /* memory.h */
> @@ -303,13 +387,97 @@
> virFree;
>
>
> + /* network_conf.h */
> + virNetworkAssignDef;
> + virNetworkDefFormat;
> + virNetworkDefFree;
> + virNetworkDefParseString;
> + virNetworkDeleteConfig;
> + virNetworkFindByName;
> + virNetworkFindByUUID;
> + virNetworkLoadAllConfigs;
> + virNetworkObjListFree;
> + virNetworkRemoveInactive;
> + virNetworkSaveConfig;
> +
> +
> + /* nodeinfo.h */
> + virNodeInfoPopulate;
> +
> +
> + /* stats_linux.h */
> + linuxDomainInterfaceStats;
> +
> +
> + /* storage_backend.h */
> + virStorageBackendForType;
> + virStorageBackendFromString;
> + virStorageBackendPartTableTypeFromString;
> + virStorageBackendPartTableTypeToString;
> + virStorageBackendRegister;
> + virStorageBackendRunProgNul;
> + virStorageBackendRunProgRegex;
> + virStorageBackendStablePath;
> + virStorageBackendUpdateVolInfo;
> + virStorageBackendUpdateVolInfoFD;
> +
> +
> + /* storage_conf.h */
> + virStoragePoolDefFormat;
> + virStoragePoolDefFree;
> + virStoragePoolDefParse;
> + virStoragePoolLoadAllConfigs;
> + virStoragePoolObjAssignDef;
> + virStoragePoolObjClearVols;
> + virStoragePoolObjDeleteDef;
> + virStoragePoolObjFindByName;
> + virStoragePoolObjFindByUUID;
> + virStoragePoolObjListFree;
> + virStoragePoolObjRemove;
> + virStoragePoolObjSaveDef;
> + virStoragePoolSourceFree;
> + virStoragePoolSourceListFormat;
> + virStorageVolDefFindByKey;
> + virStorageVolDefFindByName;
> + virStorageVolDefFindByPath;
> + virStorageVolDefFormat;
> + virStorageVolDefFree;
> + virStorageVolDefParse;
> +
> +
> /* util.h */
> virFileReadAll;
> virStrToLong_i;
> + virStrToLong_ll;
> virStrToLong_ull;
> saferead;
> safewrite;
> virMacAddrCompare;
> + virEnumFromString;
> + virEnumToString;
> + virEventAddHandle;
> + virEventRemoveHandle;
> + virExec;
> + virFileDeletePid;
> + virFileExists;
> + virFileHasSuffix;
> + virFileMakePath;
> + virFileOpenTty;
> + virFileReadLimFD;
> + virFileReadPid;
> + virRun;
> +
> +
> + /* uuid.h */
> + virUUIDFormat;
> +
> +
> + /* virterror_internal.h */
> + virReportErrorHelper;
> +
> +
> + /* xml.h */
> + virXPathString;
>
>
> /* Finally everything else is totally private */
So how many internal APIs do we end up with ? just curious ...
That's the only patch in the serie so far where I am a bit worried of
real regression for the users, unfortunately it's really needed for
the discovery patches ... I'm still very surprized that the freedesktop
guys really pushed a library like HAL as GPL instead of LGPL. I
understand that the database code be under GPL, but the accessor libs
really should be LGPL, that's weird.
I'm fine pushing everything to CVS now, others may chime in of course
:-), but maybe this patch and the following need a few more days of
thinking and maybe try to avoid this NULL open problem.
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