[libvirt] [PATCH 1/5] virerror: Fix an error message typo
--- src/util/virerror.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virerror.h b/src/util/virerror.h index 2de04f4..fe0e15e 100644 --- a/src/util/virerror.h +++ b/src/util/virerror.h @@ -99,7 +99,7 @@ void virReportSystemErrorFull(int domcode, #argname, \ NULL, \ 0, 0, \ - _("%s in %s must greater than zero"), \ + _("%s in %s must be greater than zero"), \ #argname, __FUNCTION__) # define virReportInvalidNonZeroArg(argname) \ virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__, \ -- 1.9.0
RaiseErrorFull does not prepend the static error code string (like INVALID_ARG yields "invalid arg: %(msg)s"). We should be using ReportErrorHelper. The generated error objects are slightly different, by not storing the invalid argument name in err->str2. However those fields aren't used anywhere else and aren't documented to contain anything useful, so I don't think it matters. --- src/util/virerror.h | 116 +++++++++++++++++----------------------------------- 1 file changed, 38 insertions(+), 78 deletions(-) diff --git a/src/util/virerror.h b/src/util/virerror.h index fe0e15e..872c270 100644 --- a/src/util/virerror.h +++ b/src/util/virerror.h @@ -69,92 +69,52 @@ void virReportSystemErrorFull(int domcode, (fmt), __VA_ARGS__) # define virReportInvalidNullArg(argname) \ - virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__, \ - VIR_FROM_THIS, \ - VIR_ERR_INVALID_ARG, \ - VIR_ERR_ERROR, \ - __FUNCTION__, \ - #argname, \ - NULL, \ - 0, 0, \ - _("%s in %s must be NULL"), \ - #argname, __FUNCTION__) + virReportErrorHelper(VIR_FROM_THIS, \ + VIR_ERR_INVALID_ARG, \ + __FILE__, __FUNCTION__, __LINE__, \ + _("%s in %s must be NULL"), \ + #argname, __FUNCTION__) # define virReportInvalidNonNullArg(argname) \ - virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__, \ - VIR_FROM_THIS, \ - VIR_ERR_INVALID_ARG, \ - VIR_ERR_ERROR, \ - __FUNCTION__, \ - #argname, \ - NULL, \ - 0, 0, \ - _("%s in %s must not be NULL"), \ - #argname, __FUNCTION__) + virReportErrorHelper(VIR_FROM_THIS, \ + VIR_ERR_INVALID_ARG, \ + __FILE__, __FUNCTION__, __LINE__, \ + _("%s in %s must not be NULL"), \ + #argname, __FUNCTION__) # define virReportInvalidPositiveArg(argname) \ - virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__, \ - VIR_FROM_THIS, \ - VIR_ERR_INVALID_ARG, \ - VIR_ERR_ERROR, \ - __FUNCTION__, \ - #argname, \ - NULL, \ - 0, 0, \ - _("%s in %s must be greater than zero"), \ - #argname, __FUNCTION__) + virReportErrorHelper(VIR_FROM_THIS, \ + VIR_ERR_INVALID_ARG, \ + __FILE__, __FUNCTION__, __LINE__, \ + _("%s in %s must be greater than zero"), \ + #argname, __FUNCTION__) # define virReportInvalidNonZeroArg(argname) \ - virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__, \ - VIR_FROM_THIS, \ - VIR_ERR_INVALID_ARG, \ - VIR_ERR_ERROR, \ - __FUNCTION__, \ - #argname, \ - NULL, \ - 0, 0, \ - _("%s in %s must not be zero"), \ - #argname, __FUNCTION__) + virReportErrorHelper(VIR_FROM_THIS, \ + VIR_ERR_INVALID_ARG, \ + __FILE__, __FUNCTION__, __LINE__, \ + _("%s in %s must not be zero"), \ + #argname, __FUNCTION__) # define virReportInvalidZeroArg(argname) \ - virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__, \ - VIR_FROM_THIS, \ - VIR_ERR_INVALID_ARG, \ - VIR_ERR_ERROR, \ - __FUNCTION__, \ - #argname, \ - NULL, \ - 0, 0, \ - _("%s in %s must be zero"), \ - #argname, __FUNCTION__) + virReportErrorHelper(VIR_FROM_THIS, \ + VIR_ERR_INVALID_ARG, \ + __FILE__, __FUNCTION__, __LINE__, \ + _("%s in %s must be zero"), \ + #argname, __FUNCTION__) # define virReportInvalidNonNegativeArg(argname) \ - virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__, \ - VIR_FROM_THIS, \ - VIR_ERR_INVALID_ARG, \ - VIR_ERR_ERROR, \ - __FUNCTION__, \ - #argname, \ - NULL, \ - 0, 0, \ - _("%s in %s must be zero or greater"), \ - #argname, __FUNCTION__) + virReportErrorHelper(VIR_FROM_THIS, \ + VIR_ERR_INVALID_ARG, \ + __FILE__, __FUNCTION__, __LINE__, \ + _("%s in %s must be zero or greater"), \ + #argname, __FUNCTION__) # define virReportInvalidArg(argname, fmt, ...) \ - virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__, \ - VIR_FROM_THIS, \ - VIR_ERR_INVALID_ARG, \ - VIR_ERR_ERROR, \ - __FUNCTION__, \ - #argname, \ - NULL, \ - 0, 0, \ - (fmt), __VA_ARGS__) + virReportErrorHelper(VIR_FROM_THIS, \ + VIR_ERR_INVALID_ARG, \ + __FILE__, __FUNCTION__, __LINE__, \ + (fmt), __VA_ARGS__) # define virReportDBusServiceError(message, name) \ - virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__, \ - VIR_FROM_THIS, \ - VIR_ERR_DBUS_SERVICE, \ - VIR_ERR_ERROR, \ - __FUNCTION__, \ - name, \ - NULL, \ - 0, 0, \ - "%s", message); + virReportErrorHelper(VIR_FROM_THIS, \ + VIR_ERR_DBUS_SERVICE, \ + __FILE__, __FUNCTION__, __LINE__, \ + "%s", message) # define virReportUnsupportedError() \ virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_NO_SUPPORT, \ -- 1.9.0
On Sat, May 03, 2014 at 03:59:39PM -0400, Cole Robinson wrote:
RaiseErrorFull does not prepend the static error code string (like INVALID_ARG yields "invalid arg: %(msg)s"). We should be using ReportErrorHelper.
The generated error objects are slightly different, by not storing the invalid argument name in err->str2. However those fields aren't used anywhere else and aren't documented to contain anything useful, so I don't think it matters. --- src/util/virerror.h | 116 +++++++++++++++++----------------------------------- 1 file changed, 38 insertions(+), 78 deletions(-)
diff --git a/src/util/virerror.h b/src/util/virerror.h index fe0e15e..872c270 100644 --- a/src/util/virerror.h +++ b/src/util/virerror.h @@ -69,92 +69,52 @@ void virReportSystemErrorFull(int domcode, (fmt), __VA_ARGS__)
# define virReportInvalidNullArg(argname) \ - virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__, \ - VIR_FROM_THIS, \ - VIR_ERR_INVALID_ARG, \ - VIR_ERR_ERROR, \ - __FUNCTION__, \ - #argname, \ - NULL, \ - 0, 0, \ - _("%s in %s must be NULL"), \ - #argname, __FUNCTION__) + virReportErrorHelper(VIR_FROM_THIS, \ + VIR_ERR_INVALID_ARG, \ + __FILE__, __FUNCTION__, __LINE__, \ + _("%s in %s must be NULL"), \ + #argname, __FUNCTION__) # define virReportInvalidNonNullArg(argname) \ - virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__, \ - VIR_FROM_THIS, \ - VIR_ERR_INVALID_ARG, \ - VIR_ERR_ERROR, \ - __FUNCTION__, \ - #argname, \ - NULL, \ - 0, 0, \ - _("%s in %s must not be NULL"), \ - #argname, __FUNCTION__) + virReportErrorHelper(VIR_FROM_THIS, \ + VIR_ERR_INVALID_ARG, \ + __FILE__, __FUNCTION__, __LINE__, \ + _("%s in %s must not be NULL"), \ + #argname, __FUNCTION__) # define virReportInvalidPositiveArg(argname) \ - virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__, \ - VIR_FROM_THIS, \ - VIR_ERR_INVALID_ARG, \ - VIR_ERR_ERROR, \ - __FUNCTION__, \ - #argname, \ - NULL, \ - 0, 0, \ - _("%s in %s must be greater than zero"), \ - #argname, __FUNCTION__) + virReportErrorHelper(VIR_FROM_THIS, \ + VIR_ERR_INVALID_ARG, \ + __FILE__, __FUNCTION__, __LINE__, \ + _("%s in %s must be greater than zero"), \ + #argname, __FUNCTION__) # define virReportInvalidNonZeroArg(argname) \ - virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__, \ - VIR_FROM_THIS, \ - VIR_ERR_INVALID_ARG, \ - VIR_ERR_ERROR, \ - __FUNCTION__, \ - #argname, \ - NULL, \ - 0, 0, \ - _("%s in %s must not be zero"), \ - #argname, __FUNCTION__) + virReportErrorHelper(VIR_FROM_THIS, \ + VIR_ERR_INVALID_ARG, \ + __FILE__, __FUNCTION__, __LINE__, \ + _("%s in %s must not be zero"), \ + #argname, __FUNCTION__) # define virReportInvalidZeroArg(argname) \ - virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__, \ - VIR_FROM_THIS, \ - VIR_ERR_INVALID_ARG, \ - VIR_ERR_ERROR, \ - __FUNCTION__, \ - #argname, \ - NULL, \ - 0, 0, \ - _("%s in %s must be zero"), \ - #argname, __FUNCTION__) + virReportErrorHelper(VIR_FROM_THIS, \ + VIR_ERR_INVALID_ARG, \ + __FILE__, __FUNCTION__, __LINE__, \ + _("%s in %s must be zero"), \ + #argname, __FUNCTION__) # define virReportInvalidNonNegativeArg(argname) \ - virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__, \ - VIR_FROM_THIS, \ - VIR_ERR_INVALID_ARG, \ - VIR_ERR_ERROR, \ - __FUNCTION__, \ - #argname, \ - NULL, \ - 0, 0, \ - _("%s in %s must be zero or greater"), \ - #argname, __FUNCTION__) + virReportErrorHelper(VIR_FROM_THIS, \ + VIR_ERR_INVALID_ARG, \ + __FILE__, __FUNCTION__, __LINE__, \ + _("%s in %s must be zero or greater"), \ + #argname, __FUNCTION__) # define virReportInvalidArg(argname, fmt, ...) \ - virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__, \ - VIR_FROM_THIS, \ - VIR_ERR_INVALID_ARG, \ - VIR_ERR_ERROR, \ - __FUNCTION__, \ - #argname, \ - NULL, \ - 0, 0, \ - (fmt), __VA_ARGS__) + virReportErrorHelper(VIR_FROM_THIS, \ + VIR_ERR_INVALID_ARG, \ + __FILE__, __FUNCTION__, __LINE__, \ + (fmt), __VA_ARGS__)
# define virReportDBusServiceError(message, name) \ - virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__, \ - VIR_FROM_THIS, \ - VIR_ERR_DBUS_SERVICE, \ - VIR_ERR_ERROR, \ - __FUNCTION__, \ - name, \ - NULL, \ - 0, 0, \ - "%s", message); + virReportErrorHelper(VIR_FROM_THIS, \ + VIR_ERR_DBUS_SERVICE, \ + __FILE__, __FUNCTION__, __LINE__, \ + "%s", message)
# define virReportUnsupportedError() \ virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_NO_SUPPORT, \ --
I do loke this better but might not grasp all implications. -- Guido
On 05/03/2014 01:59 PM, Cole Robinson wrote:
RaiseErrorFull does not prepend the static error code string (like INVALID_ARG yields "invalid arg: %(msg)s"). We should be using ReportErrorHelper.
I'd get Dan's opinion on this one, since he first introduced the macros in commit d91f3ef and may have had a reason not mentioned in that commit message for preferring one form over the other.
The generated error objects are slightly different, by not storing the invalid argument name in err->str2. However those fields aren't used anywhere else and aren't documented to contain anything useful, so I don't think it matters.
I think we haven't documented what the various fields of virError contain precisely because documenting it would make it ABI, but the whole object is already a nasty back-compat hack to pass over the wire. The argument that it is undocumented is indeed the best supporting argument that you can offer that making this change isn't likely to break anything.
# define virReportInvalidNullArg(argname) \ - virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__, \ - VIR_FROM_THIS, \ - VIR_ERR_INVALID_ARG, \ - VIR_ERR_ERROR, \ - __FUNCTION__, \ - #argname, \ - NULL, \ - 0, 0, \ - _("%s in %s must be NULL"), \ - #argname, __FUNCTION__) + virReportErrorHelper(VIR_FROM_THIS, \ + VIR_ERR_INVALID_ARG, \ + __FILE__, __FUNCTION__, __LINE__, \ + _("%s in %s must be NULL"), \ + #argname, __FUNCTION__)
The transformation looks sane to me, but I'm reluctant to give ACK without Dan weighing in on it. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
--- src/libvirt_private.syms | 1 - src/util/virdbus.c | 19 +++++-------------- src/util/virdbus.h | 4 ---- 3 files changed, 5 insertions(+), 19 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index cc3ae2c..8a015eb 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1148,7 +1148,6 @@ virCryptoHashString; # util/virdbus.h -virDBusCall; virDBusCallMethod; virDBusCloseSystemBus; virDBusCreateMethod; diff --git a/src/util/virdbus.c b/src/util/virdbus.c index 4217aea..709d6ee 100644 --- a/src/util/virdbus.c +++ b/src/util/virdbus.c @@ -1404,10 +1404,11 @@ int virDBusCreateReply(DBusMessage **reply, * * Returns 0 on success, or -1 upon error */ -int virDBusCall(DBusConnection *conn, - DBusMessage *call, - DBusMessage **replyout, - DBusError *error) +static int +virDBusCall(DBusConnection *conn, + DBusMessage *call, + DBusMessage **replyout, + DBusError *error) { DBusMessage *reply = NULL; DBusError localerror; @@ -1694,16 +1695,6 @@ int virDBusCreateReply(DBusMessage **reply ATTRIBUTE_UNUSED, return -1; } -int virDBusCall(DBusConnection *conn ATTRIBUTE_UNUSED, - DBusMessage *call ATTRIBUTE_UNUSED, - DBusMessage **reply ATTRIBUTE_UNUSED, - DBusError *error ATTRIBUTE_UNUSED) -{ - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("DBus support not compiled into this binary")); - return -1; -} - int virDBusCallMethod(DBusConnection *conn ATTRIBUTE_UNUSED, DBusMessage **reply ATTRIBUTE_UNUSED, DBusError *error ATTRIBUTE_UNUSED, diff --git a/src/util/virdbus.h b/src/util/virdbus.h index a4987b8..d0c7de2 100644 --- a/src/util/virdbus.h +++ b/src/util/virdbus.h @@ -68,10 +68,6 @@ int virDBusCallMethod(DBusConnection *conn, const char *iface, const char *member, const char *types, ...); -int virDBusCall(DBusConnection *conn, - DBusMessage *call, - DBusMessage **reply, - DBusError *error); int virDBusMessageRead(DBusMessage *msg, const char *types, ...); void virDBusMessageUnref(DBusMessage *msg); -- 1.9.0
On Sat, May 03, 2014 at 03:59:40PM -0400, Cole Robinson wrote:
--- src/libvirt_private.syms | 1 - src/util/virdbus.c | 19 +++++-------------- src/util/virdbus.h | 4 ---- 3 files changed, 5 insertions(+), 19 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index cc3ae2c..8a015eb 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1148,7 +1148,6 @@ virCryptoHashString;
# util/virdbus.h -virDBusCall; virDBusCallMethod; virDBusCloseSystemBus; virDBusCreateMethod; diff --git a/src/util/virdbus.c b/src/util/virdbus.c index 4217aea..709d6ee 100644 --- a/src/util/virdbus.c +++ b/src/util/virdbus.c @@ -1404,10 +1404,11 @@ int virDBusCreateReply(DBusMessage **reply, * * Returns 0 on success, or -1 upon error */ -int virDBusCall(DBusConnection *conn, - DBusMessage *call, - DBusMessage **replyout, - DBusError *error) +static int +virDBusCall(DBusConnection *conn, + DBusMessage *call, + DBusMessage **replyout, + DBusError *error) { DBusMessage *reply = NULL; DBusError localerror; @@ -1694,16 +1695,6 @@ int virDBusCreateReply(DBusMessage **reply ATTRIBUTE_UNUSED, return -1; }
-int virDBusCall(DBusConnection *conn ATTRIBUTE_UNUSED, - DBusMessage *call ATTRIBUTE_UNUSED, - DBusMessage **reply ATTRIBUTE_UNUSED, - DBusError *error ATTRIBUTE_UNUSED) -{ - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("DBus support not compiled into this binary")); - return -1; -} - int virDBusCallMethod(DBusConnection *conn ATTRIBUTE_UNUSED, DBusMessage **reply ATTRIBUTE_UNUSED, DBusError *error ATTRIBUTE_UNUSED, diff --git a/src/util/virdbus.h b/src/util/virdbus.h index a4987b8..d0c7de2 100644 --- a/src/util/virdbus.h +++ b/src/util/virdbus.h @@ -68,10 +68,6 @@ int virDBusCallMethod(DBusConnection *conn, const char *iface, const char *member, const char *types, ...); -int virDBusCall(DBusConnection *conn, - DBusMessage *call, - DBusMessage **reply, - DBusError *error); int virDBusMessageRead(DBusMessage *msg, const char *types, ...); void virDBusMessageUnref(DBusMessage *msg); --
Ack. Outside consumers use the higher level virDBusCallMethod. -- Guido
On 05/05/2014 02:53 AM, Guido Günther wrote:
On Sat, May 03, 2014 at 03:59:40PM -0400, Cole Robinson wrote:
--- src/libvirt_private.syms | 1 - src/util/virdbus.c | 19 +++++-------------- src/util/virdbus.h | 4 ---- 3 files changed, 5 insertions(+), 19 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index cc3ae2c..8a015eb 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1148,7 +1148,6 @@ virCryptoHashString;
# util/virdbus.h -virDBusCall; virDBusCallMethod; virDBusCloseSystemBus; virDBusCreateMethod; diff --git a/src/util/virdbus.c b/src/util/virdbus.c index 4217aea..709d6ee 100644 --- a/src/util/virdbus.c +++ b/src/util/virdbus.c @@ -1404,10 +1404,11 @@ int virDBusCreateReply(DBusMessage **reply, * * Returns 0 on success, or -1 upon error */ -int virDBusCall(DBusConnection *conn, - DBusMessage *call, - DBusMessage **replyout, - DBusError *error) +static int +virDBusCall(DBusConnection *conn, + DBusMessage *call, + DBusMessage **replyout, + DBusError *error) { DBusMessage *reply = NULL; DBusError localerror; @@ -1694,16 +1695,6 @@ int virDBusCreateReply(DBusMessage **reply ATTRIBUTE_UNUSED, return -1; }
-int virDBusCall(DBusConnection *conn ATTRIBUTE_UNUSED, - DBusMessage *call ATTRIBUTE_UNUSED, - DBusMessage **reply ATTRIBUTE_UNUSED, - DBusError *error ATTRIBUTE_UNUSED) -{ - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("DBus support not compiled into this binary")); - return -1; -} - int virDBusCallMethod(DBusConnection *conn ATTRIBUTE_UNUSED, DBusMessage **reply ATTRIBUTE_UNUSED, DBusError *error ATTRIBUTE_UNUSED, diff --git a/src/util/virdbus.h b/src/util/virdbus.h index a4987b8..d0c7de2 100644 --- a/src/util/virdbus.h +++ b/src/util/virdbus.h @@ -68,10 +68,6 @@ int virDBusCallMethod(DBusConnection *conn, const char *iface, const char *member, const char *types, ...); -int virDBusCall(DBusConnection *conn, - DBusMessage *call, - DBusMessage **reply, - DBusError *error); int virDBusMessageRead(DBusMessage *msg, const char *types, ...); void virDBusMessageUnref(DBusMessage *msg); --
Ack. Outside consumers use the higher level virDBusCallMethod. -- Guido
Thanks, I pushed this - Cole
This is the only callsite. We drop use of localerror.name here, because it's not actually useful to us: rather than the parameter name which received an invalid value (which was assumed), it's actually the the dbus errno equivalent. Just use the string. --- src/util/virdbus.c | 7 ++++--- src/util/virerror.h | 6 ------ 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/src/util/virdbus.c b/src/util/virdbus.c index 709d6ee..03ec028 100644 --- a/src/util/virdbus.c +++ b/src/util/virdbus.c @@ -1423,9 +1423,10 @@ virDBusCall(DBusConnection *conn, error ? error : &localerror))) { if (error) ret = 0; - else - virReportDBusServiceError(localerror.message ? localerror.message : "unknown error", - localerror.name); + else { + virReportError(VIR_ERR_DBUS_SERVICE, "%s", + localerror.message ? localerror.message : _("unknown error")); + } goto cleanup; } diff --git a/src/util/virerror.h b/src/util/virerror.h index 872c270..4b01b77 100644 --- a/src/util/virerror.h +++ b/src/util/virerror.h @@ -110,12 +110,6 @@ void virReportSystemErrorFull(int domcode, __FILE__, __FUNCTION__, __LINE__, \ (fmt), __VA_ARGS__) -# define virReportDBusServiceError(message, name) \ - virReportErrorHelper(VIR_FROM_THIS, \ - VIR_ERR_DBUS_SERVICE, \ - __FILE__, __FUNCTION__, __LINE__, \ - "%s", message) - # define virReportUnsupportedError() \ virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_NO_SUPPORT, \ __FILE__, __FUNCTION__, __LINE__, __FUNCTION__) -- 1.9.0
On 05/03/2014 01:59 PM, Cole Robinson wrote:
This is the only callsite.
We drop use of localerror.name here, because it's not actually useful to us: rather than the parameter name which received an invalid value (which was assumed), it's actually the the dbus errno equivalent. Just use the string. --- src/util/virdbus.c | 7 ++++--- src/util/virerror.h | 6 ------ 2 files changed, 4 insertions(+), 9 deletions(-)
diff --git a/src/util/virdbus.c b/src/util/virdbus.c index 709d6ee..03ec028 100644 --- a/src/util/virdbus.c +++ b/src/util/virdbus.c @@ -1423,9 +1423,10 @@ virDBusCall(DBusConnection *conn, error ? error : &localerror))) { if (error) ret = 0; - else - virReportDBusServiceError(localerror.message ? localerror.message : "unknown error", - localerror.name); + else { + virReportError(VIR_ERR_DBUS_SERVICE, "%s", + localerror.message ? localerror.message : _("unknown error")); + }
Bonus points for getting it translated. ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
If you trigger bug 1033369, we get the error message: error from service: Invalid argument Which is a bit too generic to pinpoint what is actually failing. This changes it to: error from service: CreateMachine: Invalid argument --- src/util/virdbus.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/util/virdbus.c b/src/util/virdbus.c index 03ec028..31251fe 100644 --- a/src/util/virdbus.c +++ b/src/util/virdbus.c @@ -1408,7 +1408,8 @@ static int virDBusCall(DBusConnection *conn, DBusMessage *call, DBusMessage **replyout, - DBusError *error) + DBusError *error, + const char *member) { DBusMessage *reply = NULL; DBusError localerror; @@ -1424,7 +1425,7 @@ virDBusCall(DBusConnection *conn, if (error) ret = 0; else { - virReportError(VIR_ERR_DBUS_SERVICE, "%s", + virReportError(VIR_ERR_DBUS_SERVICE, _("%s: %s"), member, localerror.message ? localerror.message : _("unknown error")); } goto cleanup; @@ -1502,7 +1503,7 @@ int virDBusCallMethod(DBusConnection *conn, ret = -1; - ret = virDBusCall(conn, call, replyout, error); + ret = virDBusCall(conn, call, replyout, error, member); cleanup: if (call) -- 1.9.0
On 05/03/2014 01:59 PM, Cole Robinson wrote:
If you trigger bug 1033369, we get the error message:
error from service: Invalid argument
Which is a bit too generic to pinpoint what is actually failing. This changes it to:
error from service: CreateMachine: Invalid argument --- src/util/virdbus.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
ACK -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Sat, May 03, 2014 at 03:59:38PM -0400, Cole Robinson wrote:
--- src/util/virerror.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virerror.h b/src/util/virerror.h index 2de04f4..fe0e15e 100644 --- a/src/util/virerror.h +++ b/src/util/virerror.h @@ -99,7 +99,7 @@ void virReportSystemErrorFull(int domcode, #argname, \ NULL, \ 0, 0, \ - _("%s in %s must greater than zero"), \ + _("%s in %s must be greater than zero"), \ #argname, __FUNCTION__) # define virReportInvalidNonZeroArg(argname) \ virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__, \ -- ACK. -- Guido
On 05/04/2014 03:37 AM, Guido Günther wrote:
On Sat, May 03, 2014 at 03:59:38PM -0400, Cole Robinson wrote:
--- src/util/virerror.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virerror.h b/src/util/virerror.h index 2de04f4..fe0e15e 100644 --- a/src/util/virerror.h +++ b/src/util/virerror.h @@ -99,7 +99,7 @@ void virReportSystemErrorFull(int domcode, #argname, \ NULL, \ 0, 0, \ - _("%s in %s must greater than zero"), \ + _("%s in %s must be greater than zero"), \ #argname, __FUNCTION__) # define virReportInvalidNonZeroArg(argname) \ virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__, \ -- ACK. -- Guido
Thanks, I pushed this one - Cole
participants (3)
-
Cole Robinson -
Eric Blake -
Guido Günther