* [PATCH v2 0/6] misc: Improve error reporting on Windows
@ 2020-02-27 16:30 Philippe Mathieu-Daudé
2020-02-27 16:30 ` [PATCH v2 1/6] chardev: Improve error report by calling error_setg_win32() Philippe Mathieu-Daudé
` (5 more replies)
0 siblings, 6 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-02-27 16:30 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Stefan Weil, Markus Armbruster, Michael Roth,
Tomáš Golembiovský,
Stefan Hajnoczi, Marc-André Lureau, Paolo Bonzini,
Philippe Mathieu-Daudé
Two patches to improve bug reports on Windows.
(i.e. https://bugs.launchpad.net/qemu/+bug/1657841)
Since v1:
- Rebase an old patch from Alistair to use error_report()
as suggested by Markus
- Fix other uses in util/osdep.c and QEMU guest-agent
Alistair Francis (1):
util: Replace fprintf(stderr, "*\n" with error_report()
Philippe Mathieu-Daudé (5):
chardev: Improve error report by calling error_setg_win32()
util/oslib-win32: Improve error report by calling error_setg_win32()
util/osdep: Improve error report by calling error_setg_win32()
qga: Fix a memory leak
qga: Improve error report by calling error_setg_win32()
chardev/char-pipe.c | 2 +-
chardev/char-win.c | 2 +-
qga/channel-win32.c | 7 ++++---
qga/commands-win32.c | 6 +++---
util/coroutine-sigaltstack.c | 3 ++-
util/mmap-alloc.c | 11 ++++++-----
util/module.c | 13 ++++++-------
util/osdep.c | 12 ++++++------
util/oslib-posix.c | 3 ++-
util/oslib-win32.c | 4 +++-
util/qemu-coroutine.c | 10 +++++-----
util/qemu-thread-posix.c | 5 +++--
util/qemu-thread-win32.c | 5 +++--
util/qemu-timer-common.c | 3 ++-
14 files changed, 47 insertions(+), 39 deletions(-)
--
2.21.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 1/6] chardev: Improve error report by calling error_setg_win32()
2020-02-27 16:30 [PATCH v2 0/6] misc: Improve error reporting on Windows Philippe Mathieu-Daudé
@ 2020-02-27 16:30 ` Philippe Mathieu-Daudé
2020-02-27 16:30 ` [PATCH v2 2/6] util: Replace fprintf(stderr, "*\n" with error_report() Philippe Mathieu-Daudé
` (4 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-02-27 16:30 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Daniel P . Berrangé,
Stefan Weil, Markus Armbruster, Michael Roth,
Tomáš Golembiovský,
Stefan Hajnoczi, Marc-André Lureau, Paolo Bonzini,
Philippe Mathieu-Daudé
Use error_setg_win32() which adds a hint similar to strerror(errno)).
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
chardev/char-pipe.c | 2 +-
chardev/char-win.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/chardev/char-pipe.c b/chardev/char-pipe.c
index 94d714ffcd..fd12c9e63b 100644
--- a/chardev/char-pipe.c
+++ b/chardev/char-pipe.c
@@ -70,7 +70,7 @@ static int win_chr_pipe_init(Chardev *chr, const char *filename,
MAXCONNECT, NSENDBUF, NRECVBUF, NTIMEOUT, NULL);
g_free(openname);
if (s->file == INVALID_HANDLE_VALUE) {
- error_setg(errp, "Failed CreateNamedPipe (%lu)", GetLastError());
+ error_setg_win32(errp, GetLastError(), "Failed CreateNamedPipe");
s->file = NULL;
goto fail;
}
diff --git a/chardev/char-win.c b/chardev/char-win.c
index 34825f683d..d4fb44c4dc 100644
--- a/chardev/char-win.c
+++ b/chardev/char-win.c
@@ -96,7 +96,7 @@ int win_chr_serial_init(Chardev *chr, const char *filename, Error **errp)
s->file = CreateFile(filename, GENERIC_READ | GENERIC_WRITE, 0, NULL,
OPEN_EXISTING, FILE_FLAG_OVERLAPPED, 0);
if (s->file == INVALID_HANDLE_VALUE) {
- error_setg(errp, "Failed CreateFile (%lu)", GetLastError());
+ error_setg_win32(errp, GetLastError(), "Failed CreateFile");
s->file = NULL;
goto fail;
}
--
2.21.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 2/6] util: Replace fprintf(stderr, "*\n" with error_report()
2020-02-27 16:30 [PATCH v2 0/6] misc: Improve error reporting on Windows Philippe Mathieu-Daudé
2020-02-27 16:30 ` [PATCH v2 1/6] chardev: Improve error report by calling error_setg_win32() Philippe Mathieu-Daudé
@ 2020-02-27 16:30 ` Philippe Mathieu-Daudé
2020-02-28 7:43 ` Markus Armbruster
2020-02-27 16:30 ` [PATCH v2 3/6] util/oslib-win32: Improve error report by calling error_setg_win32() Philippe Mathieu-Daudé
` (3 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-02-27 16:30 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Stefan Weil, Alistair Francis, Markus Armbruster,
Michael Roth, Tomáš Golembiovský,
Alistair Francis, Stefan Hajnoczi, Marc-André Lureau,
Paolo Bonzini, Alistair Francis, Philippe Mathieu-Daudé,
Philippe Mathieu-Daudé
From: Alistair Francis <alistair.francis@xilinx.com>
Replace a large number of the fprintf(stderr, "*\n" calls with
error_report(). The functions were renamed with these commands and then
compiler issues where manually fixed.
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
The error in aio_poll() was removed manually.
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <f71203227749e2afb8564b3388b2b34f6652b009.1510181732.git.alistair.francis@xilinx.com>
Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
[PMD: Rebased]
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
Cc: Alistair Francis <Alistair.Francis@wdc.com>
Cc: Alistair Francis <alistair@alistair23.me>
---
util/coroutine-sigaltstack.c | 3 ++-
util/mmap-alloc.c | 11 ++++++-----
util/module.c | 13 ++++++-------
util/osdep.c | 4 ++--
util/oslib-posix.c | 3 ++-
util/oslib-win32.c | 3 ++-
util/qemu-coroutine.c | 10 +++++-----
util/qemu-thread-posix.c | 5 +++--
util/qemu-thread-win32.c | 5 +++--
util/qemu-timer-common.c | 3 ++-
10 files changed, 33 insertions(+), 27 deletions(-)
diff --git a/util/coroutine-sigaltstack.c b/util/coroutine-sigaltstack.c
index f6fc49a0e5..63decd4d1d 100644
--- a/util/coroutine-sigaltstack.c
+++ b/util/coroutine-sigaltstack.c
@@ -29,6 +29,7 @@
#include <pthread.h>
#include "qemu-common.h"
#include "qemu/coroutine_int.h"
+#include "qemu/error-report.h"
typedef struct {
Coroutine base;
@@ -80,7 +81,7 @@ static void __attribute__((constructor)) coroutine_init(void)
ret = pthread_key_create(&thread_state_key, qemu_coroutine_thread_cleanup);
if (ret != 0) {
- fprintf(stderr, "unable to create leader key: %s\n", strerror(errno));
+ error_report("unable to create leader key: %s", strerror(errno));
abort();
}
}
diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c
index 27dcccd8ec..3ac6e10404 100644
--- a/util/mmap-alloc.c
+++ b/util/mmap-alloc.c
@@ -18,6 +18,7 @@
#endif /* CONFIG_LINUX */
#include "qemu/osdep.h"
+#include "qemu/error-report.h"
#include "qemu/mmap-alloc.h"
#include "qemu/host-utils.h"
@@ -63,7 +64,7 @@ size_t qemu_mempath_getpagesize(const char *mem_path)
} while (ret != 0 && errno == EINTR);
if (ret != 0) {
- fprintf(stderr, "Couldn't statfs() memory path: %s\n",
+ error_report("Couldn't statfs() memory path: %s",
strerror(errno));
exit(1);
}
@@ -160,10 +161,10 @@ void *qemu_ram_mmap(int fd,
len = 0;
}
file_name[len] = '\0';
- fprintf(stderr, "Warning: requesting persistence across crashes "
- "for backend file %s failed. Proceeding without "
- "persistence, data might become corrupted in case of host "
- "crash.\n", file_name);
+ error_report("Warning: requesting persistence across crashes "
+ "for backend file %s failed. Proceeding without "
+ "persistence, data might become corrupted in case "
+ "of host crash.", file_name);
g_free(proc_link);
g_free(file_name);
}
diff --git a/util/module.c b/util/module.c
index 236a7bb52a..28efa1f891 100644
--- a/util/module.c
+++ b/util/module.c
@@ -19,6 +19,7 @@
#endif
#include "qemu/queue.h"
#include "qemu/module.h"
+#include "qemu/error-report.h"
typedef struct ModuleEntry
{
@@ -130,19 +131,17 @@ static int module_load_file(const char *fname)
g_module = g_module_open(fname, G_MODULE_BIND_LAZY | G_MODULE_BIND_LOCAL);
if (!g_module) {
- fprintf(stderr, "Failed to open module: %s\n",
- g_module_error());
+ error_report("Failed to open module: %s", g_module_error());
ret = -EINVAL;
goto out;
}
if (!g_module_symbol(g_module, DSO_STAMP_FUN_STR, (gpointer *)&sym)) {
- fprintf(stderr, "Failed to initialize module: %s\n",
- fname);
+ error_report("Failed to initialize module: %s", fname);
/* Print some info if this is a QEMU module (but from different build),
* this will make debugging user problems easier. */
if (g_module_symbol(g_module, "qemu_module_dummy", (gpointer *)&sym)) {
- fprintf(stderr,
- "Note: only modules from the same build can be loaded.\n");
+ error_report("Note: "
+ "only modules from the same build can be loaded.");
}
g_module_close(g_module);
ret = -EINVAL;
@@ -178,7 +177,7 @@ bool module_load_one(const char *prefix, const char *lib_name)
static GHashTable *loaded_modules;
if (!g_module_supported()) {
- fprintf(stderr, "Module is not supported by system.\n");
+ error_report("Module is not supported by system.");
return false;
}
diff --git a/util/osdep.c b/util/osdep.c
index f7d06050f7..ef40ae512a 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -484,7 +484,7 @@ void fips_set_state(bool requested)
#endif /* __linux__ */
#ifdef _FIPS_DEBUG
- fprintf(stderr, "FIPS mode %s (requested %s)\n",
+ error_report("FIPS mode %s (requested %s)",
(fips_enabled ? "enabled" : "disabled"),
(requested ? "enabled" : "disabled"));
#endif
@@ -511,7 +511,7 @@ int socket_init(void)
ret = WSAStartup(MAKEWORD(2, 2), &Data);
if (ret != 0) {
err = WSAGetLastError();
- fprintf(stderr, "WSAStartup: %d\n", err);
+ error_report("WSAStartup: %d", err);
return -1;
}
atexit(socket_cleanup);
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 897e8f3ba6..4977594a43 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -35,6 +35,7 @@
#include "sysemu/sysemu.h"
#include "trace.h"
#include "qapi/error.h"
+#include "qemu/error-report.h"
#include "qemu/sockets.h"
#include "qemu/thread.h"
#include <libgen.h>
@@ -170,7 +171,7 @@ fail_close:
void *qemu_oom_check(void *ptr)
{
if (ptr == NULL) {
- fprintf(stderr, "Failed to allocate memory: %s\n", strerror(errno));
+ error_report("Failed to allocate memory: %s", strerror(errno));
abort();
}
return ptr;
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index e9b14ab178..84b937865a 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -39,6 +39,7 @@
#include "trace.h"
#include "qemu/sockets.h"
#include "qemu/cutils.h"
+#include "qemu/error-report.h"
/* this must come after including "trace.h" */
#include <shlobj.h>
@@ -46,7 +47,7 @@
void *qemu_oom_check(void *ptr)
{
if (ptr == NULL) {
- fprintf(stderr, "Failed to allocate memory: %lu\n", GetLastError());
+ error_report("Failed to allocate memory: %lu", GetLastError());
abort();
}
return ptr;
diff --git a/util/qemu-coroutine.c b/util/qemu-coroutine.c
index c3caa6c770..62d1dd09df 100644
--- a/util/qemu-coroutine.c
+++ b/util/qemu-coroutine.c
@@ -14,6 +14,7 @@
#include "qemu/osdep.h"
#include "trace.h"
+#include "qemu/error-report.h"
#include "qemu/thread.h"
#include "qemu/atomic.h"
#include "qemu/coroutine.h"
@@ -125,14 +126,13 @@ void qemu_aio_coroutine_enter(AioContext *ctx, Coroutine *co)
* cause us to enter it twice, potentially even after the coroutine has
* been deleted */
if (scheduled) {
- fprintf(stderr,
- "%s: Co-routine was already scheduled in '%s'\n",
- __func__, scheduled);
+ error_report("%s: Co-routine was already scheduled in '%s'",
+ __func__, scheduled);
abort();
}
if (to->caller) {
- fprintf(stderr, "Co-routine re-entered recursively\n");
+ error_report("Co-routine re-entered recursively");
abort();
}
@@ -185,7 +185,7 @@ void coroutine_fn qemu_coroutine_yield(void)
trace_qemu_coroutine_yield(self, to);
if (!to) {
- fprintf(stderr, "Co-routine is yielding to no one\n");
+ error_report("Co-routine is yielding to no one");
abort();
}
diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
index 838980aaa5..b4d8de376c 100644
--- a/util/qemu-thread-posix.c
+++ b/util/qemu-thread-posix.c
@@ -14,6 +14,7 @@
#include "qemu/thread.h"
#include "qemu/atomic.h"
#include "qemu/notify.h"
+#include "qemu/error-report.h"
#include "qemu-thread-common.h"
static bool name_threads;
@@ -25,14 +26,14 @@ void qemu_thread_naming(bool enable)
#ifndef CONFIG_THREAD_SETNAME_BYTHREAD
/* This is a debugging option, not fatal */
if (enable) {
- fprintf(stderr, "qemu: thread naming not supported on this host\n");
+ error_report("qemu: thread naming not supported on this host");
}
#endif
}
static void error_exit(int err, const char *msg)
{
- fprintf(stderr, "qemu: %s: %s\n", msg, strerror(err));
+ error_report("qemu: %s: %s", msg, strerror(err));
abort();
}
diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c
index 56a83333da..9bed338d7e 100644
--- a/util/qemu-thread-win32.c
+++ b/util/qemu-thread-win32.c
@@ -15,6 +15,7 @@
#include "qemu-common.h"
#include "qemu/thread.h"
#include "qemu/notify.h"
+#include "qemu/error-report.h"
#include "qemu-thread-common.h"
#include <process.h>
@@ -25,7 +26,7 @@ void qemu_thread_naming(bool enable)
/* But note we don't actually name them on Windows yet */
name_threads = enable;
- fprintf(stderr, "qemu: thread naming not supported on this host\n");
+ error_report("qemu: thread naming not supported on this host");
}
static void error_exit(int err, const char *msg)
@@ -34,7 +35,7 @@ static void error_exit(int err, const char *msg)
FormatMessage(FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_ALLOCATE_BUFFER,
NULL, err, 0, (LPTSTR)&pstr, 2, NULL);
- fprintf(stderr, "qemu: %s: %s\n", msg, pstr);
+ error_report("qemu: %s: %s", msg, pstr);
LocalFree(pstr);
abort();
}
diff --git a/util/qemu-timer-common.c b/util/qemu-timer-common.c
index baf3317f74..527944da1c 100644
--- a/util/qemu-timer-common.c
+++ b/util/qemu-timer-common.c
@@ -23,6 +23,7 @@
*/
#include "qemu/osdep.h"
#include "qemu/timer.h"
+#include "qemu/error-report.h"
/***********************************************************/
/* real time host monotonic timer */
@@ -37,7 +38,7 @@ static void __attribute__((constructor)) init_get_clock(void)
int ret;
ret = QueryPerformanceFrequency(&freq);
if (ret == 0) {
- fprintf(stderr, "Could not calibrate ticks\n");
+ error_report("Could not calibrate ticks");
exit(1);
}
clock_freq = freq.QuadPart;
--
2.21.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 3/6] util/oslib-win32: Improve error report by calling error_setg_win32()
2020-02-27 16:30 [PATCH v2 0/6] misc: Improve error reporting on Windows Philippe Mathieu-Daudé
2020-02-27 16:30 ` [PATCH v2 1/6] chardev: Improve error report by calling error_setg_win32() Philippe Mathieu-Daudé
2020-02-27 16:30 ` [PATCH v2 2/6] util: Replace fprintf(stderr, "*\n" with error_report() Philippe Mathieu-Daudé
@ 2020-02-27 16:30 ` Philippe Mathieu-Daudé
2020-02-27 16:30 ` [PATCH v2 4/6] util/osdep: " Philippe Mathieu-Daudé
` (2 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-02-27 16:30 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Daniel P . Berrangé,
Stefan Weil, Markus Armbruster, Michael Roth,
Tomáš Golembiovský,
Stefan Hajnoczi, Marc-André Lureau, Paolo Bonzini,
Philippe Mathieu-Daudé
Use error_setg_win32() which adds a hint similar to strerror(errno)).
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
util/oslib-win32.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index 84b937865a..6a3e6174f6 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -47,7 +47,8 @@
void *qemu_oom_check(void *ptr)
{
if (ptr == NULL) {
- error_report("Failed to allocate memory: %lu", GetLastError());
+ g_autofree gchar *emsg = g_win32_error_message(GetLastError());
+ error_report("Failed to allocate memory: %s", emsg);
abort();
}
return ptr;
--
2.21.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 4/6] util/osdep: Improve error report by calling error_setg_win32()
2020-02-27 16:30 [PATCH v2 0/6] misc: Improve error reporting on Windows Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2020-02-27 16:30 ` [PATCH v2 3/6] util/oslib-win32: Improve error report by calling error_setg_win32() Philippe Mathieu-Daudé
@ 2020-02-27 16:30 ` Philippe Mathieu-Daudé
2020-02-27 17:22 ` Marc-André Lureau
2020-02-27 16:31 ` [PATCH v2 5/6] qga: Fix a memory leak Philippe Mathieu-Daudé
2020-02-27 16:31 ` [PATCH v2 6/6] qga: Improve error report by calling error_setg_win32() Philippe Mathieu-Daudé
5 siblings, 1 reply; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-02-27 16:30 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Stefan Weil, Markus Armbruster, Michael Roth,
Tomáš Golembiovský,
Stefan Hajnoczi, Marc-André Lureau, Paolo Bonzini,
Philippe Mathieu-Daudé
Use error_setg_win32() which adds a hint similar to strerror(errno)).
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
util/osdep.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/util/osdep.c b/util/osdep.c
index ef40ae512a..144e217cb9 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -82,8 +82,8 @@ static int qemu_mprotect__osdep(void *addr, size_t size, int prot)
DWORD old_protect;
if (!VirtualProtect(addr, size, prot, &old_protect)) {
- error_report("%s: VirtualProtect failed with error code %ld",
- __func__, GetLastError());
+ g_autofree gchar *emsg = g_win32_error_message(GetLastError());
+ error_report("%s: VirtualProtect failed: %s", __func__, emsg);
return -1;
}
return 0;
@@ -506,12 +506,12 @@ int socket_init(void)
{
#ifdef _WIN32
WSADATA Data;
- int ret, err;
+ int ret;
ret = WSAStartup(MAKEWORD(2, 2), &Data);
if (ret != 0) {
- err = WSAGetLastError();
- error_report("WSAStartup: %d", err);
+ g_autofree gchar *emsg = g_win32_error_message(WSAGetLastError());
+ error_report("WSAStartup: %s", emsg);
return -1;
}
atexit(socket_cleanup);
--
2.21.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 5/6] qga: Fix a memory leak
2020-02-27 16:30 [PATCH v2 0/6] misc: Improve error reporting on Windows Philippe Mathieu-Daudé
` (3 preceding siblings ...)
2020-02-27 16:30 ` [PATCH v2 4/6] util/osdep: " Philippe Mathieu-Daudé
@ 2020-02-27 16:31 ` Philippe Mathieu-Daudé
2020-02-27 17:21 ` Marc-André Lureau
2020-02-27 16:31 ` [PATCH v2 6/6] qga: Improve error report by calling error_setg_win32() Philippe Mathieu-Daudé
5 siblings, 1 reply; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-02-27 16:31 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Stefan Weil, Markus Armbruster, Michael Roth,
Tomáš Golembiovský,
Stefan Hajnoczi, Marc-André Lureau, Paolo Bonzini,
Philippe Mathieu-Daudé
The string returned by g_win32_error_message() has to be
deallocated with g_free().
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
qga/channel-win32.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/qga/channel-win32.c b/qga/channel-win32.c
index c86f4388db..774205e017 100644
--- a/qga/channel-win32.c
+++ b/qga/channel-win32.c
@@ -302,8 +302,8 @@ static gboolean ga_channel_open(GAChannel *c, GAChannelMethod method,
OPEN_EXISTING,
FILE_FLAG_NO_BUFFERING | FILE_FLAG_OVERLAPPED, NULL);
if (c->handle == INVALID_HANDLE_VALUE) {
- g_critical("error opening path %s: %s", newpath,
- g_win32_error_message(GetLastError()));
+ g_autofree gchar *emsg = g_win32_error_message(GetLastError());
+ g_critical("error opening path %s: %s", newpath, emsg);
return false;
}
--
2.21.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 6/6] qga: Improve error report by calling error_setg_win32()
2020-02-27 16:30 [PATCH v2 0/6] misc: Improve error reporting on Windows Philippe Mathieu-Daudé
` (4 preceding siblings ...)
2020-02-27 16:31 ` [PATCH v2 5/6] qga: Fix a memory leak Philippe Mathieu-Daudé
@ 2020-02-27 16:31 ` Philippe Mathieu-Daudé
2020-02-27 17:20 ` Marc-André Lureau
5 siblings, 1 reply; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-02-27 16:31 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Stefan Weil, Markus Armbruster, Michael Roth,
Tomáš Golembiovský,
Stefan Hajnoczi, Marc-André Lureau, Paolo Bonzini,
Philippe Mathieu-Daudé
Use error_setg_win32() which adds a hint similar to strerror(errno)).
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
qga/channel-win32.c | 3 ++-
qga/commands-win32.c | 6 +++---
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/qga/channel-win32.c b/qga/channel-win32.c
index 774205e017..4f04868a76 100644
--- a/qga/channel-win32.c
+++ b/qga/channel-win32.c
@@ -308,7 +308,8 @@ static gboolean ga_channel_open(GAChannel *c, GAChannelMethod method,
}
if (method == GA_CHANNEL_ISA_SERIAL && !SetCommTimeouts(c->handle,&comTimeOut)) {
- g_critical("error setting timeout for com port: %lu",GetLastError());
+ g_autofree gchar *emsg = g_win32_error_message(GetLastError());
+ g_critical("error setting timeout for com port: %s", emsg);
CloseHandle(c->handle);
return false;
}
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 2461fd19bf..8e1f32ea23 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -315,8 +315,7 @@ void qmp_guest_shutdown(bool has_mode, const char *mode, Error **errp)
}
if (!ExitWindowsEx(shutdown_flag, SHTDN_REASON_FLAG_PLANNED)) {
- slog("guest-shutdown failed: %lu", GetLastError());
- error_setg(errp, QERR_UNDEFINED_ERROR);
+ error_setg_win32(errp, GetLastError(), "guest-shutdown failed");
}
}
@@ -1319,7 +1318,8 @@ static DWORD WINAPI do_suspend(LPVOID opaque)
DWORD ret = 0;
if (!SetSuspendState(*mode == GUEST_SUSPEND_MODE_DISK, TRUE, TRUE)) {
- slog("failed to suspend guest, %lu", GetLastError());
+ g_autofree gchar *emsg = g_win32_error_message(GetLastError());
+ slog("failed to suspend guest: %s", emsg);
ret = -1;
}
g_free(mode);
--
2.21.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 6/6] qga: Improve error report by calling error_setg_win32()
2020-02-27 16:31 ` [PATCH v2 6/6] qga: Improve error report by calling error_setg_win32() Philippe Mathieu-Daudé
@ 2020-02-27 17:20 ` Marc-André Lureau
2020-02-27 17:34 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 15+ messages in thread
From: Marc-André Lureau @ 2020-02-27 17:20 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Kevin Wolf, Michael Roth, Stefan Weil, qemu-devel,
Markus Armbruster, Tomáš Golembiovský,
Stefan Hajnoczi, Paolo Bonzini
Hi
On Thu, Feb 27, 2020 at 5:32 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> Use error_setg_win32() which adds a hint similar to strerror(errno)).
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> qga/channel-win32.c | 3 ++-
> qga/commands-win32.c | 6 +++---
> 2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/qga/channel-win32.c b/qga/channel-win32.c
> index 774205e017..4f04868a76 100644
> --- a/qga/channel-win32.c
> +++ b/qga/channel-win32.c
> @@ -308,7 +308,8 @@ static gboolean ga_channel_open(GAChannel *c, GAChannelMethod method,
> }
>
> if (method == GA_CHANNEL_ISA_SERIAL && !SetCommTimeouts(c->handle,&comTimeOut)) {
> - g_critical("error setting timeout for com port: %lu",GetLastError());
> + g_autofree gchar *emsg = g_win32_error_message(GetLastError());
> + g_critical("error setting timeout for com port: %s", emsg);
> CloseHandle(c->handle);
> return false;
> }
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index 2461fd19bf..8e1f32ea23 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -315,8 +315,7 @@ void qmp_guest_shutdown(bool has_mode, const char *mode, Error **errp)
> }
>
> if (!ExitWindowsEx(shutdown_flag, SHTDN_REASON_FLAG_PLANNED)) {
> - slog("guest-shutdown failed: %lu", GetLastError());
> - error_setg(errp, QERR_UNDEFINED_ERROR);
> + error_setg_win32(errp, GetLastError(), "guest-shutdown failed");
did you drop the slog() intentionally?
> }
> }
>
> @@ -1319,7 +1318,8 @@ static DWORD WINAPI do_suspend(LPVOID opaque)
> DWORD ret = 0;
>
> if (!SetSuspendState(*mode == GUEST_SUSPEND_MODE_DISK, TRUE, TRUE)) {
> - slog("failed to suspend guest, %lu", GetLastError());
> + g_autofree gchar *emsg = g_win32_error_message(GetLastError());
> + slog("failed to suspend guest: %s", emsg);
> ret = -1;
> }
> g_free(mode);
> --
> 2.21.1
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 5/6] qga: Fix a memory leak
2020-02-27 16:31 ` [PATCH v2 5/6] qga: Fix a memory leak Philippe Mathieu-Daudé
@ 2020-02-27 17:21 ` Marc-André Lureau
0 siblings, 0 replies; 15+ messages in thread
From: Marc-André Lureau @ 2020-02-27 17:21 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Kevin Wolf, Michael Roth, Stefan Weil, qemu-devel,
Markus Armbruster, Tomáš Golembiovský,
Stefan Hajnoczi, Paolo Bonzini
On Thu, Feb 27, 2020 at 5:32 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> The string returned by g_win32_error_message() has to be
> deallocated with g_free().
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> qga/channel-win32.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/qga/channel-win32.c b/qga/channel-win32.c
> index c86f4388db..774205e017 100644
> --- a/qga/channel-win32.c
> +++ b/qga/channel-win32.c
> @@ -302,8 +302,8 @@ static gboolean ga_channel_open(GAChannel *c, GAChannelMethod method,
> OPEN_EXISTING,
> FILE_FLAG_NO_BUFFERING | FILE_FLAG_OVERLAPPED, NULL);
> if (c->handle == INVALID_HANDLE_VALUE) {
> - g_critical("error opening path %s: %s", newpath,
> - g_win32_error_message(GetLastError()));
> + g_autofree gchar *emsg = g_win32_error_message(GetLastError());
> + g_critical("error opening path %s: %s", newpath, emsg);
> return false;
> }
>
> --
> 2.21.1
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 4/6] util/osdep: Improve error report by calling error_setg_win32()
2020-02-27 16:30 ` [PATCH v2 4/6] util/osdep: " Philippe Mathieu-Daudé
@ 2020-02-27 17:22 ` Marc-André Lureau
0 siblings, 0 replies; 15+ messages in thread
From: Marc-André Lureau @ 2020-02-27 17:22 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Kevin Wolf, Michael Roth, Stefan Weil, qemu-devel,
Markus Armbruster, Tomáš Golembiovský,
Stefan Hajnoczi, Paolo Bonzini
On Thu, Feb 27, 2020 at 5:32 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> Use error_setg_win32() which adds a hint similar to strerror(errno)).
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> util/osdep.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/util/osdep.c b/util/osdep.c
> index ef40ae512a..144e217cb9 100644
> --- a/util/osdep.c
> +++ b/util/osdep.c
> @@ -82,8 +82,8 @@ static int qemu_mprotect__osdep(void *addr, size_t size, int prot)
> DWORD old_protect;
>
> if (!VirtualProtect(addr, size, prot, &old_protect)) {
> - error_report("%s: VirtualProtect failed with error code %ld",
> - __func__, GetLastError());
> + g_autofree gchar *emsg = g_win32_error_message(GetLastError());
> + error_report("%s: VirtualProtect failed: %s", __func__, emsg);
> return -1;
> }
> return 0;
> @@ -506,12 +506,12 @@ int socket_init(void)
> {
> #ifdef _WIN32
> WSADATA Data;
> - int ret, err;
> + int ret;
>
> ret = WSAStartup(MAKEWORD(2, 2), &Data);
> if (ret != 0) {
> - err = WSAGetLastError();
> - error_report("WSAStartup: %d", err);
> + g_autofree gchar *emsg = g_win32_error_message(WSAGetLastError());
> + error_report("WSAStartup: %s", emsg);
> return -1;
> }
> atexit(socket_cleanup);
> --
> 2.21.1
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 6/6] qga: Improve error report by calling error_setg_win32()
2020-02-27 17:20 ` Marc-André Lureau
@ 2020-02-27 17:34 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-02-27 17:34 UTC (permalink / raw)
To: Marc-André Lureau
Cc: Kevin Wolf, Michael Roth, Stefan Weil, qemu-devel,
Markus Armbruster, Tomáš Golembiovský,
Stefan Hajnoczi, Paolo Bonzini
On 2/27/20 6:20 PM, Marc-André Lureau wrote:
> Hi
>
> On Thu, Feb 27, 2020 at 5:32 PM Philippe Mathieu-Daudé
> <philmd@redhat.com> wrote:
>>
>> Use error_setg_win32() which adds a hint similar to strerror(errno)).
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>> qga/channel-win32.c | 3 ++-
>> qga/commands-win32.c | 6 +++---
>> 2 files changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/qga/channel-win32.c b/qga/channel-win32.c
>> index 774205e017..4f04868a76 100644
>> --- a/qga/channel-win32.c
>> +++ b/qga/channel-win32.c
>> @@ -308,7 +308,8 @@ static gboolean ga_channel_open(GAChannel *c, GAChannelMethod method,
>> }
>>
>> if (method == GA_CHANNEL_ISA_SERIAL && !SetCommTimeouts(c->handle,&comTimeOut)) {
>> - g_critical("error setting timeout for com port: %lu",GetLastError());
>> + g_autofree gchar *emsg = g_win32_error_message(GetLastError());
>> + g_critical("error setting timeout for com port: %s", emsg);
>> CloseHandle(c->handle);
>> return false;
>> }
>> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
>> index 2461fd19bf..8e1f32ea23 100644
>> --- a/qga/commands-win32.c
>> +++ b/qga/commands-win32.c
>> @@ -315,8 +315,7 @@ void qmp_guest_shutdown(bool has_mode, const char *mode, Error **errp)
>> }
>>
>> if (!ExitWindowsEx(shutdown_flag, SHTDN_REASON_FLAG_PLANNED)) {
>> - slog("guest-shutdown failed: %lu", GetLastError());
>> - error_setg(errp, QERR_UNDEFINED_ERROR);
>> + error_setg_win32(errp, GetLastError(), "guest-shutdown failed");
>
> did you drop the slog() intentionally?
Oops no :(
>
>> }
>> }
>>
>> @@ -1319,7 +1318,8 @@ static DWORD WINAPI do_suspend(LPVOID opaque)
>> DWORD ret = 0;
>>
>> if (!SetSuspendState(*mode == GUEST_SUSPEND_MODE_DISK, TRUE, TRUE)) {
>> - slog("failed to suspend guest, %lu", GetLastError());
>> + g_autofree gchar *emsg = g_win32_error_message(GetLastError());
>> + slog("failed to suspend guest: %s", emsg);
>> ret = -1;
>> }
>> g_free(mode);
>> --
>> 2.21.1
>>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/6] util: Replace fprintf(stderr, "*\n" with error_report()
2020-02-27 16:30 ` [PATCH v2 2/6] util: Replace fprintf(stderr, "*\n" with error_report() Philippe Mathieu-Daudé
@ 2020-02-28 7:43 ` Markus Armbruster
2020-02-28 9:50 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 15+ messages in thread
From: Markus Armbruster @ 2020-02-28 7:43 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Kevin Wolf, Stefan Weil, Alistair Francis, Michael Roth,
qemu-devel, Tomáš Golembiovský,
Alistair Francis, Stefan Hajnoczi, Paolo Bonzini,
Marc-André Lureau, Alistair Francis, Markus Armbruster,
Philippe Mathieu-Daudé
Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> From: Alistair Francis <alistair.francis@xilinx.com>
>
> Replace a large number of the fprintf(stderr, "*\n" calls with
> error_report(). The functions were renamed with these commands and then
> compiler issues where manually fixed.
>
> find ./* -type f -exec sed -i \
> 'N;N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
> find ./* -type f -exec sed -i \
> 'N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
> find ./* -type f -exec sed -i \
> 'N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
> find ./* -type f -exec sed -i \
> 'N;N;N;N;N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
> find ./* -type f -exec sed -i \
> 'N;N;N;N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
> find ./* -type f -exec sed -i \
> 'N;N;N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
> find ./* -type f -exec sed -i \
> 'N;N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
> find ./* -type f -exec sed -i \
> 'N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
> find ./* -type f -exec sed -i \
> 'N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
> find ./* -type f -exec sed -i \
> 'N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
> find ./* -type f -exec sed -i \
> 'N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
>
> The error in aio_poll() was removed manually.
>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Message-Id: <f71203227749e2afb8564b3388b2b34f6652b009.1510181732.git.alistair.francis@xilinx.com>
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> [PMD: Rebased]
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> Cc: Alistair Francis <Alistair.Francis@wdc.com>
> Cc: Alistair Francis <alistair@alistair23.me>
> ---
> util/coroutine-sigaltstack.c | 3 ++-
> util/mmap-alloc.c | 11 ++++++-----
> util/module.c | 13 ++++++-------
> util/osdep.c | 4 ++--
> util/oslib-posix.c | 3 ++-
> util/oslib-win32.c | 3 ++-
> util/qemu-coroutine.c | 10 +++++-----
> util/qemu-thread-posix.c | 5 +++--
> util/qemu-thread-win32.c | 5 +++--
> util/qemu-timer-common.c | 3 ++-
> 10 files changed, 33 insertions(+), 27 deletions(-)
>
> diff --git a/util/coroutine-sigaltstack.c b/util/coroutine-sigaltstack.c
> index f6fc49a0e5..63decd4d1d 100644
> --- a/util/coroutine-sigaltstack.c
> +++ b/util/coroutine-sigaltstack.c
> @@ -29,6 +29,7 @@
> #include <pthread.h>
> #include "qemu-common.h"
> #include "qemu/coroutine_int.h"
> +#include "qemu/error-report.h"
>
> typedef struct {
> Coroutine base;
> @@ -80,7 +81,7 @@ static void __attribute__((constructor)) coroutine_init(void)
>
> ret = pthread_key_create(&thread_state_key, qemu_coroutine_thread_cleanup);
> if (ret != 0) {
> - fprintf(stderr, "unable to create leader key: %s\n", strerror(errno));
> + error_report("unable to create leader key: %s", strerror(errno));
> abort();
> }
> }
> diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c
> index 27dcccd8ec..3ac6e10404 100644
> --- a/util/mmap-alloc.c
> +++ b/util/mmap-alloc.c
> @@ -18,6 +18,7 @@
> #endif /* CONFIG_LINUX */
>
> #include "qemu/osdep.h"
> +#include "qemu/error-report.h"
> #include "qemu/mmap-alloc.h"
> #include "qemu/host-utils.h"
>
> @@ -63,7 +64,7 @@ size_t qemu_mempath_getpagesize(const char *mem_path)
> } while (ret != 0 && errno == EINTR);
>
> if (ret != 0) {
> - fprintf(stderr, "Couldn't statfs() memory path: %s\n",
> + error_report("Couldn't statfs() memory path: %s",
> strerror(errno));
Indentation is off.
> exit(1);
> }
> @@ -160,10 +161,10 @@ void *qemu_ram_mmap(int fd,
> len = 0;
> }
> file_name[len] = '\0';
> - fprintf(stderr, "Warning: requesting persistence across crashes "
> - "for backend file %s failed. Proceeding without "
> - "persistence, data might become corrupted in case of host "
> - "crash.\n", file_name);
> + error_report("Warning: requesting persistence across crashes "
> + "for backend file %s failed. Proceeding without "
> + "persistence, data might become corrupted in case "
> + "of host crash.", file_name);
This should be something like
warn_report("requesting persistence across crashes"
" for backend file %s failed",
file_name);
error_printf("Proceeding without persistence, data might"
" become corrupted in case of host crash.\n");
Precedence: commit db0754df88 "file-posix: Use error API properly".
> g_free(proc_link);
> g_free(file_name);
> }
> diff --git a/util/module.c b/util/module.c
> index 236a7bb52a..28efa1f891 100644
> --- a/util/module.c
> +++ b/util/module.c
> @@ -19,6 +19,7 @@
> #endif
> #include "qemu/queue.h"
> #include "qemu/module.h"
> +#include "qemu/error-report.h"
>
> typedef struct ModuleEntry
> {
> @@ -130,19 +131,17 @@ static int module_load_file(const char *fname)
>
> g_module = g_module_open(fname, G_MODULE_BIND_LAZY | G_MODULE_BIND_LOCAL);
> if (!g_module) {
> - fprintf(stderr, "Failed to open module: %s\n",
> - g_module_error());
> + error_report("Failed to open module: %s", g_module_error());
> ret = -EINVAL;
> goto out;
> }
> if (!g_module_symbol(g_module, DSO_STAMP_FUN_STR, (gpointer *)&sym)) {
> - fprintf(stderr, "Failed to initialize module: %s\n",
> - fname);
> + error_report("Failed to initialize module: %s", fname);
> /* Print some info if this is a QEMU module (but from different build),
> * this will make debugging user problems easier. */
> if (g_module_symbol(g_module, "qemu_module_dummy", (gpointer *)&sym)) {
> - fprintf(stderr,
> - "Note: only modules from the same build can be loaded.\n");
> + error_report("Note: "
> + "only modules from the same build can be loaded.");
Use error_printf() to print the additional note.
> }
> g_module_close(g_module);
> ret = -EINVAL;
> @@ -178,7 +177,7 @@ bool module_load_one(const char *prefix, const char *lib_name)
> static GHashTable *loaded_modules;
>
> if (!g_module_supported()) {
> - fprintf(stderr, "Module is not supported by system.\n");
> + error_report("Module is not supported by system.");
> return false;
> }
>
> diff --git a/util/osdep.c b/util/osdep.c
> index f7d06050f7..ef40ae512a 100644
> --- a/util/osdep.c
> +++ b/util/osdep.c
> @@ -484,7 +484,7 @@ void fips_set_state(bool requested)
> #endif /* __linux__ */
>
> #ifdef _FIPS_DEBUG
> - fprintf(stderr, "FIPS mode %s (requested %s)\n",
> + error_report("FIPS mode %s (requested %s)",
> (fips_enabled ? "enabled" : "disabled"),
> (requested ? "enabled" : "disabled"));
> #endif
> @@ -511,7 +511,7 @@ int socket_init(void)
> ret = WSAStartup(MAKEWORD(2, 2), &Data);
> if (ret != 0) {
> err = WSAGetLastError();
> - fprintf(stderr, "WSAStartup: %d\n", err);
> + error_report("WSAStartup: %d", err);
> return -1;
> }
> atexit(socket_cleanup);
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 897e8f3ba6..4977594a43 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -35,6 +35,7 @@
> #include "sysemu/sysemu.h"
> #include "trace.h"
> #include "qapi/error.h"
> +#include "qemu/error-report.h"
> #include "qemu/sockets.h"
> #include "qemu/thread.h"
> #include <libgen.h>
> @@ -170,7 +171,7 @@ fail_close:
> void *qemu_oom_check(void *ptr)
> {
> if (ptr == NULL) {
> - fprintf(stderr, "Failed to allocate memory: %s\n", strerror(errno));
> + error_report("Failed to allocate memory: %s", strerror(errno));
> abort();
> }
> return ptr;
> diff --git a/util/oslib-win32.c b/util/oslib-win32.c
> index e9b14ab178..84b937865a 100644
> --- a/util/oslib-win32.c
> +++ b/util/oslib-win32.c
> @@ -39,6 +39,7 @@
> #include "trace.h"
> #include "qemu/sockets.h"
> #include "qemu/cutils.h"
> +#include "qemu/error-report.h"
>
> /* this must come after including "trace.h" */
> #include <shlobj.h>
> @@ -46,7 +47,7 @@
> void *qemu_oom_check(void *ptr)
> {
> if (ptr == NULL) {
> - fprintf(stderr, "Failed to allocate memory: %lu\n", GetLastError());
> + error_report("Failed to allocate memory: %lu", GetLastError());
> abort();
> }
> return ptr;
> diff --git a/util/qemu-coroutine.c b/util/qemu-coroutine.c
> index c3caa6c770..62d1dd09df 100644
> --- a/util/qemu-coroutine.c
> +++ b/util/qemu-coroutine.c
> @@ -14,6 +14,7 @@
>
> #include "qemu/osdep.h"
> #include "trace.h"
> +#include "qemu/error-report.h"
> #include "qemu/thread.h"
> #include "qemu/atomic.h"
> #include "qemu/coroutine.h"
> @@ -125,14 +126,13 @@ void qemu_aio_coroutine_enter(AioContext *ctx, Coroutine *co)
> * cause us to enter it twice, potentially even after the coroutine has
> * been deleted */
> if (scheduled) {
> - fprintf(stderr,
> - "%s: Co-routine was already scheduled in '%s'\n",
> - __func__, scheduled);
> + error_report("%s: Co-routine was already scheduled in '%s'",
> + __func__, scheduled);
> abort();
> }
>
> if (to->caller) {
> - fprintf(stderr, "Co-routine re-entered recursively\n");
> + error_report("Co-routine re-entered recursively");
> abort();
> }
>
> @@ -185,7 +185,7 @@ void coroutine_fn qemu_coroutine_yield(void)
> trace_qemu_coroutine_yield(self, to);
>
> if (!to) {
> - fprintf(stderr, "Co-routine is yielding to no one\n");
> + error_report("Co-routine is yielding to no one");
> abort();
> }
>
> diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
> index 838980aaa5..b4d8de376c 100644
> --- a/util/qemu-thread-posix.c
> +++ b/util/qemu-thread-posix.c
> @@ -14,6 +14,7 @@
> #include "qemu/thread.h"
> #include "qemu/atomic.h"
> #include "qemu/notify.h"
> +#include "qemu/error-report.h"
> #include "qemu-thread-common.h"
>
> static bool name_threads;
> @@ -25,14 +26,14 @@ void qemu_thread_naming(bool enable)
> #ifndef CONFIG_THREAD_SETNAME_BYTHREAD
> /* This is a debugging option, not fatal */
> if (enable) {
> - fprintf(stderr, "qemu: thread naming not supported on this host\n");
> + error_report("qemu: thread naming not supported on this host");
This isn't an error. It's in response to -name debug-threads=on, and
tells the user debug-threads=on is being ignored. Let's use
warn_report().
Drop the "qemu: ", please; error_report() & friends take care of that.
More of the same below.
> }
> #endif
> }
>
> static void error_exit(int err, const char *msg)
> {
> - fprintf(stderr, "qemu: %s: %s\n", msg, strerror(err));
> + error_report("qemu: %s: %s", msg, strerror(err));
> abort();
> }
>
> diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c
> index 56a83333da..9bed338d7e 100644
> --- a/util/qemu-thread-win32.c
> +++ b/util/qemu-thread-win32.c
> @@ -15,6 +15,7 @@
> #include "qemu-common.h"
> #include "qemu/thread.h"
> #include "qemu/notify.h"
> +#include "qemu/error-report.h"
> #include "qemu-thread-common.h"
> #include <process.h>
>
> @@ -25,7 +26,7 @@ void qemu_thread_naming(bool enable)
> /* But note we don't actually name them on Windows yet */
> name_threads = enable;
>
> - fprintf(stderr, "qemu: thread naming not supported on this host\n");
> + error_report("qemu: thread naming not supported on this host");
Likewise.
> }
>
> static void error_exit(int err, const char *msg)
> @@ -34,7 +35,7 @@ static void error_exit(int err, const char *msg)
>
> FormatMessage(FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_ALLOCATE_BUFFER,
> NULL, err, 0, (LPTSTR)&pstr, 2, NULL);
> - fprintf(stderr, "qemu: %s: %s\n", msg, pstr);
> + error_report("qemu: %s: %s", msg, pstr);
> LocalFree(pstr);
> abort();
> }
> diff --git a/util/qemu-timer-common.c b/util/qemu-timer-common.c
> index baf3317f74..527944da1c 100644
> --- a/util/qemu-timer-common.c
> +++ b/util/qemu-timer-common.c
> @@ -23,6 +23,7 @@
> */
> #include "qemu/osdep.h"
> #include "qemu/timer.h"
> +#include "qemu/error-report.h"
>
> /***********************************************************/
> /* real time host monotonic timer */
> @@ -37,7 +38,7 @@ static void __attribute__((constructor)) init_get_clock(void)
> int ret;
> ret = QueryPerformanceFrequency(&freq);
> if (ret == 0) {
> - fprintf(stderr, "Could not calibrate ticks\n");
> + error_report("Could not calibrate ticks");
> exit(1);
> }
> clock_freq = freq.QuadPart;
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/6] util: Replace fprintf(stderr, "*\n" with error_report()
2020-02-28 7:43 ` Markus Armbruster
@ 2020-02-28 9:50 ` Philippe Mathieu-Daudé
2020-02-28 9:57 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-02-28 9:50 UTC (permalink / raw)
To: Markus Armbruster
Cc: Kevin Wolf, Stefan Weil, Alistair Francis, qemu-devel,
Michael Roth, Tomáš Golembiovský,
Alistair Francis, Stefan Hajnoczi, Paolo Bonzini,
Marc-André Lureau, Philippe Mathieu-Daudé
On 2/28/20 8:43 AM, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>
>> From: Alistair Francis <alistair.francis@xilinx.com>
>>
>> Replace a large number of the fprintf(stderr, "*\n" calls with
>> error_report(). The functions were renamed with these commands and then
>> compiler issues where manually fixed.
>>
>> find ./* -type f -exec sed -i \
>> 'N;N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>> {} +
>> find ./* -type f -exec sed -i \
>> 'N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>> {} +
>> find ./* -type f -exec sed -i \
>> 'N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>> {} +
>> find ./* -type f -exec sed -i \
>> 'N;N;N;N;N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>> {} +
>> find ./* -type f -exec sed -i \
>> 'N;N;N;N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>> {} +
>> find ./* -type f -exec sed -i \
>> 'N;N;N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>> {} +
>> find ./* -type f -exec sed -i \
>> 'N;N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>> {} +
>> find ./* -type f -exec sed -i \
>> 'N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>> {} +
>> find ./* -type f -exec sed -i \
>> 'N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>> {} +
>> find ./* -type f -exec sed -i \
>> 'N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>> {} +
>> find ./* -type f -exec sed -i \
>> 'N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>> {} +
>>
>> The error in aio_poll() was removed manually.
>>
>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> Message-Id: <f71203227749e2afb8564b3388b2b34f6652b009.1510181732.git.alistair.francis@xilinx.com>
>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>> [PMD: Rebased]
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>> Cc: Alistair Francis <Alistair.Francis@wdc.com>
>> Cc: Alistair Francis <alistair@alistair23.me>
>> ---
>> util/coroutine-sigaltstack.c | 3 ++-
>> util/mmap-alloc.c | 11 ++++++-----
>> util/module.c | 13 ++++++-------
>> util/osdep.c | 4 ++--
>> util/oslib-posix.c | 3 ++-
>> util/oslib-win32.c | 3 ++-
>> util/qemu-coroutine.c | 10 +++++-----
>> util/qemu-thread-posix.c | 5 +++--
>> util/qemu-thread-win32.c | 5 +++--
>> util/qemu-timer-common.c | 3 ++-
>> 10 files changed, 33 insertions(+), 27 deletions(-)
>>
>> diff --git a/util/coroutine-sigaltstack.c b/util/coroutine-sigaltstack.c
>> index f6fc49a0e5..63decd4d1d 100644
>> --- a/util/coroutine-sigaltstack.c
>> +++ b/util/coroutine-sigaltstack.c
>> @@ -29,6 +29,7 @@
>> #include <pthread.h>
>> #include "qemu-common.h"
>> #include "qemu/coroutine_int.h"
>> +#include "qemu/error-report.h"
>>
>> typedef struct {
>> Coroutine base;
>> @@ -80,7 +81,7 @@ static void __attribute__((constructor)) coroutine_init(void)
>>
>> ret = pthread_key_create(&thread_state_key, qemu_coroutine_thread_cleanup);
>> if (ret != 0) {
>> - fprintf(stderr, "unable to create leader key: %s\n", strerror(errno));
>> + error_report("unable to create leader key: %s", strerror(errno));
>> abort();
>> }
>> }
>> diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c
>> index 27dcccd8ec..3ac6e10404 100644
>> --- a/util/mmap-alloc.c
>> +++ b/util/mmap-alloc.c
>> @@ -18,6 +18,7 @@
>> #endif /* CONFIG_LINUX */
>>
>> #include "qemu/osdep.h"
>> +#include "qemu/error-report.h"
>> #include "qemu/mmap-alloc.h"
>> #include "qemu/host-utils.h"
>>
>> @@ -63,7 +64,7 @@ size_t qemu_mempath_getpagesize(const char *mem_path)
>> } while (ret != 0 && errno == EINTR);
>>
>> if (ret != 0) {
>> - fprintf(stderr, "Couldn't statfs() memory path: %s\n",
>> + error_report("Couldn't statfs() memory path: %s",
>> strerror(errno));
>
> Indentation is off.
>
>> exit(1);
>> }
>> @@ -160,10 +161,10 @@ void *qemu_ram_mmap(int fd,
>> len = 0;
>> }
>> file_name[len] = '\0';
>> - fprintf(stderr, "Warning: requesting persistence across crashes "
>> - "for backend file %s failed. Proceeding without "
>> - "persistence, data might become corrupted in case of host "
>> - "crash.\n", file_name);
>> + error_report("Warning: requesting persistence across crashes "
>> + "for backend file %s failed. Proceeding without "
>> + "persistence, data might become corrupted in case "
>> + "of host crash.", file_name);
>
> This should be something like
>
> warn_report("requesting persistence across crashes"
> " for backend file %s failed",
> file_name);
> error_printf("Proceeding without persistence, data might"
> " become corrupted in case of host crash.\n");
>
> Precedence: commit db0754df88 "file-posix: Use error API properly".
>
>> g_free(proc_link);
>> g_free(file_name);
>> }
>> diff --git a/util/module.c b/util/module.c
>> index 236a7bb52a..28efa1f891 100644
>> --- a/util/module.c
>> +++ b/util/module.c
>> @@ -19,6 +19,7 @@
>> #endif
>> #include "qemu/queue.h"
>> #include "qemu/module.h"
>> +#include "qemu/error-report.h"
>>
>> typedef struct ModuleEntry
>> {
>> @@ -130,19 +131,17 @@ static int module_load_file(const char *fname)
>>
>> g_module = g_module_open(fname, G_MODULE_BIND_LAZY | G_MODULE_BIND_LOCAL);
>> if (!g_module) {
>> - fprintf(stderr, "Failed to open module: %s\n",
>> - g_module_error());
>> + error_report("Failed to open module: %s", g_module_error());
>> ret = -EINVAL;
>> goto out;
>> }
>> if (!g_module_symbol(g_module, DSO_STAMP_FUN_STR, (gpointer *)&sym)) {
>> - fprintf(stderr, "Failed to initialize module: %s\n",
>> - fname);
>> + error_report("Failed to initialize module: %s", fname);
>> /* Print some info if this is a QEMU module (but from different build),
>> * this will make debugging user problems easier. */
>> if (g_module_symbol(g_module, "qemu_module_dummy", (gpointer *)&sym)) {
>> - fprintf(stderr,
>> - "Note: only modules from the same build can be loaded.\n");
>> + error_report("Note: "
>> + "only modules from the same build can be loaded.");
>
> Use error_printf() to print the additional note.
>
>> }
>> g_module_close(g_module);
>> ret = -EINVAL;
>> @@ -178,7 +177,7 @@ bool module_load_one(const char *prefix, const char *lib_name)
>> static GHashTable *loaded_modules;
>>
>> if (!g_module_supported()) {
>> - fprintf(stderr, "Module is not supported by system.\n");
>> + error_report("Module is not supported by system.");
>> return false;
>> }
>>
>> diff --git a/util/osdep.c b/util/osdep.c
>> index f7d06050f7..ef40ae512a 100644
>> --- a/util/osdep.c
>> +++ b/util/osdep.c
>> @@ -484,7 +484,7 @@ void fips_set_state(bool requested)
>> #endif /* __linux__ */
>>
>> #ifdef _FIPS_DEBUG
>> - fprintf(stderr, "FIPS mode %s (requested %s)\n",
>> + error_report("FIPS mode %s (requested %s)",
>> (fips_enabled ? "enabled" : "disabled"),
>> (requested ? "enabled" : "disabled"));
>> #endif
>> @@ -511,7 +511,7 @@ int socket_init(void)
>> ret = WSAStartup(MAKEWORD(2, 2), &Data);
>> if (ret != 0) {
>> err = WSAGetLastError();
>> - fprintf(stderr, "WSAStartup: %d\n", err);
>> + error_report("WSAStartup: %d", err);
>> return -1;
>> }
>> atexit(socket_cleanup);
>> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
>> index 897e8f3ba6..4977594a43 100644
>> --- a/util/oslib-posix.c
>> +++ b/util/oslib-posix.c
>> @@ -35,6 +35,7 @@
>> #include "sysemu/sysemu.h"
>> #include "trace.h"
>> #include "qapi/error.h"
>> +#include "qemu/error-report.h"
>> #include "qemu/sockets.h"
>> #include "qemu/thread.h"
>> #include <libgen.h>
>> @@ -170,7 +171,7 @@ fail_close:
>> void *qemu_oom_check(void *ptr)
>> {
>> if (ptr == NULL) {
>> - fprintf(stderr, "Failed to allocate memory: %s\n", strerror(errno));
>> + error_report("Failed to allocate memory: %s", strerror(errno));
>> abort();
>> }
>> return ptr;
>> diff --git a/util/oslib-win32.c b/util/oslib-win32.c
>> index e9b14ab178..84b937865a 100644
>> --- a/util/oslib-win32.c
>> +++ b/util/oslib-win32.c
>> @@ -39,6 +39,7 @@
>> #include "trace.h"
>> #include "qemu/sockets.h"
>> #include "qemu/cutils.h"
>> +#include "qemu/error-report.h"
>>
>> /* this must come after including "trace.h" */
>> #include <shlobj.h>
>> @@ -46,7 +47,7 @@
>> void *qemu_oom_check(void *ptr)
>> {
>> if (ptr == NULL) {
>> - fprintf(stderr, "Failed to allocate memory: %lu\n", GetLastError());
>> + error_report("Failed to allocate memory: %lu", GetLastError());
>> abort();
>> }
>> return ptr;
>> diff --git a/util/qemu-coroutine.c b/util/qemu-coroutine.c
>> index c3caa6c770..62d1dd09df 100644
>> --- a/util/qemu-coroutine.c
>> +++ b/util/qemu-coroutine.c
>> @@ -14,6 +14,7 @@
>>
>> #include "qemu/osdep.h"
>> #include "trace.h"
>> +#include "qemu/error-report.h"
>> #include "qemu/thread.h"
>> #include "qemu/atomic.h"
>> #include "qemu/coroutine.h"
>> @@ -125,14 +126,13 @@ void qemu_aio_coroutine_enter(AioContext *ctx, Coroutine *co)
>> * cause us to enter it twice, potentially even after the coroutine has
>> * been deleted */
>> if (scheduled) {
>> - fprintf(stderr,
>> - "%s: Co-routine was already scheduled in '%s'\n",
>> - __func__, scheduled);
>> + error_report("%s: Co-routine was already scheduled in '%s'",
>> + __func__, scheduled);
>> abort();
>> }
>>
>> if (to->caller) {
>> - fprintf(stderr, "Co-routine re-entered recursively\n");
>> + error_report("Co-routine re-entered recursively");
>> abort();
>> }
>>
>> @@ -185,7 +185,7 @@ void coroutine_fn qemu_coroutine_yield(void)
>> trace_qemu_coroutine_yield(self, to);
>>
>> if (!to) {
>> - fprintf(stderr, "Co-routine is yielding to no one\n");
>> + error_report("Co-routine is yielding to no one");
>> abort();
>> }
>>
>> diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
>> index 838980aaa5..b4d8de376c 100644
>> --- a/util/qemu-thread-posix.c
>> +++ b/util/qemu-thread-posix.c
>> @@ -14,6 +14,7 @@
>> #include "qemu/thread.h"
>> #include "qemu/atomic.h"
>> #include "qemu/notify.h"
>> +#include "qemu/error-report.h"
>> #include "qemu-thread-common.h"
>>
>> static bool name_threads;
>> @@ -25,14 +26,14 @@ void qemu_thread_naming(bool enable)
>> #ifndef CONFIG_THREAD_SETNAME_BYTHREAD
>> /* This is a debugging option, not fatal */
>> if (enable) {
>> - fprintf(stderr, "qemu: thread naming not supported on this host\n");
>> + error_report("qemu: thread naming not supported on this host");
>
> This isn't an error. It's in response to -name debug-threads=on, and
> tells the user debug-threads=on is being ignored. Let's use
> warn_report().
>
> Drop the "qemu: ", please; error_report() & friends take care of that.
> More of the same below.
>
>> }
>> #endif
>> }
>>
>> static void error_exit(int err, const char *msg)
>> {
>> - fprintf(stderr, "qemu: %s: %s\n", msg, strerror(err));
>> + error_report("qemu: %s: %s", msg, strerror(err));
>> abort();
>> }
>>
>> diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c
>> index 56a83333da..9bed338d7e 100644
>> --- a/util/qemu-thread-win32.c
>> +++ b/util/qemu-thread-win32.c
>> @@ -15,6 +15,7 @@
>> #include "qemu-common.h"
>> #include "qemu/thread.h"
>> #include "qemu/notify.h"
>> +#include "qemu/error-report.h"
>> #include "qemu-thread-common.h"
>> #include <process.h>
>>
>> @@ -25,7 +26,7 @@ void qemu_thread_naming(bool enable)
>> /* But note we don't actually name them on Windows yet */
>> name_threads = enable;
>>
>> - fprintf(stderr, "qemu: thread naming not supported on this host\n");
>> + error_report("qemu: thread naming not supported on this host");
>
> Likewise.
Thanks for your review. I'll drop the changes in util/oslib-win32.c for
for now, and add a note in my TODO for after the 5.0 release.
>
>> }
>>
>> static void error_exit(int err, const char *msg)
>> @@ -34,7 +35,7 @@ static void error_exit(int err, const char *msg)
>>
>> FormatMessage(FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_ALLOCATE_BUFFER,
>> NULL, err, 0, (LPTSTR)&pstr, 2, NULL);
>> - fprintf(stderr, "qemu: %s: %s\n", msg, pstr);
>> + error_report("qemu: %s: %s", msg, pstr);
>> LocalFree(pstr);
>> abort();
>> }
>> diff --git a/util/qemu-timer-common.c b/util/qemu-timer-common.c
>> index baf3317f74..527944da1c 100644
>> --- a/util/qemu-timer-common.c
>> +++ b/util/qemu-timer-common.c
>> @@ -23,6 +23,7 @@
>> */
>> #include "qemu/osdep.h"
>> #include "qemu/timer.h"
>> +#include "qemu/error-report.h"
>>
>> /***********************************************************/
>> /* real time host monotonic timer */
>> @@ -37,7 +38,7 @@ static void __attribute__((constructor)) init_get_clock(void)
>> int ret;
>> ret = QueryPerformanceFrequency(&freq);
>> if (ret == 0) {
>> - fprintf(stderr, "Could not calibrate ticks\n");
>> + error_report("Could not calibrate ticks");
>> exit(1);
>> }
>> clock_freq = freq.QuadPart;
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/6] util: Replace fprintf(stderr, "*\n" with error_report()
2020-02-28 9:50 ` Philippe Mathieu-Daudé
@ 2020-02-28 9:57 ` Philippe Mathieu-Daudé
2020-02-28 17:41 ` Markus Armbruster
0 siblings, 1 reply; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-02-28 9:57 UTC (permalink / raw)
To: Markus Armbruster
Cc: Kevin Wolf, Stefan Weil, Alistair Francis, qemu-devel,
Michael Roth, Tomáš Golembiovský,
Alistair Francis, Stefan Hajnoczi, Paolo Bonzini,
Marc-André Lureau, Philippe Mathieu-Daudé
On 2/28/20 10:50 AM, Philippe Mathieu-Daudé wrote:
> On 2/28/20 8:43 AM, Markus Armbruster wrote:
>> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>>
>>> From: Alistair Francis <alistair.francis@xilinx.com>
>>>
>>> Replace a large number of the fprintf(stderr, "*\n" calls with
>>> error_report(). The functions were renamed with these commands and then
>>> compiler issues where manually fixed.
>>>
>>> find ./* -type f -exec sed -i \
>>> 'N;N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr,
>>> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>>> {} +
>>> find ./* -type f -exec sed -i \
>>> 'N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr,
>>> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>>> {} +
>>> find ./* -type f -exec sed -i \
>>> 'N;N;N;N;N;N;N;N;N; {s|fprintf(stderr,
>>> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>>> {} +
>>> find ./* -type f -exec sed -i \
>>> 'N;N;N;N;N;N;N;N; {s|fprintf(stderr,
>>> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>>> {} +
>>> find ./* -type f -exec sed -i \
>>> 'N;N;N;N;N;N;N; {s|fprintf(stderr,
>>> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>>> {} +
>>> find ./* -type f -exec sed -i \
>>> 'N;N;N;N;N;N; {s|fprintf(stderr,
>>> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>>> {} +
>>> find ./* -type f -exec sed -i \
>>> 'N;N;N;N;N; {s|fprintf(stderr,
>>> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>>> {} +
>>> find ./* -type f -exec sed -i \
>>> 'N;N;N;N; {s|fprintf(stderr,
>>> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>>> {} +
>>> find ./* -type f -exec sed -i \
>>> 'N;N;N; {s|fprintf(stderr,
>>> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>>> {} +
>>> find ./* -type f -exec sed -i \
>>> 'N;N; {s|fprintf(stderr,
>>> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>>> {} +
>>> find ./* -type f -exec sed -i \
>>> 'N; {s|fprintf(stderr,
>>> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>>> {} +
>>>
>>> The error in aio_poll() was removed manually.
>>>
>>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> Message-Id:
>>> <f71203227749e2afb8564b3388b2b34f6652b009.1510181732.git.alistair.francis@xilinx.com>
>>>
>>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>>> [PMD: Rebased]
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> ---
>>> Cc: Alistair Francis <Alistair.Francis@wdc.com>
>>> Cc: Alistair Francis <alistair@alistair23.me>
>>> ---
>>> util/coroutine-sigaltstack.c | 3 ++-
>>> util/mmap-alloc.c | 11 ++++++-----
>>> util/module.c | 13 ++++++-------
>>> util/osdep.c | 4 ++--
>>> util/oslib-posix.c | 3 ++-
>>> util/oslib-win32.c | 3 ++-
>>> util/qemu-coroutine.c | 10 +++++-----
>>> util/qemu-thread-posix.c | 5 +++--
>>> util/qemu-thread-win32.c | 5 +++--
>>> util/qemu-timer-common.c | 3 ++-
>>> 10 files changed, 33 insertions(+), 27 deletions(-)
>>>
>>> diff --git a/util/coroutine-sigaltstack.c b/util/coroutine-sigaltstack.c
>>> index f6fc49a0e5..63decd4d1d 100644
>>> --- a/util/coroutine-sigaltstack.c
>>> +++ b/util/coroutine-sigaltstack.c
>>> @@ -29,6 +29,7 @@
>>> #include <pthread.h>
>>> #include "qemu-common.h"
>>> #include "qemu/coroutine_int.h"
>>> +#include "qemu/error-report.h"
>>> typedef struct {
>>> Coroutine base;
>>> @@ -80,7 +81,7 @@ static void __attribute__((constructor))
>>> coroutine_init(void)
>>> ret = pthread_key_create(&thread_state_key,
>>> qemu_coroutine_thread_cleanup);
>>> if (ret != 0) {
>>> - fprintf(stderr, "unable to create leader key: %s\n",
>>> strerror(errno));
>>> + error_report("unable to create leader key: %s",
>>> strerror(errno));
>>> abort();
>>> }
>>> }
>>> diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c
>>> index 27dcccd8ec..3ac6e10404 100644
>>> --- a/util/mmap-alloc.c
>>> +++ b/util/mmap-alloc.c
>>> @@ -18,6 +18,7 @@
>>> #endif /* CONFIG_LINUX */
>>> #include "qemu/osdep.h"
>>> +#include "qemu/error-report.h"
>>> #include "qemu/mmap-alloc.h"
>>> #include "qemu/host-utils.h"
>>> @@ -63,7 +64,7 @@ size_t qemu_mempath_getpagesize(const char *mem_path)
>>> } while (ret != 0 && errno == EINTR);
>>> if (ret != 0) {
>>> - fprintf(stderr, "Couldn't statfs() memory path: %s\n",
>>> + error_report("Couldn't statfs() memory path: %s",
>>> strerror(errno));
>>
>> Indentation is off.
>>
>>> exit(1);
>>> }
>>> @@ -160,10 +161,10 @@ void *qemu_ram_mmap(int fd,
>>> len = 0;
>>> }
>>> file_name[len] = '\0';
>>> - fprintf(stderr, "Warning: requesting persistence across
>>> crashes "
>>> - "for backend file %s failed. Proceeding without "
>>> - "persistence, data might become corrupted in
>>> case of host "
>>> - "crash.\n", file_name);
>>> + error_report("Warning: requesting persistence across
>>> crashes "
>>> + "for backend file %s failed. Proceeding
>>> without "
>>> + "persistence, data might become corrupted
>>> in case "
>>> + "of host crash.", file_name);
>>
>> This should be something like
>>
>> warn_report("requesting persistence across crashes"
>> " for backend file %s failed",
>> file_name);
>> error_printf("Proceeding without persistence, data might"
>> " become corrupted in case of host
>> crash.\n");
>>
>> Precedence: commit db0754df88 "file-posix: Use error API properly".
>>
>>> g_free(proc_link);
>>> g_free(file_name);
>>> }
>>> diff --git a/util/module.c b/util/module.c
>>> index 236a7bb52a..28efa1f891 100644
>>> --- a/util/module.c
>>> +++ b/util/module.c
>>> @@ -19,6 +19,7 @@
>>> #endif
>>> #include "qemu/queue.h"
>>> #include "qemu/module.h"
>>> +#include "qemu/error-report.h"
>>> typedef struct ModuleEntry
>>> {
>>> @@ -130,19 +131,17 @@ static int module_load_file(const char *fname)
>>> g_module = g_module_open(fname, G_MODULE_BIND_LAZY |
>>> G_MODULE_BIND_LOCAL);
>>> if (!g_module) {
>>> - fprintf(stderr, "Failed to open module: %s\n",
>>> - g_module_error());
>>> + error_report("Failed to open module: %s", g_module_error());
>>> ret = -EINVAL;
>>> goto out;
>>> }
>>> if (!g_module_symbol(g_module, DSO_STAMP_FUN_STR, (gpointer
>>> *)&sym)) {
>>> - fprintf(stderr, "Failed to initialize module: %s\n",
>>> - fname);
>>> + error_report("Failed to initialize module: %s", fname);
>>> /* Print some info if this is a QEMU module (but from
>>> different build),
>>> * this will make debugging user problems easier. */
>>> if (g_module_symbol(g_module, "qemu_module_dummy",
>>> (gpointer *)&sym)) {
>>> - fprintf(stderr,
>>> - "Note: only modules from the same build can be
>>> loaded.\n");
>>> + error_report("Note: "
>>> + "only modules from the same build can be
>>> loaded.");
>>
>> Use error_printf() to print the additional note.
>>
>>> }
>>> g_module_close(g_module);
>>> ret = -EINVAL;
>>> @@ -178,7 +177,7 @@ bool module_load_one(const char *prefix, const
>>> char *lib_name)
>>> static GHashTable *loaded_modules;
>>> if (!g_module_supported()) {
>>> - fprintf(stderr, "Module is not supported by system.\n");
>>> + error_report("Module is not supported by system.");
>>> return false;
>>> }
>>> diff --git a/util/osdep.c b/util/osdep.c
>>> index f7d06050f7..ef40ae512a 100644
>>> --- a/util/osdep.c
>>> +++ b/util/osdep.c
>>> @@ -484,7 +484,7 @@ void fips_set_state(bool requested)
>>> #endif /* __linux__ */
>>> #ifdef _FIPS_DEBUG
>>> - fprintf(stderr, "FIPS mode %s (requested %s)\n",
>>> + error_report("FIPS mode %s (requested %s)",
>>> (fips_enabled ? "enabled" : "disabled"),
>>> (requested ? "enabled" : "disabled"));
>>> #endif
>>> @@ -511,7 +511,7 @@ int socket_init(void)
>>> ret = WSAStartup(MAKEWORD(2, 2), &Data);
>>> if (ret != 0) {
>>> err = WSAGetLastError();
>>> - fprintf(stderr, "WSAStartup: %d\n", err);
>>> + error_report("WSAStartup: %d", err);
>>> return -1;
>>> }
>>> atexit(socket_cleanup);
>>> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
>>> index 897e8f3ba6..4977594a43 100644
>>> --- a/util/oslib-posix.c
>>> +++ b/util/oslib-posix.c
>>> @@ -35,6 +35,7 @@
>>> #include "sysemu/sysemu.h"
>>> #include "trace.h"
>>> #include "qapi/error.h"
>>> +#include "qemu/error-report.h"
>>> #include "qemu/sockets.h"
>>> #include "qemu/thread.h"
>>> #include <libgen.h>
>>> @@ -170,7 +171,7 @@ fail_close:
>>> void *qemu_oom_check(void *ptr)
>>> {
>>> if (ptr == NULL) {
>>> - fprintf(stderr, "Failed to allocate memory: %s\n",
>>> strerror(errno));
>>> + error_report("Failed to allocate memory: %s", strerror(errno));
>>> abort();
>>> }
>>> return ptr;
>>> diff --git a/util/oslib-win32.c b/util/oslib-win32.c
>>> index e9b14ab178..84b937865a 100644
>>> --- a/util/oslib-win32.c
>>> +++ b/util/oslib-win32.c
>>> @@ -39,6 +39,7 @@
>>> #include "trace.h"
>>> #include "qemu/sockets.h"
>>> #include "qemu/cutils.h"
>>> +#include "qemu/error-report.h"
>>> /* this must come after including "trace.h" */
>>> #include <shlobj.h>
>>> @@ -46,7 +47,7 @@
>>> void *qemu_oom_check(void *ptr)
>>> {
>>> if (ptr == NULL) {
>>> - fprintf(stderr, "Failed to allocate memory: %lu\n",
>>> GetLastError());
>>> + error_report("Failed to allocate memory: %lu", GetLastError());
>>> abort();
>>> }
>>> return ptr;
>>> diff --git a/util/qemu-coroutine.c b/util/qemu-coroutine.c
>>> index c3caa6c770..62d1dd09df 100644
>>> --- a/util/qemu-coroutine.c
>>> +++ b/util/qemu-coroutine.c
>>> @@ -14,6 +14,7 @@
>>> #include "qemu/osdep.h"
>>> #include "trace.h"
>>> +#include "qemu/error-report.h"
>>> #include "qemu/thread.h"
>>> #include "qemu/atomic.h"
>>> #include "qemu/coroutine.h"
>>> @@ -125,14 +126,13 @@ void qemu_aio_coroutine_enter(AioContext *ctx,
>>> Coroutine *co)
>>> * cause us to enter it twice, potentially even after the
>>> coroutine has
>>> * been deleted */
>>> if (scheduled) {
>>> - fprintf(stderr,
>>> - "%s: Co-routine was already scheduled in '%s'\n",
>>> - __func__, scheduled);
>>> + error_report("%s: Co-routine was already scheduled in
>>> '%s'",
>>> + __func__, scheduled);
>>> abort();
>>> }
>>> if (to->caller) {
>>> - fprintf(stderr, "Co-routine re-entered recursively\n");
>>> + error_report("Co-routine re-entered recursively");
>>> abort();
>>> }
>>> @@ -185,7 +185,7 @@ void coroutine_fn qemu_coroutine_yield(void)
>>> trace_qemu_coroutine_yield(self, to);
>>> if (!to) {
>>> - fprintf(stderr, "Co-routine is yielding to no one\n");
>>> + error_report("Co-routine is yielding to no one");
>>> abort();
>>> }
>>> diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
>>> index 838980aaa5..b4d8de376c 100644
>>> --- a/util/qemu-thread-posix.c
>>> +++ b/util/qemu-thread-posix.c
>>> @@ -14,6 +14,7 @@
>>> #include "qemu/thread.h"
>>> #include "qemu/atomic.h"
>>> #include "qemu/notify.h"
>>> +#include "qemu/error-report.h"
>>> #include "qemu-thread-common.h"
>>> static bool name_threads;
>>> @@ -25,14 +26,14 @@ void qemu_thread_naming(bool enable)
>>> #ifndef CONFIG_THREAD_SETNAME_BYTHREAD
>>> /* This is a debugging option, not fatal */
>>> if (enable) {
>>> - fprintf(stderr, "qemu: thread naming not supported on this
>>> host\n");
>>> + error_report("qemu: thread naming not supported on this host");
>>
>> This isn't an error. It's in response to -name debug-threads=on, and
>> tells the user debug-threads=on is being ignored. Let's use
>> warn_report().
>>
>> Drop the "qemu: ", please; error_report() & friends take care of that.
>> More of the same below.
>>
>>> }
>>> #endif
>>> }
>>> static void error_exit(int err, const char *msg)
>>> {
>>> - fprintf(stderr, "qemu: %s: %s\n", msg, strerror(err));
>>> + error_report("qemu: %s: %s", msg, strerror(err));
>>> abort();
>>> }
>>> diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c
>>> index 56a83333da..9bed338d7e 100644
>>> --- a/util/qemu-thread-win32.c
>>> +++ b/util/qemu-thread-win32.c
>>> @@ -15,6 +15,7 @@
>>> #include "qemu-common.h"
>>> #include "qemu/thread.h"
>>> #include "qemu/notify.h"
>>> +#include "qemu/error-report.h"
>>> #include "qemu-thread-common.h"
>>> #include <process.h>
>>> @@ -25,7 +26,7 @@ void qemu_thread_naming(bool enable)
>>> /* But note we don't actually name them on Windows yet */
>>> name_threads = enable;
>>> - fprintf(stderr, "qemu: thread naming not supported on this
>>> host\n");
>>> + error_report("qemu: thread naming not supported on this host");
>>
>> Likewise.
>
> Thanks for your review. I'll drop the changes in util/oslib-win32.c for
> for now, and add a note in my TODO for after the 5.0 release.
Well if I follow this line, I'v to drop the changes in util/osdep.c too.
Maybe we can keep fprintf() for now and improve the error message, and
do the fprintf -> error_report cleanup later?
>
>>
>>> }
>>> static void error_exit(int err, const char *msg)
>>> @@ -34,7 +35,7 @@ static void error_exit(int err, const char *msg)
>>> FormatMessage(FORMAT_MESSAGE_FROM_SYSTEM |
>>> FORMAT_MESSAGE_ALLOCATE_BUFFER,
>>> NULL, err, 0, (LPTSTR)&pstr, 2, NULL);
>>> - fprintf(stderr, "qemu: %s: %s\n", msg, pstr);
>>> + error_report("qemu: %s: %s", msg, pstr);
>>> LocalFree(pstr);
>>> abort();
>>> }
>>> diff --git a/util/qemu-timer-common.c b/util/qemu-timer-common.c
>>> index baf3317f74..527944da1c 100644
>>> --- a/util/qemu-timer-common.c
>>> +++ b/util/qemu-timer-common.c
>>> @@ -23,6 +23,7 @@
>>> */
>>> #include "qemu/osdep.h"
>>> #include "qemu/timer.h"
>>> +#include "qemu/error-report.h"
>>> /***********************************************************/
>>> /* real time host monotonic timer */
>>> @@ -37,7 +38,7 @@ static void __attribute__((constructor))
>>> init_get_clock(void)
>>> int ret;
>>> ret = QueryPerformanceFrequency(&freq);
>>> if (ret == 0) {
>>> - fprintf(stderr, "Could not calibrate ticks\n");
>>> + error_report("Could not calibrate ticks");
>>> exit(1);
>>> }
>>> clock_freq = freq.QuadPart;
>>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/6] util: Replace fprintf(stderr, "*\n" with error_report()
2020-02-28 9:57 ` Philippe Mathieu-Daudé
@ 2020-02-28 17:41 ` Markus Armbruster
0 siblings, 0 replies; 15+ messages in thread
From: Markus Armbruster @ 2020-02-28 17:41 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Kevin Wolf, Stefan Weil, Alistair Francis, qemu-devel,
Michael Roth, Tomáš Golembiovský,
Alistair Francis, Stefan Hajnoczi, Marc-André Lureau,
Paolo Bonzini, Philippe Mathieu-Daudé
Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> On 2/28/20 10:50 AM, Philippe Mathieu-Daudé wrote:
[...]
>> Thanks for your review. I'll drop the changes in util/oslib-win32.c
>> for for now, and add a note in my TODO for after the 5.0 release.
>
> Well if I follow this line, I'v to drop the changes in util/osdep.c too.
> Maybe we can keep fprintf() for now and improve the error message, and
> do the fprintf -> error_report cleanup later?
I recommend to convert from fprintf() to error_report() & friends and
improve the message all in one go.
Separating different kinds of changes makes sense when some kinds are
mechanical and the resulting mechanical patches are large. These
patches aren't large.
But it's really up to you. I'm not going to veto an improvement only
because further improvement is called for.
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2020-02-28 17:42 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-27 16:30 [PATCH v2 0/6] misc: Improve error reporting on Windows Philippe Mathieu-Daudé
2020-02-27 16:30 ` [PATCH v2 1/6] chardev: Improve error report by calling error_setg_win32() Philippe Mathieu-Daudé
2020-02-27 16:30 ` [PATCH v2 2/6] util: Replace fprintf(stderr, "*\n" with error_report() Philippe Mathieu-Daudé
2020-02-28 7:43 ` Markus Armbruster
2020-02-28 9:50 ` Philippe Mathieu-Daudé
2020-02-28 9:57 ` Philippe Mathieu-Daudé
2020-02-28 17:41 ` Markus Armbruster
2020-02-27 16:30 ` [PATCH v2 3/6] util/oslib-win32: Improve error report by calling error_setg_win32() Philippe Mathieu-Daudé
2020-02-27 16:30 ` [PATCH v2 4/6] util/osdep: " Philippe Mathieu-Daudé
2020-02-27 17:22 ` Marc-André Lureau
2020-02-27 16:31 ` [PATCH v2 5/6] qga: Fix a memory leak Philippe Mathieu-Daudé
2020-02-27 17:21 ` Marc-André Lureau
2020-02-27 16:31 ` [PATCH v2 6/6] qga: Improve error report by calling error_setg_win32() Philippe Mathieu-Daudé
2020-02-27 17:20 ` Marc-André Lureau
2020-02-27 17:34 ` Philippe Mathieu-Daudé
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.