We previously added "-z nodelete" to the build of libvirt.so to prevent crashes when thread local destructors run which point to a code that has been dlclose()d: commit 384b9a76a5e387f64cfe8f83f4a518bb302e80f7 Author: Daniel P. Berrangé <berrange(a)redhat.com&gt; Date: Thu Apr 19 11:42:22 2018 +0100 driver: prevent unloading of dlopen'd modules We forgot to copy this protection into the libvirt-qemu.so, libvirt-lxc.so and libvirt-admin.so libraries when we introduced them. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/Makefile.am | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Makefile.am b/src/Makefile.am index a6667228b6..2f8e5f6908 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -529,7 +529,9 @@ libvirt_admin_la_SOURCES = \ libvirt_admin_la_LDFLAGS = \ $(VERSION_SCRIPT_FLAGS)$(LIBVIRT_ADMIN_SYMBOL_FILE) \ -version-info $(LIBVIRT_VERSION_INFO) \ - $(AM_LDFLAGS) + $(LIBVIRT_NODELETE) \ + $(AM_LDFLAGS) \ + $(NULL) libvirt_admin_la_LIBADD = \ libvirt.la \ @@ -644,6 +646,7 @@ libvirt_qemu_la_SOURCES = libvirt-qemu.c libvirt_qemu_la_LDFLAGS = \ $(VERSION_SCRIPT_FLAGS)$(LIBVIRT_QEMU_SYMBOL_FILE) \ -version-info $(LIBVIRT_VERSION_INFO) \ + $(LIBVIRT_NODELETE) \ $(AM_LDFLAGS) \ $(NULL) libvirt_qemu_la_CFLAGS = $(AM_CFLAGS) @@ -653,6 +656,7 @@ libvirt_lxc_la_SOURCES = libvirt-lxc.c libvirt_lxc_la_LDFLAGS = \ $(VERSION_SCRIPT_FLAGS)$(LIBVIRT_LXC_SYMBOL_FILE) \ -version-info $(LIBVIRT_VERSION_INFO) \ + $(LIBVIRT_NODELETE) \ $(AM_LDFLAGS) \ $(NULL) libvirt_lxc_la_CFLAGS = $(AM_CFLAGS) -- 2.14.3

On 04/19/2018 07:09 PM, Daniel P. Berrangé wrote: ACK please. Also might be worth removing the following from virsh (in a separate patch perhaps?) diff --git i/tools/virsh.c w/tools/virsh.c index 5f8352e861..62226eea4c 100644 --- i/tools/virsh.c +++ w/tools/virsh.c @@ -526,9 +526,6 @@ virshShowVersion(vshControl *ctl ATTRIBUTE_UNUSED) #ifdef WITH_UML vshPrint(ctl, " UML"); #endif -#ifdef WITH_XEN - vshPrint(ctl, " Xen"); -#endif #ifdef WITH_LIBXL vshPrint(ctl, " LibXL"); #endif Michal

We previously added "-z nodelete" to the build of libvirt.so to prevent crashes when thread local destructors run which point to a code that has been dlclose()d: commit 384b9a76a5e387f64cfe8f83f4a518bb302e80f7 Author: Daniel P. Berrangé <berrange(a)redhat.com&gt; Date: Thu Apr 19 11:42:22 2018 +0100 driver: prevent unloading of dlopen'd modules The libvirtd loadable modules can suffer from the same problem if they were ever unloaded. Fortunately we don't ever call dlclose() on them, but lets add a second layer of protection by linking them with the "-z nodelete" flag. While we're doing this, lets add a third layer of protection by passing RTLD_NODELETE to dlopen(). Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/Makefile.am | 6 +++++- src/driver.c | 7 ++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 2f8e5f6908..0d8d380df1 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -51,7 +51,11 @@ AM_LDFLAGS = $(DRIVER_MODULES_LDFLAGS) \ $(CYGWIN_EXTRA_LDFLAGS) \ $(MINGW_EXTRA_LDFLAGS) \ $(NULL) -AM_LDFLAGS_MOD = -module -avoid-version $(AM_LDFLAGS) +AM_LDFLAGS_MOD = \ + -module \ + -avoid-version \ + $(LIBVIRT_NODELETE) \ + $(AM_LDFLAGS) AM_LDFLAGS_MOD_NOUNDEF = $(AM_LDFLAGS_MOD) $(NO_UNDEFINED_LDFLAGS) POD2MAN = pod2man -c "Virtualization Support" -r "$(PACKAGE)-$(VERSION)" diff --git a/src/driver.c b/src/driver.c index 52e1ae345a..7d4c78eaaa 100644 --- a/src/driver.c +++ b/src/driver.c @@ -45,6 +45,11 @@ static void * virDriverLoadModuleFile(const char *file) { void *handle = NULL; + int flags = RTLD_NOW | RTLD_GLOBAL; + +# ifdef RTLD_NODELETE + flags |= RTLD_NODELETE; +# endif VIR_DEBUG("Load module file '%s'", file); @@ -55,7 +60,7 @@ virDriverLoadModuleFile(const char *file) virUpdateSelfLastChanged(file); - if (!(handle = dlopen(file, RTLD_NOW | RTLD_GLOBAL))) + if (!(handle = dlopen(file, flags))) VIR_ERROR(_("failed to load module %s %s"), file, dlerror()); return handle; -- 2.14.3

Now that we've activated two hacks to prevent unloading of modules, there is no point passing back a pointer to the loaded library handle. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/driver.c | 23 +++++++---------------- src/driver.h | 5 ++--- src/storage/storage_backend.c | 2 +- 3 files changed, 10 insertions(+), 20 deletions(-) diff --git a/src/driver.c b/src/driver.c index 7d4c78eaaa..4d314b3870 100644 --- a/src/driver.c +++ b/src/driver.c @@ -86,12 +86,11 @@ virDriverLoadModuleFunc(void *handle, * virDriverLoadModuleFull: * @path: filename of module to load * @regfunc: name of the function that registers the module - * @handle: Returns handle of the loaded library if not NULL * * Loads a loadable module named @path and calls the - * registration function @regfunc. If @handle is not NULL the handle is returned - * in the variable. Otherwise the handle is leaked so that the module stays - * loaded forever. + * registration function @regfunc. The module will never + * be unloaded because unloading is not safe in a multi-threaded + * application. * * The module is automatically looked up in the appropriate place (git or * installed directory). @@ -100,8 +99,7 @@ virDriverLoadModuleFunc(void *handle, */ int virDriverLoadModuleFull(const char *path, - const char *regfunc, - void **handle) + const char *regfunc) { void *rethandle = NULL; int (*regsym)(void); @@ -120,11 +118,7 @@ virDriverLoadModuleFull(const char *path, goto cleanup; } - if (handle) - VIR_STEAL_PTR(*handle, rethandle); - else - rethandle = NULL; - + rethandle = NULL; ret = 0; cleanup: @@ -136,12 +130,9 @@ virDriverLoadModuleFull(const char *path, #else /* ! HAVE_DLFCN_H */ int virDriverLoadModuleFull(const char *path ATTRIBUTE_UNUSED, - const char *regfunc ATTRIBUTE_UNUSED, - void **handle) + const char *regfunc ATTRIBUTE_UNUSED) { VIR_DEBUG("dlopen not available on this platform"); - if (handle) - *handle = NULL; return -1; } #endif /* ! HAVE_DLFCN_H */ @@ -164,7 +155,7 @@ virDriverLoadModule(const char *name, "LIBVIRT_DRIVER_DIR"))) return 1; - ret = virDriverLoadModuleFull(modfile, regfunc, NULL); + ret = virDriverLoadModuleFull(modfile, regfunc); VIR_FREE(modfile); diff --git a/src/driver.h b/src/driver.h index b071a3a782..e28c63ecc2 100644 --- a/src/driver.h +++ b/src/driver.h @@ -109,9 +109,8 @@ int virSetSharedStorageDriver(virStorageDriverPtr driver) ATTRIBUTE_RETURN_CHECK int virDriverLoadModule(const char *name, const char *regfunc); -int virDriverLoadModuleFull(const char *name, - const char *regfunc, - void **handle); +int virDriverLoadModuleFull(const char *path, + const char *regfunc); virConnectPtr virGetConnectInterface(void); virConnectPtr virGetConnectNetwork(void); diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 053f4ecf26..aac2f53b01 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -97,7 +97,7 @@ virStorageDriverLoadBackendModule(const char *name, "LIBVIRT_STORAGE_BACKEND_DIR"))) return 1; - if ((ret = virDriverLoadModuleFull(modfile, regfunc, NULL)) != 0) { + if ((ret = virDriverLoadModuleFull(modfile, regfunc)) != 0) { if (forceload) { virReportError(VIR_ERR_INTERNAL_ERROR, _("failed to load storage backend module '%s'"), -- 2.14.3

