diff --git a/po/POTFILES.in b/po/POTFILES.in index 26c098f18c..3064037fcc 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -217,7 +217,6 @@ src/util/virstorageencryption.c src/util/virstoragefile.c src/util/virstring.c src/util/virsysinfo.c -src/util/virsystemd.c src/util/virerror.c src/util/virerror.h src/util/virtime.c diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a2eec83253..528e93c20e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1284,6 +1284,7 @@ virErrorInitialize; virErrorSetErrnoFromLastError; virLastErrorIsSystemErrno; virRaiseErrorFull; +virRaiseErrorObject; virReportErrorHelper; virReportOOMErrorFull; virReportSystemErrorFull; diff --git a/src/util/virdbus.c b/src/util/virdbus.c index d9665c1f7f..3522ae02e2 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 d0c7de29dc..e2b8d2b1bc 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/virerror.c b/src/util/virerror.c index f5d7f54fd8..9635c8212f 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -618,6 +618,39 @@ virDispatchError(virConnectPtr conn) } +/* + * Reports an error through the logging subsystem + */ +static +void virRaiseErrorLog(const char *filename, + const char *funcname, + size_t linenr, + virErrorPtr err, + virLogMetadata *meta) +{ + int priority; + + /* + * Hook up the error or warning to the logging facility + */ + priority = virErrorLevelPriority(err->level); + if (virErrorLogPriorityFilter) + priority = virErrorLogPriorityFilter(err, priority); + + /* We don't want to pollute stderr if no logging outputs + * are explicitly requested by the user, since the default + * error function already pollutes stderr and most apps + * hate & thus disable that too. If the daemon has set + * a priority filter though, we should always forward + * all errors to the logging code. + */ + if (virLogGetNbOutputs() > 0 || + virErrorLogPriorityFilter) + virLogMessage(&virLogSelf, + priority, + filename, linenr, funcname, + meta, "%s", err->message); +} /** * virRaiseErrorFull: @@ -639,7 +672,7 @@ virDispatchError(virConnectPtr conn) * immediately if a callback is found and store it for later handling. */ void -virRaiseErrorFull(const char *filename ATTRIBUTE_UNUSED, +virRaiseErrorFull(const char *filename, const char *funcname, size_t linenr, int domain, @@ -655,7 +688,6 @@ virRaiseErrorFull(const char *filename ATTRIBUTE_UNUSED, int save_errno = errno; virErrorPtr to; char *str; - int priority; virLogMetadata meta[] = { { .key = "LIBVIRT_DOMAIN", .s = NULL, .iv = domain }, { .key = "LIBVIRT_CODE", .s = NULL, .iv = code }, @@ -709,30 +741,58 @@ virRaiseErrorFull(const char *filename ATTRIBUTE_UNUSED, to->int1 = int1; to->int2 = int2; - /* - * Hook up the error or warning to the logging facility - */ - priority = virErrorLevelPriority(level); - if (virErrorLogPriorityFilter) - priority = virErrorLogPriorityFilter(to, priority); - - /* We don't want to pollute stderr if no logging outputs - * are explicitly requested by the user, since the default - * error function already pollutes stderr and most apps - * hate & thus disable that too. If the daemon has set - * a priority filter though, we should always forward - * all errors to the logging code. - */ - if (virLogGetNbOutputs() > 0 || - virErrorLogPriorityFilter) - virLogMessage(&virLogSelf, - priority, - filename, linenr, funcname, - meta, "%s", str); + virRaiseErrorLog(filename, funcname, linenr, + to, meta); errno = save_errno; } + +/** + * virRaiseErrorObject: + * @filename: filename where error was raised + * @funcname: function name where error was raised + * @linenr: line number where error was raised + * @newerr: the error object to report + * + * Sets the thread local error object to be a copy of + * @newerr and logs the error + * + * This is like virRaiseErrorFull, except that it accepts the + * error information via a pre-filled virErrorPtr object + * + * This is like virSetError, except that it will trigger the + * logging callbacks. + * + * The caller must clear the @newerr instance afterwards, since + * it will be copied into the thread local error. + */ +void virRaiseErrorObject(const char *filename, + const char *funcname, + size_t linenr, + virErrorPtr newerr) +{ + int saved_errno = errno; + virErrorPtr err; + virLogMetadata meta[] = { + { .key = "LIBVIRT_DOMAIN", .s = NULL, .iv = newerr->domain }, + { .key = "LIBVIRT_CODE", .s = NULL, .iv = newerr->code }, + { .key = NULL }, + }; + + err = virLastErrorObject(); + if (!err) + goto cleanup; + + virResetError(err); + virCopyError(newerr, err); + virRaiseErrorLog(filename, funcname, linenr, + err, meta); + cleanup: + errno = saved_errno; +} + + /** * virErrorMsg: * @error: the virErrorNumber diff --git a/src/util/virerror.h b/src/util/virerror.h index 5c8578f791..ad3a9464ae 100644 --- a/src/util/virerror.h +++ b/src/util/virerror.h @@ -47,6 +47,11 @@ void virRaiseErrorFull(const char *filename, const char *fmt, ...) ATTRIBUTE_FMT_PRINTF(12, 13); +void virRaiseErrorObject(const char *filename, + const char *funcname, + size_t linenr, + virErrorPtr err); + void virReportErrorHelper(int domcode, int errcode, const char *filename, const char *funcname, @@ -165,6 +170,9 @@ void virReportOOMErrorFull(int domcode, virReportErrorHelper(VIR_FROM_THIS, code, __FILE__, \ __FUNCTION__, __LINE__, __VA_ARGS__) +# define virReportErrorObject(obj) \ + virRaiseErrorObject(__FILE__, __FUNCTION__, __LINE__, obj) + int virSetError(virErrorPtr newerr); void virDispatchError(virConnectPtr conn); const char *virStrerror(int theerrno, char *errBuf, size_t errBufLen); diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c index b536912767..cd7afa53bc 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); + virReportErrorObject(&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 3eea5c2700..6de265be59 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")); + virReportErrorObject(&error); + virResetError(&error); goto cleanup; } } else {