2:36 p.m.
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(a)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