This patch adds the source and header for a new driver, named config, which provides the 'config:///' connection URI. This driver provides access to non-domain-related configuration information, such as secret or network definitions. This helps prevent circular dependencies between drivers, including the current case between the QEMU and storage drivers. Signed-off-by: Adam Walters <adam(a)pandorasboxen.com&gt; --- include/libvirt/virterror.h | 2 + src/config/config_driver.c | 238 ++++++++++++++++++++++++++++++++++++++++++++ src/config/config_driver.h | 44 ++++++++ src/util/virerror.c | 2 + 4 files changed, 286 insertions(+) create mode 100644 src/config/config_driver.c create mode 100644 src/config/config_driver.h diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index e31e9c4..0f25a65 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -121,6 +121,8 @@ typedef enum { VIR_FROM_ACCESS = 55, /* Error from access control manager */ VIR_FROM_SYSTEMD = 56, /* Error from systemd code */ + VIR_FROM_CONFIG = 57, /* Error from config driver */ + # ifdef VIR_ENUM_SENTINELS VIR_ERR_DOMAIN_LAST # endif diff --git a/src/config/config_driver.c b/src/config/config_driver.c new file mode 100644 index 0000000..c64b532 --- /dev/null +++ b/src/config/config_driver.c @@ -0,0 +1,238 @@ +/* + * config_driver.c: core driver methods for accessing configs + * + * Copyright (C) 2013 Adam Walters + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + * + * Author: Adam Walters <adam(a)pandorasboxen.com&gt; + */ +#include <config.h> +#include <string.h> + +#include "internal.h" +#include "datatypes.h" +#include "driver.h" +#include "virlog.h" +#include "viralloc.h" +#include "virerror.h" +#include "virstring.h" +#include "viraccessapicheck.h" +#include "nodeinfo.h" +#include "config_driver.h" + +#define VIR_FROM_THIS VIR_FROM_CONFIG + +virConfigDriverPtr config_driver = NULL; + +static int configStateCleanup(void) { + if (config_driver == NULL) + return -1; + + virObjectUnref(config_driver->closeCallbacks); + + virSysinfoDefFree(config_driver->hostsysinfo); + + virMutexDestroy(&config_driver->lock); + VIR_FREE(config_driver); + + return 0; +} + +static int configStateInitialize(bool privileged, + virStateInhibitCallback callback ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) +{ + if (!privileged) { + VIR_INFO("Not running privileged, disabling driver"); + return 0; + } + + if (VIR_ALLOC(config_driver) < 0) + return -1; + + if (virMutexInit(&config_driver->lock) < 0) { + VIR_ERROR(_("cannot initialize mutex")); + VIR_FREE(config_driver); + return -1; + } + + config_driver->hostsysinfo = virSysinfoRead(); + + if (!(config_driver->closeCallbacks = virCloseCallbacksNew())) + goto cleanup; + + return 0; + +cleanup: + configStateCleanup(); + return -1; +} + +static int configStateReload(void) { + return 0; +} + +static virDrvOpenStatus configConnectOpen(virConnectPtr conn, + virConnectAuthPtr auth ATTRIBUTE_UNUSED, + unsigned int flags) +{ + virDrvOpenStatus ret = VIR_DRV_OPEN_ERROR; + virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); + + if (conn->uri != NULL) { + /* If URI isn't 'config', decline the connection */ + if (conn->uri->scheme == NULL || + STRNEQ(conn->uri->scheme, "config")) { + ret = VIR_DRV_OPEN_DECLINED; + goto cleanup; + } + + /* Allow remote driver to deal with URIs with hostname set */ + if (conn->uri->server != NULL) { + ret = VIR_DRV_OPEN_DECLINED; + goto cleanup; + } + + if (config_driver == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("config state driver is not active")); + goto cleanup; + } + } else { + ret = VIR_DRV_OPEN_DECLINED; + goto cleanup; + } + + if (virConnectOpenEnsureACL(conn) < 0) + goto cleanup; + + conn->privateData = config_driver; + + ret = VIR_DRV_OPEN_SUCCESS; +cleanup: + return ret; +} + +static int configConnectClose(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + /* Cleanup callbacks on config_driver */ + /* Set conn->privateData to NULL */ + return 0; +} + +static int +configConnectSupportsFeature(virConnectPtr conn, int feature ATTRIBUTE_UNUSED) +{ + if (virConnectSupportsFeatureEnsureACL(conn) < 0) + return -1; + + return 0; +} + +static const char *configConnectGetType(virConnectPtr conn ATTRIBUTE_UNUSED) { + if (virConnectGetTypeEnsureACL(conn) < 0) + return NULL; + + return "config"; +} + +static int configConnectIsSecure(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + return 1; +} + +static int configConnectIsEncrypted(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + return 0; +} + +static char *configConnectGetHostname(virConnectPtr conn) +{ + if (virConnectGetHostnameEnsureACL(conn) < 0) + return NULL; + + return virGetHostname(); +} + +static char * +configConnectGetSysinfo(virConnectPtr conn, unsigned int flags) +{ + virConfigDriverPtr driver = conn->privateData; + virBuffer buf = VIR_BUFFER_INITIALIZER; + + virCheckFlags(0, NULL); + + if (virConnectGetSysinfoEnsureACL(conn) < 0) + return NULL; + + if (!driver->hostsysinfo) { + return NULL; + } + + if (virSysinfoFormat(&buf, driver->hostsysinfo) < 0) + return NULL; + + if (virBufferError(&buf)) { + virReportOOMError(); + return NULL; + } + + return virBufferContentAndReset(&buf); +} + +static int +configNodeGetInfo(virConnectPtr conn, + virNodeInfoPtr nodeinfo) +{ + if (virNodeGetInfoEnsureACL(conn) < 0) + return -1; + + return nodeGetInfo(nodeinfo); +} + +static int configConnectIsAlive(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + return 1; +} + +static virDriver configDriver = { + .name = "config", + .connectOpen = configConnectOpen, /* 1.2.2 */ + .connectClose = configConnectClose, /* 1.2.2 */ + .connectSupportsFeature = configConnectSupportsFeature, /* 1.2.2 */ + .connectGetType = configConnectGetType, /* 1.2.2 */ + .connectGetHostname = configConnectGetHostname, /* 1.2.2 */ + .connectGetSysinfo = configConnectGetSysinfo, /* 1.2.2 */ + .nodeGetInfo = configNodeGetInfo, /* 1.2.2 */ + .connectIsEncrypted = configConnectIsEncrypted, /* 1.2.2 */ + .connectIsSecure = configConnectIsSecure, /* 1.2.2 */ + .connectIsAlive = configConnectIsAlive, /* 1.2.2 */ +}; + + +static virStateDriver configStateDriver = { + .name = "config", + .stateInitialize = configStateInitialize, + .stateCleanup = configStateCleanup, + .stateReload = configStateReload, + .stateDrvType = VIR_DRV_STATE_DRV_LIBVIRT, +}; + +int configRegister(void) { + virRegisterDriver(&configDriver); + virRegisterStateDriver(&configStateDriver); + return 0; +} diff --git a/src/config/config_driver.h b/src/config/config_driver.h new file mode 100644 index 0000000..c74af71 --- /dev/null +++ b/src/config/config_driver.h @@ -0,0 +1,44 @@ +/* + * config_driver.h: core driver methods for accessing configs + * + * Copyright (C) 2013 Adam Walters + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + * + * Author: Adam Walters <adam(a)pandorasboxen.com&gt; + */ + +#ifndef __CONFIG_DRIVER_H__ +# define __CONFIG_DRIVER_H__ + +# include "virsysinfo.h" +# include "virclosecallbacks.h" + +typedef struct _virConfigDriver virConfigDriver; +typedef virConfigDriver *virConfigDriverPtr; + +struct _virConfigDriver { + virMutex lock; + + /* Immutable pointer, lockless APIs*/ + virSysinfoDefPtr hostsysinfo; + + /* Immutable pointer, self-locking APIs*/ + virCloseCallbacksPtr closeCallbacks; +}; + +int configRegister(void); + +#endif /* __CONFIG_DRIVER_H__ */ diff --git a/src/util/virerror.c b/src/util/virerror.c index f0c159f..a16a517 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -124,6 +124,8 @@ VIR_ENUM_IMPL(virErrorDomain, VIR_ERR_DOMAIN_LAST, "Access Manager", /* 55 */ "Systemd", + + "Config Driver", /* 57 */ ) -- 1.8.5.2

This patch changes the order in which the drivers are loaded in libvirtd.c. Per the comment at the top of the daemonInitialize function, order is important. Since the storage driver may depend on the secrets driver, but the secrets driver can't depend on the storage driver, I moved the loading of the secrets driver to just above the storage driver. Signed-off-by: Adam Walters <adam(a)pandorasboxen.com&gt; --- daemon/libvirtd.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 1e76f5a..251c2f4 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -378,15 +378,15 @@ static void daemonInitialize(void) # ifdef WITH_NETWORK virDriverLoadModule("network"); # endif +# ifdef WITH_SECRETS + virDriverLoadModule("secret"); +# endif # ifdef WITH_STORAGE virDriverLoadModule("storage"); # endif # ifdef WITH_NODE_DEVICES virDriverLoadModule("nodedev"); # endif -# ifdef WITH_SECRETS - virDriverLoadModule("secret"); -# endif # ifdef WITH_NWFILTER virDriverLoadModule("nwfilter"); # endif @@ -421,15 +421,15 @@ static void daemonInitialize(void) # ifdef WITH_INTERFACE interfaceRegister(); # endif +# ifdef WITH_SECRETS + secretRegister(); +# endif # ifdef WITH_STORAGE storageRegister(); # endif # ifdef WITH_NODE_DEVICES nodedevRegister(); # endif -# ifdef WITH_SECRETS - secretRegister(); -# endif # ifdef WITH_NWFILTER nwfilterRegister(); # endif -- 1.8.5.2

On Thu, Jan 23, 2014 at 03:14:19PM -0500, Adam Walters wrote: So I had an idea about one possible alternative way to deal with this. Basically have a single global virConnectPtr instance which each non-virt driver wires up its hooks to when it starts. I've not fully tested this approach, but I've got a example patch which better illustrates what I mean: diff --git a/src/datatypes.c b/src/datatypes.c index 73f17e7..9da2ff4 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -124,6 +124,26 @@ error: return NULL; } +virConnectPtr virGetGlobalConnect(void) +{ + static virConnectPtr globalconn; + virConnectPtr conn; + + if (globalconn) + return globalconn; + + if (!(conn = virGetConnect())) + return NULL; + + if (!(conn->uri = virURIParse("global:///"))) { + virObjectUnref(conn); + return NULL; + } + + globalconn = conn; + return globalconn; +} + /** * virConnectDispose: * @conn: the hypervisor connection to release diff --git a/src/datatypes.h b/src/datatypes.h index 9621c55..6806832 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -521,6 +521,8 @@ struct _virNWFilter { */ virConnectPtr virGetConnect(void); +virConnectPtr virGetGlobalConnect(void); + virDomainPtr virGetDomain(virConnectPtr conn, const char *name, const unsigned char *uuid); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0c16343..8ac2bf5 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -762,6 +762,7 @@ virDomainSnapshotClass; virGetConnect; virGetDomain; virGetDomainSnapshot; +virGetGlobalConnect; virGetInterface; virGetNetwork; virGetNodeDevice; diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c index 9f7f946..a88ea23 100644 --- a/src/secret/secret_driver.c +++ b/src/secret/secret_driver.c @@ -1102,6 +1102,10 @@ secretStateInitialize(bool privileged, void *opaque ATTRIBUTE_UNUSED) { char *base = NULL; + virConnectPtr conn; + + if ((conn = virGetGlobalConnect()) == NULL) + return -1; if (VIR_ALLOC(driverState) < 0) return -1; @@ -1127,6 +1131,7 @@ secretStateInitialize(bool privileged, if (loadSecrets(driverState, &driverState->secrets) < 0) goto error; + conn->secretPrivateData = driverState; secretDriverUnlock(driverState); return 0; diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index c83aa8a..017a74d 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -68,14 +68,7 @@ static void storageDriverUnlock(virStorageDriverStatePtr driver) static void storageDriverAutostart(virStorageDriverStatePtr driver) { size_t i; - virConnectPtr conn = NULL; - - /* XXX Remove hardcoding of QEMU URI */ - if (driverState->privileged) - conn = virConnectOpen("qemu:///system"); - else - conn = virConnectOpen("qemu:///session"); - /* Ignoring NULL conn - let backends decide */ + virConnectPtr conn = virGetGlobalConnect(); for (i = 0; i < driver->pools.count; i++) { virStoragePoolObjPtr pool = driver->pools.objs[i]; @@ -129,8 +122,6 @@ storageDriverAutostart(virStorageDriverStatePtr driver) { } virStoragePoolObjUnlock(pool); } - - virObjectUnref(conn); } /** It is based on the similar idea that you had, that we don't actually need to have a full connection with virt drivers active. The difference with my approach is that we're not exposing a new URI externally - this hack is only visible inside libvirtd, so we're free to change it any time we want. If you think this approach will work I'll look at doing a complete patch for it for all non-virt drivers. 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 :|

On Wed, Jan 29, 2014 at 07:01:31AM -0500, Adam Walters wrote: Ah don't worry about it too much, as I'm off to FOSDEM on friday. I'll continue to explore this idea and see about reating a full patch to test next week. 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 :|