[libvirt] [PATCHv3 0/3] Fix two issues when DBus isn't enabled or compiled in

Fix regression in starting VMs and inability to start libvirtd when firewalld support is compiled in. Diff to v2: virDBusHasSystemBus is now a bool function and adaptations to that. 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 | 9 +++++++-- src/util/virdbus.c | 33 ++++++++++++++++++++++++++++++--- src/util/virdbus.h | 1 + src/util/virsystemd.c | 6 ++++-- 5 files changed, 43 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 | 33 ++++++++++++++++++++++++++++++--- src/util/virdbus.h | 1 + 3 files changed, 32 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..1370a5d 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,32 @@ 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; +} + + +bool virDBusHasSystemBus(void) +{ + if (virDBusGetSystemBusInternal()) + return true; + + VIR_DEBUG("System DBus not available: %s", NULLSTR(systemdbuserr.message)); + return false; } @@ -1195,6 +1214,14 @@ DBusConnection *virDBusGetSystemBus(void) return NULL; } + +bool virDBusHasSystemBus(void) +{ + VIR_DEBUG("DBus support not compiled into this binary"); + return false; +} + + DBusConnection *virDBusGetSessionBus(void) { virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/util/virdbus.h b/src/util/virdbus.h index 39de479..a5aab56 100644 --- a/src/util/virdbus.h +++ b/src/util/virdbus.h @@ -32,6 +32,7 @@ # include "internal.h" DBusConnection *virDBusGetSystemBus(void); +bool virDBusHasSystemBus(void); DBusConnection *virDBusGetSessionBus(void); int virDBusCallMethod(DBusConnection *conn, -- 1.8.3.2

On 08/19/2013 06:58 AM, 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 | 33 ++++++++++++++++++++++++++++++--- src/util/virdbus.h | 1 + 3 files changed, 32 insertions(+), 3 deletions(-)
+++ 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)
Wrapped incorrectly. Should be static DBusConnection * virDBusGetSystemBusInternal(void) (The * is part of the return type, not part of the function name.)
+ + +bool virDBusHasSystemBus(void)
Odd to see wrapped and non-wrapped in the same patch. bool virDBusHasSystemBus(void) -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

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 | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c index 251b846..3e69ef6 100644 --- a/src/util/virsystemd.c +++ b/src/util/virsystemd.c @@ -145,8 +145,10 @@ int virSystemdCreateMachine(const char *name, char *username = NULL; char *slicename = NULL; - if (!(conn = virDBusGetSystemBus())) - return -1; + if (!virDBusHasSystemBus()) + return -2; + + conn = virDBusGetSystemBus(); 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 | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index 9bb4b4e..1ed28a2 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -175,7 +175,8 @@ nwfilterStateInitialize(bool privileged, DBusConnection *sysbus = NULL; #if WITH_DBUS - sysbus = virDBusGetSystemBus(); + if (virDBusHasSystemBus()) + sysbus = virDBusGetSystemBus(); #endif /* WITH_DBUS */ if (VIR_ALLOC(driverState) < 0) @@ -184,6 +185,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 +210,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 +219,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

On 08/19/2013 06:58 AM, Peter Krempa wrote:
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.
Does this mean we need to ensure a dependency between dbus and libvirtd, to make sure DBus is up before we try to use it? /me checks... [Unit] Description=Virtualization daemon Before=libvirt-guests.service After=network.target After=dbus.service After=iscsid.service Good - we already have it in libvirtd.service.in. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 08/19/2013 06:58 AM, Peter Krempa wrote:
Fix regression in starting VMs and inability to start libvirtd when firewalld support is compiled in.
Diff to v2: virDBusHasSystemBus is now a bool function and adaptations to that.
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
ACK series, if you fix the formatting nits in patch 1/3. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 08/19/13 15:35, Eric Blake wrote:
On 08/19/2013 06:58 AM, Peter Krempa wrote:
Fix regression in starting VMs and inability to start libvirtd when firewalld support is compiled in.
Diff to v2: virDBusHasSystemBus is now a bool function and adaptations to that.
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
ACK series, if you fix the formatting nits in patch 1/3.
I fixed the formatting and pushed the series. Thanks. Peter
participants (2)
-
Eric Blake
-
Peter Krempa