3:48 p.m.
V2 fixes gettext and simplifies some parts. Patch 4/9 is new and the
last two were dropped in comparison to v1.
Peter Krempa (9):
include: error: Add enum sentinel for virErrorNumber enum
util: error: Fix error message strings to play well with additional
info
util: error: Add error message versions with info for some error codes
util: error: Reword some unused error messages
util: error: Export virErrorMsg for use in testsuite
tests: Add test for virErrorMsg message constraints
util: error: Improve docs for virErrorMsg
DO NOT PUSH: Make sure that error messages are not moved around
util: error: Put error code messages into an array
include/libvirt/virterror.h | 5 +
src/libvirt_private.syms | 2 +
src/util/Makefile.inc.am | 1 +
src/util/virerror.c | 738 +++++++-----------------------------
src/util/virerrorpriv.h | 28 ++
tests/Makefile.am | 6 +
tests/virerrormessages.txt | 101 +++++
tests/virerrortest.c | 126 ++++++
8 files changed, 405 insertions(+), 602 deletions(-)
create mode 100644 src/util/virerrorpriv.h
create mode 100644 tests/virerrormessages.txt
create mode 100644 tests/virerrortest.c
--
2.19.2
3:48 p.m.
New subject: [libvirt] [PATCH v2 1/9] include: error: Add enum sentinel for virErrorNumber enum
We do have one for the error domain but not for the error number itself.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
---
include/libvirt/virterror.h | 5 +++++
src/util/virerror.c | 1 +
2 files changed, 6 insertions(+)
diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
index 57aadb8d16..9867c05450 100644
--- a/include/libvirt/virterror.h
+++ b/include/libvirt/virterror.h
@@ -323,6 +323,11 @@ typedef enum {
VIR_ERR_DEVICE_MISSING = 99, /* fail to find the desired device */
VIR_ERR_INVALID_NWFILTER_BINDING = 100, /* invalid nwfilter binding */
VIR_ERR_NO_NWFILTER_BINDING = 101, /* no nwfilter binding */
+
+# ifdef VIR_ENUM_SENTINELS
+ VIR_ERR_NUMBER_LAST
+# endif
+
} virErrorNumber;
/**
diff --git a/src/util/virerror.c b/src/util/virerror.c
index 683e51aa19..eca6ddf7d1 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -916,6 +916,7 @@ virErrorMsg(virErrorNumber error, const char *info)
const char *errmsg = NULL;
switch (error) {
+ case VIR_ERR_NUMBER_LAST:
case VIR_ERR_OK:
return NULL;
case VIR_ERR_INTERNAL_ERROR:
--
2.19.2
3:48 p.m.
New subject: [libvirt] [PATCH v2 2/9] util: error: Fix error message strings to play well with additional info
Additional information for an error message is either in form of a
string or empty. Fix two offenders. One used %d as the format modifier
and the second one always expected a string.
Thankfully, neither of the offenders are currently in effect.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
---
src/util/virerror.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/util/virerror.c b/src/util/virerror.c
index eca6ddf7d1..5d515bde57 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -977,7 +977,10 @@ virErrorMsg(virErrorNumber error, const char *info)
errmsg = _("POST operation failed");
break;
case VIR_ERR_HTTP_ERROR:
- errmsg = _("got unknown HTTP error code %d");
+ if (info != NULL)
+ errmsg = _("got unknown HTTP error code %s");
+ else
+ errmsg = _("got unknown HTTP error code");
break;
case VIR_ERR_UNKNOWN_HOST:
if (info != NULL)
@@ -1004,7 +1007,10 @@ virErrorMsg(virErrorNumber error, const char *info)
errmsg = _("could not connect to Xen Store %s");
break;
case VIR_ERR_XEN_CALL:
- errmsg = _("failed Xen syscall %s");
+ if (info == NULL)
+ errmsg = _("failed Xen syscall");
+ else
+ errmsg = _("failed Xen syscall %s");
break;
case VIR_ERR_OS_TYPE:
if (info == NULL)
--
2.19.2
3:48 p.m.
New subject: [libvirt] [PATCH v2 3/9] util: error: Add error message versions with info for some error codes
Few error codes were missing the version of the message with additional
info. In case of the modified messages it's not very likely they'll ever
report any additional data, but for the sake of consistency we should
provide them.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
---
src/util/virerror.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/src/util/virerror.c b/src/util/virerror.c
index 5d515bde57..64be00dc75 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -926,7 +926,10 @@ virErrorMsg(virErrorNumber error, const char *info)
errmsg = _("internal error");
break;
case VIR_ERR_NO_MEMORY:
- errmsg = _("out of memory");
+ if (info == NULL)
+ errmsg = _("out of memory");
+ else
+ errmsg = _("out of memory: %s");
break;
case VIR_ERR_NO_SUPPORT:
if (info == NULL)
@@ -1019,7 +1022,10 @@ virErrorMsg(virErrorNumber error, const char *info)
errmsg = _("unknown OS type %s");
break;
case VIR_ERR_NO_KERNEL:
- errmsg = _("missing kernel information");
+ if (info == NULL)
+ errmsg = _("missing kernel information");
+ else
+ errmsg = _("missing kernel information: %s");
break;
case VIR_ERR_NO_ROOT:
if (info == NULL)
@@ -1472,7 +1478,10 @@ virErrorMsg(virErrorNumber error, const char *info)
errmsg = _("XML document failed to validate against schema:
%s");
break;
case VIR_ERR_MIGRATE_FINISH_OK:
- errmsg = _("migration successfully aborted");
+ if (info == NULL)
+ errmsg = _("migration successfully aborted");
+ else
+ errmsg = _("migration successfully aborted: %s");
break;
case VIR_ERR_NO_SERVER:
if (info == NULL)
--
2.19.2
3:48 p.m.
New subject: [libvirt] [PATCH v2 4/9] util: error: Reword some unused error messages
Simplify wording of the error string for VIR_ERR_OPEN_FAILED and
VIR_ERR_CALL_FAILED. The error codes itself are currently unused so it
will not impact any client.
This will simplify upcomming patch which refactors how we convert these.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/util/virerror.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/util/virerror.c b/src/util/virerror.c
index 64be00dc75..eed65e8f6a 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -1071,9 +1071,9 @@ virErrorMsg(virErrorNumber error, const char *info)
break;
case VIR_ERR_CALL_FAILED: /* DEPRECATED, use VIR_ERR_NO_SUPPORT */
if (info == NULL)
- errmsg = _("library call failed, possibly not supported");
+ errmsg = _("library call failed");
else
- errmsg = _("library call %s failed, possibly not supported");
+ errmsg = _("library call failed: %s");
break;
case VIR_ERR_XML_ERROR:
if (info == NULL)
@@ -1095,9 +1095,9 @@ virErrorMsg(virErrorNumber error, const char *info)
break;
case VIR_ERR_OPEN_FAILED:
if (info == NULL)
- errmsg = _("failed to open configuration file for reading");
+ errmsg = _("failed to open configuration file");
else
- errmsg = _("failed to open %s for reading");
+ errmsg = _("failed to open configuration file %s");
break;
case VIR_ERR_READ_FAILED:
if (info == NULL)
--
2.19.2
9:59 a.m.
New subject: [libvirt] [PATCH v2 4/9] util: error: Reword some unused error messages
On Thu, Dec 13, 2018 at 03:48:51PM +0100, Peter Krempa wrote:
Simplify wording of the error string for VIR_ERR_OPEN_FAILED and
VIR_ERR_CALL_FAILED. The error codes itself are currently unused so it
will not impact any client.
This will simplify upcomming patch which refactors how we convert these.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
3:48 p.m.
New subject: [libvirt] [PATCH v2 5/9] util: error: Export virErrorMsg for use in testsuite
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
---
src/libvirt_private.syms | 1 +
src/util/Makefile.inc.am | 1 +
src/util/virerror.c | 6 +++++-
src/util/virerrorpriv.h | 28 ++++++++++++++++++++++++++++
4 files changed, 35 insertions(+), 1 deletion(-)
create mode 100644 src/util/virerrorpriv.h
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index fd63c9ca61..6184030d59 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1752,6 +1752,7 @@ ebtablesRemoveForwardAllowIn;
virDispatchError;
virErrorCopyNew;
virErrorInitialize;
+virErrorMsg;
virErrorPreserveLast;
virErrorRestore;
virErrorSetErrnoFromLastError;
diff --git a/src/util/Makefile.inc.am b/src/util/Makefile.inc.am
index cffbb357bc..4295babac3 100644
--- a/src/util/Makefile.inc.am
+++ b/src/util/Makefile.inc.am
@@ -50,6 +50,7 @@ UTIL_SOURCES = \
util/virendian.h \
util/virerror.c \
util/virerror.h \
+ util/virerrorpriv.h \
util/virevent.c \
util/virevent.h \
util/vireventpoll.c \
diff --git a/src/util/virerror.c b/src/util/virerror.c
index eed65e8f6a..1e7c8bdb90 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -31,6 +31,10 @@
#include "virutil.h"
#include "virstring.h"
+#define __VIR_ERROR_ALLOW_INCLUDE_PRIV_H__
+#include "virerrorpriv.h"
+#undef __VIR_ERROR_ALLOW_INCLUDE_PRIV_H__
+
VIR_LOG_INIT("util.error");
virThreadLocal virLastErr;
@@ -910,7 +914,7 @@ void virRaiseErrorObject(const char *filename,
*
* Returns the constant string associated to @error
*/
-static const char *
+const char *
virErrorMsg(virErrorNumber error, const char *info)
{
const char *errmsg = NULL;
diff --git a/src/util/virerrorpriv.h b/src/util/virerrorpriv.h
new file mode 100644
index 0000000000..bc214393e6
--- /dev/null
+++ b/src/util/virerrorpriv.h
@@ -0,0 +1,28 @@
+/*
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * <http://www.gnu.org/licenses/>;.
+ */
+
+#ifndef __VIR_ERROR_ALLOW_INCLUDE_PRIV_H__
+# error "virerrorpriv.h may only be included by virerror.c or its test suite"
+#endif
+
+#ifndef __VIR_ERROR_PRIV_H__
+# define __VIR_ERROR_PRIV_H__
+
+const char *
+virErrorMsg(virErrorNumber error,
+ const char *info);
+
+#endif /* __VIR_ERROR_PRIV_H__ */
--
2.19.2
3:48 p.m.
New subject: [libvirt] [PATCH v2 6/9] tests: Add test for virErrorMsg message constraints
Make sure that we don't add any broken error message strings any more.
This ensures that both the version with and without additional info is
populated, the version without info does not have any formatting
modifiers and the version with info has exactly one.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
---
tests/Makefile.am | 6 +++
tests/virerrortest.c | 101 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 107 insertions(+)
create mode 100644 tests/virerrortest.c
diff --git a/tests/Makefile.am b/tests/Makefile.am
index d7ec7e3a6f..f2eef2293f 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -207,6 +207,7 @@ test_programs = virshtest sockettest \
virnetdevtest \
virtypedparamtest \
vshtabletest \
+ virerrortest \
$(NULL)
test_libraries = libshunload.la \
@@ -940,6 +941,11 @@ metadatatest_SOURCES = \
testutils.c testutils.h
metadatatest_LDADD = $(LDADDS) $(LIBXML_LIBS)
+virerrortest_SOURCES = \
+ virerrortest.c \
+ testutils.c testutils.h
+virerrortest_LDADD = $(LDADDS)
+
vshtabletest_SOURCES = \
vshtabletest.c \
testutils.c testutils.h
diff --git a/tests/virerrortest.c b/tests/virerrortest.c
new file mode 100644
index 0000000000..0d0377bfa8
--- /dev/null
+++ b/tests/virerrortest.c
@@ -0,0 +1,101 @@
+/*
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; If not, see
+ * <http://www.gnu.org/licenses/>;.
+ */
+
+#include <config.h>
+
+#include "testutils.h"
+
+#define __VIR_ERROR_ALLOW_INCLUDE_PRIV_H__
+#include "virerrorpriv.h"
+
+static int
+virErrorTestMsgFormatInfoOne(const char *msg)
+{
+ bool found = false;
+ char *next;
+ int ret = 0;
+
+ for (next = (char *)msg; (next = strchr(next, '%')); next++) {
+ if (next[1] != 's') {
+ VIR_TEST_VERBOSE("\nerror message '%s' contains disallowed
printf modifiers\n", msg);
+ ret = -1;
+ } else {
+ if (found) {
+ VIR_TEST_VERBOSE("\nerror message '%s' contains multiple %%s
modifiers\n", msg);
+ ret = -1;
+ } else {
+ found = true;
+ }
+ }
+ }
+
+ if (!found) {
+ VIR_TEST_VERBOSE("\nerror message '%s' does not contain any %%s
modifiers\n", msg);
+ ret = -1;
+ }
+
+ return ret;
+}
+
+
+static int
+virErrorTestMsgs(const void *opaque ATTRIBUTE_UNUSED)
+{
+ const char *err_noinfo;
+ const char *err_info;
+ size_t i;
+ int ret = 0;
+
+ for (i = 1; i < VIR_ERR_NUMBER_LAST; i++) {
+ err_noinfo = virErrorMsg(i, NULL);
+ err_info = virErrorMsg(i, "");
+
+ if (!err_noinfo) {
+ VIR_TEST_VERBOSE("\nmissing string without info for error id
%zu\n", i);
+ ret = -1;
+ }
+
+ if (!err_info) {
+ VIR_TEST_VERBOSE("\nmissing string with info for error id %zu\n",
i);
+ ret = -1;
+ }
+
+ if (strchr(err_noinfo, '%')) {
+ VIR_TEST_VERBOSE("\nerror message id %zu contains formatting characters:
'%s'\n",
+ i, err_noinfo);
+ ret = -1;
+ }
+
+ if (virErrorTestMsgFormatInfoOne(err_info) < 0)
+ ret = -1;
+ }
+
+ return ret;
+}
+
+
+static int
+mymain(void)
+{
+ int ret = 0;
+
+ if (virTestRun("error message strings ", virErrorTestMsgs, NULL) < 0)
+ ret = -1;
+
+ return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
+}
+
+VIR_TEST_MAIN(mymain)
--
2.19.2
3:48 p.m.
New subject: [libvirt] [PATCH v2 7/9] util: error: Improve docs for virErrorMsg
Clarify how @info is used and what the returned values look like.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
---
src/util/virerror.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/util/virerror.c b/src/util/virerror.c
index 1e7c8bdb90..03166d85d2 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -907,12 +907,14 @@ void virRaiseErrorObject(const char *filename,
/**
* virErrorMsg:
* @error: the virErrorNumber
- * @info: usually the first parameter string
+ * @info: additional info string
*
- * Internal routine to get the message associated to an error raised
- * from the library
+ * Internal routine to get the message associated to @error raised
+ * from the library.
*
- * Returns the constant string associated to @error
+ * Returns a *printf format string which describes @error. The returned string
+ * contains exactly one '%s' modifier if @info is non-NULL, or no modifiers at
+ * all if @info is NULL. If @error is invalid NULL is returned.
*/
const char *
virErrorMsg(virErrorNumber error, const char *info)
--
2.19.2
3:48 p.m.
New subject: [libvirt] [PATCH v2 8/9] DO NOT PUSH: Make sure that error messages are not moved around
Add a virerrortest case that makes sure that all error messages stay the
same while refactoring. This patch will not be pushed upstream to avoid
polluting the git history.
---
tests/virerrormessages.txt | 101 +++++++++++++++++++++++++++++++++++++
tests/virerrortest.c | 25 +++++++++
2 files changed, 126 insertions(+)
create mode 100644 tests/virerrormessages.txt
diff --git a/tests/virerrormessages.txt b/tests/virerrormessages.txt
new file mode 100644
index 0000000000..8da486638e
--- /dev/null
+++ b/tests/virerrormessages.txt
@@ -0,0 +1,101 @@
+1-internal error-internal error: %s
+2-out of memory-out of memory: %s
+3-this function is not supported by the connection driver-this function is not supported
by the connection driver: %s
+4-unknown host-unknown host %s
+5-no connection driver available-no connection driver available for %s
+6-invalid connection pointer in-invalid connection pointer in %s
+7-invalid domain pointer in-invalid domain pointer in %s
+8-invalid argument-invalid argument: %s
+9-operation failed-operation failed: %s
+10-GET operation failed-GET operation failed: %s
+11-POST operation failed-POST operation failed: %s
+12-got unknown HTTP error code-got unknown HTTP error code %s
+13-failed to serialize S-Expr-failed to serialize S-Expr: %s
+14-could not use Xen hypervisor entry-could not use Xen hypervisor entry %s
+15-failed Xen syscall-failed Xen syscall %s
+16-unknown OS type-unknown OS type %s
+17-missing kernel information-missing kernel information: %s
+18-missing root device information-missing root device information in %s
+19-missing source information for device-missing source information for device %s
+20-missing target information for device-missing target information for device %s
+21-missing name information-missing name information in %s
+22-missing operating system information-missing operating system information for %s
+23-missing devices information-missing devices information for %s
+24-could not connect to Xen Store-could not connect to Xen Store %s
+25-too many drivers registered-too many drivers registered in %s
+26-library call failed-library call failed: %s
+27-XML description is invalid or not well formed-XML error: %s
+28-this domain exists already-domain %s exists already
+29-operation forbidden for read only access-operation forbidden: %s
+30-failed to open configuration file-failed to open configuration file %s
+31-failed to read configuration file-failed to read configuration file %s
+32-failed to parse configuration file-failed to parse configuration file %s
+33-configuration file syntax error-configuration file syntax error: %s
+34-failed to write configuration file-failed to write configuration file: %s
+35-parser error-%s
+36-invalid network pointer in-invalid network pointer in %s
+37-this network exists already-network %s exists already
+38-system call error-%s
+39-RPC error-%s
+40-GNUTLS call error-%s
+41-Failed to find the network-Failed to find the network: %s
+42-Domain not found-Domain not found: %s
+43-Network not found-Network not found: %s
+44-invalid MAC address-invalid MAC address: %s
+45-authentication failed-authentication failed: %s
+46-invalid storage pool pointer in-invalid storage pool pointer in %s
+47-invalid storage volume pointer in-invalid storage volume pointer in %s
+48-Failed to find a storage driver-Failed to find a storage driver: %s
+49-Storage pool not found-Storage pool not found: %s
+50-Storage volume not found-Storage volume not found: %s
+51-Failed to find a node driver-Failed to find a node driver: %s
+52-invalid node device pointer-invalid node device pointer in %s
+53-Node device not found-Node device not found: %s
+54-Security model not found-Security model not found: %s
+55-Requested operation is not valid-Requested operation is not valid: %s
+56-Failed to find the interface-Failed to find the interface: %s
+57-Interface not found-Interface not found: %s
+58-invalid interface pointer in-invalid interface pointer in %s
+59-multiple matching interfaces found-multiple matching interfaces found: %s
+60-Failed to start the nwfilter driver-Failed to start the nwfilter driver: %s
+61-Invalid network filter-Invalid network filter: %s
+62-Network filter not found-Network filter not found: %s
+63-Error while building firewall-Error while building firewall: %s
+64-Failed to find a secret storage driver-Failed to find a secret storage driver: %s
+65-Invalid secret-Invalid secret: %s
+66-Secret not found-Secret not found: %s
+67-unsupported configuration-unsupported configuration: %s
+68-Timed out during operation-Timed out during operation: %s
+69-Failed to make domain persistent after migration-Failed to make domain persistent
after migration: %s
+70-Hook script execution failed-Hook script execution failed: %s
+71-Invalid snapshot-Invalid snapshot: %s
+72-Domain snapshot not found-Domain snapshot not found: %s
+73-invalid stream pointer-invalid stream pointer in %s
+74-argument unsupported-argument unsupported: %s
+75-Storage pool probe failed-Storage pool probe failed: %s
+76-Storage pool already built-Storage pool already built: %s
+77-revert requires force-revert requires force: %s
+78-operation aborted-operation aborted: %s
+79-authentication cancelled-authentication cancelled: %s
+80-metadata not found-metadata not found: %s
+81-Unsafe migration-Unsafe migration: %s
+82-numerical overflow-numerical overflow: %s
+83-block copy still active-block copy still active: %s
+84-Operation not supported-Operation not supported: %s
+85-SSH transport error-SSH transport error: %s
+86-Guest agent is not responding-Guest agent is not responding: %s
+87-resource busy-resource busy: %s
+88-access denied-access denied: %s
+89-error from service-error from service: %s
+90-this storage volume exists already-storage volume %s exists already
+91-the CPU is incompatible with host CPU-the CPU is incompatible with host CPU: %s
+92-XML document failed to validate against schema-XML document failed to validate against
schema: %s
+93-migration successfully aborted-migration successfully aborted: %s
+94-authentication unavailable-authentication unavailable: %s
+95-Server not found-Server not found: %s
+96-Client not found-Client not found: %s
+97-guest agent replied with wrong id to guest-sync command-guest agent replied with wrong
id to guest-sync command: %s
+98-libssh transport error-libssh transport error: %s
+99-device not found-device not found: %s
+100-Invalid network filter binding-Invalid network filter binding: %s
+101-Network filter binding not found-Network filter binding not found: %s
diff --git a/tests/virerrortest.c b/tests/virerrortest.c
index 0d0377bfa8..e985ca743b 100644
--- a/tests/virerrortest.c
+++ b/tests/virerrortest.c
@@ -87,6 +87,29 @@ virErrorTestMsgs(const void *opaque ATTRIBUTE_UNUSED)
}
+static int
+virErrorTestMsgsStable(const void *opaque ATTRIBUTE_UNUSED)
+{
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+ char *actual = NULL;
+ size_t i;
+ int ret = 0;
+
+ for (i = 1; i < VIR_ERR_NUMBER_LAST; i++) {
+ virBufferAsprintf(&buf, "%zu-", i);
+ virBufferStrcat(&buf, virErrorMsg(i, NULL), "-", virErrorMsg(i,
""), "\n", NULL);
+ }
+
+ actual = virBufferContentAndReset(&buf);
+
+ if (virTestCompareToFile(actual, abs_srcdir "/virerrormessages.txt") <
0)
+ ret = -1;
+
+ VIR_FREE(actual);
+ return ret;
+}
+
+
static int
mymain(void)
{
@@ -94,6 +117,8 @@ mymain(void)
if (virTestRun("error message strings ", virErrorTestMsgs, NULL) < 0)
ret = -1;
+ if (virTestRun("error message strings stability ", virErrorTestMsgsStable,
NULL) < 0)
+ ret = -1;
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
}
--
2.19.2
3:48 p.m.
New subject: [libvirt] [PATCH v2 9/9] util: error: Put error code messages into an array
Simplify adding of new errors by just adding them to the array of
messages rather than having to add conversion code.
Additionally most of the messages add the format string part as a suffix
so we can avoid some of the duplication by using a macro which adds the
suffix to the original string. This way most messages fit into the 80
column limit and only 3 exceed 100 colums.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
Notes:
v2:
- use positional initializers
- add macros for shortening the messages
- make it gettext-friendly, since the last version was not
src/libvirt_private.syms | 1 +
src/util/virerror.c | 738 +++++++--------------------------------
2 files changed, 126 insertions(+), 613 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 6184030d59..775b33e151 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1753,6 +1753,7 @@ virDispatchError;
virErrorCopyNew;
virErrorInitialize;
virErrorMsg;
+virErrorMsgStrings;
virErrorPreserveLast;
virErrorRestore;
virErrorSetErrnoFromLastError;
diff --git a/src/util/virerror.c b/src/util/virerror.c
index 03166d85d2..d3f1d7d0e1 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -904,6 +904,124 @@ void virRaiseErrorObject(const char *filename,
}
+typedef struct {
+ const char *msg;
+ const char *msginfo;
+} virErrorMsgTuple;
+
+#define MSG(msg, sfx) \
+ { N_(msg), N_(msg sfx) }
+#define MSG_EXISTS(object) \
+ { N_("this " object " exists already"), N_(object " %s exists
already") }
+
+const virErrorMsgTuple virErrorMsgStrings[VIR_ERR_NUMBER_LAST] = {
+ [VIR_ERR_OK] = { NULL, NULL },
+ [VIR_ERR_INTERNAL_ERROR] = MSG("internal error", ": %s"),
+ [VIR_ERR_NO_MEMORY] = MSG("out of memory", ": %s"),
+ [VIR_ERR_NO_SUPPORT] = MSG("this function is not supported by the connection
driver", ": %s"),
+ [VIR_ERR_UNKNOWN_HOST] = MSG("unknown host", " %s"),
+ [VIR_ERR_NO_CONNECT] = MSG("no connection driver available", " for
%s"),
+ [VIR_ERR_INVALID_CONN] = MSG("invalid connection pointer in", "
%s"),
+ [VIR_ERR_INVALID_DOMAIN] = MSG("invalid domain pointer in", "
%s"),
+ [VIR_ERR_INVALID_ARG] = MSG("invalid argument", ": %s"),
+ [VIR_ERR_OPERATION_FAILED] = MSG("operation failed", ": %s"),
+ [VIR_ERR_GET_FAILED] = MSG("GET operation failed", ": %s"),
+ [VIR_ERR_POST_FAILED] = MSG("POST operation failed", ": %s"),
+ [VIR_ERR_HTTP_ERROR] = MSG("got unknown HTTP error code", "
%s"),
+ [VIR_ERR_SEXPR_SERIAL] = MSG("failed to serialize S-Expr", ":
%s"),
+ [VIR_ERR_NO_XEN] = MSG("could not use Xen hypervisor entry", "
%s"),
+ [VIR_ERR_XEN_CALL] = MSG("failed Xen syscall", " %s"),
+ [VIR_ERR_OS_TYPE] = MSG("unknown OS type", " %s"),
+ [VIR_ERR_NO_KERNEL] = MSG("missing kernel information", ": %s"),
+ [VIR_ERR_NO_ROOT] = MSG("missing root device information", " in
%s"),
+ [VIR_ERR_NO_SOURCE] = MSG("missing source information for device", "
%s"),
+ [VIR_ERR_NO_TARGET] = MSG("missing target information for device", "
%s"),
+ [VIR_ERR_NO_NAME] = MSG("missing name information", " in %s"),
+ [VIR_ERR_NO_OS] = MSG("missing operating system information", " for
%s"),
+ [VIR_ERR_NO_DEVICE] = MSG("missing devices information", " for
%s"),
+ [VIR_ERR_NO_XENSTORE] = MSG("could not connect to Xen Store", "
%s"),
+ [VIR_ERR_DRIVER_FULL] = MSG("too many drivers registered", " in
%s"),
+ [VIR_ERR_CALL_FAILED] = MSG("library call failed", ": %s"),
+ [VIR_ERR_XML_ERROR] = { "XML description is invalid or not well formed",
"XML error: %s" },
+ [VIR_ERR_DOM_EXIST] = MSG_EXISTS("domain"),
+ [VIR_ERR_OPERATION_DENIED] = { "operation forbidden for read only access",
"operation forbidden: %s" },
+ [VIR_ERR_OPEN_FAILED] = MSG("failed to open configuration file", "
%s"),
+ [VIR_ERR_READ_FAILED] = MSG("failed to read configuration file", "
%s"),
+ [VIR_ERR_PARSE_FAILED] = MSG("failed to parse configuration file", "
%s"),
+ [VIR_ERR_CONF_SYNTAX] = MSG("configuration file syntax error", ":
%s"),
+ [VIR_ERR_WRITE_FAILED] = MSG("failed to write configuration file", ":
%s"),
+ [VIR_ERR_XML_DETAIL] = { "parser error", "%s" },
+ [VIR_ERR_INVALID_NETWORK] = MSG("invalid network pointer in", "
%s"),
+ [VIR_ERR_NETWORK_EXIST] = MSG_EXISTS("network"),
+ [VIR_ERR_SYSTEM_ERROR] = { "system call error", "%s" },
+ [VIR_ERR_RPC] = { "RPC error", "%s" },
+ [VIR_ERR_GNUTLS_ERROR] = { "GNUTLS call error", "%s" },
+ [VIR_WAR_NO_NETWORK] = MSG("Failed to find the network", ":
%s"),
+ [VIR_ERR_NO_DOMAIN] = MSG("Domain not found", ": %s"),
+ [VIR_ERR_NO_NETWORK] = MSG("Network not found", ": %s"),
+ [VIR_ERR_INVALID_MAC] = MSG("invalid MAC address", ": %s"),
+ [VIR_ERR_AUTH_FAILED] = MSG("authentication failed", ": %s"),
+ [VIR_ERR_INVALID_STORAGE_POOL] = MSG("invalid storage pool pointer in",
" %s"),
+ [VIR_ERR_INVALID_STORAGE_VOL] = MSG("invalid storage volume pointer in",
" %s"),
+ [VIR_WAR_NO_STORAGE] = MSG("Failed to find a storage driver", ":
%s"),
+ [VIR_ERR_NO_STORAGE_POOL] = MSG("Storage pool not found", ":
%s"),
+ [VIR_ERR_NO_STORAGE_VOL] = MSG("Storage volume not found", ":
%s"),
+ [VIR_WAR_NO_NODE] = MSG("Failed to find a node driver", ": %s"),
+ [VIR_ERR_INVALID_NODE_DEVICE] = MSG("invalid node device pointer", "
in %s"),
+ [VIR_ERR_NO_NODE_DEVICE] = MSG("Node device not found", ": %s"),
+ [VIR_ERR_NO_SECURITY_MODEL] = MSG("Security model not found", ":
%s"),
+ [VIR_ERR_OPERATION_INVALID] = MSG("Requested operation is not valid",
": %s"),
+ [VIR_WAR_NO_INTERFACE] = MSG("Failed to find the interface", ":
%s"),
+ [VIR_ERR_NO_INTERFACE] = MSG("Interface not found", ": %s"),
+ [VIR_ERR_INVALID_INTERFACE] = MSG("invalid interface pointer in", "
%s"),
+ [VIR_ERR_MULTIPLE_INTERFACES] = MSG("multiple matching interfaces found",
": %s"),
+ [VIR_WAR_NO_NWFILTER] = MSG("Failed to start the nwfilter driver", ":
%s"),
+ [VIR_ERR_INVALID_NWFILTER] = MSG("Invalid network filter", ":
%s"),
+ [VIR_ERR_NO_NWFILTER] = MSG("Network filter not found", ": %s"),
+ [VIR_ERR_BUILD_FIREWALL] = MSG("Error while building firewall", ":
%s"),
+ [VIR_WAR_NO_SECRET] = MSG("Failed to find a secret storage driver", ":
%s"),
+ [VIR_ERR_INVALID_SECRET] = MSG("Invalid secret", ": %s"),
+ [VIR_ERR_NO_SECRET] = MSG("Secret not found", ": %s"),
+ [VIR_ERR_CONFIG_UNSUPPORTED] = MSG("unsupported configuration", ":
%s"),
+ [VIR_ERR_OPERATION_TIMEOUT] = MSG("Timed out during operation", ":
%s"),
+ [VIR_ERR_MIGRATE_PERSIST_FAILED] = MSG("Failed to make domain persistent after
migration", ": %s"),
+ [VIR_ERR_HOOK_SCRIPT_FAILED] = MSG("Hook script execution failed", ":
%s"),
+ [VIR_ERR_INVALID_DOMAIN_SNAPSHOT] = MSG("Invalid snapshot", ":
%s"),
+ [VIR_ERR_NO_DOMAIN_SNAPSHOT] = MSG("Domain snapshot not found", ":
%s"),
+ [VIR_ERR_INVALID_STREAM] = MSG("invalid stream pointer", " in
%s"),
+ [VIR_ERR_ARGUMENT_UNSUPPORTED] = MSG("argument unsupported", ":
%s"),
+ [VIR_ERR_STORAGE_PROBE_FAILED] = MSG("Storage pool probe failed", ":
%s"),
+ [VIR_ERR_STORAGE_POOL_BUILT] = MSG("Storage pool already built", ":
%s"),
+ [VIR_ERR_SNAPSHOT_REVERT_RISKY] = MSG("revert requires force", ":
%s"),
+ [VIR_ERR_OPERATION_ABORTED] = MSG("operation aborted", ": %s"),
+ [VIR_ERR_AUTH_CANCELLED] = MSG("authentication cancelled", ":
%s"),
+ [VIR_ERR_NO_DOMAIN_METADATA] = MSG("metadata not found", ":
%s"),
+ [VIR_ERR_MIGRATE_UNSAFE] = MSG("Unsafe migration", ": %s"),
+ [VIR_ERR_OVERFLOW] = MSG("numerical overflow", ": %s"),
+ [VIR_ERR_BLOCK_COPY_ACTIVE] = MSG("block copy still active", ":
%s"),
+ [VIR_ERR_OPERATION_UNSUPPORTED] = MSG("Operation not supported", ":
%s"),
+ [VIR_ERR_SSH] = MSG("SSH transport error", ": %s"),
+ [VIR_ERR_AGENT_UNRESPONSIVE] = MSG("Guest agent is not responding", ":
%s"),
+ [VIR_ERR_RESOURCE_BUSY] = MSG("resource busy", ": %s"),
+ [VIR_ERR_ACCESS_DENIED] = MSG("access denied", ": %s"),
+ [VIR_ERR_DBUS_SERVICE] = MSG("error from service", ": %s"),
+ [VIR_ERR_STORAGE_VOL_EXIST] = MSG_EXISTS("storage volume"),
+ [VIR_ERR_CPU_INCOMPATIBLE] = MSG("the CPU is incompatible with host CPU",
": %s"),
+ [VIR_ERR_XML_INVALID_SCHEMA] = MSG("XML document failed to validate against
schema", ": %s"),
+ [VIR_ERR_MIGRATE_FINISH_OK] = MSG("migration successfully aborted", ":
%s"),
+ [VIR_ERR_AUTH_UNAVAILABLE] = MSG("authentication unavailable", ":
%s"),
+ [VIR_ERR_NO_SERVER] = MSG("Server not found", ": %s"),
+ [VIR_ERR_NO_CLIENT] = MSG("Client not found", ": %s"),
+ [VIR_ERR_AGENT_UNSYNCED] = MSG("guest agent replied with wrong id to guest-sync
command", ": %s"),
+ [VIR_ERR_LIBSSH] = MSG("libssh transport error", ": %s"),
+ [VIR_ERR_DEVICE_MISSING] = MSG("device not found", ": %s"),
+ [VIR_ERR_INVALID_NWFILTER_BINDING] = MSG("Invalid network filter binding",
": %s"),
+ [VIR_ERR_NO_NWFILTER_BINDING] = MSG("Network filter binding not found",
": %s"),
+};
+
+#undef MSG
+#undef MSG_EXISTS
+
/**
* virErrorMsg:
* @error: the virErrorNumber
@@ -919,622 +1037,16 @@ void virRaiseErrorObject(const char *filename,
const char *
virErrorMsg(virErrorNumber error, const char *info)
{
- const char *errmsg = NULL;
+ if (error >= VIR_ERR_NUMBER_LAST)
+ return NULL;
- switch (error) {
- case VIR_ERR_NUMBER_LAST:
- case VIR_ERR_OK:
- return NULL;
- case VIR_ERR_INTERNAL_ERROR:
- if (info != NULL)
- errmsg = _("internal error: %s");
- else
- errmsg = _("internal error");
- break;
- case VIR_ERR_NO_MEMORY:
- if (info == NULL)
- errmsg = _("out of memory");
- else
- errmsg = _("out of memory: %s");
- break;
- case VIR_ERR_NO_SUPPORT:
- if (info == NULL)
- errmsg = _("this function is not supported by the connection
driver");
- else
- errmsg = _("this function is not supported by the connection driver:
%s");
- break;
- case VIR_ERR_NO_CONNECT:
- if (info == NULL)
- errmsg = _("no connection driver available");
- else
- errmsg = _("no connection driver available for %s");
- break;
- case VIR_ERR_INVALID_CONN:
- if (info == NULL)
- errmsg = _("invalid connection pointer in");
- else
- errmsg = _("invalid connection pointer in %s");
- break;
- case VIR_ERR_INVALID_DOMAIN:
- if (info == NULL)
- errmsg = _("invalid domain pointer in");
- else
- errmsg = _("invalid domain pointer in %s");
- break;
- case VIR_ERR_INVALID_ARG:
- if (info == NULL)
- errmsg = _("invalid argument");
- else
- errmsg = _("invalid argument: %s");
- break;
- case VIR_ERR_OPERATION_FAILED:
- if (info != NULL)
- errmsg = _("operation failed: %s");
- else
- errmsg = _("operation failed");
- break;
- case VIR_ERR_GET_FAILED:
- if (info != NULL)
- errmsg = _("GET operation failed: %s");
- else
- errmsg = _("GET operation failed");
- break;
- case VIR_ERR_POST_FAILED:
- if (info != NULL)
- errmsg = _("POST operation failed: %s");
- else
- errmsg = _("POST operation failed");
- break;
- case VIR_ERR_HTTP_ERROR:
- if (info != NULL)
- errmsg = _("got unknown HTTP error code %s");
- else
- errmsg = _("got unknown HTTP error code");
- break;
- case VIR_ERR_UNKNOWN_HOST:
- if (info != NULL)
- errmsg = _("unknown host %s");
- else
- errmsg = _("unknown host");
- break;
- case VIR_ERR_SEXPR_SERIAL:
- if (info != NULL)
- errmsg = _("failed to serialize S-Expr: %s");
- else
- errmsg = _("failed to serialize S-Expr");
- break;
- case VIR_ERR_NO_XEN:
- if (info == NULL)
- errmsg = _("could not use Xen hypervisor entry");
- else
- errmsg = _("could not use Xen hypervisor entry %s");
- break;
- case VIR_ERR_NO_XENSTORE:
- if (info == NULL)
- errmsg = _("could not connect to Xen Store");
- else
- errmsg = _("could not connect to Xen Store %s");
- break;
- case VIR_ERR_XEN_CALL:
- if (info == NULL)
- errmsg = _("failed Xen syscall");
- else
- errmsg = _("failed Xen syscall %s");
- break;
- case VIR_ERR_OS_TYPE:
- if (info == NULL)
- errmsg = _("unknown OS type");
- else
- errmsg = _("unknown OS type %s");
- break;
- case VIR_ERR_NO_KERNEL:
- if (info == NULL)
- errmsg = _("missing kernel information");
- else
- errmsg = _("missing kernel information: %s");
- break;
- case VIR_ERR_NO_ROOT:
- if (info == NULL)
- errmsg = _("missing root device information");
- else
- errmsg = _("missing root device information in %s");
- break;
- case VIR_ERR_NO_SOURCE:
- if (info == NULL)
- errmsg = _("missing source information for device");
- else
- errmsg = _("missing source information for device %s");
- break;
- case VIR_ERR_NO_TARGET:
- if (info == NULL)
- errmsg = _("missing target information for device");
- else
- errmsg = _("missing target information for device %s");
- break;
- case VIR_ERR_NO_NAME:
- if (info == NULL)
- errmsg = _("missing name information");
- else
- errmsg = _("missing name information in %s");
- break;
- case VIR_ERR_NO_OS:
- if (info == NULL)
- errmsg = _("missing operating system information");
- else
- errmsg = _("missing operating system information for %s");
- break;
- case VIR_ERR_NO_DEVICE:
- if (info == NULL)
- errmsg = _("missing devices information");
- else
- errmsg = _("missing devices information for %s");
- break;
- case VIR_ERR_DRIVER_FULL:
- if (info == NULL)
- errmsg = _("too many drivers registered");
- else
- errmsg = _("too many drivers registered in %s");
- break;
- case VIR_ERR_CALL_FAILED: /* DEPRECATED, use VIR_ERR_NO_SUPPORT */
- if (info == NULL)
- errmsg = _("library call failed");
- else
- errmsg = _("library call failed: %s");
- break;
- case VIR_ERR_XML_ERROR:
- if (info == NULL)
- errmsg = _("XML description is invalid or not well formed");
- else
- errmsg = _("XML error: %s");
- break;
- case VIR_ERR_DOM_EXIST:
- if (info == NULL)
- errmsg = _("this domain exists already");
- else
- errmsg = _("domain %s exists already");
- break;
- case VIR_ERR_OPERATION_DENIED:
- if (info == NULL)
- errmsg = _("operation forbidden for read only access");
- else
- errmsg = _("operation forbidden: %s");
- break;
- case VIR_ERR_OPEN_FAILED:
- if (info == NULL)
- errmsg = _("failed to open configuration file");
- else
- errmsg = _("failed to open configuration file %s");
- break;
- case VIR_ERR_READ_FAILED:
- if (info == NULL)
- errmsg = _("failed to read configuration file");
- else
- errmsg = _("failed to read configuration file %s");
- break;
- case VIR_ERR_PARSE_FAILED:
- if (info == NULL)
- errmsg = _("failed to parse configuration file");
- else
- errmsg = _("failed to parse configuration file %s");
- break;
- case VIR_ERR_CONF_SYNTAX:
- if (info == NULL)
- errmsg = _("configuration file syntax error");
- else
- errmsg = _("configuration file syntax error: %s");
- break;
- case VIR_ERR_WRITE_FAILED:
- if (info == NULL)
- errmsg = _("failed to write configuration file");
- else
- errmsg = _("failed to write configuration file: %s");
- break;
- case VIR_ERR_XML_DETAIL:
- if (info == NULL)
- errmsg = _("parser error");
- else
- errmsg = "%s";
- break;
- case VIR_ERR_INVALID_NETWORK:
- if (info == NULL)
- errmsg = _("invalid network pointer in");
- else
- errmsg = _("invalid network pointer in %s");
- break;
- case VIR_ERR_NETWORK_EXIST:
- if (info == NULL)
- errmsg = _("this network exists already");
- else
- errmsg = _("network %s exists already");
- break;
- case VIR_ERR_SYSTEM_ERROR:
- if (info == NULL)
- errmsg = _("system call error");
- else
- errmsg = "%s";
- break;
- case VIR_ERR_RPC:
- if (info == NULL)
- errmsg = _("RPC error");
- else
- errmsg = "%s";
- break;
- case VIR_ERR_GNUTLS_ERROR:
- if (info == NULL)
- errmsg = _("GNUTLS call error");
- else
- errmsg = "%s";
- break;
- case VIR_WAR_NO_NETWORK:
- if (info == NULL)
- errmsg = _("Failed to find the network");
- else
- errmsg = _("Failed to find the network: %s");
- break;
- case VIR_ERR_NO_DOMAIN:
- if (info == NULL)
- errmsg = _("Domain not found");
- else
- errmsg = _("Domain not found: %s");
- break;
- case VIR_ERR_NO_NETWORK:
- if (info == NULL)
- errmsg = _("Network not found");
- else
- errmsg = _("Network not found: %s");
- break;
- case VIR_ERR_INVALID_MAC:
- if (info == NULL)
- errmsg = _("invalid MAC address");
- else
- errmsg = _("invalid MAC address: %s");
- break;
- case VIR_ERR_AUTH_FAILED:
- if (info == NULL)
- errmsg = _("authentication failed");
- else
- errmsg = _("authentication failed: %s");
- break;
- case VIR_ERR_AUTH_CANCELLED:
- if (info == NULL)
- errmsg = _("authentication cancelled");
- else
- errmsg = _("authentication cancelled: %s");
- break;
- case VIR_ERR_AUTH_UNAVAILABLE:
- if (info == NULL)
- errmsg = _("authentication unavailable");
- else
- errmsg = _("authentication unavailable: %s");
- break;
- case VIR_ERR_NO_STORAGE_POOL:
- if (info == NULL)
- errmsg = _("Storage pool not found");
- else
- errmsg = _("Storage pool not found: %s");
- break;
- case VIR_ERR_NO_STORAGE_VOL:
- if (info == NULL)
- errmsg = _("Storage volume not found");
- else
- errmsg = _("Storage volume not found: %s");
- break;
- case VIR_ERR_STORAGE_VOL_EXIST:
- if (info == NULL)
- errmsg = _("this storage volume exists already");
- else
- errmsg = _("storage volume %s exists already");
- break;
- case VIR_ERR_STORAGE_PROBE_FAILED:
- if (info == NULL)
- errmsg = _("Storage pool probe failed");
- else
- errmsg = _("Storage pool probe failed: %s");
- break;
- case VIR_ERR_STORAGE_POOL_BUILT:
- if (info == NULL)
- errmsg = _("Storage pool already built");
- else
- errmsg = _("Storage pool already built: %s");
- break;
- case VIR_ERR_INVALID_STORAGE_POOL:
- if (info == NULL)
- errmsg = _("invalid storage pool pointer in");
- else
- errmsg = _("invalid storage pool pointer in %s");
- break;
- case VIR_ERR_INVALID_STORAGE_VOL:
- if (info == NULL)
- errmsg = _("invalid storage volume pointer in");
- else
- errmsg = _("invalid storage volume pointer in %s");
- break;
- case VIR_WAR_NO_STORAGE:
- if (info == NULL)
- errmsg = _("Failed to find a storage driver");
- else
- errmsg = _("Failed to find a storage driver: %s");
- break;
- case VIR_WAR_NO_NODE:
- if (info == NULL)
- errmsg = _("Failed to find a node driver");
- else
- errmsg = _("Failed to find a node driver: %s");
- break;
- case VIR_ERR_INVALID_NODE_DEVICE:
- if (info == NULL)
- errmsg = _("invalid node device pointer");
- else
- errmsg = _("invalid node device pointer in %s");
- break;
- case VIR_ERR_NO_NODE_DEVICE:
- if (info == NULL)
- errmsg = _("Node device not found");
- else
- errmsg = _("Node device not found: %s");
- break;
- case VIR_ERR_NO_SECURITY_MODEL:
- if (info == NULL)
- errmsg = _("Security model not found");
- else
- errmsg = _("Security model not found: %s");
- break;
- case VIR_ERR_OPERATION_INVALID:
- if (info == NULL)
- errmsg = _("Requested operation is not valid");
- else
- errmsg = _("Requested operation is not valid: %s");
- break;
- case VIR_WAR_NO_INTERFACE:
- if (info == NULL)
- errmsg = _("Failed to find the interface");
- else
- errmsg = _("Failed to find the interface: %s");
- break;
- case VIR_ERR_NO_INTERFACE:
- if (info == NULL)
- errmsg = _("Interface not found");
- else
- errmsg = _("Interface not found: %s");
- break;
- case VIR_ERR_INVALID_INTERFACE:
- if (info == NULL)
- errmsg = _("invalid interface pointer in");
- else
- errmsg = _("invalid interface pointer in %s");
- break;
- case VIR_ERR_MULTIPLE_INTERFACES:
- if (info == NULL)
- errmsg = _("multiple matching interfaces found");
- else
- errmsg = _("multiple matching interfaces found: %s");
- break;
- case VIR_WAR_NO_SECRET:
- if (info == NULL)
- errmsg = _("Failed to find a secret storage driver");
- else
- errmsg = _("Failed to find a secret storage driver: %s");
- break;
- case VIR_ERR_INVALID_SECRET:
- if (info == NULL)
- errmsg = _("Invalid secret");
- else
- errmsg = _("Invalid secret: %s");
- break;
- case VIR_ERR_NO_SECRET:
- if (info == NULL)
- errmsg = _("Secret not found");
- else
- errmsg = _("Secret not found: %s");
- break;
- case VIR_WAR_NO_NWFILTER:
- if (info == NULL)
- errmsg = _("Failed to start the nwfilter driver");
- else
- errmsg = _("Failed to start the nwfilter driver: %s");
- break;
- case VIR_ERR_INVALID_NWFILTER:
- if (info == NULL)
- errmsg = _("Invalid network filter");
- else
- errmsg = _("Invalid network filter: %s");
- break;
- case VIR_ERR_NO_NWFILTER:
- if (info == NULL)
- errmsg = _("Network filter not found");
- else
- errmsg = _("Network filter not found: %s");
- break;
- case VIR_ERR_BUILD_FIREWALL:
- if (info == NULL)
- errmsg = _("Error while building firewall");
- else
- errmsg = _("Error while building firewall: %s");
- break;
- case VIR_ERR_CONFIG_UNSUPPORTED:
- if (info == NULL)
- errmsg = _("unsupported configuration");
- else
- errmsg = _("unsupported configuration: %s");
- break;
- case VIR_ERR_OPERATION_TIMEOUT:
- if (info == NULL)
- errmsg = _("Timed out during operation");
- else
- errmsg = _("Timed out during operation: %s");
- break;
- case VIR_ERR_MIGRATE_PERSIST_FAILED:
- if (info == NULL)
- errmsg = _("Failed to make domain persistent after
migration");
- else
- errmsg = _("Failed to make domain persistent after migration:
%s");
- break;
- case VIR_ERR_HOOK_SCRIPT_FAILED:
- if (info == NULL)
- errmsg = _("Hook script execution failed");
- else
- errmsg = _("Hook script execution failed: %s");
- break;
- case VIR_ERR_INVALID_DOMAIN_SNAPSHOT:
- if (info == NULL)
- errmsg = _("Invalid snapshot");
- else
- errmsg = _("Invalid snapshot: %s");
- break;
- case VIR_ERR_NO_DOMAIN_SNAPSHOT:
- if (info == NULL)
- errmsg = _("Domain snapshot not found");
- else
- errmsg = _("Domain snapshot not found: %s");
- break;
- case VIR_ERR_INVALID_STREAM:
- if (info == NULL)
- errmsg = _("invalid stream pointer");
- else
- errmsg = _("invalid stream pointer in %s");
- break;
- case VIR_ERR_ARGUMENT_UNSUPPORTED:
- if (info == NULL)
- errmsg = _("argument unsupported");
- else
- errmsg = _("argument unsupported: %s");
- break;
- case VIR_ERR_SNAPSHOT_REVERT_RISKY:
- if (info == NULL)
- errmsg = _("revert requires force");
- else
- errmsg = _("revert requires force: %s");
- break;
- case VIR_ERR_OPERATION_ABORTED:
- if (info == NULL)
- errmsg = _("operation aborted");
- else
- errmsg = _("operation aborted: %s");
- break;
- case VIR_ERR_NO_DOMAIN_METADATA:
- if (info == NULL)
- errmsg = _("metadata not found");
- else
- errmsg = _("metadata not found: %s");
- break;
- case VIR_ERR_MIGRATE_UNSAFE:
- if (!info)
- errmsg = _("Unsafe migration");
- else
- errmsg = _("Unsafe migration: %s");
- break;
- case VIR_ERR_OVERFLOW:
- if (!info)
- errmsg = _("numerical overflow");
- else
- errmsg = _("numerical overflow: %s");
- break;
- case VIR_ERR_BLOCK_COPY_ACTIVE:
- if (!info)
- errmsg = _("block copy still active");
- else
- errmsg = _("block copy still active: %s");
- break;
- case VIR_ERR_OPERATION_UNSUPPORTED:
- if (!info)
- errmsg = _("Operation not supported");
- else
- errmsg = _("Operation not supported: %s");
- break;
- case VIR_ERR_SSH:
- if (info == NULL)
- errmsg = _("SSH transport error");
- else
- errmsg = _("SSH transport error: %s");
- break;
- case VIR_ERR_AGENT_UNRESPONSIVE:
- if (info == NULL)
- errmsg = _("Guest agent is not responding");
- else
- errmsg = _("Guest agent is not responding: %s");
- break;
- case VIR_ERR_RESOURCE_BUSY:
- if (info == NULL)
- errmsg = _("resource busy");
- else
- errmsg = _("resource busy: %s");
- break;
- case VIR_ERR_ACCESS_DENIED:
- if (info == NULL)
- errmsg = _("access denied");
- else
- errmsg = _("access denied: %s");
- break;
- case VIR_ERR_DBUS_SERVICE:
- if (info == NULL)
- errmsg = _("error from service");
- else
- errmsg = _("error from service: %s");
- break;
- case VIR_ERR_CPU_INCOMPATIBLE:
- if (info == NULL)
- errmsg = _("the CPU is incompatible with host CPU");
- else
- errmsg = _("the CPU is incompatible with host CPU: %s");
- break;
- case VIR_ERR_XML_INVALID_SCHEMA:
- if (info == NULL)
- errmsg = _("XML document failed to validate against schema");
- else
- errmsg = _("XML document failed to validate against schema:
%s");
- break;
- case VIR_ERR_MIGRATE_FINISH_OK:
- if (info == NULL)
- errmsg = _("migration successfully aborted");
- else
- errmsg = _("migration successfully aborted: %s");
- break;
- case VIR_ERR_NO_SERVER:
- if (info == NULL)
- errmsg = _("Server not found");
- else
- errmsg = _("Server not found: %s");
- break;
- case VIR_ERR_NO_CLIENT:
- if (info == NULL)
- errmsg = _("Client not found");
- else
- errmsg = _("Client not found: %s");
- break;
- case VIR_ERR_AGENT_UNSYNCED: /* DEPRECATED */
- if (info == NULL)
- errmsg = _("guest agent replied with wrong id to guest-sync
command");
- else
- errmsg = _("guest agent replied with wrong id to guest-sync command:
%s");
- break;
- case VIR_ERR_LIBSSH:
- if (info == NULL)
- errmsg = _("libssh transport error");
- else
- errmsg = _("libssh transport error: %s");
- break;
- case VIR_ERR_DEVICE_MISSING:
- if (info == NULL)
- errmsg = _("device not found");
- else
- errmsg = _("device not found: %s");
- break;
- case VIR_ERR_INVALID_NWFILTER_BINDING:
- if (info == NULL)
- errmsg = _("Invalid network filter binding");
- else
- errmsg = _("Invalid network filter binding: %s");
- break;
- case VIR_ERR_NO_NWFILTER_BINDING:
- if (info == NULL)
- errmsg = _("Network filter binding not found");
- else
- errmsg = _("Network filter binding not found: %s");
- break;
- }
- return errmsg;
+ if (info)
+ return _(virErrorMsgStrings[error].msginfo);
+ else
+ return _(virErrorMsgStrings[error].msg);
}
+
/**
* virReportErrorHelper:
*
--
2.19.2
5:02 p.m.
New subject: [libvirt] [PATCH v2 9/9] util: error: Put error code messages into an array
On Thu, Dec 13, 2018 at 03:48:56PM +0100, Peter Krempa wrote:
Simplify adding of new errors by just adding them to the array of
messages rather than having to add conversion code.
Additionally most of the messages add the format string part as a suffix
so we can avoid some of the duplication by using a macro which adds the
suffix to the original string. This way most messages fit into the 80
column limit and only 3 exceed 100 colums.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
Notes:
v2:
- use positional initializers
- add macros for shortening the messages
- make it gettext-friendly, since the last version was not
src/libvirt_private.syms | 1 +
src/util/virerror.c | 738 +++++++--------------------------------
2 files changed, 126 insertions(+), 613 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 6184030d59..775b33e151 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1753,6 +1753,7 @@ virDispatchError;
virErrorCopyNew;
virErrorInitialize;
virErrorMsg;
+virErrorMsgStrings;
virErrorPreserveLast;
virErrorRestore;
virErrorSetErrnoFromLastError;
diff --git a/src/util/virerror.c b/src/util/virerror.c
index 03166d85d2..d3f1d7d0e1 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -904,6 +904,124 @@ void virRaiseErrorObject(const char *filename,
}
+typedef struct {
+ const char *msg;
+ const char *msginfo;
+} virErrorMsgTuple;
+
+#define MSG(msg, sfx) \
+ { N_(msg), N_(msg sfx) }
So ^this is the majority of messages, therefore I think we could introduce one
more macro MSG_FULL which the ones you introduced could link to and we might
get rid of the ": %s" suffix which is repeated over and over again.
Since you incorporated Dan's points, there are no further comments from my
side:
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
10:35 a.m.
New subject: [libvirt] [PATCH v2 9/9] util: error: Put error code messages into an array
On Thu, Dec 13, 2018 at 17:02:55 +0100, Erik Skultety wrote:
On Thu, Dec 13, 2018 at 03:48:56PM +0100, Peter Krempa wrote:
> Simplify adding of new errors by just adding them to the array of
> messages rather than having to add conversion code.
>
> Additionally most of the messages add the format string part as a suffix
> so we can avoid some of the duplication by using a macro which adds the
> suffix to the original string. This way most messages fit into the 80
> column limit and only 3 exceed 100 colums.
>
> Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
> ---
>
> Notes:
> v2:
> - use positional initializers
> - add macros for shortening the messages
> - make it gettext-friendly, since the last version was not
>
> src/libvirt_private.syms | 1 +
> src/util/virerror.c | 738 +++++++--------------------------------
> 2 files changed, 126 insertions(+), 613 deletions(-)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 6184030d59..775b33e151 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1753,6 +1753,7 @@ virDispatchError;
> virErrorCopyNew;
> virErrorInitialize;
> virErrorMsg;
> +virErrorMsgStrings;
This hunk will be dropped.
> virErrorPreserveLast;
> virErrorRestore;
> virErrorSetErrnoFromLastError;
> diff --git a/src/util/virerror.c b/src/util/virerror.c
> index 03166d85d2..d3f1d7d0e1 100644
> --- a/src/util/virerror.c
> +++ b/src/util/virerror.c
> @@ -904,6 +904,124 @@ void virRaiseErrorObject(const char *filename,
> }
>
>
> +typedef struct {
> + const char *msg;
> + const char *msginfo;
> +} virErrorMsgTuple;
> +
> +#define MSG(msg, sfx) \
> + { N_(msg), N_(msg sfx) }
So ^this is the majority of messages, therefore I think we could introduce one
more macro MSG_FULL which the ones you introduced could link to and we might
get rid of the ": %s" suffix which is repeated over and over again.
Soo, will it be okay with the following macro definitions? I've also
added some explanation:
/**
* These macros expand to the following format of error message:
* MSG2("error message", " suffix %s")
* - no info: "error message"
* - info: "error message suffix %s"
*
* MSG("error message")
* - no info: "error message"
* - info: "error message: %s"
*
* MSG_EXISTS("sausage")
* - no info: "this sausage exists already"
* - info: "sausage %s exists already"
*/
#define MSG2(msg, sfx) \
{ N_(msg), N_(msg sfx) }
#define MSG(msg) \
MSG2(msg, ": %s")
#define MSG_EXISTS(object) \
{ N_("this " object " exists already"), N_(object " %s exists
already") }
Followed by the appropriate changes:
[VIR_ERR_NO_MEMORY] = MSG("out of memory"),
[VIR_ERR_NO_CONNECT] = MSG2("no connection driver available", " for
%s"),
Since you incorporated Dan's points, there are no further comments from my
side:
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
10:38 a.m.
New subject: [libvirt] [PATCH v2 9/9] util: error: Put error code messages into an array
On Fri, Dec 14, 2018 at 10:35:50AM +0100, Peter Krempa wrote:
On Thu, Dec 13, 2018 at 17:02:55 +0100, Erik Skultety wrote:
> On Thu, Dec 13, 2018 at 03:48:56PM +0100, Peter Krempa wrote:
> > Simplify adding of new errors by just adding them to the array of
> > messages rather than having to add conversion code.
> >
> > Additionally most of the messages add the format string part as a suffix
> > so we can avoid some of the duplication by using a macro which adds the
> > suffix to the original string. This way most messages fit into the 80
> > column limit and only 3 exceed 100 colums.
> >
> > Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
> > ---
> >
> > Notes:
> > v2:
> > - use positional initializers
> > - add macros for shortening the messages
> > - make it gettext-friendly, since the last version was not
> >
> > src/libvirt_private.syms | 1 +
> > src/util/virerror.c | 738 +++++++--------------------------------
> > 2 files changed, 126 insertions(+), 613 deletions(-)
> >
> > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> > index 6184030d59..775b33e151 100644
> > --- a/src/libvirt_private.syms
> > +++ b/src/libvirt_private.syms
> > @@ -1753,6 +1753,7 @@ virDispatchError;
> > virErrorCopyNew;
> > virErrorInitialize;
> > virErrorMsg;
> > +virErrorMsgStrings;
This hunk will be dropped.
> > virErrorPreserveLast;
> > virErrorRestore;
> > virErrorSetErrnoFromLastError;
> > diff --git a/src/util/virerror.c b/src/util/virerror.c
> > index 03166d85d2..d3f1d7d0e1 100644
> > --- a/src/util/virerror.c
> > +++ b/src/util/virerror.c
> > @@ -904,6 +904,124 @@ void virRaiseErrorObject(const char *filename,
> > }
> >
> >
> > +typedef struct {
> > + const char *msg;
> > + const char *msginfo;
> > +} virErrorMsgTuple;
> > +
> > +#define MSG(msg, sfx) \
> > + { N_(msg), N_(msg sfx) }
>
> So ^this is the majority of messages, therefore I think we could introduce one
> more macro MSG_FULL which the ones you introduced could link to and we might
> get rid of the ": %s" suffix which is repeated over and over again.
Soo, will it be okay with the following macro definitions? I've also
added some explanation:
/**
* These macros expand to the following format of error message:
* MSG2("error message", " suffix %s")
* - no info: "error message"
* - info: "error message suffix %s"
*
* MSG("error message")
* - no info: "error message"
* - info: "error message: %s"
*
* MSG_EXISTS("sausage")
* - no info: "this sausage exists already"
* - info: "sausage %s exists already"
*/
#define MSG2(msg, sfx) \
{ N_(msg), N_(msg sfx) }
#define MSG(msg) \
MSG2(msg, ": %s")
if you make ^this N_(msg) then sure.
Erik
2106
days inactive
2107
days old
13 comments
2 participants
participants (2)
-
Erik Skultety -
Peter Krempa