The virFileFindResource method merely builds up the expected fully qualified path to the resource. It does not actually check if it exists on disk. The loadable module callers were mistakenly thinking a NULL indicates the file doesn't exist on disk, whereas it in fact indicates an out of memory error. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/driver.c | 2 +- src/storage/storage_backend.c | 2 +- src/util/virfile.c | 4 ++++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/driver.c b/src/driver.c index 4d314b3870..ddda1e71f7 100644 --- a/src/driver.c +++ b/src/driver.c @@ -153,7 +153,7 @@ virDriverLoadModule(const char *name, abs_topbuilddir "/src/.libs", DEFAULT_DRIVER_DIR, "LIBVIRT_DRIVER_DIR"))) - return 1; + return -1; ret = virDriverLoadModuleFull(modfile, regfunc); diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index aac2f53b01..8c1dcf31b1 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -95,7 +95,7 @@ virStorageDriverLoadBackendModule(const char *name, abs_topbuilddir "/src/.libs", STORAGE_BACKEND_MODULE_DIR, "LIBVIRT_STORAGE_BACKEND_DIR"))) - return 1; + return -1; if ((ret = virDriverLoadModuleFull(modfile, regfunc)) != 0) { if (forceload) { diff --git a/src/util/virfile.c b/src/util/virfile.c index 5e9bd2007a..e12a584ca1 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -1736,6 +1736,10 @@ static bool useDirOverride; * run from the source tree. Otherwise it will return the * path in the installed location. * + * Note that this function does not actually check whether + * the file exists on disk, it merely builds the fully + * qualified path where it is supposed to exist. + * * If @envname is non-NULL it will override all other * directory lookup. * -- 2.14.3

Currently we do a access(R_OK) check to see whether a loadable module exists, treating failure as non-fatal. This is unreasonably loose, as a module which exists but has had incorrect permissions set will turn into a silent skip. We only want to skip loading if the module genuinely does not exist on disk, due to the optional package not being installed. Furthermore, checking the return value of virDriverLoadModuleFile() is not a suitable witness that the module does not exist. This method can return NULL if dlopen() fails, for example due to being unable to resolve symbols in the library. This is should always be reported as an error because it is a sign of the bad installation where either the module build doesn't match the libvirtd build, or where some 3rd party libraries are missing or broken. Both these problems can be fixed by using virFileExists in the caller instead. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/driver.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/driver.c b/src/driver.c index ddda1e71f7..e02efe2615 100644 --- a/src/driver.c +++ b/src/driver.c @@ -53,11 +53,6 @@ virDriverLoadModuleFile(const char *file) VIR_DEBUG("Load module file '%s'", file); - if (access(file, R_OK) < 0) { - VIR_INFO("Module %s not accessible", file); - return NULL; - } - virUpdateSelfLastChanged(file); if (!(handle = dlopen(file, flags))) @@ -105,11 +100,14 @@ virDriverLoadModuleFull(const char *path, int (*regsym)(void); int ret = -1; - if (!(rethandle = virDriverLoadModuleFile(path))) { - ret = 1; - goto cleanup; + if (!virFileExists(path)) { + VIR_INFO("Module '%s' does not exists", path); + return 1; } + if (!(rethandle = virDriverLoadModuleFile(path))) + goto cleanup; + if (!(regsym = virDriverLoadModuleFunc(rethandle, regfunc))) goto cleanup; -- 2.14.3

The driver module loading code is one of the few places that still uses VIR_ERROR for reporting failures. Convert it to normal error reporting APIs. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/driver.c | 34 +++++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/src/driver.c b/src/driver.c index e02efe2615..9b137c39e4 100644 --- a/src/driver.c +++ b/src/driver.c @@ -33,6 +33,7 @@ VIR_LOG_INIT("driver"); +#define VIR_FROM_THIS VIR_FROM_NONE /* XXX re-implement this for other OS, or use libtools helper lib ? */ #define DEFAULT_DRIVER_DIR LIBDIR "/libvirt/connection-driver" @@ -55,8 +56,11 @@ virDriverLoadModuleFile(const char *file) virUpdateSelfLastChanged(file); - if (!(handle = dlopen(file, flags))) - VIR_ERROR(_("failed to load module %s %s"), file, dlerror()); + if (!(handle = dlopen(file, flags))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to load module '%s': %s"), file, dlerror()); + return NULL; + } return handle; } @@ -64,14 +68,19 @@ virDriverLoadModuleFile(const char *file) static void * virDriverLoadModuleFunc(void *handle, + const char *file, const char *funcname) { void *regsym; VIR_DEBUG("Lookup function '%s'", funcname); - if (!(regsym = dlsym(handle, funcname))) - VIR_ERROR(_("Missing module registration symbol %s"), funcname); + if (!(regsym = dlsym(handle, funcname))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to find symbol '%s' in module '%s': %s"), + funcname, file, dlerror()); + return NULL; + } return regsym; } @@ -108,11 +117,18 @@ virDriverLoadModuleFull(const char *path, if (!(rethandle = virDriverLoadModuleFile(path))) goto cleanup; - if (!(regsym = virDriverLoadModuleFunc(rethandle, regfunc))) + if (!(regsym = virDriverLoadModuleFunc(rethandle, path, regfunc))) goto cleanup; if ((*regsym)() < 0) { - VIR_ERROR(_("Failed module registration %s"), regfunc); + /* regsym() should report an error itself, but lets + * just make sure */ + virErrorPtr err = virGetLastError(); + if (err == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to execute symbol '%s' in module '%s'"), + regfunc, path); + } goto cleanup; } @@ -131,7 +147,11 @@ virDriverLoadModuleFull(const char *path ATTRIBUTE_UNUSED, const char *regfunc ATTRIBUTE_UNUSED) { VIR_DEBUG("dlopen not available on this platform"); - return -1; + /* Since we have no dlopen(), but definition we have no + * loadable modules on disk, so we can resaonably + * return '1' instead of an error. + */ + return 1; } #endif /* ! HAVE_DLFCN_H */ -- 2.14.3

