[libvirt] [PATCHv2 0/3] Fix two DBus related failures
Some places in the code assumed that either DBus was compiled in or running: 1: systemd cgroup code failed to start a VM on hosts without DBus installed or running 2: nwfilter driver failed to initialize on hosts without DBus running This series fixes those issues as we have fallbacks in case DBus isn't available that we can use. Peter Krempa (3): virdbus: Add virDBusHasSystemBus() virsystemd: Don't fail to start VM if DBus isn't available or compiled in nwfilter: Don't fail to start if DBus isn't available src/libvirt_private.syms | 1 + src/nwfilter/nwfilter_driver.c | 8 ++++++-- src/util/virdbus.c | 34 +++++++++++++++++++++++++++++++--- src/util/virdbus.h | 1 + src/util/virsystemd.c | 4 ++-- 5 files changed, 41 insertions(+), 7 deletions(-) -- 1.8.3.2
Some systems may not use DBus in their system. Add a method to check if the system bus is available that doesn't print error messages so that code can later check for this condition and use an alternative approach. --- src/libvirt_private.syms | 1 + src/util/virdbus.c | 34 +++++++++++++++++++++++++++++++--- src/util/virdbus.h | 1 + 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a958d94..c25a61f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1294,6 +1294,7 @@ virConfWriteMem; virDBusCallMethod; virDBusGetSessionBus; virDBusGetSystemBus; +virDBusHasSystemBus; virDBusMessageDecode; virDBusMessageEncode; virDBusMessageRead; diff --git a/src/util/virdbus.c b/src/util/virdbus.c index e88dc26..1ef78ce 100644 --- a/src/util/virdbus.c +++ b/src/util/virdbus.c @@ -73,7 +73,8 @@ static void virDBusSystemBusInit(void) systembus = virDBusBusInit(DBUS_BUS_SYSTEM, &systemdbuserr); } -DBusConnection *virDBusGetSystemBus(void) +static DBusConnection +*virDBusGetSystemBusInternal(void) { if (virOnce(&systemonce, virDBusSystemBusInit) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -81,14 +82,33 @@ DBusConnection *virDBusGetSystemBus(void) return NULL; } - if (!systembus) { + return systembus; +} + + +DBusConnection *virDBusGetSystemBus(void) +{ + DBusConnection *bus; + + if (!(bus = virDBusGetSystemBusInternal())) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to get DBus system bus connection: %s"), systemdbuserr.message ? systemdbuserr.message : "watch setup failed"); return NULL; } - return systembus; + return bus; +} + + +DBusConnection *virDBusHasSystemBus(void) +{ + DBusConnection *bus; + + if (!(bus = virDBusGetSystemBusInternal())) + VIR_DEBUG("System DBus not available: %s", NULLSTR(systemdbuserr.message)); + + return bus; } @@ -1195,6 +1215,14 @@ DBusConnection *virDBusGetSystemBus(void) return NULL; } + +DBusConnection *virDBusHasSystemBus(void) +{ + VIR_DEBUG("DBus support not compiled into this binary"); + return NULL; +} + + DBusConnection *virDBusGetSessionBus(void) { virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/util/virdbus.h b/src/util/virdbus.h index 39de479..63908fe 100644 --- a/src/util/virdbus.h +++ b/src/util/virdbus.h @@ -32,6 +32,7 @@ # include "internal.h" DBusConnection *virDBusGetSystemBus(void); +DBusConnection *virDBusHasSystemBus(void); DBusConnection *virDBusGetSessionBus(void); int virDBusCallMethod(DBusConnection *conn, -- 1.8.3.2
On Mon, Aug 19, 2013 at 12:32:05PM +0200, Peter Krempa wrote:
Some systems may not use DBus in their system. Add a method to check if the system bus is available that doesn't print error messages so that code can later check for this condition and use an alternative approach. --- src/libvirt_private.syms | 1 + src/util/virdbus.c | 34 +++++++++++++++++++++++++++++++--- src/util/virdbus.h | 1 + 3 files changed, 33 insertions(+), 3 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a958d94..c25a61f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1294,6 +1294,7 @@ virConfWriteMem; virDBusCallMethod; virDBusGetSessionBus; virDBusGetSystemBus; +virDBusHasSystemBus; virDBusMessageDecode; virDBusMessageEncode; virDBusMessageRead; diff --git a/src/util/virdbus.c b/src/util/virdbus.c index e88dc26..1ef78ce 100644 --- a/src/util/virdbus.c +++ b/src/util/virdbus.c @@ -73,7 +73,8 @@ static void virDBusSystemBusInit(void) systembus = virDBusBusInit(DBUS_BUS_SYSTEM, &systemdbuserr); }
-DBusConnection *virDBusGetSystemBus(void) +static DBusConnection +*virDBusGetSystemBusInternal(void) { if (virOnce(&systemonce, virDBusSystemBusInit) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -81,14 +82,33 @@ DBusConnection *virDBusGetSystemBus(void) return NULL; }
- if (!systembus) { + return systembus; +} + + +DBusConnection *virDBusGetSystemBus(void) +{ + DBusConnection *bus; + + if (!(bus = virDBusGetSystemBusInternal())) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to get DBus system bus connection: %s"), systemdbuserr.message ? systemdbuserr.message : "watch setup failed"); return NULL; }
- return systembus; + return bus; +} + + +DBusConnection *virDBusHasSystemBus(void) +{ + DBusConnection *bus; + + if (!(bus = virDBusGetSystemBusInternal())) + VIR_DEBUG("System DBus not available: %s", NULLSTR(systemdbuserr.message)); + + return bus; }
@@ -1195,6 +1215,14 @@ DBusConnection *virDBusGetSystemBus(void) return NULL; }
+ +DBusConnection *virDBusHasSystemBus(void) +{ + VIR_DEBUG("DBus support not compiled into this binary"); + return NULL; +} + + DBusConnection *virDBusGetSessionBus(void) { virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/util/virdbus.h b/src/util/virdbus.h index 39de479..63908fe 100644 --- a/src/util/virdbus.h +++ b/src/util/virdbus.h @@ -32,6 +32,7 @@ # include "internal.h"
DBusConnection *virDBusGetSystemBus(void); +DBusConnection *virDBusHasSystemBus(void);
This should be a returning a 'bool' IMHO. eg usage would be if (virDBusHasSystemBus()) { DBusConnection conn = virDBusGetSystemBus() ....dbus code path... } else { ... non-dbus code path... } 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 08/19/13 12:45, Daniel P. Berrange wrote:
On Mon, Aug 19, 2013 at 12:32:05PM +0200, Peter Krempa wrote:
Some systems may not use DBus in their system. Add a method to check if the system bus is available that doesn't print error messages so that code can later check for this condition and use an alternative approach. --- src/libvirt_private.syms | 1 + src/util/virdbus.c | 34 +++++++++++++++++++++++++++++++--- src/util/virdbus.h | 1 + 3 files changed, 33 insertions(+), 3 deletions(-)
....
DBusConnection *virDBusGetSystemBus(void); +DBusConnection *virDBusHasSystemBus(void);
This should be a returning a 'bool' IMHO. eg usage would be
if (virDBusHasSystemBus()) { DBusConnection conn = virDBusGetSystemBus() ....dbus code path... } else { ... non-dbus code path... }
That was my initial implementation but it resulted in a few more lines of code. I'll repost with that changed. Peter
On hosts that don't have the DBus service running or installed the new systemd cgroups code failed with hard error instead of falling back to "manual" cgroup creation. Use the new helper to check for the system bus and use the fallback code in case it isn't available. --- src/util/virsystemd.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c index 251b846..1c41ffc 100644 --- a/src/util/virsystemd.c +++ b/src/util/virsystemd.c @@ -145,8 +145,8 @@ int virSystemdCreateMachine(const char *name, char *username = NULL; char *slicename = NULL; - if (!(conn = virDBusGetSystemBus())) - return -1; + if (!(conn = virDBusHasSystemBus())) + return -2; if (privileged) { if (virAsprintf(&machinename, "%s-%s", drivername, name) < 0) -- 1.8.3.2
When the daemon is compiled with firewalld support but the DBus message bus isn't started in the system, the initialization of the nwfilter driver fails even if there are fallback options. --- src/nwfilter/nwfilter_driver.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index 9bb4b4e..ac28dc9 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -175,7 +175,7 @@ nwfilterStateInitialize(bool privileged, DBusConnection *sysbus = NULL; #if WITH_DBUS - sysbus = virDBusGetSystemBus(); + sysbus = virDBusHasSystemBus(); #endif /* WITH_DBUS */ if (VIR_ALLOC(driverState) < 0) @@ -184,6 +184,7 @@ nwfilterStateInitialize(bool privileged, if (virMutexInit(&driverState->lock) < 0) goto err_free_driverstate; + /* remember that we are going to use firewalld */ driverState->watchingFirewallD = (sysbus != NULL); driverState->privileged = privileged; @@ -208,7 +209,8 @@ nwfilterStateInitialize(bool privileged, * startup the DBus late so we don't get a reload signal while * initializing */ - if (nwfilterDriverInstallDBusMatches(sysbus) < 0) { + if (sysbus && + nwfilterDriverInstallDBusMatches(sysbus) < 0) { VIR_ERROR(_("DBus matches could not be installed. Disabling nwfilter " "driver")); /* @@ -216,6 +218,8 @@ nwfilterStateInitialize(bool privileged, * may have caused the ebiptables driver to use the firewall tool * but now that the watches don't work, we just disable the nwfilter * driver + * + * This may only happen if the system bus is available. */ goto error; } -- 1.8.3.2
participants (2)
-
Daniel P. Berrange -
Peter Krempa