[libvirt] [PATCH v2] systemd: fix build without dbus

Daniel P. Berrange berrange at redhat.com
Mon Jan 19 13:36:12 UTC 2015


The virDBusMethodCall method has a DBusError as one of its
parameters. If the caller wants to pass a non-NULL value
for this, it immediately makes the calling code require
DBus at build time. This has led to breakage of non-DBus
builds several times. It is desirable that only the virdbus.c
file should need WITH_DBUS conditionals, so we must ideally
remove the DBusError parameter from the method.

We can't simply raise a libvirt error, since the whole point
of this parameter is to give the callers a way to check if
the error is one they want to ignore, without having the logs
polluted with an error message. So, we add a virErrorPtr
parameter which the caller can then either ignore or raise
using virSetError.

Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
---
 src/util/virdbus.c     | 31 +++++++++++++++++++------------
 src/util/virdbus.h     |  4 ++--
 src/util/virfirewall.c | 29 ++++++-----------------------
 src/util/virsystemd.c  | 15 +++++++--------
 4 files changed, 34 insertions(+), 45 deletions(-)

In v2:

 - Removed left over debug statements

diff --git a/src/util/virdbus.c b/src/util/virdbus.c
index d9665c1..3522ae0 100644
--- a/src/util/virdbus.c
+++ b/src/util/virdbus.c
@@ -1522,7 +1522,7 @@ static int
 virDBusCall(DBusConnection *conn,
             DBusMessage *call,
             DBusMessage **replyout,
-            DBusError *error)
+            virErrorPtr error)
 
 {
     DBusMessage *reply = NULL;
@@ -1530,8 +1530,9 @@ virDBusCall(DBusConnection *conn,
     int ret = -1;
     const char *iface, *member, *path, *dest;
 
-    if (!error)
-        dbus_error_init(&localerror);
+    dbus_error_init(&localerror);
+    if (error)
+        memset(error, 0, sizeof(*error));
 
     iface = dbus_message_get_interface(call);
     member = dbus_message_get_member(call);
@@ -1545,13 +1546,20 @@ virDBusCall(DBusConnection *conn,
     if (!(reply = dbus_connection_send_with_reply_and_block(conn,
                                                             call,
                                                             VIR_DBUS_METHOD_CALL_TIMEOUT_MILLIS,
-                                                            error ? error : &localerror))) {
+                                                            &localerror))) {
         PROBE(DBUS_METHOD_ERROR,
               "'%s.%s' on '%s' at '%s' error %s: %s",
               iface, member, path, dest,
-              error ? error->name : localerror.name,
-              error ? error->message : localerror.message);
+              localerror.name,
+              localerror.message);
         if (error) {
+            error->level = VIR_ERR_ERROR;
+            error->code = VIR_ERR_DBUS_SERVICE;
+            error->domain = VIR_FROM_DBUS;
+            if (VIR_STRDUP(error->message, localerror.message) < 0)
+                goto cleanup;
+            if (VIR_STRDUP(error->str1, localerror.name) < 0)
+                goto cleanup;
             ret = 0;
         } else {
             virReportError(VIR_ERR_DBUS_SERVICE, _("%s: %s"), member,
@@ -1567,8 +1575,9 @@ virDBusCall(DBusConnection *conn,
     ret = 0;
 
  cleanup:
-    if (!error)
-        dbus_error_free(&localerror);
+    if (ret < 0 && error)
+        virResetError(error);
+    dbus_error_free(&localerror);
     if (reply) {
         if (ret == 0 && replyout)
             *replyout = reply;
@@ -1616,7 +1625,7 @@ virDBusCall(DBusConnection *conn,
  */
 int virDBusCallMethod(DBusConnection *conn,
                       DBusMessage **replyout,
-                      DBusError *error,
+                      virErrorPtr error,
                       const char *destination,
                       const char *path,
                       const char *iface,
@@ -1634,8 +1643,6 @@ int virDBusCallMethod(DBusConnection *conn,
     if (ret < 0)
         goto cleanup;
 
-    ret = -1;
-
     ret = virDBusCall(conn, call, replyout, error);
 
  cleanup:
@@ -1832,7 +1839,7 @@ int virDBusCreateReply(DBusMessage **reply ATTRIBUTE_UNUSED,
 
 int virDBusCallMethod(DBusConnection *conn ATTRIBUTE_UNUSED,
                       DBusMessage **reply ATTRIBUTE_UNUSED,
-                      DBusError *error ATTRIBUTE_UNUSED,
+                      virErrorPtr error ATTRIBUTE_UNUSED,
                       const char *destination ATTRIBUTE_UNUSED,
                       const char *path ATTRIBUTE_UNUSED,
                       const char *iface ATTRIBUTE_UNUSED,
diff --git a/src/util/virdbus.h b/src/util/virdbus.h
index d0c7de2..e2b8d2b 100644
--- a/src/util/virdbus.h
+++ b/src/util/virdbus.h
@@ -28,7 +28,7 @@
 # else
 #  define DBusConnection void
 #  define DBusMessage void
-#  define DBusError void
+#  define dbus_message_unref(m) do {} while (0)
 # endif
 # include "internal.h"
 
@@ -62,7 +62,7 @@ int virDBusCreateReplyV(DBusMessage **reply,
 
 int virDBusCallMethod(DBusConnection *conn,
                       DBusMessage **reply,
-                      DBusError *error,
+                      virErrorPtr error,
                       const char *destination,
                       const char *path,
                       const char *iface,
diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index b536912..2fa7f65 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -156,7 +156,6 @@ static int
 virFirewallValidateBackend(virFirewallBackend backend)
 {
     VIR_DEBUG("Validating backend %d", backend);
-#if WITH_DBUS
     if (backend == VIR_FIREWALL_BACKEND_AUTOMATIC ||
         backend == VIR_FIREWALL_BACKEND_FIREWALLD) {
         int rv = virDBusIsServiceRegistered(VIR_FIREWALL_FIREWALLD_SERVICE);
@@ -180,16 +179,6 @@ virFirewallValidateBackend(virFirewallBackend backend)
             backend = VIR_FIREWALL_BACKEND_FIREWALLD;
         }
     }
-#else
-    if (backend == VIR_FIREWALL_BACKEND_AUTOMATIC) {
-        VIR_DEBUG("DBus support disabled, trying direct backend");
-        backend = VIR_FIREWALL_BACKEND_DIRECT;
-    } else if (backend == VIR_FIREWALL_BACKEND_FIREWALLD) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("firewalld firewall backend requested, but DBus support disabled"));
-        return -1;
-    }
-#endif
 
     if (backend == VIR_FIREWALL_BACKEND_DIRECT) {
         const char *commands[] = {
@@ -755,7 +744,6 @@ virFirewallApplyRuleDirect(virFirewallRulePtr rule,
 }
 
 
-#ifdef WITH_DBUS
 static int
 virFirewallApplyRuleFirewallD(virFirewallRulePtr rule,
                               bool ignoreErrors,
@@ -764,13 +752,13 @@ virFirewallApplyRuleFirewallD(virFirewallRulePtr rule,
     const char *ipv = virFirewallLayerFirewallDTypeToString(rule->layer);
     DBusConnection *sysbus = virDBusGetSystemBus();
     DBusMessage *reply = NULL;
-    DBusError error;
+    virError error;
     int ret = -1;
 
     if (!sysbus)
         return -1;
 
-    dbus_error_init(&error);
+    memset(&error, 0, sizeof(error));
 
     if (!ipv) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -792,7 +780,7 @@ virFirewallApplyRuleFirewallD(virFirewallRulePtr rule,
                           rule->args) < 0)
         goto cleanup;
 
-    if (dbus_error_is_set(&error)) {
+    if (error.level == VIR_ERR_ERROR) {
         /*
          * As of firewalld-0.3.9.3-1.fc20.noarch the name and
          * message fields in the error look like
@@ -820,11 +808,9 @@ virFirewallApplyRuleFirewallD(virFirewallRulePtr rule,
          */
         if (ignoreErrors) {
             VIR_DEBUG("Ignoring error '%s': '%s'",
-                      error.name, error.message);
+                      error.str1, error.message);
         } else {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("Unable to apply rule '%s'"),
-                           error.message);
+            virSetError(&error);
             goto cleanup;
         }
     } else {
@@ -835,12 +821,11 @@ virFirewallApplyRuleFirewallD(virFirewallRulePtr rule,
     ret = 0;
 
  cleanup:
-    dbus_error_free(&error);
+    virResetError(&error);
     if (reply)
         dbus_message_unref(reply);
     return ret;
 }
-#endif
 
 static int
 virFirewallApplyRule(virFirewallPtr firewall,
@@ -862,12 +847,10 @@ virFirewallApplyRule(virFirewallPtr firewall,
         if (virFirewallApplyRuleDirect(rule, ignoreErrors, &output) < 0)
             return -1;
         break;
-#if WITH_DBUS
     case VIR_FIREWALL_BACKEND_FIREWALLD:
         if (virFirewallApplyRuleFirewallD(rule, ignoreErrors, &output) < 0)
             return -1;
         break;
-#endif
     default:
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        _("Unexpected firewall engine backend"));
diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c
index 3eea5c2..0b71b26 100644
--- a/src/util/virsystemd.c
+++ b/src/util/virsystemd.c
@@ -252,8 +252,8 @@ int virSystemdCreateMachine(const char *name,
 
     VIR_DEBUG("Attempting to create machine via systemd");
     if (virAtomicIntGet(&hasCreateWithNetwork)) {
-        DBusError error;
-        dbus_error_init(&error);
+        virError error;
+        memset(&error, 0, sizeof(error));
 
         if (virDBusCallMethod(conn,
                               NULL,
@@ -280,21 +280,20 @@ int virSystemdCreateMachine(const char *name,
                               "Before", "as", 1, "libvirt-guests.service") < 0)
             goto cleanup;
 
-        if (dbus_error_is_set(&error)) {
+        if (error.level == VIR_ERR_ERROR) {
             if (STREQ_NULLABLE("org.freedesktop.DBus.Error.UnknownMethod",
-                               error.name)) {
+                               error.str1)) {
                 VIR_INFO("CreateMachineWithNetwork isn't supported, switching "
                          "to legacy CreateMachine method for systemd-machined");
-                dbus_error_free(&error);
+                virResetError(&error);
                 virAtomicIntSet(&hasCreateWithNetwork, 0);
                 /* Could re-structure without Using goto, but this
                  * avoids another atomic read which would trigger
                  * another memory barrier */
                 goto fallback;
             }
-            virReportError(VIR_ERR_DBUS_SERVICE,
-                           _("CreateMachineWithNetwork: %s"),
-                           error.message ? error.message : _("unknown error"));
+            virSetError(&error);
+            virResetError(&error);
             goto cleanup;
         }
     } else {
-- 
2.1.0




More information about the libvir-list mailing list