Currently the driver module loading code does not report an error if the driver module is physically missing on disk. This is useful for distro packaging optional pieces. When the daemons are split up into one daemon per driver, we will expect module loading to always succeed. If a driver is not desired, the entire daemon should not be installed. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/driver.c | 37 ++++++++++++++++++++++++++----------- src/driver.h | 6 ++++-- src/remote/remote_daemon.c | 2 +- src/storage/storage_backend.c | 9 +-------- tests/virdrivermoduletest.c | 2 +- 5 files changed, 33 insertions(+), 23 deletions(-) diff --git a/src/driver.c b/src/driver.c index 9b137c39e4..447f61d554 100644 --- a/src/driver.c +++ b/src/driver.c @@ -103,15 +103,22 @@ virDriverLoadModuleFunc(void *handle, */ int virDriverLoadModuleFull(const char *path, - const char *regfunc) + const char *regfunc, + bool required) { void *rethandle = NULL; int (*regsym)(void); int ret = -1; if (!virFileExists(path)) { - VIR_INFO("Module '%s' does not exists", path); - return 1; + if (required) { + virReportSystemError(errno, + _("Failed to find module '%s'"), path); + return -1; + } else { + VIR_INFO("Module '%s' does not exist", path); + return 1; + } } if (!(rethandle = virDriverLoadModuleFile(path))) @@ -144,21 +151,29 @@ virDriverLoadModuleFull(const char *path, #else /* ! HAVE_DLFCN_H */ int virDriverLoadModuleFull(const char *path ATTRIBUTE_UNUSED, - const char *regfunc ATTRIBUTE_UNUSED) + const char *regfunc ATTRIBUTE_UNUSED, + bool required) { VIR_DEBUG("dlopen not available on this platform"); - /* Since we have no dlopen(), but definition we have no - * loadable modules on disk, so we can resaonably - * return '1' instead of an error. - */ - return 1; + if (required) { + virReportSystemError(ENOSYS, + _("Failed to find module '%s': %s"), path); + return -1; + } else { + /* Since we have no dlopen(), but definition we have no + * loadable modules on disk, so we can resaonably + * return '1' instead of an error. + */ + return 1; + } } #endif /* ! HAVE_DLFCN_H */ int virDriverLoadModule(const char *name, - const char *regfunc) + const char *regfunc, + bool required) { char *modfile = NULL; int ret; @@ -173,7 +188,7 @@ virDriverLoadModule(const char *name, "LIBVIRT_DRIVER_DIR"))) return -1; - ret = virDriverLoadModuleFull(modfile, regfunc); + ret = virDriverLoadModuleFull(modfile, regfunc, required); VIR_FREE(modfile); diff --git a/src/driver.h b/src/driver.h index e28c63ecc2..b4e50ab987 100644 --- a/src/driver.h +++ b/src/driver.h @@ -108,9 +108,11 @@ int virSetSharedSecretDriver(virSecretDriverPtr driver) ATTRIBUTE_RETURN_CHECK; int virSetSharedStorageDriver(virStorageDriverPtr driver) ATTRIBUTE_RETURN_CHECK; int virDriverLoadModule(const char *name, - const char *regfunc); + const char *regfunc, + bool required); int virDriverLoadModuleFull(const char *path, - const char *regfunc); + const char *regfunc, + bool required); virConnectPtr virGetConnectInterface(void); virConnectPtr virGetConnectNetwork(void); diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c index 3e02297eee..b4f89d4fd7 100644 --- a/src/remote/remote_daemon.c +++ b/src/remote/remote_daemon.c @@ -295,7 +295,7 @@ static int daemonErrorLogFilter(virErrorPtr err, int priority) #define VIR_DAEMON_LOAD_MODULE(func, module) \ - virDriverLoadModule(module, #func) + virDriverLoadModule(module, #func, false) static void daemonInitialize(void) { /* diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 8c1dcf31b1..cb1bcc0944 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -97,14 +97,7 @@ virStorageDriverLoadBackendModule(const char *name, "LIBVIRT_STORAGE_BACKEND_DIR"))) return -1; - if ((ret = virDriverLoadModuleFull(modfile, regfunc)) != 0) { - if (forceload) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("failed to load storage backend module '%s'"), - name); - ret = -1; - } - } + ret = virDriverLoadModuleFull(modfile, regfunc, forceload); VIR_FREE(modfile); diff --git a/tests/virdrivermoduletest.c b/tests/virdrivermoduletest.c index 6c63ecd52b..125183327b 100644 --- a/tests/virdrivermoduletest.c +++ b/tests/virdrivermoduletest.c @@ -41,7 +41,7 @@ static int testDriverModule(const void *args) const struct testDriverModuleData *data = args; /* coverity[leaked_storage] */ - if (virDriverLoadModule(data->module, data->regfunc) != 0) + if (virDriverLoadModule(data->module, data->regfunc, true) != 0) return -1; return 0; -- 2.14.3

The libvirtd daemon currently ignores the return status of virDriverLoadModule entirely. This is way too loose, resulting in many important problems going undiagnosed, resulting in a libvirtd that may never work correctly. We should only ignore a non-existant module, and pass back any fatal errors. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/remote/remote_daemon.c | 61 +++++++++++++++++++++++++++------------------- 1 file changed, 36 insertions(+), 25 deletions(-) diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c index b4f89d4fd7..27377fe3bc 100644 --- a/src/remote/remote_daemon.c +++ b/src/remote/remote_daemon.c @@ -87,6 +87,7 @@ enum { VIR_DAEMON_ERR_CONFIG, VIR_DAEMON_ERR_HOOKS, VIR_DAEMON_ERR_AUDIT, + VIR_DAEMON_ERR_DRIVER, VIR_DAEMON_ERR_LAST }; @@ -102,7 +103,8 @@ VIR_ENUM_IMPL(virDaemonErr, VIR_DAEMON_ERR_LAST, "Unable to initialize network sockets", "Unable to load configuration file", "Unable to look for hook scripts", - "Unable to initialize audit system") + "Unable to initialize audit system", + "Unable to initialize driver") static int daemonForkIntoBackground(const char *argv0) { @@ -294,9 +296,7 @@ static int daemonErrorLogFilter(virErrorPtr err, int priority) } -#define VIR_DAEMON_LOAD_MODULE(func, module) \ - virDriverLoadModule(module, #func, false) -static void daemonInitialize(void) +static int daemonInitialize(void) { /* * Note that the order is important: the first ones have a higher @@ -305,52 +305,60 @@ static void daemonInitialize(void) * driver, since their resources must be auto-started before any * domains can be auto-started. */ - /* We don't care if any of these fail, because the whole point - * is to allow users to only install modules they want to use. - * If they try to open a connection for a module that - * is not loaded they'll get a suitable error at that point - */ #ifdef WITH_NETWORK - VIR_DAEMON_LOAD_MODULE(networkRegister, "network"); + if (virDriverLoadModule("network", "networkRegister", false) < 0) + return -1; #endif #ifdef WITH_INTERFACE - VIR_DAEMON_LOAD_MODULE(interfaceRegister, "interface"); + if (virDriverLoadModule("interface", "interfaceRegister", false) < 0) + return -1; #endif #ifdef WITH_STORAGE - VIR_DAEMON_LOAD_MODULE(storageRegister, "storage"); + if (virDriverLoadModule("storage", "storageRegister", false) < 0) + return -1; #endif #ifdef WITH_NODE_DEVICES - VIR_DAEMON_LOAD_MODULE(nodedevRegister, "nodedev"); + if (virDriverLoadModule("nodedev", "nodedevRegister", false) < 0) + return -1; #endif #ifdef WITH_SECRETS - VIR_DAEMON_LOAD_MODULE(secretRegister, "secret"); + if (virDriverLoadModule("secret", "secretRegister", false) < 0) + return -1; #endif #ifdef WITH_NWFILTER - VIR_DAEMON_LOAD_MODULE(nwfilterRegister, "nwfilter"); + if (virDriverLoadModule("nwfilter", "nwfilterRegister", false) < 0) + return -1; #endif #ifdef WITH_LIBXL - VIR_DAEMON_LOAD_MODULE(libxlRegister, "libxl"); + if (virDriverLoadModule("libxl", "libxlRegister", false) < 0) + return -1; #endif #ifdef WITH_QEMU - VIR_DAEMON_LOAD_MODULE(qemuRegister, "qemu"); + if (virDriverLoadModule("qemu", "qemuRegister", false) < 0) + return -1; #endif #ifdef WITH_LXC - VIR_DAEMON_LOAD_MODULE(lxcRegister, "lxc"); + if (virDriverLoadModule("lxc", "lxcRegister", false) < 0) + return -1; #endif #ifdef WITH_UML - VIR_DAEMON_LOAD_MODULE(umlRegister, "uml"); + if (virDriverLoadModule("uml", "umlRegister", false) < 0) + return -1; #endif #ifdef WITH_VBOX - VIR_DAEMON_LOAD_MODULE(vboxRegister, "vbox"); + if (virDriverLoadModule("vbox", "vboxRegister", false) < 0) + return -1; #endif #ifdef WITH_BHYVE - VIR_DAEMON_LOAD_MODULE(bhyveRegister, "bhyve"); + if (virDriverLoadModule("bhyve", "bhyveRegister", false) < 0) + return -1; #endif #ifdef WITH_VZ - VIR_DAEMON_LOAD_MODULE(vzRegister, "vz"); + if (virDriverLoadModule("vz", "vzRegister", false) < 0) + return -1; #endif + return 0; } -#undef VIR_DAEMON_LOAD_MODULE static int ATTRIBUTE_NONNULL(3) @@ -1283,7 +1291,7 @@ int main(int argc, char **argv) { } if (!(dmn = virNetDaemonNew())) { - ret = VIR_DAEMON_ERR_INIT; + ret = VIR_DAEMON_ERR_DRIVER; goto cleanup; } @@ -1309,7 +1317,10 @@ int main(int argc, char **argv) { goto cleanup; } - daemonInitialize(); + if (daemonInitialize() < 0) { + ret = VIR_DAEMON_ERR_INIT; + goto cleanup; + } remoteProcs[REMOTE_PROC_AUTH_LIST].needAuth = false; remoteProcs[REMOTE_PROC_AUTH_SASL_INIT].needAuth = false; -- 2.14.3

Currently the remote driver extracts the transport from URI scheme and plays games to temporarily hide the driver part when formatting URIs. Refactor the code to split the URI scheme upfront so the two pieces are easily available where needed. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/remote/remote_driver.c | 73 ++++++++++++++++++++++++++-------------------- 1 file changed, 42 insertions(+), 31 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index d3b588c374..436324d252 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -24,7 +24,6 @@ #include <config.h> #include <unistd.h> -#include <assert.h> #include "virnetclient.h" #include "virnetclientprogram.h" @@ -162,7 +161,26 @@ static void make_nonnull_domain_snapshot(remote_nonnull_domain_snapshot *snapsho /*----------------------------------------------------------------------*/ /* Helper functions for remoteOpen. */ -static char *get_transport_from_scheme(char *scheme); +static int remoteSplitURIScheme(virURIPtr uri, + char **driver, + char **transport) +{ + char *p = strchr(uri->scheme, '+'); + + *driver = *transport = NULL; + + if (VIR_STRNDUP(*driver, uri->scheme, p ? p - uri->scheme : -1) < 0) + return -1; + + if (p && + VIR_STRDUP(*transport, p + 1) < 0) { + VIR_FREE(*driver); + return -1; + } + + return 0; +} + static int remoteStateInitialize(bool privileged ATTRIBUTE_UNUSED, @@ -715,11 +733,12 @@ remoteConnectSupportsFeatureUnlocked(virConnectPtr conn, static int doRemoteOpen(virConnectPtr conn, struct private_data *priv, + const char *driver_str, + const char *transport_str, virConnectAuthPtr auth ATTRIBUTE_UNUSED, virConfPtr conf, unsigned int flags) { - char *transport_str = NULL; enum { trans_tls, trans_unix, @@ -738,8 +757,6 @@ doRemoteOpen(virConnectPtr conn, * URIs we don't care about */ if (conn->uri) { - transport_str = get_transport_from_scheme(conn->uri->scheme); - if (!transport_str) { if (conn->uri->server) transport = trans_tls; @@ -873,26 +890,16 @@ doRemoteOpen(virConnectPtr conn, goto failed; } else { virURI tmpuri = { - .scheme = conn->uri->scheme, + .scheme = (char *)driver_str, .query = virURIFormatParams(conn->uri), .path = conn->uri->path, .fragment = conn->uri->fragment, }; - /* Evil, blank out transport scheme temporarily */ - if (transport_str) { - assert(transport_str[-1] == '+'); - transport_str[-1] = '\0'; - } - name = virURIFormat(&tmpuri); VIR_FREE(tmpuri.query); - /* Restore transport scheme */ - if (transport_str) - transport_str[-1] = '+'; - if (!name) goto failed; } @@ -1297,14 +1304,23 @@ remoteConnectOpen(virConnectPtr conn, unsigned int flags) { struct private_data *priv; - int ret, rflags = 0; + int ret = VIR_DRV_OPEN_ERROR; + int rflags = 0; const char *autostart = virGetEnvBlockSUID("LIBVIRT_AUTOSTART"); + char *driver = NULL; + char *transport = NULL; - if (inside_daemon && (!conn->uri || !conn->uri->server)) - return VIR_DRV_OPEN_DECLINED; + if (conn->uri && + remoteSplitURIScheme(conn->uri, &driver, &transport) < 0) + goto cleanup; + + if (inside_daemon && (!conn->uri || !conn->uri->server)) { + ret = VIR_DRV_OPEN_DECLINED; + goto cleanup; + } if (!(priv = remoteAllocPrivateData())) - return VIR_DRV_OPEN_ERROR; + goto cleanup; if (flags & VIR_CONNECT_RO) rflags |= VIR_DRV_OPEN_REMOTE_RO; @@ -1319,8 +1335,7 @@ remoteConnectOpen(virConnectPtr conn, !conn->uri->server && conn->uri->path && conn->uri->scheme && - ((strchr(conn->uri->scheme, '+') == 0)|| - (strstr(conn->uri->scheme, "+unix") != NULL)) && + (transport == NULL || STREQ(transport, "unix")) && (STREQ(conn->uri->path, "/session") || STRPREFIX(conn->uri->scheme, "test+")) && geteuid() > 0) { @@ -1348,7 +1363,7 @@ remoteConnectOpen(virConnectPtr conn, } } - ret = doRemoteOpen(conn, priv, auth, conf, rflags); + ret = doRemoteOpen(conn, priv, driver, transport, auth, conf, rflags); if (ret != VIR_DRV_OPEN_SUCCESS) { conn->privateData = NULL; remoteDriverUnlock(priv); @@ -1357,18 +1372,14 @@ remoteConnectOpen(virConnectPtr conn, conn->privateData = priv; remoteDriverUnlock(priv); } + + cleanup: + VIR_FREE(driver); + VIR_FREE(transport); return ret; } -/* In a string "driver+transport" return a pointer to "transport". */ -static char * -get_transport_from_scheme(char *scheme) -{ - char *p = strchr(scheme, '+'); - return p ? p + 1 : NULL; -} - /*----------------------------------------------------------------------*/ -- 2.14.3

The code for building UNIX socket paths will be getting more complex to cope with accessing various different daemons. Refactor it to eliminate the code duplication and isolation the logic for constructing paths. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/remote/remote_driver.c | 69 ++++++++++++++++++++++++++++------------------ 1 file changed, 42 insertions(+), 27 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 436324d252..b4b034423a 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -710,6 +710,39 @@ remoteConnectSupportsFeatureUnlocked(virConnectPtr conn, var->ignore = 1; \ continue; \ } + + +static char *remoteGetUNIXSocketNonRoot(void) +{ + char *sockname = NULL; + char *userdir = virGetUserRuntimeDirectory(); + + if (!userdir) + return NULL; + + if (virAsprintf(&sockname, "%s/" LIBVIRTD_USER_UNIX_SOCKET, userdir) < 0) { + VIR_FREE(userdir); + return NULL; + } + VIR_FREE(userdir); + + VIR_DEBUG("Chosen UNIX sockname %s", sockname); + return sockname; +} + +static char *remoteGetUNIXSocketRoot(unsigned int flags) +{ + char *sockname = NULL; + + if (VIR_STRDUP(sockname, + flags & VIR_DRV_OPEN_REMOTE_RO ? + LIBVIRTD_PRIV_UNIX_SOCKET_RO : LIBVIRTD_PRIV_UNIX_SOCKET) < 0) + return NULL; + + VIR_DEBUG("Chosen UNIX sockname %s", sockname); + return sockname; +} + /* * URIs that this driver needs to handle: * @@ -971,9 +1004,7 @@ doRemoteOpen(virConnectPtr conn, goto failed; } - if (VIR_STRDUP(sockname, - flags & VIR_DRV_OPEN_REMOTE_RO ? - LIBVIRTD_PRIV_UNIX_SOCKET_RO : LIBVIRTD_PRIV_UNIX_SOCKET) < 0) + if (!(sockname = remoteGetUNIXSocketRoot(flags))) goto failed; } @@ -1008,9 +1039,7 @@ doRemoteOpen(virConnectPtr conn, goto failed; } - if (VIR_STRDUP(sockname, - flags & VIR_DRV_OPEN_REMOTE_RO ? - LIBVIRTD_PRIV_UNIX_SOCKET_RO : LIBVIRTD_PRIV_UNIX_SOCKET) < 0) + if (!(sockname = remoteGetUNIXSocketRoot(flags))) goto failed; } @@ -1037,24 +1066,12 @@ doRemoteOpen(virConnectPtr conn, #ifndef WIN32 case trans_unix: if (!sockname) { - if (flags & VIR_DRV_OPEN_REMOTE_USER) { - char *userdir = virGetUserRuntimeDirectory(); - - if (!userdir) - goto failed; - - if (virAsprintf(&sockname, "%s/" LIBVIRTD_USER_UNIX_SOCKET, userdir) < 0) { - VIR_FREE(userdir); - goto failed; - } - VIR_FREE(userdir); - } else { - if (VIR_STRDUP(sockname, - flags & VIR_DRV_OPEN_REMOTE_RO ? - LIBVIRTD_PRIV_UNIX_SOCKET_RO : LIBVIRTD_PRIV_UNIX_SOCKET) < 0) - goto failed; - } - VIR_DEBUG("Proceeding with sockname %s", sockname); + if (flags & VIR_DRV_OPEN_REMOTE_USER) + sockname = remoteGetUNIXSocketNonRoot(); + else + sockname = remoteGetUNIXSocketRoot(flags); + if (!sockname) + goto failed; } if ((flags & VIR_DRV_OPEN_REMOTE_AUTOSTART) && @@ -1087,9 +1104,7 @@ doRemoteOpen(virConnectPtr conn, goto failed; } - if (VIR_STRDUP(sockname, - flags & VIR_DRV_OPEN_REMOTE_RO ? - LIBVIRTD_PRIV_UNIX_SOCKET_RO : LIBVIRTD_PRIV_UNIX_SOCKET) < 0) + if (!(sockname = remoteGetUNIXSocketRoot(flags))) goto failed; } -- 2.14.3

Prepare for reusing libvirtd source to create other daemons by making various bits conditionalized. This will avoid cut+paste of huge amount of source code for each new daemon. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/remote/Makefile.inc.am | 21 +++++ src/remote/remote_daemon.c | 160 +++++++++++++++++++++++--------------- src/remote/remote_daemon_config.c | 36 +++++++-- src/remote/remote_daemon_config.h | 9 ++- src/remote/remote_driver.h | 1 - 5 files changed, 153 insertions(+), 74 deletions(-) diff --git a/src/remote/Makefile.inc.am b/src/remote/Makefile.inc.am index 12600b8bb5..a6c60a35b3 100644 --- a/src/remote/Makefile.inc.am +++ b/src/remote/Makefile.inc.am @@ -140,6 +140,27 @@ libvirtd_CFLAGS = \ -I$(srcdir)/access \ -I$(srcdir)/conf \ -I$(srcdir)/rpc \ + -DSOCK_NAME="\"libvirt-sock\"" \ + -DSOCK_NAME_RO="\"libvirt-sock-ro\"" \ + -DSOCK_NAME_ADMIN="\"libvirt-admin-sock\"" \ + -DAPP_NAME="\"libvirtd\"" \ + -DLOAD_NETWORK \ + -DLOAD_INTERFACE \ + -DLOAD_STORAGE \ + -DLOAD_NODE_DEVICES \ + -DLOAD_SECRETS \ + -DLOAD_NWFILTER \ + -DLOAD_XEN \ + -DLOAD_LIBXL \ + -DLOAD_QEMU \ + -DLOAD_LXC \ + -DLOAD_UML \ + -DLOAD_VBOX \ + -DLOAD_BHYVE \ + -DLOAD_VZ \ + -DCONFIG_MIGRATE \ + -DWITH_NET_IP \ + -DREQUIRE_MODULE=false \ $(NULL) libvirtd_LDFLAGS = \ diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c index 27377fe3bc..0f53b7989d 100644 --- a/src/remote/remote_daemon.c +++ b/src/remote/remote_daemon.c @@ -221,19 +221,19 @@ daemonUnixSocketPaths(struct daemonConfig *config, char *rundir = NULL; if (config->unix_sock_dir) { - if (virAsprintf(sockfile, "%s/libvirt-sock", config->unix_sock_dir) < 0) + if (virAsprintf(sockfile, "%s/" SOCK_NAME, config->unix_sock_dir) < 0) goto cleanup; if (privileged) { - if (virAsprintf(rosockfile, "%s/libvirt-sock-ro", config->unix_sock_dir) < 0 || - virAsprintf(admsockfile, "%s/libvirt-admin-sock", config->unix_sock_dir) < 0) + if (virAsprintf(rosockfile, "%s/" SOCK_NAME_RO, config->unix_sock_dir) < 0 || + virAsprintf(admsockfile, "%s/" SOCK_NAME_ADMIN, config->unix_sock_dir) < 0) goto cleanup; } } else { if (privileged) { - if (VIR_STRDUP(*sockfile, LOCALSTATEDIR "/run/libvirt/libvirt-sock") < 0 || - VIR_STRDUP(*rosockfile, LOCALSTATEDIR "/run/libvirt/libvirt-sock-ro") < 0 || - VIR_STRDUP(*admsockfile, LOCALSTATEDIR "/run/libvirt/libvirt-admin-sock") < 0) + if (VIR_STRDUP(*sockfile, LOCALSTATEDIR "/run/libvirt/" SOCK_NAME) < 0 || + VIR_STRDUP(*rosockfile, LOCALSTATEDIR "/run/libvirt/" SOCK_NAME_RO) < 0 || + VIR_STRDUP(*admsockfile, LOCALSTATEDIR "/run/libvirt/" SOCK_NAME_ADMIN) < 0) goto cleanup; } else { mode_t old_umask; @@ -248,8 +248,8 @@ daemonUnixSocketPaths(struct daemonConfig *config, } umask(old_umask); - if (virAsprintf(sockfile, "%s/libvirt-sock", rundir) < 0 || - virAsprintf(admsockfile, "%s/libvirt-admin-sock", rundir) < 0) + if (virAsprintf(sockfile, "%s/" SOCK_NAME, rundir) < 0 || + virAsprintf(admsockfile, "%s/" SOCK_NAME_ADMIN, rundir) < 0) goto cleanup; } } @@ -305,56 +305,56 @@ static int daemonInitialize(void) * driver, since their resources must be auto-started before any * domains can be auto-started. */ -#ifdef WITH_NETWORK - if (virDriverLoadModule("network", "networkRegister", false) < 0) +#if defined(WITH_NETWORK) && defined(LOAD_NETWORK) + if (virDriverLoadModule("network", "networkRegister", REQUIRE_MODULE) < 0) return -1; #endif -#ifdef WITH_INTERFACE - if (virDriverLoadModule("interface", "interfaceRegister", false) < 0) +#if defined(WITH_INTERFACE) && defined(LOAD_INTERFACE) + if (virDriverLoadModule("interface", "interfaceRegister", REQUIRE_MODULE) < 0) return -1; #endif -#ifdef WITH_STORAGE - if (virDriverLoadModule("storage", "storageRegister", false) < 0) +#if defined(WITH_STORAGE) && defined(LOAD_STORAGE) + if (virDriverLoadModule("storage", "storageRegister", REQUIRE_MODULE) < 0) return -1; #endif -#ifdef WITH_NODE_DEVICES - if (virDriverLoadModule("nodedev", "nodedevRegister", false) < 0) +#if defined(WITH_NODE_DEVICES) && defined(LOAD_NODE_DEVICES) + if (virDriverLoadModule("nodedev", "nodedevRegister", REQUIRE_MODULE) < 0) return -1; #endif -#ifdef WITH_SECRETS - if (virDriverLoadModule("secret", "secretRegister", false) < 0) +#if defined(WITH_SECRETS) && defined(LOAD_SECRETS) + if (virDriverLoadModule("secret", "secretRegister", REQUIRE_MODULE) < 0) return -1; #endif -#ifdef WITH_NWFILTER - if (virDriverLoadModule("nwfilter", "nwfilterRegister", false) < 0) +#if defined(WITH_NWFILTER) && defined(LOAD_NWFILTER) + if (virDriverLoadModule("nwfilter", "nwfilterRegister", REQUIRE_MODULE) < 0) return -1; #endif -#ifdef WITH_LIBXL - if (virDriverLoadModule("libxl", "libxlRegister", false) < 0) +#if defined(WITH_LIBXL) && defined(LOAD_LIBXL) + if (virDriverLoadModule("libxl", "libxlRegister", REQUIRE_MODULE) < 0) return -1; #endif -#ifdef WITH_QEMU - if (virDriverLoadModule("qemu", "qemuRegister", false) < 0) +#if defined(WITH_QEMU) && defined(LOAD_QEMU) + if (virDriverLoadModule("qemu", "qemuRegister", REQUIRE_MODULE) < 0) return -1; #endif -#ifdef WITH_LXC - if (virDriverLoadModule("lxc", "lxcRegister", false) < 0) +#if defined(WITH_LXC) && defined(LOAD_LXC) + if (virDriverLoadModule("lxc", "lxcRegister", REQUIRE_MODULE) < 0) return -1; #endif -#ifdef WITH_UML - if (virDriverLoadModule("uml", "umlRegister", false) < 0) +#if defined(WITH_UML) && defined(LOAD_UML) + if (virDriverLoadModule("uml", "umlRegister", REQUIRE_MODULE) < 0) return -1; #endif -#ifdef WITH_VBOX - if (virDriverLoadModule("vbox", "vboxRegister", false) < 0) +#if defined(WITH_VBOX) && defined(LOAD_VBOX) + if (virDriverLoadModule("vbox", "vboxRegister", REQUIRE_MODULE) < 0) return -1; #endif -#ifdef WITH_BHYVE - if (virDriverLoadModule("bhyve", "bhyveRegister", false) < 0) +#if defined(WITH_BHYVE) && defined(LOAD_BHYVE) + if (virDriverLoadModule("bhyve", "bhyveRegister", REQUIRE_MODULE) < 0) return -1; #endif -#ifdef WITH_VZ - if (virDriverLoadModule("vz", "vzRegister", false) < 0) +#if defined(WITH_VZ) && defined(LOAD_VZ) + if (virDriverLoadModule("vz", "vzRegister", REQUIRE_MODULE) < 0) return -1; #endif return 0; @@ -368,15 +368,19 @@ daemonSetupNetworking(virNetServerPtr srv, const char *sock_path, const char *sock_path_ro, const char *sock_path_adm, +#ifdef WITH_NET_IP bool ipsock, - bool privileged) +#endif + bool privileged ATTRIBUTE_UNUSED) { virNetServerServicePtr svc = NULL; virNetServerServicePtr svcAdm = NULL; virNetServerServicePtr svcRO = NULL; +#ifdef WITH_NET_IP virNetServerServicePtr svcTCP = NULL; -#if WITH_GNUTLS +# if WITH_GNUTLS virNetServerServicePtr svcTLS = NULL; +# endif #endif gid_t unix_sock_gid = 0; int unix_sock_ro_mask = 0; @@ -440,8 +444,10 @@ daemonSetupNetworking(virNetServerPtr srv, } if (virNetServerAddService(srv, svc, +#ifdef WITH_NET_IP config->mdns_adv && !ipsock ? "_libvirt._tcp" : +#endif NULL) < 0) goto cleanup; @@ -467,6 +473,7 @@ daemonSetupNetworking(virNetServerPtr srv, goto cleanup; } +#ifdef WITH_NET_IP if (ipsock) { if (config->listen_tcp) { VIR_DEBUG("Registering TCP socket %s:%s", @@ -475,9 +482,9 @@ daemonSetupNetworking(virNetServerPtr srv, config->tcp_port, AF_UNSPEC, config->auth_tcp, -#if WITH_GNUTLS +# if WITH_GNUTLS NULL, -#endif +# endif false, config->max_queued_clients, config->max_client_requests))) @@ -488,7 +495,7 @@ daemonSetupNetworking(virNetServerPtr srv, goto cleanup; } -#if WITH_GNUTLS +# if WITH_GNUTLS if (config->listen_tls) { virNetTLSContextPtr ctxt = NULL; @@ -552,23 +559,26 @@ daemonSetupNetworking(virNetServerPtr srv, virObjectUnref(ctxt); } -#else +# else (void)privileged; if (config->listen_tls) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("This libvirtd build does not support TLS")); goto cleanup; } -#endif +# endif } +#endif #if WITH_SASL - if (config->auth_unix_rw == REMOTE_AUTH_SASL || - (sock_path_ro && config->auth_unix_ro == REMOTE_AUTH_SASL) || -# if WITH_GNUTLS - (ipsock && config->listen_tls && config->auth_tls == REMOTE_AUTH_SASL) || + if (config->auth_unix_rw == REMOTE_AUTH_SASL +# ifdef WITH_NET_IP +# if WITH_GNUTLS + || (ipsock && config->listen_tls && config->auth_tls == REMOTE_AUTH_SASL) +# endif + || (ipsock && config->listen_tcp && config->auth_tcp == REMOTE_AUTH_SASL) # endif - (ipsock && config->listen_tcp && config->auth_tcp == REMOTE_AUTH_SASL)) { + || (sock_path_ro && config->auth_unix_ro == REMOTE_AUTH_SASL)) { saslCtxt = virNetSASLContextNewServer( (const char *const*)config->sasl_allowed_username_list); if (!saslCtxt) @@ -579,10 +589,12 @@ daemonSetupNetworking(virNetServerPtr srv, ret = 0; cleanup: -#if WITH_GNUTLS +#ifdef WITH_NET_IP +# if WITH_GNUTLS virObjectUnref(svcTLS); -#endif +# endif virObjectUnref(svcTCP); +#endif virObjectUnref(svcRO); virObjectUnref(svcAdm); virObjectUnref(svc); @@ -643,7 +655,7 @@ daemonSetupLogging(struct daemonConfig *config, /* Define the default output. This is only applied if there was no setting * from either the config or the environment. */ - if (virLogSetDefaultOutput("libvirtd.log", godaemon, privileged) < 0) + if (virLogSetDefaultOutput(APP_NAME ".log", godaemon, privileged) < 0) return -1; if (virLogGetNbOutputs() == 0) @@ -872,6 +884,7 @@ static int daemonStateInit(virNetDaemonPtr dmn) return 0; } +#ifdef CONFIG_MIGRATE static int migrateProfile(void) { char *old_base = NULL; @@ -944,6 +957,7 @@ static int migrateProfile(void) return ret; } +#endif static int daemonSetupHostUUID(const struct daemonConfig *config) @@ -1006,46 +1020,55 @@ daemonUsage(const char *argv0, bool privileged) " Default paths:\n" "\n" " Configuration file (unless overridden by -f):\n" - " %s\n" + " %s.conf\n" "\n" " Sockets:\n" " %s\n" " %s\n" "\n" +#ifdef WITH_NET_IP " TLS:\n" " CA certificate: %s\n" " Server certificate: %s\n" " Server private key: %s\n" +#endif "\n" " PID file (unless overridden by -p):\n" - " %s/run/libvirtd.pid\n" + " %s/run/%s.pid\n" "\n"), - LIBVIRTD_CONFIGURATION_FILE, - LIBVIRTD_PRIV_UNIX_SOCKET, - LIBVIRTD_PRIV_UNIX_SOCKET_RO, + SYSCONFDIR "/libvirt/" APP_NAME, + LOCALSTATEDIR "/run/libvirt/" SOCK_NAME, + LOCALSTATEDIR "/run/libvirt/" SOCK_NAME_RO, +#ifdef WITH_NET_IP LIBVIRT_CACERT, LIBVIRT_SERVERCERT, LIBVIRT_SERVERKEY, - LOCALSTATEDIR); +#endif + LOCALSTATEDIR, APP_NAME); } else { - fprintf(stderr, "%s", + fprintf(stderr, _("\n" " Default paths:\n" "\n" " Configuration file (unless overridden by -f):\n" - " $XDG_CONFIG_HOME/libvirt/libvirtd.conf\n" + " $XDG_CONFIG_HOME/libvirt/%s.conf\n" "\n" " Sockets:\n" - " $XDG_RUNTIME_DIR/libvirt/libvirt-sock\n" + " $XDG_RUNTIME_DIR/libvirt/%s\n" "\n" +#ifdef WITH_NET_IP " TLS:\n" " CA certificate: $HOME/.pki/libvirt/cacert.pem\n" " Server certificate: $HOME/.pki/libvirt/servercert.pem\n" " Server private key: $HOME/.pki/libvirt/serverkey.pem\n" +#endif "\n" " PID file:\n" - " $XDG_RUNTIME_DIR/libvirt/libvirtd.pid\n" - "\n")); + " $XDG_RUNTIME_DIR/libvirt/%s.pid\n" + "\n"), + APP_NAME, + SOCK_NAME, + APP_NAME); } } @@ -1189,11 +1212,13 @@ int main(int argc, char **argv) { exit(EXIT_FAILURE); } +#ifdef CONFIG_MIGRATE if (!privileged && migrateProfile() < 0) { VIR_ERROR(_("Exiting due to failure to migrate profile")); exit(EXIT_FAILURE); } +#endif if (daemonSetupHostUUID(config) < 0) { VIR_ERROR(_("Can't setup host uuid")); @@ -1215,7 +1240,7 @@ int main(int argc, char **argv) { if (!pid_file && virPidFileConstructPath(privileged, LOCALSTATEDIR, - "libvirtd", + APP_NAME, &pid_file) < 0) { VIR_ERROR(_("Can't determine pid file path.")); exit(EXIT_FAILURE); @@ -1295,7 +1320,7 @@ int main(int argc, char **argv) { goto cleanup; } - if (!(srv = virNetServerNew("libvirtd", 1, + if (!(srv = virNetServerNew(APP_NAME, 1, config->min_workers, config->max_workers, config->prio_workers, @@ -1303,7 +1328,11 @@ int main(int argc, char **argv) { config->max_anonymous_clients, config->keepalive_interval, config->keepalive_count, +#ifdef WITH_NET_IP config->mdns_adv ? config->mdns_name : NULL, +#else + NULL, +#endif remoteClientNew, NULL, remoteClientFree, @@ -1318,7 +1347,7 @@ int main(int argc, char **argv) { } if (daemonInitialize() < 0) { - ret = VIR_DAEMON_ERR_INIT; + ret = VIR_DAEMON_ERR_DRIVER; goto cleanup; } @@ -1442,7 +1471,10 @@ int main(int argc, char **argv) { sock_file, sock_file_ro, sock_file_adm, - ipsock, privileged) < 0) { +#ifdef WITH_NET_IP + ipsock, +#endif + privileged) < 0) { ret = VIR_DAEMON_ERR_NETWORK; goto cleanup; } diff --git a/src/remote/remote_daemon_config.c b/src/remote/remote_daemon_config.c index b1516befb4..9759cd9438 100644 --- a/src/remote/remote_daemon_config.c +++ b/src/remote/remote_daemon_config.c @@ -79,7 +79,7 @@ int daemonConfigFilePath(bool privileged, char **configfile) { if (privileged) { - if (VIR_STRDUP(*configfile, SYSCONFDIR "/libvirt/libvirtd.conf") < 0) + if (VIR_STRDUP(*configfile, SYSCONFDIR "/libvirt/" APP_NAME ".conf") < 0) goto error; } else { char *configdir = NULL; @@ -87,7 +87,7 @@ daemonConfigFilePath(bool privileged, char **configfile) if (!(configdir = virGetUserConfigDirectory())) goto error; - if (virAsprintf(configfile, "%s/libvirtd.conf", configdir) < 0) { + if (virAsprintf(configfile, "%s/" APP_NAME ".conf", configdir) < 0) { VIR_FREE(configdir); goto error; } @@ -104,18 +104,22 @@ struct daemonConfig* daemonConfigNew(bool privileged ATTRIBUTE_UNUSED) { struct daemonConfig *data; +#ifdef WITH_NET_IP char *localhost; int ret; +#endif if (VIR_ALLOC(data) < 0) return NULL; +#ifdef WITH_NET_IP data->listen_tls = 1; data->listen_tcp = 0; if (VIR_STRDUP(data->tls_port, LIBVIRTD_TLS_PORT) < 0 || VIR_STRDUP(data->tcp_port, LIBVIRTD_TCP_PORT) < 0) goto error; +#endif /* Only default to PolicyKit if running as root */ #if WITH_POLKIT @@ -136,14 +140,16 @@ daemonConfigNew(bool privileged ATTRIBUTE_UNUSED) VIR_STRDUP(data->unix_sock_admin_perms, "0700") < 0) goto error; -#if WITH_SASL +#ifdef WITH_NET_IP +# if WITH_SASL data->auth_tcp = REMOTE_AUTH_SASL; -#else +# else data->auth_tcp = REMOTE_AUTH_NONE; -#endif +# endif data->auth_tls = REMOTE_AUTH_NONE; data->mdns_adv = 0; +#endif data->min_workers = 5; data->max_workers = 20; @@ -172,6 +178,7 @@ daemonConfigNew(bool privileged ATTRIBUTE_UNUSED) data->ovs_timeout = VIR_NETDEV_OVS_DEFAULT_TIMEOUT; +#ifdef WITH_NET_IP localhost = virGetHostname(); if (localhost == NULL) { /* we couldn't resolve the hostname; assume that we are @@ -190,6 +197,7 @@ daemonConfigNew(bool privileged ATTRIBUTE_UNUSED) VIR_FREE(localhost); if (ret < 0) goto error; +#endif return data; @@ -206,9 +214,11 @@ daemonConfigFree(struct daemonConfig *data) if (!data) return; +#ifdef WITH_NET_IP VIR_FREE(data->listen_addr); VIR_FREE(data->tls_port); VIR_FREE(data->tcp_port); +#endif tmp = data->access_drivers; while (tmp && *tmp) { VIR_FREE(*tmp); @@ -221,6 +231,7 @@ daemonConfigFree(struct daemonConfig *data) VIR_FREE(data->unix_sock_rw_perms); VIR_FREE(data->unix_sock_group); VIR_FREE(data->unix_sock_dir); +#ifdef WITH_NET_IP VIR_FREE(data->mdns_name); tmp = data->tls_allowed_dn_list; @@ -229,6 +240,7 @@ daemonConfigFree(struct daemonConfig *data) tmp++; } VIR_FREE(data->tls_allowed_dn_list); +#endif tmp = data->sasl_allowed_username_list; while (tmp && *tmp) { @@ -236,12 +248,15 @@ daemonConfigFree(struct daemonConfig *data) tmp++; } VIR_FREE(data->sasl_allowed_username_list); + +#ifdef WITH_NET_IP VIR_FREE(data->tls_priority); VIR_FREE(data->key_file); VIR_FREE(data->ca_file); VIR_FREE(data->cert_file); VIR_FREE(data->crl_file); +#endif VIR_FREE(data->host_uuid); VIR_FREE(data->host_uuid_source); @@ -256,6 +271,7 @@ daemonConfigLoadOptions(struct daemonConfig *data, const char *filename, virConfPtr conf) { +#ifdef WITH_NET_IP if (virConfGetValueBool(conf, "listen_tcp", &data->listen_tcp) < 0) goto error; if (virConfGetValueBool(conf, "listen_tls", &data->listen_tls) < 0) @@ -266,6 +282,7 @@ daemonConfigLoadOptions(struct daemonConfig *data, goto error; if (virConfGetValueString(conf, "listen_addr", &data->listen_addr) < 0) goto error; +#endif if (remoteConfigGetAuth(conf, filename, "auth_unix_rw", &data->auth_unix_rw) < 0) goto error; @@ -281,10 +298,12 @@ daemonConfigLoadOptions(struct daemonConfig *data, #endif if (remoteConfigGetAuth(conf, filename, "auth_unix_ro", &data->auth_unix_ro) < 0) goto error; +#ifdef WITH_NET_IP if (remoteConfigGetAuth(conf, filename, "auth_tcp", &data->auth_tcp) < 0) goto error; if (remoteConfigGetAuth(conf, filename, "auth_tls", &data->auth_tls) < 0) goto error; +#endif if (virConfGetValueStringList(conf, "access_drivers", false, &data->access_drivers) < 0) @@ -302,6 +321,7 @@ daemonConfigLoadOptions(struct daemonConfig *data, if (virConfGetValueString(conf, "unix_sock_dir", &data->unix_sock_dir) < 0) goto error; +#ifdef WITH_NET_IP if (virConfGetValueBool(conf, "mdns_adv", &data->mdns_adv) < 0) goto error; if (virConfGetValueString(conf, "mdns_name", &data->mdns_name) < 0) @@ -325,14 +345,14 @@ daemonConfigLoadOptions(struct daemonConfig *data, &data->tls_allowed_dn_list) < 0) goto error; + if (virConfGetValueString(conf, "tls_priority", &data->tls_priority) < 0) + goto error; +#endif if (virConfGetValueStringList(conf, "sasl_allowed_username_list", false, &data->sasl_allowed_username_list) < 0) goto error; - if (virConfGetValueString(conf, "tls_priority", &data->tls_priority) < 0) - goto error; - if (virConfGetValueUInt(conf, "min_workers", &data->min_workers) < 0) goto error; if (virConfGetValueUInt(conf, "max_workers", &data->max_workers) < 0) diff --git a/src/remote/remote_daemon_config.h b/src/remote/remote_daemon_config.h index 49ea80104b..86f78e756e 100644 --- a/src/remote/remote_daemon_config.h +++ b/src/remote/remote_daemon_config.h @@ -30,11 +30,13 @@ struct daemonConfig { char *host_uuid; char *host_uuid_source; +# ifdef WITH_NET_IP bool listen_tls; bool listen_tcp; char *listen_addr; char *tls_port; char *tcp_port; +# endif char *unix_sock_admin_perms; char *unix_sock_ro_perms; @@ -44,24 +46,29 @@ struct daemonConfig { int auth_unix_rw; int auth_unix_ro; +# ifdef WITH_NET_IP int auth_tcp; int auth_tls; +# endif char **access_drivers; +# ifdef WITH_NET_IP bool mdns_adv; char *mdns_name; bool tls_no_verify_certificate; bool tls_no_sanity_certificate; char **tls_allowed_dn_list; - char **sasl_allowed_username_list; char *tls_priority; char *key_file; char *cert_file; char *ca_file; char *crl_file; +# endif + + char **sasl_allowed_username_list; unsigned int min_workers; unsigned int max_workers; diff --git a/src/remote/remote_driver.h b/src/remote/remote_driver.h index 4033a3cd2c..ef53179833 100644 --- a/src/remote/remote_driver.h +++ b/src/remote/remote_driver.h @@ -37,7 +37,6 @@ unsigned long remoteVersion(void); # define LIBVIRTD_PRIV_UNIX_SOCKET LOCALSTATEDIR "/run/libvirt/libvirt-sock" # define LIBVIRTD_PRIV_UNIX_SOCKET_RO LOCALSTATEDIR "/run/libvirt/libvirt-sock-ro" # define LIBVIRTD_USER_UNIX_SOCKET "libvirt-sock" -# define LIBVIRTD_CONFIGURATION_FILE SYSCONFDIR "/libvirt/libvirtd.conf" /* Defaults for PKI directory. */ # define LIBVIRT_PKI_DIR SYSCONFDIR "/pki" -- 2.14.3

The virtsecretd daemon will be responsible for providing the secret API driver functionality. The secret driver is now disabled for the main libvirtd daemon. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/remote/Makefile.inc.am | 1 - src/secret/Makefile.inc.am | 54 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/src/remote/Makefile.inc.am b/src/remote/Makefile.inc.am index a6c60a35b3..0a041b5638 100644 --- a/src/remote/Makefile.inc.am +++ b/src/remote/Makefile.inc.am @@ -148,7 +148,6 @@ libvirtd_CFLAGS = \ -DLOAD_INTERFACE \ -DLOAD_STORAGE \ -DLOAD_NODE_DEVICES \ - -DLOAD_SECRETS \ -DLOAD_NWFILTER \ -DLOAD_XEN \ -DLOAD_LIBXL \ diff --git a/src/secret/Makefile.inc.am b/src/secret/Makefile.inc.am index 305c4a1ead..aa947e85e9 100644 --- a/src/secret/Makefile.inc.am +++ b/src/secret/Makefile.inc.am @@ -35,4 +35,58 @@ libvirt_driver_secret_la_LIBADD = \ $(NULL) libvirt_driver_secret_la_LDFLAGS = $(AM_LDFLAGS_MOD_NOUNDEF) libvirt_driver_secret_la_SOURCES = $(SECRET_DRIVER_SOURCES) + +sbin_PROGRAMS += virtsecretd + +virtsecretd_SOURCES = $(LIBVIRTD_SOURCES) + +virtsecretd_CFLAGS = \ + $(LIBXML_CFLAGS) \ + $(GNUTLS_CFLAGS) \ + $(SASL_CFLAGS) \ + $(XDR_CFLAGS) \ + $(DBUS_CFLAGS) \ + $(LIBNL_CFLAGS) \ + $(WARN_CFLAGS) \ + $(PIE_CFLAGS) \ + $(COVERAGE_CFLAGS) \ + -I$(srcdir)/access \ + -I$(srcdir)/conf \ + -I$(srcdir)/rpc \ + -DSOCK_NAME="\"virtsecretd-sock\"" \ + -DSOCK_NAME_RO="\"virtsecretd-sock-ro\"" \ + -DSOCK_NAME_ADMIN="\"virtsecretd-admin-sock\"" \ + -DAPP_NAME="\"virtsecretd\"" \ + -DLOAD_SECRETS \ + -DREQUIRE_MODULE=true \ + $(NULL) + +virtsecretd_LDFLAGS = \ + $(RELRO_LDFLAGS) \ + $(PIE_LDFLAGS) \ + $(COVERAGE_LDFLAGS) \ + $(NO_INDIRECT_LDFLAGS) \ + $(NO_UNDEFINED_LDFLAGS) \ + $(NULL) + +virtsecretd_LDADD = \ + libvirt_driver_admin.la \ + libvirt-lxc.la \ + libvirt-qemu.la \ + libvirt.la \ + $(LIBXML_LIBS) \ + $(GNUTLS_LIBS) \ + $(SASL_LIBS) \ + $(DBUS_LIBS) \ + $(LIBNL_LIBS) \ + $(NULL) + +if WITH_DTRACE_PROBES +virtsecretd_LDADD += ../src/libvirt_probes.lo +endif WITH_DTRACE_PROBES + +virtsecretd_LDADD += \ + ../gnulib/lib/libgnu.la \ + $(LIBSOCKET) \ + $(NULL) endif WITH_SECRETS -- 2.14.3