* [Qemu-devel] [PATCH v3 0/8] Implement a warning_report function
@ 2017-07-11 12:07 Alistair Francis
2017-07-11 12:07 ` [Qemu-devel] [PATCH v3 1/8] util/qemu-error: Rename error_print_loc() to be more generic Alistair Francis
` (8 more replies)
0 siblings, 9 replies; 23+ messages in thread
From: Alistair Francis @ 2017-07-11 12:07 UTC (permalink / raw)
To: qemu-devel; +Cc: alistair.francis, alistair23, philippe, berrange, armbru
QEMU currently has a standard method to report errors with
error_repot(). This ensure a sane and standard format when printing
errors. This series is attempting to extend this functionality for
warnings and information as well.
This patch renames error_print_loc() function to be more clear, but I
didn't bother renaming the others. It seems silly to change
error_printf() to error_warning_printf() and printf is already taken so
I just left it as is.
v3:
- Regenerate the replacement patch
v1:
- Convert all of the existing warning messages in QEMU
- Add a error_report*_err() functions as well
RFCv3:
- Use more detailed enum and function names
- Add wrapper functions for the info and warning reporting
RFCv2:
- Use enums for ERROR, WARN and INFO with a generic report() function
instead of adding new functions
Alistair Francis (8):
util/qemu-error: Rename error_print_loc() to be more generic
error: Functions to report warnings and informational messages
Convert error_report() to warn_report()
hw/i386: Improve some of the warning messages
char-socket: Report TCP socket waiting as information
error: Implement the warn and free Error functions
Convert error_report*_err() to warn_report*_err()
error: Add a 'error: ' prefix to error_report()
block/backup.c | 10 ++--
block/gluster.c | 4 +-
block/iscsi.c | 6 +--
block/nfs.c | 12 ++---
block/rbd.c | 6 +--
block/ssh.c | 4 +-
blockdev.c | 2 +-
chardev/char-socket.c | 4 +-
cpus.c | 2 +-
hw/9pfs/9p.c | 2 +-
hw/arm/highbank.c | 6 +--
hw/arm/imx25_pdk.c | 6 +--
hw/arm/kzm.c | 6 +--
hw/core/machine.c | 10 ++--
hw/core/qdev-properties.c | 10 ++--
hw/i386/acpi-build.c | 11 +++--
hw/i386/kvm/pci-assign.c | 6 +--
hw/i386/pc.c | 16 +++---
hw/i386/pc_piix.c | 8 +--
hw/i386/pc_q35.c | 6 +--
hw/misc/aspeed_sdmc.c | 8 +--
hw/nvram/fw_cfg.c | 2 +-
hw/pci-host/piix.c | 2 +-
hw/ppc/pnv.c | 6 +--
hw/ppc/spapr.c | 4 +-
hw/ppc/spapr_iommu.c | 2 +-
hw/scsi/scsi-bus.c | 6 +--
hw/usb/dev-smartcard-reader.c | 4 +-
hw/usb/redirect.c | 2 +-
include/qapi/error.h | 11 +++++
include/qemu/error-report.h | 7 +++
net/tap-linux.c | 2 +-
scripts/checkpatch.pl | 7 ++-
target/i386/cpu.c | 8 +--
target/i386/kvm.c | 4 +-
target/s390x/cpu_models.c | 2 +-
target/s390x/kvm.c | 2 +-
tests/test-qdev-global-props.c | 6 +--
trace/control.c | 4 +-
util/error.c | 20 ++++++++
util/qemu-error.c | 107 ++++++++++++++++++++++++++++++++++++++---
vl.c | 20 ++++----
42 files changed, 254 insertions(+), 119 deletions(-)
--
2.11.0
^ permalink raw reply [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH v3 1/8] util/qemu-error: Rename error_print_loc() to be more generic
2017-07-11 12:07 [Qemu-devel] [PATCH v3 0/8] Implement a warning_report function Alistair Francis
@ 2017-07-11 12:07 ` Alistair Francis
2017-07-11 12:07 ` [Qemu-devel] [PATCH v3 2/8] error: Functions to report warnings and informational messages Alistair Francis
` (7 subsequent siblings)
8 siblings, 0 replies; 23+ messages in thread
From: Alistair Francis @ 2017-07-11 12:07 UTC (permalink / raw)
To: qemu-devel; +Cc: alistair.francis, alistair23, philippe, berrange, armbru
Rename the error_print_loc() function in preparation for using it to
print warnings as well.
Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
util/qemu-error.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/util/qemu-error.c b/util/qemu-error.c
index b331f8f4a4..1c5e35ecdb 100644
--- a/util/qemu-error.c
+++ b/util/qemu-error.c
@@ -146,7 +146,7 @@ const char *error_get_progname(void)
/*
* Print current location to current monitor if we have one, else to stderr.
*/
-static void error_print_loc(void)
+static void print_loc(void)
{
const char *sep = "";
int i;
@@ -197,7 +197,7 @@ void error_vreport(const char *fmt, va_list ap)
g_free(timestr);
}
- error_print_loc();
+ print_loc();
error_vprintf(fmt, ap);
error_printf("\n");
}
--
2.11.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH v3 2/8] error: Functions to report warnings and informational messages
2017-07-11 12:07 [Qemu-devel] [PATCH v3 0/8] Implement a warning_report function Alistair Francis
2017-07-11 12:07 ` [Qemu-devel] [PATCH v3 1/8] util/qemu-error: Rename error_print_loc() to be more generic Alistair Francis
@ 2017-07-11 12:07 ` Alistair Francis
2017-07-12 7:57 ` Markus Armbruster
2017-07-11 12:07 ` [Qemu-devel] [PATCH v3 3/8] Convert error_report() to warn_report() Alistair Francis
` (6 subsequent siblings)
8 siblings, 1 reply; 23+ messages in thread
From: Alistair Francis @ 2017-07-11 12:07 UTC (permalink / raw)
To: qemu-devel; +Cc: alistair.francis, alistair23, philippe, berrange, armbru
Add warn_report(), warn_vreport() for reporting warnings, and
info_report(), info_vreport() for informational messages.
These are implemented them with a helper function factored out of
error_vreport(), suitably generalized. This patch makes no changes
to the output of the original error_report() function.
Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
v2:
- Don't add *vreport() functions to checkpatch
- Maintain original comments for the reporting functions
- Don't change the error report output in this patch
v1:
- Don't expose the generic report and vreport() functions
- Prefix error messages
- Use vreport instead of qmsg_vreport()
RFC V3:
- Change the function and enum names to be more descriptive
- Add wrapper functions for *_report() and *_vreport()
include/qemu/error-report.h | 7 +++
scripts/checkpatch.pl | 6 ++-
util/qemu-error.c | 102 +++++++++++++++++++++++++++++++++++++++++---
3 files changed, 109 insertions(+), 6 deletions(-)
diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h
index 3001865896..e1c8ae1a52 100644
--- a/include/qemu/error-report.h
+++ b/include/qemu/error-report.h
@@ -35,8 +35,15 @@ void error_printf(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
void error_vprintf_unless_qmp(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0);
void error_printf_unless_qmp(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
void error_set_progname(const char *argv0);
+
void error_vreport(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0);
+void warn_vreport(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0);
+void info_vreport(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0);
+
void error_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
+void warn_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
+void info_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
+
const char *error_get_progname(void);
extern bool enable_timestamp_msg;
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 73efc927a9..60b1f320a1 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2535,7 +2535,11 @@ sub process {
error_prepend|
error_reportf_err|
error_vreport|
- error_report}x;
+ report_vreport|
+ info_vreport|
+ error_report|
+ warn_report|
+ info_report}x;
if ($rawline =~ /\b(?:$qemu_error_funcs)\s*\(.*\".*\\n/) {
ERROR("Error messages should not contain newlines\n" . $herecurr);
diff --git a/util/qemu-error.c b/util/qemu-error.c
index 1c5e35ecdb..c557c6ae47 100644
--- a/util/qemu-error.c
+++ b/util/qemu-error.c
@@ -14,6 +14,16 @@
#include "monitor/monitor.h"
#include "qemu/error-report.h"
+/*
+ * @report_type is the type of message: error, warning or
+ * informational.
+ */
+typedef enum {
+ REPORT_TYPE_ERROR,
+ REPORT_TYPE_WARNING,
+ REPORT_TYPE_INFO,
+} report_type;
+
void error_printf(const char *fmt, ...)
{
va_list ap;
@@ -179,17 +189,29 @@ static void print_loc(void)
bool enable_timestamp_msg;
/*
- * Print an error message to current monitor if we have one, else to stderr.
+ * Print a message to current monitor if we have one, else to stderr.
+ * @report_type is the type of message: error, warning or informational.
* Format arguments like vsprintf(). The resulting message should be
* a single phrase, with no newline or trailing punctuation.
* Prepend the current location and append a newline.
* It's wrong to call this in a QMP monitor. Use error_setg() there.
*/
-void error_vreport(const char *fmt, va_list ap)
+static void vreport(report_type type, const char *fmt, va_list ap)
{
GTimeVal tv;
gchar *timestr;
+ switch (type) {
+ case REPORT_TYPE_ERROR:
+ break;
+ case REPORT_TYPE_WARNING:
+ error_printf("warning: ");
+ break;
+ case REPORT_TYPE_INFO:
+ error_printf("info: ");
+ break;
+ }
+
if (enable_timestamp_msg && !cur_mon) {
g_get_current_time(&tv);
timestr = g_time_val_to_iso8601(&tv);
@@ -204,8 +226,45 @@ void error_vreport(const char *fmt, va_list ap)
/*
* Print an error message to current monitor if we have one, else to stderr.
- * Format arguments like sprintf(). The resulting message should be a
- * single phrase, with no newline or trailing punctuation.
+ * Format arguments like vsprintf(). The resulting message should be
+ * a single phrase, with no newline or trailing punctuation.
+ * Prepend the current location and append a newline.
+ * It's wrong to call this in a QMP monitor. Use error_setg() there.
+ */
+void error_vreport(const char *fmt, va_list ap)
+{
+ vreport(REPORT_TYPE_ERROR, fmt, ap);
+}
+
+/*
+ * Print a warning message to current monitor if we have one, else to stderr.
+ * Format arguments like vsprintf(). The resulting message should be
+ * a single phrase, with no newline or trailing punctuation.
+ * Prepend the current location and append a newline.
+ * It's wrong to call this in a QMP monitor. Use error_setg() there.
+ */
+void warn_vreport(const char *fmt, va_list ap)
+{
+ vreport(REPORT_TYPE_WARNING, fmt, ap);
+}
+
+/*
+ * Print an information message to current monitor if we have one, else to
+ * stderr.
+ * Format arguments like vsprintf(). The resulting message should be
+ * a single phrase, with no newline or trailing punctuation.
+ * Prepend the current location and append a newline.
+ * It's wrong to call this in a QMP monitor. Use error_setg() there.
+ */
+void info_vreport(const char *fmt, va_list ap)
+{
+ vreport(REPORT_TYPE_INFO, fmt, ap);
+}
+
+/*
+ * Print an error message to current monitor if we have one, else to stderr.
+ * Format arguments like sprintf(). The resulting message should be
+ * a single phrase, with no newline or trailing punctuation.
* Prepend the current location and append a newline.
* It's wrong to call this in a QMP monitor. Use error_setg() there.
*/
@@ -214,6 +273,39 @@ void error_report(const char *fmt, ...)
va_list ap;
va_start(ap, fmt);
- error_vreport(fmt, ap);
+ vreport(REPORT_TYPE_ERROR, fmt, ap);
+ va_end(ap);
+}
+
+/*
+ * Print a warning message to current monitor if we have one, else to stderr.
+ * Format arguments like sprintf(). The resulting message should be a
+ * single phrase, with no newline or trailing punctuation.
+ * Prepend the current location and append a newline.
+ * It's wrong to call this in a QMP monitor. Use error_setg() there.
+ */
+void warn_report(const char *fmt, ...)
+{
+ va_list ap;
+
+ va_start(ap, fmt);
+ vreport(REPORT_TYPE_WARNING, fmt, ap);
+ va_end(ap);
+}
+
+/*
+ * Print an information message to current monitor if we have one, else to
+ * stderr.
+ * Format arguments like sprintf(). The resulting message should be a
+ * single phrase, with no newline or trailing punctuation.
+ * Prepend the current location and append a newline.
+ * It's wrong to call this in a QMP monitor. Use error_setg() there.
+ */
+void info_report(const char *fmt, ...)
+{
+ va_list ap;
+
+ va_start(ap, fmt);
+ vreport(REPORT_TYPE_INFO, fmt, ap);
va_end(ap);
}
--
2.11.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH v3 3/8] Convert error_report() to warn_report()
2017-07-11 12:07 [Qemu-devel] [PATCH v3 0/8] Implement a warning_report function Alistair Francis
2017-07-11 12:07 ` [Qemu-devel] [PATCH v3 1/8] util/qemu-error: Rename error_print_loc() to be more generic Alistair Francis
2017-07-11 12:07 ` [Qemu-devel] [PATCH v3 2/8] error: Functions to report warnings and informational messages Alistair Francis
@ 2017-07-11 12:07 ` Alistair Francis
2017-07-11 16:51 ` Max Reitz
2017-07-12 8:34 ` Markus Armbruster
2017-07-11 12:07 ` [Qemu-devel] [PATCH v3 4/8] hw/i386: Improve some of the warning messages Alistair Francis
` (5 subsequent siblings)
8 siblings, 2 replies; 23+ messages in thread
From: Alistair Francis @ 2017-07-11 12:07 UTC (permalink / raw)
To: qemu-devel
Cc: alistair.francis, alistair23, philippe, berrange, armbru,
Jeff Cody, Kevin Wolf, Max Reitz, Ronnie Sahlberg, Paolo Bonzini,
Peter Lieven, Josh Durgin, Richard W.M. Jones, Peter Crosthwaite,
Richard Henderson, Aneesh Kumar K.V, Greg Kurz, Rob Herring,
Peter Maydell, Peter Chubb, Eduardo Habkost, Marcel Apfelbaum,
Michael S. Tsirkin, Igor Mammedov, David Gibson, Alexander Graf,
Gerd Hoffmann, Jason Wang, Marcelo Tosatti,
Christian Borntraeger, Cornelia Huck, Stefan Hajnoczi
Convert all uses of error_report("warning:"... to use warn_report()
instead. This helps standardise on a single method of printing warnings
to the user.
All of the warnings where changed using these two commands:
find ./* -type f -exec sed -i \
's|error_report(".*warning[,:] |warn_report("|Ig' {} +
Then the white space changes where manually fixed afterwards.
The test-qdev-global-props test case was manually updated to ensure that
this patch passes make check (as the test cases are case sensitive).
Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
Suggested-by: Thomas Huth <thuth@redhat.com>
Cc: Jeff Cody <jcody@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Lieven <pl@kamp.de>
Cc: Josh Durgin <jdurgin@redhat.com>
Cc: "Richard W.M. Jones" <rjones@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Peter Crosthwaite <crosthwaite.peter@gmail.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
Cc: Greg Kurz <groug@kaod.org>
Cc: Rob Herring <robh@kernel.org>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Peter Chubb <peter.chubb@nicta.com.au>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Marcel Apfelbaum <marcel@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: David Gibson <david@gibson.dropbear.id.au>
Cc: Alexander Graf <agraf@suse.de>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Marcelo Tosatti <mtosatti@redhat.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Cornelia Huck <cohuck@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
Acked-by: Greg Kurz <groug@kaod.org>
Acked-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed by: Peter Chubb <peter.chubb@data61.csiro.au>
Acked-by: Max Reitz <mreitz@redhat.com>
Acked-by: Marcel Apfelbaum <marcel@redhat.com>
---
V3:
- Regenerate patch to ensure no manual edits where made
- Tighten regex matches
V2:
- Fix quotation issues
- Update commit message
- Include full command
Just a note:
We will need to do a similar thing for fprintf. There are patches on
the list at the moment that conflict with this series (by adding
error_report() calls that shsould be warning), so I think I'm giong to
have to do a follow up series converting more cases. I'll cover the
fprintf cases when I do that. There are some manual cases as
well that don't say warning or info, but really should. They will have
to be converted as well.
block/backup.c | 10 +++++-----
block/gluster.c | 4 ++--
block/iscsi.c | 6 +++---
block/nfs.c | 12 ++++++------
block/rbd.c | 6 +++---
block/ssh.c | 4 ++--
blockdev.c | 2 +-
cpus.c | 2 +-
hw/9pfs/9p.c | 2 +-
hw/arm/highbank.c | 6 +++---
hw/arm/imx25_pdk.c | 6 +++---
hw/arm/kzm.c | 6 +++---
hw/core/machine.c | 10 +++++-----
hw/core/qdev-properties.c | 8 ++++----
hw/i386/acpi-build.c | 10 +++++-----
hw/i386/kvm/pci-assign.c | 6 +++---
hw/i386/pc.c | 10 +++++-----
hw/i386/pc_piix.c | 8 ++++----
hw/i386/pc_q35.c | 6 +++---
hw/misc/aspeed_sdmc.c | 8 ++++----
hw/nvram/fw_cfg.c | 2 +-
hw/pci-host/piix.c | 2 +-
hw/ppc/pnv.c | 6 +++---
hw/ppc/spapr.c | 4 ++--
hw/ppc/spapr_iommu.c | 2 +-
hw/scsi/scsi-bus.c | 6 +++---
hw/usb/dev-smartcard-reader.c | 4 ++--
hw/usb/redirect.c | 2 +-
net/tap-linux.c | 2 +-
target/i386/cpu.c | 8 ++++----
target/i386/kvm.c | 4 ++--
target/s390x/cpu_models.c | 2 +-
target/s390x/kvm.c | 2 +-
tests/test-qdev-global-props.c | 6 +++---
trace/control.c | 4 ++--
vl.c | 20 ++++++++++----------
36 files changed, 104 insertions(+), 104 deletions(-)
diff --git a/block/backup.c b/block/backup.c
index b69184eac5..44cc2b22ab 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -639,11 +639,11 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
ret = bdrv_get_info(target, &bdi);
if (ret == -ENOTSUP && !target->backing) {
/* Cluster size is not defined */
- error_report("WARNING: The target block device doesn't provide "
- "information about the block size and it doesn't have a "
- "backing file. The default block size of %u bytes is "
- "used. If the actual block size of the target exceeds "
- "this default, the backup may be unusable",
+ warn_report("The target block device doesn't provide "
+ "information about the block size and it doesn't have a "
+ "backing file. The default block size of %u bytes is "
+ "used. If the actual block size of the target exceeds "
+ "this default, the backup may be unusable",
BACKUP_CLUSTER_SIZE_DEFAULT);
job->cluster_size = BACKUP_CLUSTER_SIZE_DEFAULT;
} else if (ret < 0 && !target->backing) {
diff --git a/block/gluster.c b/block/gluster.c
index addceed6eb..ba88fe116e 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -345,8 +345,8 @@ static int qemu_gluster_parse_uri(BlockdevOptionsGluster *gconf,
is_unix = true;
} else if (!strcmp(uri->scheme, "gluster+rdma")) {
gsconf->type = SOCKET_ADDRESS_TYPE_INET;
- error_report("Warning: rdma feature is not supported, falling "
- "back to tcp");
+ warn_report("rdma feature is not supported, falling "
+ "back to tcp");
} else {
ret = -EINVAL;
goto out;
diff --git a/block/iscsi.c b/block/iscsi.c
index 54067e2620..3aa438a0b7 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1761,9 +1761,9 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
* filename encoded options */
filename = qdict_get_try_str(options, "filename");
if (filename) {
- error_report("Warning: 'filename' option specified. "
- "This is an unsupported option, and may be deprecated "
- "in the future");
+ warn_report("'filename' option specified. "
+ "This is an unsupported option, and may be deprecated "
+ "in the future");
iscsi_parse_filename(filename, options, &local_err);
if (local_err) {
ret = -EINVAL;
diff --git a/block/nfs.c b/block/nfs.c
index c3c5de0113..43929c6f23 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -558,8 +558,8 @@ static int64_t nfs_client_open(NFSClient *client, QDict *options,
}
client->readahead = qemu_opt_get_number(opts, "readahead-size", 0);
if (client->readahead > QEMU_NFS_MAX_READAHEAD_SIZE) {
- error_report("NFS Warning: Truncating NFS readahead "
- "size to %d", QEMU_NFS_MAX_READAHEAD_SIZE);
+ warn_report("Truncating NFS readahead "
+ "size to %d", QEMU_NFS_MAX_READAHEAD_SIZE);
client->readahead = QEMU_NFS_MAX_READAHEAD_SIZE;
}
nfs_set_readahead(client->context, client->readahead);
@@ -579,8 +579,8 @@ static int64_t nfs_client_open(NFSClient *client, QDict *options,
}
client->pagecache = qemu_opt_get_number(opts, "page-cache-size", 0);
if (client->pagecache > QEMU_NFS_MAX_PAGECACHE_SIZE) {
- error_report("NFS Warning: Truncating NFS pagecache "
- "size to %d pages", QEMU_NFS_MAX_PAGECACHE_SIZE);
+ warn_report("Truncating NFS pagecache "
+ "size to %d pages", QEMU_NFS_MAX_PAGECACHE_SIZE);
client->pagecache = QEMU_NFS_MAX_PAGECACHE_SIZE;
}
nfs_set_pagecache(client->context, client->pagecache);
@@ -595,8 +595,8 @@ static int64_t nfs_client_open(NFSClient *client, QDict *options,
/* limit the maximum debug level to avoid potential flooding
* of our log files. */
if (client->debug > QEMU_NFS_MAX_DEBUG_LEVEL) {
- error_report("NFS Warning: Limiting NFS debug level "
- "to %d", QEMU_NFS_MAX_DEBUG_LEVEL);
+ warn_report("Limiting NFS debug level "
+ "to %d", QEMU_NFS_MAX_DEBUG_LEVEL);
client->debug = QEMU_NFS_MAX_DEBUG_LEVEL;
}
nfs_set_debug(client->context, client->debug);
diff --git a/block/rbd.c b/block/rbd.c
index 9da02cdceb..d461f7dc87 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -555,9 +555,9 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,
* filename encoded options */
filename = qdict_get_try_str(options, "filename");
if (filename) {
- error_report("Warning: 'filename' option specified. "
- "This is an unsupported option, and may be deprecated "
- "in the future");
+ warn_report("'filename' option specified. "
+ "This is an unsupported option, and may be deprecated "
+ "in the future");
qemu_rbd_parse_filename(filename, options, &local_err);
if (local_err) {
r = -EINVAL;
diff --git a/block/ssh.c b/block/ssh.c
index 52964416da..07a57eb466 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -1114,8 +1114,8 @@ static coroutine_fn int ssh_co_writev(BlockDriverState *bs,
static void unsafe_flush_warning(BDRVSSHState *s, const char *what)
{
if (!s->unsafe_flush_warning) {
- error_report("warning: ssh server %s does not support fsync",
- s->inet->host);
+ warn_report("ssh server %s does not support fsync",
+ s->inet->host);
if (what) {
error_report("to support fsync, you need %s", what);
}
diff --git a/blockdev.c b/blockdev.c
index e2016b6f37..a521c85956 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -914,7 +914,7 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
copy_on_read = qemu_opt_get_bool(legacy_opts, "copy-on-read", false);
if (read_only && copy_on_read) {
- error_report("warning: disabling copy-on-read on read-only drive");
+ warn_report("disabling copy-on-read on read-only drive");
copy_on_read = false;
}
diff --git a/cpus.c b/cpus.c
index 14bb8d552e..9bed61eefc 100644
--- a/cpus.c
+++ b/cpus.c
@@ -557,7 +557,7 @@ void qemu_start_warp_timer(void)
if (deadline < 0) {
static bool notified;
if (!icount_sleep && !notified) {
- error_report("WARNING: icount sleep disabled and no active timers");
+ warn_report("icount sleep disabled and no active timers");
notified = true;
}
return;
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 6c92bad5b3..333dbb6f8e 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -2376,7 +2376,7 @@ static void coroutine_fn v9fs_flush(void *opaque)
trace_v9fs_flush(pdu->tag, pdu->id, tag);
if (pdu->tag == tag) {
- error_report("Warning: the guest sent a self-referencing 9P flush request");
+ warn_report("the guest sent a self-referencing 9P flush request");
} else {
QLIST_FOREACH(cancel_pdu, &s->active_list, next) {
if (cancel_pdu->tag == tag) {
diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
index d209b97dee..750c463e2a 100644
--- a/hw/arm/highbank.c
+++ b/hw/arm/highbank.c
@@ -383,9 +383,9 @@ static void calxeda_init(MachineState *machine, enum cxmachines machine_id)
highbank_binfo.write_board_setup = hb_write_board_setup;
highbank_binfo.secure_board_setup = true;
} else {
- error_report("WARNING: cannot load built-in Monitor support "
- "if KVM is enabled. Some guests (such as Linux) "
- "may not boot.");
+ warn_report("cannot load built-in Monitor support "
+ "if KVM is enabled. Some guests (such as Linux) "
+ "may not boot.");
}
arm_load_kernel(ARM_CPU(first_cpu), &highbank_binfo);
diff --git a/hw/arm/imx25_pdk.c b/hw/arm/imx25_pdk.c
index 44e741fde3..7d42c74001 100644
--- a/hw/arm/imx25_pdk.c
+++ b/hw/arm/imx25_pdk.c
@@ -80,9 +80,9 @@ static void imx25_pdk_init(MachineState *machine)
/* We need to initialize our memory */
if (machine->ram_size > (FSL_IMX25_SDRAM0_SIZE + FSL_IMX25_SDRAM1_SIZE)) {
- error_report("WARNING: RAM size " RAM_ADDR_FMT " above max supported, "
- "reduced to %x", machine->ram_size,
- FSL_IMX25_SDRAM0_SIZE + FSL_IMX25_SDRAM1_SIZE);
+ warn_report("RAM size " RAM_ADDR_FMT " above max supported, "
+ "reduced to %x", machine->ram_size,
+ FSL_IMX25_SDRAM0_SIZE + FSL_IMX25_SDRAM1_SIZE);
machine->ram_size = FSL_IMX25_SDRAM0_SIZE + FSL_IMX25_SDRAM1_SIZE;
}
diff --git a/hw/arm/kzm.c b/hw/arm/kzm.c
index 2c96ee33b6..3ed6577a55 100644
--- a/hw/arm/kzm.c
+++ b/hw/arm/kzm.c
@@ -79,9 +79,9 @@ static void kzm_init(MachineState *machine)
/* Check the amount of memory is compatible with the SOC */
if (machine->ram_size > (FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE)) {
- error_report("WARNING: RAM size " RAM_ADDR_FMT " above max supported, "
- "reduced to %x", machine->ram_size,
- FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE);
+ warn_report("RAM size " RAM_ADDR_FMT " above max supported, "
+ "reduced to %x", machine->ram_size,
+ FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE);
machine->ram_size = FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE;
}
diff --git a/hw/core/machine.c b/hw/core/machine.c
index ecb55528e8..dc431fabf5 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -741,11 +741,11 @@ static void machine_numa_finish_init(MachineState *machine)
}
}
if (s->len && !qtest_enabled()) {
- error_report("warning: CPU(s) not present in any NUMA nodes: %s",
- s->str);
- error_report("warning: All CPU(s) up to maxcpus should be described "
- "in NUMA config, ability to start up with partial NUMA "
- "mappings is obsoleted and will be removed in future");
+ warn_report("CPU(s) not present in any NUMA nodes: %s",
+ s->str);
+ warn_report("All CPU(s) up to maxcpus should be described "
+ "in NUMA config, ability to start up with partial NUMA "
+ "mappings is obsoleted and will be removed in future");
}
g_string_free(s, true);
}
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index f11d57831b..f5983c83da 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -1132,15 +1132,15 @@ int qdev_prop_check_globals(void)
oc = object_class_by_name(prop->driver);
oc = object_class_dynamic_cast(oc, TYPE_DEVICE);
if (!oc) {
- error_report("Warning: global %s.%s has invalid class name",
- prop->driver, prop->property);
+ warn_report("global %s.%s has invalid class name",
+ prop->driver, prop->property);
ret = 1;
continue;
}
dc = DEVICE_CLASS(oc);
if (!dc->hotpluggable && !prop->used) {
- error_report("Warning: global %s.%s=%s not used",
- prop->driver, prop->property, prop->value);
+ warn_report("global %s.%s=%s not used",
+ prop->driver, prop->property, prop->value);
ret = 1;
continue;
}
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 5464977424..6b7bade183 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2766,17 +2766,17 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
ACPI_BUILD_ALIGN_SIZE);
if (tables_blob->len > legacy_table_size) {
/* Should happen only with PCI bridges and -M pc-i440fx-2.0. */
- error_report("Warning: migration may not work.");
+ warn_report("migration may not work.");
}
g_array_set_size(tables_blob, legacy_table_size);
} else {
/* Make sure we have a buffer in case we need to resize the tables. */
if (tables_blob->len > ACPI_BUILD_TABLE_SIZE / 2) {
/* As of QEMU 2.1, this fires with 160 VCPUs and 255 memory slots. */
- error_report("Warning: ACPI tables are larger than 64k.");
- error_report("Warning: migration may not work.");
- error_report("Warning: please remove CPUs, NUMA nodes, "
- "memory slots or PCI bridges.");
+ warn_report("ACPI tables are larger than 64k.");
+ warn_report("migration may not work.");
+ warn_report("please remove CPUs, NUMA nodes, "
+ "memory slots or PCI bridges.");
}
acpi_align_size(tables_blob, ACPI_BUILD_TABLE_SIZE);
}
diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index 9f2615cbe0..33e20cb3e8 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -1353,9 +1353,9 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev, Error **errp)
PCI_CAP_ID_EXP);
return -EINVAL;
} else if (size != 0x3c) {
- error_report("WARNING, %s: PCIe cap-id 0x%x has "
- "non-standard size 0x%x; std size should be 0x3c",
- __func__, PCI_CAP_ID_EXP, size);
+ warn_report("%s: PCIe cap-id 0x%x has "
+ "non-standard size 0x%x; std size should be 0x3c",
+ __func__, PCI_CAP_ID_EXP, size);
}
} else if (version == 0) {
uint16_t vid, did;
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 224fe58fe7..465e91cc5b 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -381,8 +381,8 @@ ISADevice *pc_find_fdc0(void)
}
if (state.multiple) {
- error_report("warning: multiple floppy disk controllers with "
- "iobase=0x3f0 have been found");
+ warn_report("multiple floppy disk controllers with "
+ "iobase=0x3f0 have been found");
error_printf("the one being picked for CMOS setup might not reflect "
"your intent\n");
}
@@ -2087,9 +2087,9 @@ static void pc_machine_set_max_ram_below_4g(Object *obj, Visitor *v,
}
if (value < (1ULL << 20)) {
- error_report("Warning: small max_ram_below_4g(%"PRIu64
- ") less than 1M. BIOS may not work..",
- value);
+ warn_report("small max_ram_below_4g(%"PRIu64
+ ") less than 1M. BIOS may not work..",
+ value);
}
pcms->max_ram_below_4g = value;
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 22dbef64c6..11b4336a42 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -131,10 +131,10 @@ static void pc_init1(MachineState *machine,
lowmem = 0xc0000000;
}
if (lowmem & ((1ULL << 30) - 1)) {
- error_report("Warning: Large machine and max_ram_below_4g "
- "(%" PRIu64 ") not a multiple of 1G; "
- "possible bad performance.",
- pcms->max_ram_below_4g);
+ warn_report("Large machine and max_ram_below_4g "
+ "(%" PRIu64 ") not a multiple of 1G; "
+ "possible bad performance.",
+ pcms->max_ram_below_4g);
}
}
}
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 8f696b7cb6..1653a47f0a 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -101,9 +101,9 @@ static void pc_q35_init(MachineState *machine)
lowmem = pcms->max_ram_below_4g;
if (machine->ram_size - lowmem > lowmem &&
lowmem & ((1ULL << 30) - 1)) {
- error_report("Warning: Large machine and max_ram_below_4g(%"PRIu64
- ") not a multiple of 1G; possible bad performance.",
- pcms->max_ram_below_4g);
+ warn_report("Large machine and max_ram_below_4g(%"PRIu64
+ ") not a multiple of 1G; possible bad performance.",
+ pcms->max_ram_below_4g);
}
}
diff --git a/hw/misc/aspeed_sdmc.c b/hw/misc/aspeed_sdmc.c
index 5f3ac0b6f6..633fa4510e 100644
--- a/hw/misc/aspeed_sdmc.c
+++ b/hw/misc/aspeed_sdmc.c
@@ -157,8 +157,8 @@ static int ast2400_rambits(AspeedSDMCState *s)
}
/* use a common default */
- error_report("warning: Invalid RAM size 0x%" PRIx64
- ". Using default 256M", s->ram_size);
+ warn_report("Invalid RAM size 0x%" PRIx64
+ ". Using default 256M", s->ram_size);
s->ram_size = 256 << 20;
return ASPEED_SDMC_DRAM_256MB;
}
@@ -179,8 +179,8 @@ static int ast2500_rambits(AspeedSDMCState *s)
}
/* use a common default */
- error_report("warning: Invalid RAM size 0x%" PRIx64
- ". Using default 512M", s->ram_size);
+ warn_report("Invalid RAM size 0x%" PRIx64
+ ". Using default 512M", s->ram_size);
s->ram_size = 512 << 20;
return ASPEED_SDMC_AST2500_512MB;
}
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 99bdbc2233..e881e3b812 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -781,7 +781,7 @@ static int get_fw_cfg_order(FWCfgState *s, const char *name)
}
/* Stick unknown stuff at the end. */
- error_report("warning: Unknown firmware file in legacy mode: %s", name);
+ warn_report("Unknown firmware file in legacy mode: %s", name);
return FW_CFG_ORDER_OVERRIDE_LAST;
}
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index a2c1033dbe..072a04e318 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -307,7 +307,7 @@ static void i440fx_realize(PCIDevice *dev, Error **errp)
dev->config[I440FX_SMRAM] = 0x02;
if (object_property_get_bool(qdev_get_machine(), "iommu", NULL)) {
- error_report("warning: i440fx doesn't support emulated iommu");
+ warn_report("i440fx doesn't support emulated iommu");
}
}
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index a4cd733cba..47221158d4 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -160,13 +160,13 @@ static void powernv_create_core_node(PnvChip *chip, PnvCore *pc, void *fdt)
_FDT((fdt_setprop_cell(fdt, offset, "d-cache-size",
pcc->l1_dcache_size)));
} else {
- error_report("Warning: Unknown L1 dcache size for cpu");
+ warn_report("Unknown L1 dcache size for cpu");
}
if (pcc->l1_icache_size) {
_FDT((fdt_setprop_cell(fdt, offset, "i-cache-size",
pcc->l1_icache_size)));
} else {
- error_report("Warning: Unknown L1 icache size for cpu");
+ warn_report("Unknown L1 icache size for cpu");
}
_FDT((fdt_setprop_cell(fdt, offset, "timebase-frequency", tbfreq)));
@@ -556,7 +556,7 @@ static void ppc_powernv_init(MachineState *machine)
/* allocate RAM */
if (machine->ram_size < (1 * G_BYTE)) {
- error_report("Warning: skiboot may not work with < 1GB of RAM");
+ warn_report("skiboot may not work with < 1GB of RAM");
}
ram = g_new(MemoryRegion, 1);
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 0ee9fac50b..fdd55d4820 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -534,13 +534,13 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
_FDT((fdt_setprop_cell(fdt, offset, "d-cache-size",
pcc->l1_dcache_size)));
} else {
- error_report("Warning: Unknown L1 dcache size for cpu");
+ warn_report("Unknown L1 dcache size for cpu");
}
if (pcc->l1_icache_size) {
_FDT((fdt_setprop_cell(fdt, offset, "i-cache-size",
pcc->l1_icache_size)));
} else {
- error_report("Warning: Unknown L1 icache size for cpu");
+ warn_report("Unknown L1 icache size for cpu");
}
_FDT((fdt_setprop_cell(fdt, offset, "timebase-frequency", tbfreq)));
diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
index 8656a54a3e..583afc1a46 100644
--- a/hw/ppc/spapr_iommu.c
+++ b/hw/ppc/spapr_iommu.c
@@ -334,7 +334,7 @@ void spapr_tce_table_enable(sPAPRTCETable *tcet,
uint32_t nb_table)
{
if (tcet->nb_table) {
- error_report("Warning: trying to enable already enabled TCE table");
+ warn_report("trying to enable already enabled TCE table");
return;
}
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index f5574469c8..23c51de66a 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -282,9 +282,9 @@ void scsi_bus_legacy_handle_cmdline(SCSIBus *bus, bool deprecated)
continue; /* claimed */
}
if (!dinfo->is_default) {
- error_report("warning: bus=%d,unit=%d is deprecated with this"
- " machine type",
- bus->busnr, unit);
+ warn_report("bus=%d,unit=%d is deprecated with this"
+ " machine type",
+ bus->busnr, unit);
}
}
scsi_bus_legacy_add_drive(bus, blk_by_legacy_dinfo(dinfo),
diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
index 49cb1829b5..79cd2758a5 100644
--- a/hw/usb/dev-smartcard-reader.c
+++ b/hw/usb/dev-smartcard-reader.c
@@ -1314,12 +1314,12 @@ static int ccid_card_init(DeviceState *qdev)
int ret = 0;
if (card->slot != 0) {
- error_report("Warning: usb-ccid supports one slot, can't add %d",
+ warn_report("usb-ccid supports one slot, can't add %d",
card->slot);
return -1;
}
if (s->card != NULL) {
- error_report("Warning: usb-ccid card already full, not adding");
+ warn_report("usb-ccid card already full, not adding");
return -1;
}
ret = ccid_card_initfn(card);
diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index aa22d69216..5b65965cc2 100644
--- a/hw/usb/redirect.c
+++ b/hw/usb/redirect.c
@@ -193,7 +193,7 @@ static void usbredir_handle_status(USBRedirDevice *dev, USBPacket *p,
#define WARNING(...) \
do { \
if (dev->debug >= usbredirparser_warning) { \
- error_report("usb-redir warning: " __VA_ARGS__); \
+ warn_report("" __VA_ARGS__); \
} \
} while (0)
#define INFO(...) \
diff --git a/net/tap-linux.c b/net/tap-linux.c
index a503fa9c6e..535b1ddb61 100644
--- a/net/tap-linux.c
+++ b/net/tap-linux.c
@@ -55,7 +55,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
ifr.ifr_flags = IFF_TAP | IFF_NO_PI;
if (ioctl(fd, TUNGETFEATURES, &features) == -1) {
- error_report("warning: TUNGETFEATURES failed: %s", strerror(errno));
+ warn_report("TUNGETFEATURES failed: %s", strerror(errno));
features = 0;
}
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index c57177278b..c078c8e7f1 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -2060,13 +2060,13 @@ static void x86_cpu_parse_featurestr(const char *typename, char *features,
name = featurestr;
if (g_list_find_custom(plus_features, name, compare_string)) {
- error_report("warning: Ambiguous CPU model string. "
+ warn_report("Ambiguous CPU model string. "
"Don't mix both \"+%s\" and \"%s=%s\"",
name, name, val);
ambiguous = true;
}
if (g_list_find_custom(minus_features, name, compare_string)) {
- error_report("warning: Ambiguous CPU model string. "
+ warn_report("Ambiguous CPU model string. "
"Don't mix both \"-%s\" and \"%s=%s\"",
name, name, val);
ambiguous = true;
@@ -2096,7 +2096,7 @@ static void x86_cpu_parse_featurestr(const char *typename, char *features,
}
if (ambiguous) {
- error_report("warning: Compatibility of ambiguous CPU model "
+ warn_report("Compatibility of ambiguous CPU model "
"strings won't be kept on future QEMU versions");
}
}
@@ -3547,7 +3547,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
*/
if (cpu->phys_bits != host_phys_bits && cpu->phys_bits != 0 &&
!warned) {
- error_report("Warning: Host physical bits (%u)"
+ warn_report("Host physical bits (%u)"
" does not match phys-bits property (%u)",
host_phys_bits, cpu->phys_bits);
warned = true;
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index f84a49d366..2ea1d8c728 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -600,7 +600,7 @@ static int kvm_arch_set_tsc_khz(CPUState *cs)
kvm_vcpu_ioctl(cs, KVM_GET_TSC_KHZ) :
-ENOTSUP;
if (cur_freq <= 0 || cur_freq != env->tsc_khz) {
- error_report("warning: TSC frequency mismatch between "
+ warn_report("TSC frequency mismatch between "
"VM (%" PRId64 " kHz) and host (%d kHz), "
"and TSC scaling unavailable",
env->tsc_khz, cur_freq);
@@ -919,7 +919,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
error_report("kvm: LMCE not supported");
return -ENOTSUP;
}
- error_report("warning: Unsupported MCG_CAP bits: 0x%" PRIx64,
+ warn_report("Unsupported MCG_CAP bits: 0x%" PRIx64,
unsupported_caps);
}
diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 7cb55dc7e3..8db2dfd6d4 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -677,7 +677,7 @@ static void check_consistency(const S390CPUModel *model)
for (i = 0; i < ARRAY_SIZE(dep); i++) {
if (test_bit(dep[i][0], model->features) &&
!test_bit(dep[i][1], model->features)) {
- error_report("Warning: \'%s\' requires \'%s\'.",
+ warn_report("\'%s\' requires \'%s\'.",
s390_feat_def(dep[i][0])->name,
s390_feat_def(dep[i][1])->name);
}
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index a3d00196f4..8e956ae39c 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -2675,7 +2675,7 @@ void kvm_s390_apply_cpu_model(const S390CPUModel *model, Error **errp)
/* enable CMM via CMMA - disable on hugetlbfs */
if (test_bit(S390_FEAT_CMM, model->features)) {
if (mem_path) {
- error_report("Warning: CMM will not be enabled because it is not "
+ warn_report("CMM will not be enabled because it is not "
"compatible to hugetlbfs.");
} else {
kvm_s390_enable_cmma();
diff --git a/tests/test-qdev-global-props.c b/tests/test-qdev-global-props.c
index 48e5b7315f..b25fe892ed 100644
--- a/tests/test-qdev-global-props.c
+++ b/tests/test-qdev-global-props.c
@@ -232,10 +232,10 @@ static void test_dynamic_globalprop(void)
g_test_trap_assert_passed();
g_test_trap_assert_stderr_unmatched("*prop1*");
g_test_trap_assert_stderr_unmatched("*prop2*");
- g_test_trap_assert_stderr("*Warning: global dynamic-prop-type-bad.prop3 has invalid class name\n*");
+ g_test_trap_assert_stderr("*warning: global dynamic-prop-type-bad.prop3 has invalid class name\n*");
g_test_trap_assert_stderr_unmatched("*prop4*");
- g_test_trap_assert_stderr("*Warning: global nohotplug-type.prop5=105 not used\n*");
- g_test_trap_assert_stderr("*Warning: global nondevice-type.prop6 has invalid class name\n*");
+ g_test_trap_assert_stderr("*warning: global nohotplug-type.prop5=105 not used\n*");
+ g_test_trap_assert_stderr("*warning: global nondevice-type.prop6 has invalid class name\n*");
g_test_trap_assert_stdout("");
}
diff --git a/trace/control.c b/trace/control.c
index 9b157b0ca7..bda1000554 100644
--- a/trace/control.c
+++ b/trace/control.c
@@ -171,7 +171,7 @@ static void do_trace_enable_events(const char *line_buf)
while ((ev = trace_event_iter_next(&iter)) != NULL) {
if (!trace_event_get_state_static(ev)) {
if (!is_pattern) {
- error_report("WARNING: trace event '%s' is not traceable",
+ warn_report("trace event '%s' is not traceable",
line_ptr);
return;
}
@@ -186,7 +186,7 @@ static void do_trace_enable_events(const char *line_buf)
}
if (!is_pattern) {
- error_report("WARNING: trace event '%s' does not exist",
+ warn_report("trace event '%s' does not exist",
line_ptr);
}
}
diff --git a/vl.c b/vl.c
index d17c863409..d5342fe816 100644
--- a/vl.c
+++ b/vl.c
@@ -952,8 +952,8 @@ static void bt_vhci_add(int vlan_id)
struct bt_scatternet_s *vlan = qemu_find_bt_vlan(vlan_id);
if (!vlan->slave)
- error_report("warning: adding a VHCI to an empty scatternet %i",
- vlan_id);
+ warn_report("adding a VHCI to an empty scatternet %i",
+ vlan_id);
bt_vhci_init(bt_new_hci(vlan));
}
@@ -979,8 +979,8 @@ static struct bt_device_s *bt_device_add(const char *opt)
vlan = qemu_find_bt_vlan(vlan_id);
if (!vlan->slave)
- error_report("warning: adding a slave device to an empty scatternet %i",
- vlan_id);
+ warn_report("adding a slave device to an empty scatternet %i",
+ vlan_id);
if (!strcmp(devname, "keyboard"))
return bt_keyboard_init(vlan);
@@ -2302,8 +2302,8 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts, Error **errp)
return -1;
}
if (strncmp(name, "opt/", 4) != 0) {
- error_report("warning: externally provided fw_cfg item names "
- "should be prefixed with \"opt/\"");
+ warn_report("externally provided fw_cfg item names "
+ "should be prefixed with \"opt/\"");
}
if (nonempty_str(str)) {
size = strlen(str); /* NUL terminator NOT included in fw_cfg blob */
@@ -3760,7 +3760,7 @@ int main(int argc, char **argv, char **envp)
qemu_opts_parse_noisily(olist, "accel=tcg", false);
break;
case QEMU_OPTION_no_kvm_pit: {
- error_report("warning: ignoring deprecated option");
+ warn_report("ignoring deprecated option");
break;
}
case QEMU_OPTION_no_kvm_pit_reinjection: {
@@ -3770,8 +3770,8 @@ int main(int argc, char **argv, char **envp)
.value = "discard",
};
- error_report("warning: deprecated, replaced by "
- "-global kvm-pit.lost_tick_policy=discard");
+ warn_report("deprecated, replaced by "
+ "-global kvm-pit.lost_tick_policy=discard");
qdev_prop_register_global(&kvm_pit_lost_tick_policy);
break;
}
@@ -3896,7 +3896,7 @@ int main(int argc, char **argv, char **envp)
}
break;
case QEMU_OPTION_tdf:
- error_report("warning: ignoring deprecated option");
+ warn_report("ignoring deprecated option");
break;
case QEMU_OPTION_name:
opts = qemu_opts_parse_noisily(qemu_find_opts("name"),
--
2.11.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH v3 4/8] hw/i386: Improve some of the warning messages
2017-07-11 12:07 [Qemu-devel] [PATCH v3 0/8] Implement a warning_report function Alistair Francis
` (2 preceding siblings ...)
2017-07-11 12:07 ` [Qemu-devel] [PATCH v3 3/8] Convert error_report() to warn_report() Alistair Francis
@ 2017-07-11 12:07 ` Alistair Francis
2017-07-12 9:39 ` Markus Armbruster
2017-07-11 12:07 ` [Qemu-devel] [PATCH v3 5/8] char-socket: Report TCP socket waiting as information Alistair Francis
` (4 subsequent siblings)
8 siblings, 1 reply; 23+ messages in thread
From: Alistair Francis @ 2017-07-11 12:07 UTC (permalink / raw)
To: qemu-devel; +Cc: alistair.francis, alistair23, philippe, berrange, armbru
Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
---
hw/i386/acpi-build.c | 7 ++++---
hw/i386/pc.c | 9 ++++-----
hw/i386/pc_q35.c | 4 ++--
3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 6b7bade183..f9efb6be41 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2766,7 +2766,8 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
ACPI_BUILD_ALIGN_SIZE);
if (tables_blob->len > legacy_table_size) {
/* Should happen only with PCI bridges and -M pc-i440fx-2.0. */
- warn_report("migration may not work.");
+ warn_report("ACPI tables are larger than legacy_table_size");
+ warn_report("migration may not work");
}
g_array_set_size(tables_blob, legacy_table_size);
} else {
@@ -2774,9 +2775,9 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
if (tables_blob->len > ACPI_BUILD_TABLE_SIZE / 2) {
/* As of QEMU 2.1, this fires with 160 VCPUs and 255 memory slots. */
warn_report("ACPI tables are larger than 64k.");
- warn_report("migration may not work.");
+ warn_report("migration may not work");
warn_report("please remove CPUs, NUMA nodes, "
- "memory slots or PCI bridges.");
+ "memory slots or PCI bridges");
}
acpi_align_size(tables_blob, ACPI_BUILD_TABLE_SIZE);
}
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 465e91cc5b..084ca796c2 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -383,8 +383,8 @@ ISADevice *pc_find_fdc0(void)
if (state.multiple) {
warn_report("multiple floppy disk controllers with "
"iobase=0x3f0 have been found");
- error_printf("the one being picked for CMOS setup might not reflect "
- "your intent\n");
+ warn_report("the one being picked for CMOS setup might not reflect "
+ "your intent");
}
return state.floppy;
@@ -2087,9 +2087,8 @@ static void pc_machine_set_max_ram_below_4g(Object *obj, Visitor *v,
}
if (value < (1ULL << 20)) {
- warn_report("small max_ram_below_4g(%"PRIu64
- ") less than 1M. BIOS may not work..",
- value);
+ warn_report("max_ram_below_4g (%" PRIu64 ") is less than 1M; "
+ "BIOS may not work.", value);
}
pcms->max_ram_below_4g = value;
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 1653a47f0a..682c576cf1 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -101,8 +101,8 @@ static void pc_q35_init(MachineState *machine)
lowmem = pcms->max_ram_below_4g;
if (machine->ram_size - lowmem > lowmem &&
lowmem & ((1ULL << 30) - 1)) {
- warn_report("Large machine and max_ram_below_4g(%"PRIu64
- ") not a multiple of 1G; possible bad performance.",
+ warn_report("Large machine and max_ram_below_4g (%"PRIu64") not a "
+ "multiple of 1G; possible bad performance.",
pcms->max_ram_below_4g);
}
}
--
2.11.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH v3 5/8] char-socket: Report TCP socket waiting as information
2017-07-11 12:07 [Qemu-devel] [PATCH v3 0/8] Implement a warning_report function Alistair Francis
` (3 preceding siblings ...)
2017-07-11 12:07 ` [Qemu-devel] [PATCH v3 4/8] hw/i386: Improve some of the warning messages Alistair Francis
@ 2017-07-11 12:07 ` Alistair Francis
2017-07-11 12:07 ` [Qemu-devel] [PATCH v3 6/8] error: Implement the warn and free Error functions Alistair Francis
` (3 subsequent siblings)
8 siblings, 0 replies; 23+ messages in thread
From: Alistair Francis @ 2017-07-11 12:07 UTC (permalink / raw)
To: qemu-devel; +Cc: alistair.francis, alistair23, philippe, berrange, armbru
When QEMU is waiting for a TCP socket connection it reports that message as
an error. This isn't an error it is just information so let's change the
report to use info_report() instead.
Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
chardev/char-socket.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index ccc499cfa1..a050a686ea 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -765,8 +765,8 @@ static int tcp_chr_wait_connected(Chardev *chr, Error **errp)
* in TLS and telnet cases, only wait for an accepted socket */
while (!s->ioc) {
if (s->is_listen) {
- error_report("QEMU waiting for connection on: %s",
- chr->filename);
+ info_report("QEMU waiting for connection on: %s",
+ chr->filename);
qio_channel_set_blocking(QIO_CHANNEL(s->listen_ioc), true, NULL);
tcp_chr_accept(QIO_CHANNEL(s->listen_ioc), G_IO_IN, chr);
qio_channel_set_blocking(QIO_CHANNEL(s->listen_ioc), false, NULL);
--
2.11.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH v3 6/8] error: Implement the warn and free Error functions
2017-07-11 12:07 [Qemu-devel] [PATCH v3 0/8] Implement a warning_report function Alistair Francis
` (4 preceding siblings ...)
2017-07-11 12:07 ` [Qemu-devel] [PATCH v3 5/8] char-socket: Report TCP socket waiting as information Alistair Francis
@ 2017-07-11 12:07 ` Alistair Francis
2017-07-11 12:07 ` [Qemu-devel] [PATCH v3 7/8] Convert error_report*_err() to warn_report*_err() Alistair Francis
` (2 subsequent siblings)
8 siblings, 0 replies; 23+ messages in thread
From: Alistair Francis @ 2017-07-11 12:07 UTC (permalink / raw)
To: qemu-devel; +Cc: alistair.francis, alistair23, philippe, berrange, armbru
Implement warn_report_err() and warn_reportf_err() functions which
are the same as the error_report_err() and error_reportf_err()
functions except report a warning instead of an error.
Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
include/qapi/error.h | 11 +++++++++++
scripts/checkpatch.pl | 1 +
util/error.c | 20 ++++++++++++++++++++
3 files changed, 32 insertions(+)
diff --git a/include/qapi/error.h b/include/qapi/error.h
index 7e532d00e9..af53b34410 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -267,11 +267,22 @@ void error_free(Error *err);
void error_free_or_abort(Error **errp);
/*
+ * Convenience function to warn_report() and free @err.
+ */
+void warn_report_err(Error *err);
+
+/*
* Convenience function to error_report() and free @err.
*/
void error_report_err(Error *err);
/*
+ * Convenience function to error_prepend(), warn_report() and free @err.
+ */
+void warn_reportf_err(Error *err, const char *fmt, ...)
+ GCC_FMT_ATTR(2, 3);
+
+/*
* Convenience function to error_prepend(), error_report() and free @err.
*/
void error_reportf_err(Error *err, const char *fmt, ...)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 60b1f320a1..dba8c84ca5 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2533,6 +2533,7 @@ sub process {
error_setg_file_open|
error_set|
error_prepend|
+ warn_reportf_err|
error_reportf_err|
error_vreport|
report_vreport|
diff --git a/util/error.c b/util/error.c
index 020b86b9f0..3efdd69162 100644
--- a/util/error.c
+++ b/util/error.c
@@ -232,6 +232,15 @@ void error_report_err(Error *err)
error_free(err);
}
+void warn_report_err(Error *err)
+{
+ warn_report("%s", error_get_pretty(err));
+ if (err->hint) {
+ error_printf_unless_qmp("%s", err->hint->str);
+ }
+ error_free(err);
+}
+
void error_reportf_err(Error *err, const char *fmt, ...)
{
va_list ap;
@@ -242,6 +251,17 @@ void error_reportf_err(Error *err, const char *fmt, ...)
error_report_err(err);
}
+
+void warn_reportf_err(Error *err, const char *fmt, ...)
+{
+ va_list ap;
+
+ va_start(ap, fmt);
+ error_vprepend(&err, fmt, ap);
+ va_end(ap);
+ warn_report_err(err);
+}
+
void error_free(Error *err)
{
if (err) {
--
2.11.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH v3 7/8] Convert error_report*_err() to warn_report*_err()
2017-07-11 12:07 [Qemu-devel] [PATCH v3 0/8] Implement a warning_report function Alistair Francis
` (5 preceding siblings ...)
2017-07-11 12:07 ` [Qemu-devel] [PATCH v3 6/8] error: Implement the warn and free Error functions Alistair Francis
@ 2017-07-11 12:07 ` Alistair Francis
2017-07-11 12:07 ` [Qemu-devel] [PATCH v3 8/8] error: Add a 'error: ' prefix to error_report() Alistair Francis
2017-07-11 13:58 ` [Qemu-devel] [PATCH v3 0/8] Implement a warning_report function no-reply
8 siblings, 0 replies; 23+ messages in thread
From: Alistair Francis @ 2017-07-11 12:07 UTC (permalink / raw)
To: qemu-devel
Cc: alistair.francis, alistair23, philippe, berrange, armbru,
Paolo Bonzini, Richard Henderson, Eduardo Habkost,
Michael S. Tsirkin
Convert all uses of error_report*_err("Warning:"... to use
warn_report*_err() instead. This helps standardise on a single
method of printing warnings to the user.
Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
hw/core/qdev-properties.c | 2 +-
hw/i386/pc.c | 3 +--
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index f5983c83da..3d0bba21a2 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -1169,7 +1169,7 @@ static void qdev_prop_set_globals_for_type(DeviceState *dev,
error_propagate(prop->errp, err);
} else {
assert(prop->user_provided);
- error_reportf_err(err, "Warning: ");
+ warn_report_err(err);
}
}
}
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 084ca796c2..4d5bc6e5f3 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1320,8 +1320,7 @@ void pc_acpi_init(const char *default_dsdt)
acpi_table_add_builtin(opts, &err);
if (err) {
- error_reportf_err(err, "WARNING: failed to load %s: ",
- filename);
+ warn_reportf_err(err, "failed to load %s: ", filename);
}
g_free(filename);
}
--
2.11.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH v3 8/8] error: Add a 'error: ' prefix to error_report()
2017-07-11 12:07 [Qemu-devel] [PATCH v3 0/8] Implement a warning_report function Alistair Francis
` (6 preceding siblings ...)
2017-07-11 12:07 ` [Qemu-devel] [PATCH v3 7/8] Convert error_report*_err() to warn_report*_err() Alistair Francis
@ 2017-07-11 12:07 ` Alistair Francis
2017-07-11 17:44 ` Max Reitz
2017-07-11 13:58 ` [Qemu-devel] [PATCH v3 0/8] Implement a warning_report function no-reply
8 siblings, 1 reply; 23+ messages in thread
From: Alistair Francis @ 2017-07-11 12:07 UTC (permalink / raw)
To: qemu-devel; +Cc: alistair.francis, alistair23, philippe, berrange, armbru
As we don't regard error messages as a stable API the let's add a
'error: ' prefix to the original error_report() messages.
Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
---
util/qemu-error.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/util/qemu-error.c b/util/qemu-error.c
index c557c6ae47..3a3372b68b 100644
--- a/util/qemu-error.c
+++ b/util/qemu-error.c
@@ -203,6 +203,7 @@ static void vreport(report_type type, const char *fmt, va_list ap)
switch (type) {
case REPORT_TYPE_ERROR:
+ error_printf("error: ");
break;
case REPORT_TYPE_WARNING:
error_printf("warning: ");
--
2.11.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/8] Implement a warning_report function
2017-07-11 12:07 [Qemu-devel] [PATCH v3 0/8] Implement a warning_report function Alistair Francis
` (7 preceding siblings ...)
2017-07-11 12:07 ` [Qemu-devel] [PATCH v3 8/8] error: Add a 'error: ' prefix to error_report() Alistair Francis
@ 2017-07-11 13:58 ` no-reply
8 siblings, 0 replies; 23+ messages in thread
From: no-reply @ 2017-07-11 13:58 UTC (permalink / raw)
To: alistair.francis; +Cc: famz, qemu-devel, alistair23, philippe, armbru
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Subject: [Qemu-devel] [PATCH v3 0/8] Implement a warning_report function
Message-id: cover.1499774331.git.alistair.francis@xilinx.com
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0
git config --local diff.renamelimit 0
git config --local diff.renames True
commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done
exit $failed
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
60686e6 error: Add a 'error: ' prefix to error_report()
5364f27 Convert error_report*_err() to warn_report*_err()
a893104 error: Implement the warn and free Error functions
acb3d0e char-socket: Report TCP socket waiting as information
49d68cd hw/i386: Improve some of the warning messages
ec1a246 Convert error_report() to warn_report()
23f4974 error: Functions to report warnings and informational messages
fdc3c8a util/qemu-error: Rename error_print_loc() to be more generic
=== OUTPUT BEGIN ===
Checking PATCH 1/8: util/qemu-error: Rename error_print_loc() to be more generic...
Checking PATCH 2/8: error: Functions to report warnings and informational messages...
Checking PATCH 3/8: Convert error_report() to warn_report()...
ERROR: line over 90 characters
#694: FILE: tests/test-qdev-global-props.c:235:
+ g_test_trap_assert_stderr("*warning: global dynamic-prop-type-bad.prop3 has invalid class name\n*");
WARNING: line over 80 characters
#698: FILE: tests/test-qdev-global-props.c:237:
+ g_test_trap_assert_stderr("*warning: global nohotplug-type.prop5=105 not used\n*");
ERROR: line over 90 characters
#699: FILE: tests/test-qdev-global-props.c:238:
+ g_test_trap_assert_stderr("*warning: global nondevice-type.prop6 has invalid class name\n*");
total: 2 errors, 1 warnings, 539 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 4/8: hw/i386: Improve some of the warning messages...
Checking PATCH 5/8: char-socket: Report TCP socket waiting as information...
Checking PATCH 6/8: error: Implement the warn and free Error functions...
Checking PATCH 7/8: Convert error_report*_err() to warn_report*_err()...
Checking PATCH 8/8: error: Add a 'error: ' prefix to error_report()...
=== OUTPUT END ===
Test command exited with code: 1
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v3 3/8] Convert error_report() to warn_report()
2017-07-11 12:07 ` [Qemu-devel] [PATCH v3 3/8] Convert error_report() to warn_report() Alistair Francis
@ 2017-07-11 16:51 ` Max Reitz
2017-07-12 8:34 ` Markus Armbruster
1 sibling, 0 replies; 23+ messages in thread
From: Max Reitz @ 2017-07-11 16:51 UTC (permalink / raw)
To: Alistair Francis, qemu-devel
Cc: alistair23, philippe, berrange, armbru, Jeff Cody, Kevin Wolf,
Ronnie Sahlberg, Paolo Bonzini, Peter Lieven, Josh Durgin,
Richard W.M. Jones, Peter Crosthwaite, Richard Henderson,
Aneesh Kumar K.V, Greg Kurz, Rob Herring, Peter Maydell,
Peter Chubb, Eduardo Habkost, Marcel Apfelbaum,
Michael S. Tsirkin, Igor Mammedov, David Gibson, Alexander Graf,
Gerd Hoffmann, Jason Wang, Marcelo Tosatti,
Christian Borntraeger, Cornelia Huck, Stefan Hajnoczi
[-- Attachment #1: Type: text/plain, Size: 6086 bytes --]
On 2017-07-11 14:07, Alistair Francis wrote:
> Convert all uses of error_report("warning:"... to use warn_report()
> instead. This helps standardise on a single method of printing warnings
> to the user.
>
> All of the warnings where changed using these two commands:
> find ./* -type f -exec sed -i \
> 's|error_report(".*warning[,:] |warn_report("|Ig' {} +
>
> Then the white space changes where manually fixed afterwards.
>
> The test-qdev-global-props test case was manually updated to ensure that
> this patch passes make check (as the test cases are case sensitive).
>
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> Suggested-by: Thomas Huth <thuth@redhat.com>
> Cc: Jeff Cody <jcody@redhat.com>
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Max Reitz <mreitz@redhat.com>
> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Peter Lieven <pl@kamp.de>
> Cc: Josh Durgin <jdurgin@redhat.com>
> Cc: "Richard W.M. Jones" <rjones@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Peter Crosthwaite <crosthwaite.peter@gmail.com>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
> Cc: Greg Kurz <groug@kaod.org>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Peter Chubb <peter.chubb@nicta.com.au>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Cc: Marcel Apfelbaum <marcel@redhat.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: Marcelo Tosatti <mtosatti@redhat.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Acked-by: David Gibson <david@gibson.dropbear.id.au>
> Acked-by: Greg Kurz <groug@kaod.org>
> Acked-by: Cornelia Huck <cohuck@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Reviewed by: Peter Chubb <peter.chubb@data61.csiro.au>
> Acked-by: Max Reitz <mreitz@redhat.com>
> Acked-by: Marcel Apfelbaum <marcel@redhat.com>
> ---
> V3:
> - Regenerate patch to ensure no manual edits where made
> - Tighten regex matches
> V2:
> - Fix quotation issues
> - Update commit message
> - Include full command
>
> Just a note:
> We will need to do a similar thing for fprintf. There are patches on
> the list at the moment that conflict with this series (by adding
> error_report() calls that shsould be warning), so I think I'm giong to
> have to do a follow up series converting more cases. I'll cover the
> fprintf cases when I do that. There are some manual cases as
> well that don't say warning or info, but really should. They will have
> to be converted as well.
>
>
> block/backup.c | 10 +++++-----
> block/gluster.c | 4 ++--
> block/iscsi.c | 6 +++---
> block/nfs.c | 12 ++++++------
> block/rbd.c | 6 +++---
> block/ssh.c | 4 ++--
> blockdev.c | 2 +-
> cpus.c | 2 +-
> hw/9pfs/9p.c | 2 +-
> hw/arm/highbank.c | 6 +++---
> hw/arm/imx25_pdk.c | 6 +++---
> hw/arm/kzm.c | 6 +++---
> hw/core/machine.c | 10 +++++-----
> hw/core/qdev-properties.c | 8 ++++----
> hw/i386/acpi-build.c | 10 +++++-----
> hw/i386/kvm/pci-assign.c | 6 +++---
> hw/i386/pc.c | 10 +++++-----
> hw/i386/pc_piix.c | 8 ++++----
> hw/i386/pc_q35.c | 6 +++---
> hw/misc/aspeed_sdmc.c | 8 ++++----
> hw/nvram/fw_cfg.c | 2 +-
> hw/pci-host/piix.c | 2 +-
> hw/ppc/pnv.c | 6 +++---
> hw/ppc/spapr.c | 4 ++--
> hw/ppc/spapr_iommu.c | 2 +-
> hw/scsi/scsi-bus.c | 6 +++---
> hw/usb/dev-smartcard-reader.c | 4 ++--
> hw/usb/redirect.c | 2 +-
> net/tap-linux.c | 2 +-
> target/i386/cpu.c | 8 ++++----
> target/i386/kvm.c | 4 ++--
> target/s390x/cpu_models.c | 2 +-
> target/s390x/kvm.c | 2 +-
> tests/test-qdev-global-props.c | 6 +++---
> trace/control.c | 4 ++--
> vl.c | 20 ++++++++++----------
> 36 files changed, 104 insertions(+), 104 deletions(-)
>
> diff --git a/block/backup.c b/block/backup.c
> index b69184eac5..44cc2b22ab 100644
> --- a/block/backup.c
> +++ b/block/backup.c
> @@ -639,11 +639,11 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
> ret = bdrv_get_info(target, &bdi);
> if (ret == -ENOTSUP && !target->backing) {
> /* Cluster size is not defined */
> - error_report("WARNING: The target block device doesn't provide "
> - "information about the block size and it doesn't have a "
> - "backing file. The default block size of %u bytes is "
> - "used. If the actual block size of the target exceeds "
> - "this default, the backup may be unusable",
> + warn_report("The target block device doesn't provide "
> + "information about the block size and it doesn't have a "
> + "backing file. The default block size of %u bytes is "
> + "used. If the actual block size of the target exceeds "
> + "this default, the backup may be unusable",
> BACKUP_CLUSTER_SIZE_DEFAULT);
Sorry I didn't see this before, but this line may need re-indentation as
well.
But I'm not too picky (even though maybe I should be O:-)), so my
Acked-by can stay either way.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v3 8/8] error: Add a 'error: ' prefix to error_report()
2017-07-11 12:07 ` [Qemu-devel] [PATCH v3 8/8] error: Add a 'error: ' prefix to error_report() Alistair Francis
@ 2017-07-11 17:44 ` Max Reitz
2017-07-12 12:27 ` Alistair Francis
0 siblings, 1 reply; 23+ messages in thread
From: Max Reitz @ 2017-07-11 17:44 UTC (permalink / raw)
To: Alistair Francis, qemu-devel; +Cc: alistair23, philippe, armbru
[-- Attachment #1: Type: text/plain, Size: 369 bytes --]
On 2017-07-11 14:07, Alistair Francis wrote:
> As we don't regard error messages as a stable API the let's add a
> 'error: ' prefix to the original error_report() messages.
>
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> ---
>
> util/qemu-error.c | 1 +
> 1 file changed, 1 insertion(+)
This breaks quite a few qemu-iotests.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v3 2/8] error: Functions to report warnings and informational messages
2017-07-11 12:07 ` [Qemu-devel] [PATCH v3 2/8] error: Functions to report warnings and informational messages Alistair Francis
@ 2017-07-12 7:57 ` Markus Armbruster
2017-07-12 10:48 ` Alistair Francis
0 siblings, 1 reply; 23+ messages in thread
From: Markus Armbruster @ 2017-07-12 7:57 UTC (permalink / raw)
To: Alistair Francis; +Cc: qemu-devel, alistair23, philippe
Alistair Francis <alistair.francis@xilinx.com> writes:
> Add warn_report(), warn_vreport() for reporting warnings, and
> info_report(), info_vreport() for informational messages.
>
> These are implemented them with a helper function factored out of
> error_vreport(), suitably generalized. This patch makes no changes
> to the output of the original error_report() function.
>
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> ---
> v2:
> - Don't add *vreport() functions to checkpatch
> - Maintain original comments for the reporting functions
> - Don't change the error report output in this patch
> v1:
> - Don't expose the generic report and vreport() functions
> - Prefix error messages
> - Use vreport instead of qmsg_vreport()
> RFC V3:
> - Change the function and enum names to be more descriptive
> - Add wrapper functions for *_report() and *_vreport()
>
> include/qemu/error-report.h | 7 +++
> scripts/checkpatch.pl | 6 ++-
> util/qemu-error.c | 102 +++++++++++++++++++++++++++++++++++++++++---
> 3 files changed, 109 insertions(+), 6 deletions(-)
[...]
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 73efc927a9..60b1f320a1 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2535,7 +2535,11 @@ sub process {
> error_prepend|
> error_reportf_err|
> error_vreport|
> - error_report}x;
> + report_vreport|
Should be warn_vreport, oops. I can fix this on commit.
> + info_vreport|
> + error_report|
> + warn_report|
> + info_report}x;
>
> if ($rawline =~ /\b(?:$qemu_error_funcs)\s*\(.*\".*\\n/) {
> ERROR("Error messages should not contain newlines\n" . $herecurr);
[...]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v3 3/8] Convert error_report() to warn_report()
2017-07-11 12:07 ` [Qemu-devel] [PATCH v3 3/8] Convert error_report() to warn_report() Alistair Francis
2017-07-11 16:51 ` Max Reitz
@ 2017-07-12 8:34 ` Markus Armbruster
2017-07-12 10:46 ` Alistair Francis
1 sibling, 1 reply; 23+ messages in thread
From: Markus Armbruster @ 2017-07-12 8:34 UTC (permalink / raw)
To: Alistair Francis
Cc: qemu-devel, Peter Maydell, Cornelia Huck, Stefan Hajnoczi,
Michael S. Tsirkin, Jeff Cody, Alexander Graf, Gerd Hoffmann,
Eduardo Habkost, Rob Herring, Josh Durgin, Christian Borntraeger,
Marcel Apfelbaum, David Gibson, Jason Wang, philippe,
Peter Lieven, Greg Kurz, Peter Chubb, Ronnie Sahlberg,
Igor Mammedov, alistair23, Richard Henderson, Kevin Wolf,
Peter Crosthwaite, Marcelo Tosatti, Richard W.M. Jones,
Max Reitz, Aneesh Kumar K.V, Paolo Bonzini
Alistair Francis <alistair.francis@xilinx.com> writes:
> Convert all uses of error_report("warning:"... to use warn_report()
> instead. This helps standardise on a single method of printing warnings
> to the user.
>
> All of the warnings where changed using these two commands:
s/where/were/
> find ./* -type f -exec sed -i \
> 's|error_report(".*warning[,:] |warn_report("|Ig' {} +
>
> Then the white space changes where manually fixed afterwards.
>
> The test-qdev-global-props test case was manually updated to ensure that
> this patch passes make check (as the test cases are case sensitive).
>
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> Suggested-by: Thomas Huth <thuth@redhat.com>
> Cc: Jeff Cody <jcody@redhat.com>
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Max Reitz <mreitz@redhat.com>
> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Peter Lieven <pl@kamp.de>
> Cc: Josh Durgin <jdurgin@redhat.com>
> Cc: "Richard W.M. Jones" <rjones@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Peter Crosthwaite <crosthwaite.peter@gmail.com>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
> Cc: Greg Kurz <groug@kaod.org>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Peter Chubb <peter.chubb@nicta.com.au>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Cc: Marcel Apfelbaum <marcel@redhat.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: Marcelo Tosatti <mtosatti@redhat.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Acked-by: David Gibson <david@gibson.dropbear.id.au>
> Acked-by: Greg Kurz <groug@kaod.org>
> Acked-by: Cornelia Huck <cohuck@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Reviewed by: Peter Chubb <peter.chubb@data61.csiro.au>
> Acked-by: Max Reitz <mreitz@redhat.com>
> Acked-by: Marcel Apfelbaum <marcel@redhat.com>
> ---
> V3:
> - Regenerate patch to ensure no manual edits where made
> - Tighten regex matches
> V2:
> - Fix quotation issues
> - Update commit message
> - Include full command
>
> Just a note:
> We will need to do a similar thing for fprintf. There are patches on
> the list at the moment that conflict with this series (by adding
> error_report() calls that shsould be warning), so I think I'm giong to
> have to do a follow up series converting more cases. I'll cover the
> fprintf cases when I do that. There are some manual cases as
> well that don't say warning or info, but really should. They will have
> to be converted as well.
>
>
> block/backup.c | 10 +++++-----
> block/gluster.c | 4 ++--
> block/iscsi.c | 6 +++---
> block/nfs.c | 12 ++++++------
> block/rbd.c | 6 +++---
> block/ssh.c | 4 ++--
> blockdev.c | 2 +-
> cpus.c | 2 +-
> hw/9pfs/9p.c | 2 +-
> hw/arm/highbank.c | 6 +++---
> hw/arm/imx25_pdk.c | 6 +++---
> hw/arm/kzm.c | 6 +++---
> hw/core/machine.c | 10 +++++-----
> hw/core/qdev-properties.c | 8 ++++----
> hw/i386/acpi-build.c | 10 +++++-----
> hw/i386/kvm/pci-assign.c | 6 +++---
> hw/i386/pc.c | 10 +++++-----
> hw/i386/pc_piix.c | 8 ++++----
> hw/i386/pc_q35.c | 6 +++---
> hw/misc/aspeed_sdmc.c | 8 ++++----
> hw/nvram/fw_cfg.c | 2 +-
> hw/pci-host/piix.c | 2 +-
> hw/ppc/pnv.c | 6 +++---
> hw/ppc/spapr.c | 4 ++--
> hw/ppc/spapr_iommu.c | 2 +-
> hw/scsi/scsi-bus.c | 6 +++---
> hw/usb/dev-smartcard-reader.c | 4 ++--
> hw/usb/redirect.c | 2 +-
> net/tap-linux.c | 2 +-
> target/i386/cpu.c | 8 ++++----
> target/i386/kvm.c | 4 ++--
> target/s390x/cpu_models.c | 2 +-
> target/s390x/kvm.c | 2 +-
> tests/test-qdev-global-props.c | 6 +++---
> trace/control.c | 4 ++--
> vl.c | 20 ++++++++++----------
> 36 files changed, 104 insertions(+), 104 deletions(-)
>
> diff --git a/block/backup.c b/block/backup.c
> index b69184eac5..44cc2b22ab 100644
> --- a/block/backup.c
> +++ b/block/backup.c
> @@ -639,11 +639,11 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
> ret = bdrv_get_info(target, &bdi);
> if (ret == -ENOTSUP && !target->backing) {
> /* Cluster size is not defined */
> - error_report("WARNING: The target block device doesn't provide "
> - "information about the block size and it doesn't have a "
> - "backing file. The default block size of %u bytes is "
> - "used. If the actual block size of the target exceeds "
> - "this default, the backup may be unusable",
> + warn_report("The target block device doesn't provide "
> + "information about the block size and it doesn't have a "
> + "backing file. The default block size of %u bytes is "
> + "used. If the actual block size of the target exceeds "
> + "this default, the backup may be unusable",
> BACKUP_CLUSTER_SIZE_DEFAULT);
As Max noted, indentation's off here.
Preexisting, not this patch's problem: an error or warning message
should be a (short!) phrase, not a novel. If additional explanations
are needed, they go on separate lines, like this:
warn_report("The target block device doesn't provide "
"information about the block size and it doesn't have a "
"backing file");
error_printf("The default block size of %u bytes is used."
" If the actual block size of the target exceeds "
"this default, the backup may be unusable.",
BACKUP_CLUSTER_SIZE_DEFAULT);
Note that the warning message is a *phrase*, not a sentence, let alone
three, and its followed by additional hints, which *are* sentences.
More of the same below. Followup patches welcome.
> job->cluster_size = BACKUP_CLUSTER_SIZE_DEFAULT;
> } else if (ret < 0 && !target->backing) {
> diff --git a/block/gluster.c b/block/gluster.c
> index addceed6eb..ba88fe116e 100644
> --- a/block/gluster.c
> +++ b/block/gluster.c
> @@ -345,8 +345,8 @@ static int qemu_gluster_parse_uri(BlockdevOptionsGluster *gconf,
> is_unix = true;
> } else if (!strcmp(uri->scheme, "gluster+rdma")) {
> gsconf->type = SOCKET_ADDRESS_TYPE_INET;
> - error_report("Warning: rdma feature is not supported, falling "
> - "back to tcp");
> + warn_report("rdma feature is not supported, falling "
> + "back to tcp");
Further cleanup:
warn_report("rdma feature is not supported, falling back to tcp");
Could be done as a separate patch to keep this one as mechanical as
possible, but I think squashing it in would be okay, too. If we do,
then the commit message needs to updated. I think
Indentation fixed up manually afterwards.
instead of
Then the white space changes where manually fixed afterwards.
should do.
> } else {
> ret = -EINVAL;
> goto out;
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 54067e2620..3aa438a0b7 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -1761,9 +1761,9 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
> * filename encoded options */
> filename = qdict_get_try_str(options, "filename");
> if (filename) {
> - error_report("Warning: 'filename' option specified. "
> - "This is an unsupported option, and may be deprecated "
> - "in the future");
> + warn_report("'filename' option specified. "
> + "This is an unsupported option, and may be deprecated "
> + "in the future");
> iscsi_parse_filename(filename, options, &local_err);
> if (local_err) {
> ret = -EINVAL;
> diff --git a/block/nfs.c b/block/nfs.c
> index c3c5de0113..43929c6f23 100644
> --- a/block/nfs.c
> +++ b/block/nfs.c
> @@ -558,8 +558,8 @@ static int64_t nfs_client_open(NFSClient *client, QDict *options,
> }
> client->readahead = qemu_opt_get_number(opts, "readahead-size", 0);
> if (client->readahead > QEMU_NFS_MAX_READAHEAD_SIZE) {
> - error_report("NFS Warning: Truncating NFS readahead "
> - "size to %d", QEMU_NFS_MAX_READAHEAD_SIZE);
> + warn_report("Truncating NFS readahead "
> + "size to %d", QEMU_NFS_MAX_READAHEAD_SIZE);
Similarly:
warn_report("Truncating NFS readahead size to %d",
QEMU_NFS_MAX_READAHEAD_SIZE);
> client->readahead = QEMU_NFS_MAX_READAHEAD_SIZE;
> }
> nfs_set_readahead(client->context, client->readahead);
> @@ -579,8 +579,8 @@ static int64_t nfs_client_open(NFSClient *client, QDict *options,
> }
> client->pagecache = qemu_opt_get_number(opts, "page-cache-size", 0);
> if (client->pagecache > QEMU_NFS_MAX_PAGECACHE_SIZE) {
> - error_report("NFS Warning: Truncating NFS pagecache "
> - "size to %d pages", QEMU_NFS_MAX_PAGECACHE_SIZE);
> + warn_report("Truncating NFS pagecache "
> + "size to %d pages", QEMU_NFS_MAX_PAGECACHE_SIZE);
Similarly:
warn_report("Truncating NFS pagecache size to %d pages",
QEMU_NFS_MAX_PAGECACHE_SIZE);
> client->pagecache = QEMU_NFS_MAX_PAGECACHE_SIZE;
> }
> nfs_set_pagecache(client->context, client->pagecache);
> @@ -595,8 +595,8 @@ static int64_t nfs_client_open(NFSClient *client, QDict *options,
> /* limit the maximum debug level to avoid potential flooding
> * of our log files. */
> if (client->debug > QEMU_NFS_MAX_DEBUG_LEVEL) {
> - error_report("NFS Warning: Limiting NFS debug level "
> - "to %d", QEMU_NFS_MAX_DEBUG_LEVEL);
> + warn_report("Limiting NFS debug level "
> + "to %d", QEMU_NFS_MAX_DEBUG_LEVEL);
warn_report("Limiting NFS debug level to %d",
QEMU_NFS_MAX_DEBUG_LEVEL);
> client->debug = QEMU_NFS_MAX_DEBUG_LEVEL;
> }
> nfs_set_debug(client->context, client->debug);
> diff --git a/block/rbd.c b/block/rbd.c
> index 9da02cdceb..d461f7dc87 100644
> --- a/block/rbd.c
> +++ b/block/rbd.c
> @@ -555,9 +555,9 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,
> * filename encoded options */
> filename = qdict_get_try_str(options, "filename");
> if (filename) {
> - error_report("Warning: 'filename' option specified. "
> - "This is an unsupported option, and may be deprecated "
> - "in the future");
> + warn_report("'filename' option specified. "
> + "This is an unsupported option, and may be deprecated "
> + "in the future");
> qemu_rbd_parse_filename(filename, options, &local_err);
> if (local_err) {
> r = -EINVAL;
> diff --git a/block/ssh.c b/block/ssh.c
> index 52964416da..07a57eb466 100644
> --- a/block/ssh.c
> +++ b/block/ssh.c
> @@ -1114,8 +1114,8 @@ static coroutine_fn int ssh_co_writev(BlockDriverState *bs,
> static void unsafe_flush_warning(BDRVSSHState *s, const char *what)
> {
> if (!s->unsafe_flush_warning) {
> - error_report("warning: ssh server %s does not support fsync",
> - s->inet->host);
> + warn_report("ssh server %s does not support fsync",
> + s->inet->host);
> if (what) {
> error_report("to support fsync, you need %s", what);
> }
> diff --git a/blockdev.c b/blockdev.c
> index e2016b6f37..a521c85956 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -914,7 +914,7 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
> copy_on_read = qemu_opt_get_bool(legacy_opts, "copy-on-read", false);
>
> if (read_only && copy_on_read) {
> - error_report("warning: disabling copy-on-read on read-only drive");
> + warn_report("disabling copy-on-read on read-only drive");
> copy_on_read = false;
> }
>
> diff --git a/cpus.c b/cpus.c
> index 14bb8d552e..9bed61eefc 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -557,7 +557,7 @@ void qemu_start_warp_timer(void)
> if (deadline < 0) {
> static bool notified;
> if (!icount_sleep && !notified) {
> - error_report("WARNING: icount sleep disabled and no active timers");
> + warn_report("icount sleep disabled and no active timers");
> notified = true;
> }
> return;
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 6c92bad5b3..333dbb6f8e 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -2376,7 +2376,7 @@ static void coroutine_fn v9fs_flush(void *opaque)
> trace_v9fs_flush(pdu->tag, pdu->id, tag);
>
> if (pdu->tag == tag) {
> - error_report("Warning: the guest sent a self-referencing 9P flush request");
> + warn_report("the guest sent a self-referencing 9P flush request");
> } else {
> QLIST_FOREACH(cancel_pdu, &s->active_list, next) {
> if (cancel_pdu->tag == tag) {
> diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
> index d209b97dee..750c463e2a 100644
> --- a/hw/arm/highbank.c
> +++ b/hw/arm/highbank.c
> @@ -383,9 +383,9 @@ static void calxeda_init(MachineState *machine, enum cxmachines machine_id)
> highbank_binfo.write_board_setup = hb_write_board_setup;
> highbank_binfo.secure_board_setup = true;
> } else {
> - error_report("WARNING: cannot load built-in Monitor support "
> - "if KVM is enabled. Some guests (such as Linux) "
> - "may not boot.");
> + warn_report("cannot load built-in Monitor support "
> + "if KVM is enabled. Some guests (such as Linux) "
> + "may not boot.");
> }
>
> arm_load_kernel(ARM_CPU(first_cpu), &highbank_binfo);
> diff --git a/hw/arm/imx25_pdk.c b/hw/arm/imx25_pdk.c
> index 44e741fde3..7d42c74001 100644
> --- a/hw/arm/imx25_pdk.c
> +++ b/hw/arm/imx25_pdk.c
> @@ -80,9 +80,9 @@ static void imx25_pdk_init(MachineState *machine)
>
> /* We need to initialize our memory */
> if (machine->ram_size > (FSL_IMX25_SDRAM0_SIZE + FSL_IMX25_SDRAM1_SIZE)) {
> - error_report("WARNING: RAM size " RAM_ADDR_FMT " above max supported, "
> - "reduced to %x", machine->ram_size,
> - FSL_IMX25_SDRAM0_SIZE + FSL_IMX25_SDRAM1_SIZE);
> + warn_report("RAM size " RAM_ADDR_FMT " above max supported, "
> + "reduced to %x", machine->ram_size,
> + FSL_IMX25_SDRAM0_SIZE + FSL_IMX25_SDRAM1_SIZE);
> machine->ram_size = FSL_IMX25_SDRAM0_SIZE + FSL_IMX25_SDRAM1_SIZE;
> }
>
> diff --git a/hw/arm/kzm.c b/hw/arm/kzm.c
> index 2c96ee33b6..3ed6577a55 100644
> --- a/hw/arm/kzm.c
> +++ b/hw/arm/kzm.c
> @@ -79,9 +79,9 @@ static void kzm_init(MachineState *machine)
>
> /* Check the amount of memory is compatible with the SOC */
> if (machine->ram_size > (FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE)) {
> - error_report("WARNING: RAM size " RAM_ADDR_FMT " above max supported, "
> - "reduced to %x", machine->ram_size,
> - FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE);
> + warn_report("RAM size " RAM_ADDR_FMT " above max supported, "
> + "reduced to %x", machine->ram_size,
> + FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE);
> machine->ram_size = FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE;
> }
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index ecb55528e8..dc431fabf5 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -741,11 +741,11 @@ static void machine_numa_finish_init(MachineState *machine)
> }
> }
> if (s->len && !qtest_enabled()) {
> - error_report("warning: CPU(s) not present in any NUMA nodes: %s",
> - s->str);
> - error_report("warning: All CPU(s) up to maxcpus should be described "
> - "in NUMA config, ability to start up with partial NUMA "
> - "mappings is obsoleted and will be removed in future");
> + warn_report("CPU(s) not present in any NUMA nodes: %s",
> + s->str);
> + warn_report("All CPU(s) up to maxcpus should be described "
> + "in NUMA config, ability to start up with partial NUMA "
> + "mappings is obsoleted and will be removed in future");
Preexisting, not this patch's problem: the resulting output looks like
two independent issues instead of one. Multiple error_report() /
warn_report() in a row are almost always inappropriate.
warn_report("CPU(s) not present in any NUMA nodes: %s", s->str);
error_printf("All CPU(s) up to maxcpus should be described "
"in NUMA config, ability to start up with partial NUMA "
"mappings is obsoleted and will be removed in future");
More of the same below. Followup patches welcome.
> }
> g_string_free(s, true);
> }
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index f11d57831b..f5983c83da 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -1132,15 +1132,15 @@ int qdev_prop_check_globals(void)
> oc = object_class_by_name(prop->driver);
> oc = object_class_dynamic_cast(oc, TYPE_DEVICE);
> if (!oc) {
> - error_report("Warning: global %s.%s has invalid class name",
> - prop->driver, prop->property);
> + warn_report("global %s.%s has invalid class name",
> + prop->driver, prop->property);
> ret = 1;
> continue;
> }
> dc = DEVICE_CLASS(oc);
> if (!dc->hotpluggable && !prop->used) {
> - error_report("Warning: global %s.%s=%s not used",
> - prop->driver, prop->property, prop->value);
> + warn_report("global %s.%s=%s not used",
> + prop->driver, prop->property, prop->value);
> ret = 1;
> continue;
> }
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 5464977424..6b7bade183 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2766,17 +2766,17 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
> ACPI_BUILD_ALIGN_SIZE);
> if (tables_blob->len > legacy_table_size) {
> /* Should happen only with PCI bridges and -M pc-i440fx-2.0. */
> - error_report("Warning: migration may not work.");
> + warn_report("migration may not work.");
Could use a tree-wide sweep to drop periods from the end of error
messages. I'm not asking you to do this now.
> }
> g_array_set_size(tables_blob, legacy_table_size);
> } else {
> /* Make sure we have a buffer in case we need to resize the tables. */
> if (tables_blob->len > ACPI_BUILD_TABLE_SIZE / 2) {
> /* As of QEMU 2.1, this fires with 160 VCPUs and 255 memory slots. */
> - error_report("Warning: ACPI tables are larger than 64k.");
> - error_report("Warning: migration may not work.");
> - error_report("Warning: please remove CPUs, NUMA nodes, "
> - "memory slots or PCI bridges.");
> + warn_report("ACPI tables are larger than 64k.");
> + warn_report("migration may not work.");
> + warn_report("please remove CPUs, NUMA nodes, "
> + "memory slots or PCI bridges.");
> }
> acpi_align_size(tables_blob, ACPI_BUILD_TABLE_SIZE);
> }
> diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
> index 9f2615cbe0..33e20cb3e8 100644
> --- a/hw/i386/kvm/pci-assign.c
> +++ b/hw/i386/kvm/pci-assign.c
> @@ -1353,9 +1353,9 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev, Error **errp)
> PCI_CAP_ID_EXP);
> return -EINVAL;
> } else if (size != 0x3c) {
> - error_report("WARNING, %s: PCIe cap-id 0x%x has "
> - "non-standard size 0x%x; std size should be 0x3c",
> - __func__, PCI_CAP_ID_EXP, size);
> + warn_report("%s: PCIe cap-id 0x%x has "
> + "non-standard size 0x%x; std size should be 0x3c",
> + __func__, PCI_CAP_ID_EXP, size);
> }
> } else if (version == 0) {
> uint16_t vid, did;
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 224fe58fe7..465e91cc5b 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -381,8 +381,8 @@ ISADevice *pc_find_fdc0(void)
> }
>
> if (state.multiple) {
> - error_report("warning: multiple floppy disk controllers with "
> - "iobase=0x3f0 have been found");
> + warn_report("multiple floppy disk controllers with "
> + "iobase=0x3f0 have been found");
> error_printf("the one being picked for CMOS setup might not reflect "
> "your intent\n");
> }
> @@ -2087,9 +2087,9 @@ static void pc_machine_set_max_ram_below_4g(Object *obj, Visitor *v,
> }
>
> if (value < (1ULL << 20)) {
> - error_report("Warning: small max_ram_below_4g(%"PRIu64
> - ") less than 1M. BIOS may not work..",
> - value);
> + warn_report("small max_ram_below_4g(%"PRIu64
Let's insert a space between string literal and PRIu64 while there.
> + ") less than 1M. BIOS may not work..",
> + value);
> }
>
> pcms->max_ram_below_4g = value;
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 22dbef64c6..11b4336a42 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -131,10 +131,10 @@ static void pc_init1(MachineState *machine,
> lowmem = 0xc0000000;
> }
> if (lowmem & ((1ULL << 30) - 1)) {
> - error_report("Warning: Large machine and max_ram_below_4g "
> - "(%" PRIu64 ") not a multiple of 1G; "
> - "possible bad performance.",
> - pcms->max_ram_below_4g);
> + warn_report("Large machine and max_ram_below_4g "
> + "(%" PRIu64 ") not a multiple of 1G; "
> + "possible bad performance.",
> + pcms->max_ram_below_4g);
> }
> }
> }
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 8f696b7cb6..1653a47f0a 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -101,9 +101,9 @@ static void pc_q35_init(MachineState *machine)
> lowmem = pcms->max_ram_below_4g;
> if (machine->ram_size - lowmem > lowmem &&
> lowmem & ((1ULL << 30) - 1)) {
> - error_report("Warning: Large machine and max_ram_below_4g(%"PRIu64
> - ") not a multiple of 1G; possible bad performance.",
> - pcms->max_ram_below_4g);
> + warn_report("Large machine and max_ram_below_4g(%"PRIu64
> + ") not a multiple of 1G; possible bad performance.",
> + pcms->max_ram_below_4g);
Let's insert a space between string literal and PRIu64 while there.
> }
> }
>
> diff --git a/hw/misc/aspeed_sdmc.c b/hw/misc/aspeed_sdmc.c
> index 5f3ac0b6f6..633fa4510e 100644
> --- a/hw/misc/aspeed_sdmc.c
> +++ b/hw/misc/aspeed_sdmc.c
> @@ -157,8 +157,8 @@ static int ast2400_rambits(AspeedSDMCState *s)
> }
>
> /* use a common default */
> - error_report("warning: Invalid RAM size 0x%" PRIx64
> - ". Using default 256M", s->ram_size);
> + warn_report("Invalid RAM size 0x%" PRIx64
> + ". Using default 256M", s->ram_size);
warn_report("Invalid RAM size 0x%" PRIx64 ". Using default 256M",
s->ram_size);
> s->ram_size = 256 << 20;
> return ASPEED_SDMC_DRAM_256MB;
> }
> @@ -179,8 +179,8 @@ static int ast2500_rambits(AspeedSDMCState *s)
> }
>
> /* use a common default */
> - error_report("warning: Invalid RAM size 0x%" PRIx64
> - ". Using default 512M", s->ram_size);
> + warn_report("Invalid RAM size 0x%" PRIx64
> + ". Using default 512M", s->ram_size);
warn_report("Invalid RAM size 0x%" PRIx64 ". Using default 512M",
s->ram_size);
> s->ram_size = 512 << 20;
> return ASPEED_SDMC_AST2500_512MB;
> }
> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> index 99bdbc2233..e881e3b812 100644
> --- a/hw/nvram/fw_cfg.c
> +++ b/hw/nvram/fw_cfg.c
> @@ -781,7 +781,7 @@ static int get_fw_cfg_order(FWCfgState *s, const char *name)
> }
>
> /* Stick unknown stuff at the end. */
> - error_report("warning: Unknown firmware file in legacy mode: %s", name);
> + warn_report("Unknown firmware file in legacy mode: %s", name);
> return FW_CFG_ORDER_OVERRIDE_LAST;
> }
>
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index a2c1033dbe..072a04e318 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -307,7 +307,7 @@ static void i440fx_realize(PCIDevice *dev, Error **errp)
> dev->config[I440FX_SMRAM] = 0x02;
>
> if (object_property_get_bool(qdev_get_machine(), "iommu", NULL)) {
> - error_report("warning: i440fx doesn't support emulated iommu");
> + warn_report("i440fx doesn't support emulated iommu");
> }
> }
>
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index a4cd733cba..47221158d4 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -160,13 +160,13 @@ static void powernv_create_core_node(PnvChip *chip, PnvCore *pc, void *fdt)
> _FDT((fdt_setprop_cell(fdt, offset, "d-cache-size",
> pcc->l1_dcache_size)));
> } else {
> - error_report("Warning: Unknown L1 dcache size for cpu");
> + warn_report("Unknown L1 dcache size for cpu");
> }
> if (pcc->l1_icache_size) {
> _FDT((fdt_setprop_cell(fdt, offset, "i-cache-size",
> pcc->l1_icache_size)));
> } else {
> - error_report("Warning: Unknown L1 icache size for cpu");
> + warn_report("Unknown L1 icache size for cpu");
> }
>
> _FDT((fdt_setprop_cell(fdt, offset, "timebase-frequency", tbfreq)));
> @@ -556,7 +556,7 @@ static void ppc_powernv_init(MachineState *machine)
>
> /* allocate RAM */
> if (machine->ram_size < (1 * G_BYTE)) {
> - error_report("Warning: skiboot may not work with < 1GB of RAM");
> + warn_report("skiboot may not work with < 1GB of RAM");
> }
>
> ram = g_new(MemoryRegion, 1);
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 0ee9fac50b..fdd55d4820 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -534,13 +534,13 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
> _FDT((fdt_setprop_cell(fdt, offset, "d-cache-size",
> pcc->l1_dcache_size)));
> } else {
> - error_report("Warning: Unknown L1 dcache size for cpu");
> + warn_report("Unknown L1 dcache size for cpu");
> }
> if (pcc->l1_icache_size) {
> _FDT((fdt_setprop_cell(fdt, offset, "i-cache-size",
> pcc->l1_icache_size)));
> } else {
> - error_report("Warning: Unknown L1 icache size for cpu");
> + warn_report("Unknown L1 icache size for cpu");
> }
>
> _FDT((fdt_setprop_cell(fdt, offset, "timebase-frequency", tbfreq)));
> diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
> index 8656a54a3e..583afc1a46 100644
> --- a/hw/ppc/spapr_iommu.c
> +++ b/hw/ppc/spapr_iommu.c
> @@ -334,7 +334,7 @@ void spapr_tce_table_enable(sPAPRTCETable *tcet,
> uint32_t nb_table)
> {
> if (tcet->nb_table) {
> - error_report("Warning: trying to enable already enabled TCE table");
> + warn_report("trying to enable already enabled TCE table");
> return;
> }
>
> diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
> index f5574469c8..23c51de66a 100644
> --- a/hw/scsi/scsi-bus.c
> +++ b/hw/scsi/scsi-bus.c
> @@ -282,9 +282,9 @@ void scsi_bus_legacy_handle_cmdline(SCSIBus *bus, bool deprecated)
> continue; /* claimed */
> }
> if (!dinfo->is_default) {
> - error_report("warning: bus=%d,unit=%d is deprecated with this"
> - " machine type",
> - bus->busnr, unit);
> + warn_report("bus=%d,unit=%d is deprecated with this"
> + " machine type",
> + bus->busnr, unit);
> }
> }
> scsi_bus_legacy_add_drive(bus, blk_by_legacy_dinfo(dinfo),
> diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
> index 49cb1829b5..79cd2758a5 100644
> --- a/hw/usb/dev-smartcard-reader.c
> +++ b/hw/usb/dev-smartcard-reader.c
> @@ -1314,12 +1314,12 @@ static int ccid_card_init(DeviceState *qdev)
> int ret = 0;
>
> if (card->slot != 0) {
> - error_report("Warning: usb-ccid supports one slot, can't add %d",
> + warn_report("usb-ccid supports one slot, can't add %d",
> card->slot);
Let's fix indentation while there.
> return -1;
> }
> if (s->card != NULL) {
> - error_report("Warning: usb-ccid card already full, not adding");
> + warn_report("usb-ccid card already full, not adding");
> return -1;
> }
> ret = ccid_card_initfn(card);
> diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
> index aa22d69216..5b65965cc2 100644
> --- a/hw/usb/redirect.c
> +++ b/hw/usb/redirect.c
> @@ -193,7 +193,7 @@ static void usbredir_handle_status(USBRedirDevice *dev, USBPacket *p,
> #define WARNING(...) \
> do { \
> if (dev->debug >= usbredirparser_warning) { \
> - error_report("usb-redir warning: " __VA_ARGS__); \
> + warn_report("" __VA_ARGS__); \
> } \
> } while (0)
> #define INFO(...) \
> diff --git a/net/tap-linux.c b/net/tap-linux.c
> index a503fa9c6e..535b1ddb61 100644
> --- a/net/tap-linux.c
> +++ b/net/tap-linux.c
> @@ -55,7 +55,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
> ifr.ifr_flags = IFF_TAP | IFF_NO_PI;
>
> if (ioctl(fd, TUNGETFEATURES, &features) == -1) {
> - error_report("warning: TUNGETFEATURES failed: %s", strerror(errno));
> + warn_report("TUNGETFEATURES failed: %s", strerror(errno));
> features = 0;
> }
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index c57177278b..c078c8e7f1 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -2060,13 +2060,13 @@ static void x86_cpu_parse_featurestr(const char *typename, char *features,
> name = featurestr;
>
> if (g_list_find_custom(plus_features, name, compare_string)) {
> - error_report("warning: Ambiguous CPU model string. "
> + warn_report("Ambiguous CPU model string. "
> "Don't mix both \"+%s\" and \"%s=%s\"",
> name, name, val);
Indentation's off.
> ambiguous = true;
> }
> if (g_list_find_custom(minus_features, name, compare_string)) {
> - error_report("warning: Ambiguous CPU model string. "
> + warn_report("Ambiguous CPU model string. "
> "Don't mix both \"-%s\" and \"%s=%s\"",
> name, name, val);
Likewise.
> ambiguous = true;
> @@ -2096,7 +2096,7 @@ static void x86_cpu_parse_featurestr(const char *typename, char *features,
> }
>
> if (ambiguous) {
> - error_report("warning: Compatibility of ambiguous CPU model "
> + warn_report("Compatibility of ambiguous CPU model "
> "strings won't be kept on future QEMU versions");
Likewise.
> }
> }
> @@ -3547,7 +3547,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
> */
> if (cpu->phys_bits != host_phys_bits && cpu->phys_bits != 0 &&
> !warned) {
> - error_report("Warning: Host physical bits (%u)"
> + warn_report("Host physical bits (%u)"
> " does not match phys-bits property (%u)",
> host_phys_bits, cpu->phys_bits);
Let's fix indentation while there.
> warned = true;
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index f84a49d366..2ea1d8c728 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -600,7 +600,7 @@ static int kvm_arch_set_tsc_khz(CPUState *cs)
> kvm_vcpu_ioctl(cs, KVM_GET_TSC_KHZ) :
> -ENOTSUP;
> if (cur_freq <= 0 || cur_freq != env->tsc_khz) {
> - error_report("warning: TSC frequency mismatch between "
> + warn_report("TSC frequency mismatch between "
> "VM (%" PRId64 " kHz) and host (%d kHz), "
> "and TSC scaling unavailable",
> env->tsc_khz, cur_freq);
Indentation's off.
> @@ -919,7 +919,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
> error_report("kvm: LMCE not supported");
> return -ENOTSUP;
> }
> - error_report("warning: Unsupported MCG_CAP bits: 0x%" PRIx64,
> + warn_report("Unsupported MCG_CAP bits: 0x%" PRIx64,
> unsupported_caps);
Likewise.
> }
>
> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> index 7cb55dc7e3..8db2dfd6d4 100644
> --- a/target/s390x/cpu_models.c
> +++ b/target/s390x/cpu_models.c
> @@ -677,7 +677,7 @@ static void check_consistency(const S390CPUModel *model)
> for (i = 0; i < ARRAY_SIZE(dep); i++) {
> if (test_bit(dep[i][0], model->features) &&
> !test_bit(dep[i][1], model->features)) {
> - error_report("Warning: \'%s\' requires \'%s\'.",
> + warn_report("\'%s\' requires \'%s\'.",
> s390_feat_def(dep[i][0])->name,
> s390_feat_def(dep[i][1])->name);
Likewise.
> }
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index a3d00196f4..8e956ae39c 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -2675,7 +2675,7 @@ void kvm_s390_apply_cpu_model(const S390CPUModel *model, Error **errp)
> /* enable CMM via CMMA - disable on hugetlbfs */
> if (test_bit(S390_FEAT_CMM, model->features)) {
> if (mem_path) {
> - error_report("Warning: CMM will not be enabled because it is not "
> + warn_report("CMM will not be enabled because it is not "
> "compatible to hugetlbfs.");
Likewise.
> } else {
> kvm_s390_enable_cmma();
> diff --git a/tests/test-qdev-global-props.c b/tests/test-qdev-global-props.c
> index 48e5b7315f..b25fe892ed 100644
> --- a/tests/test-qdev-global-props.c
> +++ b/tests/test-qdev-global-props.c
> @@ -232,10 +232,10 @@ static void test_dynamic_globalprop(void)
> g_test_trap_assert_passed();
> g_test_trap_assert_stderr_unmatched("*prop1*");
> g_test_trap_assert_stderr_unmatched("*prop2*");
> - g_test_trap_assert_stderr("*Warning: global dynamic-prop-type-bad.prop3 has invalid class name\n*");
> + g_test_trap_assert_stderr("*warning: global dynamic-prop-type-bad.prop3 has invalid class name\n*");
> g_test_trap_assert_stderr_unmatched("*prop4*");
> - g_test_trap_assert_stderr("*Warning: global nohotplug-type.prop5=105 not used\n*");
> - g_test_trap_assert_stderr("*Warning: global nondevice-type.prop6 has invalid class name\n*");
> + g_test_trap_assert_stderr("*warning: global nohotplug-type.prop5=105 not used\n*");
> + g_test_trap_assert_stderr("*warning: global nondevice-type.prop6 has invalid class name\n*");
> g_test_trap_assert_stdout("");
> }
>
> diff --git a/trace/control.c b/trace/control.c
> index 9b157b0ca7..bda1000554 100644
> --- a/trace/control.c
> +++ b/trace/control.c
> @@ -171,7 +171,7 @@ static void do_trace_enable_events(const char *line_buf)
> while ((ev = trace_event_iter_next(&iter)) != NULL) {
> if (!trace_event_get_state_static(ev)) {
> if (!is_pattern) {
> - error_report("WARNING: trace event '%s' is not traceable",
> + warn_report("trace event '%s' is not traceable",
> line_ptr);
Likewise.
> return;
> }
> @@ -186,7 +186,7 @@ static void do_trace_enable_events(const char *line_buf)
> }
>
> if (!is_pattern) {
> - error_report("WARNING: trace event '%s' does not exist",
> + warn_report("trace event '%s' does not exist",
> line_ptr);
Likewise.
> }
> }
> diff --git a/vl.c b/vl.c
> index d17c863409..d5342fe816 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -952,8 +952,8 @@ static void bt_vhci_add(int vlan_id)
> struct bt_scatternet_s *vlan = qemu_find_bt_vlan(vlan_id);
>
> if (!vlan->slave)
> - error_report("warning: adding a VHCI to an empty scatternet %i",
> - vlan_id);
> + warn_report("adding a VHCI to an empty scatternet %i",
> + vlan_id);
>
> bt_vhci_init(bt_new_hci(vlan));
> }
> @@ -979,8 +979,8 @@ static struct bt_device_s *bt_device_add(const char *opt)
> vlan = qemu_find_bt_vlan(vlan_id);
>
> if (!vlan->slave)
> - error_report("warning: adding a slave device to an empty scatternet %i",
> - vlan_id);
> + warn_report("adding a slave device to an empty scatternet %i",
> + vlan_id);
>
> if (!strcmp(devname, "keyboard"))
> return bt_keyboard_init(vlan);
> @@ -2302,8 +2302,8 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts, Error **errp)
> return -1;
> }
> if (strncmp(name, "opt/", 4) != 0) {
> - error_report("warning: externally provided fw_cfg item names "
> - "should be prefixed with \"opt/\"");
> + warn_report("externally provided fw_cfg item names "
> + "should be prefixed with \"opt/\"");
> }
> if (nonempty_str(str)) {
> size = strlen(str); /* NUL terminator NOT included in fw_cfg blob */
> @@ -3760,7 +3760,7 @@ int main(int argc, char **argv, char **envp)
> qemu_opts_parse_noisily(olist, "accel=tcg", false);
> break;
> case QEMU_OPTION_no_kvm_pit: {
> - error_report("warning: ignoring deprecated option");
> + warn_report("ignoring deprecated option");
> break;
> }
> case QEMU_OPTION_no_kvm_pit_reinjection: {
> @@ -3770,8 +3770,8 @@ int main(int argc, char **argv, char **envp)
> .value = "discard",
> };
>
> - error_report("warning: deprecated, replaced by "
> - "-global kvm-pit.lost_tick_policy=discard");
> + warn_report("deprecated, replaced by "
> + "-global kvm-pit.lost_tick_policy=discard");
> qdev_prop_register_global(&kvm_pit_lost_tick_policy);
> break;
> }
> @@ -3896,7 +3896,7 @@ int main(int argc, char **argv, char **envp)
> }
> break;
> case QEMU_OPTION_tdf:
> - error_report("warning: ignoring deprecated option");
> + warn_report("ignoring deprecated option");
> break;
> case QEMU_OPTION_name:
> opts = qemu_opts_parse_noisily(qemu_find_opts("name"),
Nothing remotely serious here, but too many little touch-ups for me to
volunteer doing them on commit, sorry.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v3 4/8] hw/i386: Improve some of the warning messages
2017-07-11 12:07 ` [Qemu-devel] [PATCH v3 4/8] hw/i386: Improve some of the warning messages Alistair Francis
@ 2017-07-12 9:39 ` Markus Armbruster
2017-07-12 19:45 ` Eduardo Habkost
0 siblings, 1 reply; 23+ messages in thread
From: Markus Armbruster @ 2017-07-12 9:39 UTC (permalink / raw)
To: Alistair Francis
Cc: qemu-devel, alistair23, philippe, Michael S. Tsirkin,
Igor Mammedov, Eduardo Habkost
Alistair Francis <alistair.francis@xilinx.com> writes:
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
You forgot to cc: Eduardo. Fixed.
> ---
>
> hw/i386/acpi-build.c | 7 ++++---
> hw/i386/pc.c | 9 ++++-----
> hw/i386/pc_q35.c | 4 ++--
> 3 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 6b7bade183..f9efb6be41 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2766,7 +2766,8 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
> ACPI_BUILD_ALIGN_SIZE);
> if (tables_blob->len > legacy_table_size) {
> /* Should happen only with PCI bridges and -M pc-i440fx-2.0. */
> - warn_report("migration may not work.");
> + warn_report("ACPI tables are larger than legacy_table_size");
> + warn_report("migration may not work");
The user has no idea what legacy_table_size means, what its value might
be, or what he can do to reduce it.
Recommend
warn_report("ACPI tables too large, migration may not work");
If the user can do something to reduce the table size, printing suitable
hints would be nice. Printing both tables_blob->len and
legacy_table_size might also help then.
> }
> g_array_set_size(tables_blob, legacy_table_size);
> } else {
> @@ -2774,9 +2775,9 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
> if (tables_blob->len > ACPI_BUILD_TABLE_SIZE / 2) {
> /* As of QEMU 2.1, this fires with 160 VCPUs and 255 memory slots. */
> warn_report("ACPI tables are larger than 64k.");
The warning text hardcodes the value of ACPI_BUILD_TABLE_SIZE / 2. Not
nice. Clean up while there?
> - warn_report("migration may not work.");
> + warn_report("migration may not work");
> warn_report("please remove CPUs, NUMA nodes, "
> - "memory slots or PCI bridges.");
> + "memory slots or PCI bridges");
Aha, here's what the user can do.
What about:
warn_report("ACPI tables are large, migration may not work");
error_printf("Try removing CPUs, NUMA nodes, memory slots"
" or PCI bridges.");
If we want to show actual size and limit, then this might do instead:
warn_report("ACPI table size %u exceeds %d bytes,"
" migration may not work",
tables_blob->len, ACPI_BUILD_TABLE_SIZE / 2);
error_printf("Try removing CPUs, NUMA nodes, memory slots"
" or PCI bridges.");
> }
> acpi_align_size(tables_blob, ACPI_BUILD_TABLE_SIZE);
> }
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 465e91cc5b..084ca796c2 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -383,8 +383,8 @@ ISADevice *pc_find_fdc0(void)
> if (state.multiple) {
> warn_report("multiple floppy disk controllers with "
> "iobase=0x3f0 have been found");
> - error_printf("the one being picked for CMOS setup might not reflect "
> - "your intent\n");
> + warn_report("the one being picked for CMOS setup might not reflect "
> + "your intent");
Please keep error_printf() here.
> }
>
> return state.floppy;
> @@ -2087,9 +2087,8 @@ static void pc_machine_set_max_ram_below_4g(Object *obj, Visitor *v,
> }
>
> if (value < (1ULL << 20)) {
> - warn_report("small max_ram_below_4g(%"PRIu64
> - ") less than 1M. BIOS may not work..",
> - value);
> + warn_report("max_ram_below_4g (%" PRIu64 ") is less than 1M; "
> + "BIOS may not work.", value);
The user has no idea what max_ram_below_4g might be. Suggest:
warn_report("Only %" PRIu64 " bytes of RAM below the 4GiB boundary,"
"BIOS may not work with less than 1MiB");
> }
>
> pcms->max_ram_below_4g = value;
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 1653a47f0a..682c576cf1 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -101,8 +101,8 @@ static void pc_q35_init(MachineState *machine)
> lowmem = pcms->max_ram_below_4g;
> if (machine->ram_size - lowmem > lowmem &&
> lowmem & ((1ULL << 30) - 1)) {
> - warn_report("Large machine and max_ram_below_4g(%"PRIu64
> - ") not a multiple of 1G; possible bad performance.",
> + warn_report("Large machine and max_ram_below_4g (%"PRIu64") not a "
> + "multiple of 1G; possible bad performance.",
Space between string literal and PRIu64, please.
The user has no idea what max_ram_below_4g might be, or what makes the
machine "large".
> pcms->max_ram_below_4g);
> }
> }
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v3 3/8] Convert error_report() to warn_report()
2017-07-12 8:34 ` Markus Armbruster
@ 2017-07-12 10:46 ` Alistair Francis
0 siblings, 0 replies; 23+ messages in thread
From: Alistair Francis @ 2017-07-12 10:46 UTC (permalink / raw)
To: Markus Armbruster
Cc: Alistair Francis, qemu-devel@nongnu.org Developers,
Peter Maydell, Cornelia Huck, Stefan Hajnoczi,
Michael S. Tsirkin, Jeff Cody, Alexander Graf, Gerd Hoffmann,
Eduardo Habkost, Rob Herring, Josh Durgin, Christian Borntraeger,
Marcel Apfelbaum, David Gibson, Jason Wang, philippe,
Peter Lieven, Greg Kurz, Peter Chubb, Ronnie Sahlberg,
Igor Mammedov, Richard Henderson, Kevin Wolf, Peter Crosthwaite,
Marcelo Tosatti, Richard W.M. Jones, Max Reitz, Aneesh Kumar K.V,
Paolo Bonzini
On Wed, Jul 12, 2017 at 10:34 AM, Markus Armbruster <armbru@redhat.com> wrote:
> Alistair Francis <alistair.francis@xilinx.com> writes:
>
>> Convert all uses of error_report("warning:"... to use warn_report()
>> instead. This helps standardise on a single method of printing warnings
>> to the user.
>>
>> All of the warnings where changed using these two commands:
>
> s/where/were/
>
>> find ./* -type f -exec sed -i \
>> 's|error_report(".*warning[,:] |warn_report("|Ig' {} +
>>
>> Then the white space changes where manually fixed afterwards.
>>
>> The test-qdev-global-props test case was manually updated to ensure that
>> this patch passes make check (as the test cases are case sensitive).
>>
>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>> Suggested-by: Thomas Huth <thuth@redhat.com>
>> Cc: Jeff Cody <jcody@redhat.com>
>> Cc: Kevin Wolf <kwolf@redhat.com>
>> Cc: Max Reitz <mreitz@redhat.com>
>> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Peter Lieven <pl@kamp.de>
>> Cc: Josh Durgin <jdurgin@redhat.com>
>> Cc: "Richard W.M. Jones" <rjones@redhat.com>
>> Cc: Markus Armbruster <armbru@redhat.com>
>> Cc: Peter Crosthwaite <crosthwaite.peter@gmail.com>
>> Cc: Richard Henderson <rth@twiddle.net>
>> Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>> Cc: Greg Kurz <groug@kaod.org>
>> Cc: Rob Herring <robh@kernel.org>
>> Cc: Peter Maydell <peter.maydell@linaro.org>
>> Cc: Peter Chubb <peter.chubb@nicta.com.au>
>> Cc: Eduardo Habkost <ehabkost@redhat.com>
>> Cc: Marcel Apfelbaum <marcel@redhat.com>
>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>> Cc: Igor Mammedov <imammedo@redhat.com>
>> Cc: David Gibson <david@gibson.dropbear.id.au>
>> Cc: Alexander Graf <agraf@suse.de>
>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>> Cc: Jason Wang <jasowang@redhat.com>
>> Cc: Marcelo Tosatti <mtosatti@redhat.com>
>> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
>> Cc: Cornelia Huck <cohuck@redhat.com>
>> Cc: Stefan Hajnoczi <stefanha@redhat.com>
>> Acked-by: David Gibson <david@gibson.dropbear.id.au>
>> Acked-by: Greg Kurz <groug@kaod.org>
>> Acked-by: Cornelia Huck <cohuck@redhat.com>
>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>> Reviewed by: Peter Chubb <peter.chubb@data61.csiro.au>
>> Acked-by: Max Reitz <mreitz@redhat.com>
>> Acked-by: Marcel Apfelbaum <marcel@redhat.com>
>> ---
>> V3:
>> - Regenerate patch to ensure no manual edits where made
>> - Tighten regex matches
>> V2:
>> - Fix quotation issues
>> - Update commit message
>> - Include full command
>>
>> Just a note:
>> We will need to do a similar thing for fprintf. There are patches on
>> the list at the moment that conflict with this series (by adding
>> error_report() calls that shsould be warning), so I think I'm giong to
>> have to do a follow up series converting more cases. I'll cover the
>> fprintf cases when I do that. There are some manual cases as
>> well that don't say warning or info, but really should. They will have
>> to be converted as well.
>>
>>
>> block/backup.c | 10 +++++-----
>> block/gluster.c | 4 ++--
>> block/iscsi.c | 6 +++---
>> block/nfs.c | 12 ++++++------
>> block/rbd.c | 6 +++---
>> block/ssh.c | 4 ++--
>> blockdev.c | 2 +-
>> cpus.c | 2 +-
>> hw/9pfs/9p.c | 2 +-
>> hw/arm/highbank.c | 6 +++---
>> hw/arm/imx25_pdk.c | 6 +++---
>> hw/arm/kzm.c | 6 +++---
>> hw/core/machine.c | 10 +++++-----
>> hw/core/qdev-properties.c | 8 ++++----
>> hw/i386/acpi-build.c | 10 +++++-----
>> hw/i386/kvm/pci-assign.c | 6 +++---
>> hw/i386/pc.c | 10 +++++-----
>> hw/i386/pc_piix.c | 8 ++++----
>> hw/i386/pc_q35.c | 6 +++---
>> hw/misc/aspeed_sdmc.c | 8 ++++----
>> hw/nvram/fw_cfg.c | 2 +-
>> hw/pci-host/piix.c | 2 +-
>> hw/ppc/pnv.c | 6 +++---
>> hw/ppc/spapr.c | 4 ++--
>> hw/ppc/spapr_iommu.c | 2 +-
>> hw/scsi/scsi-bus.c | 6 +++---
>> hw/usb/dev-smartcard-reader.c | 4 ++--
>> hw/usb/redirect.c | 2 +-
>> net/tap-linux.c | 2 +-
>> target/i386/cpu.c | 8 ++++----
>> target/i386/kvm.c | 4 ++--
>> target/s390x/cpu_models.c | 2 +-
>> target/s390x/kvm.c | 2 +-
>> tests/test-qdev-global-props.c | 6 +++---
>> trace/control.c | 4 ++--
>> vl.c | 20 ++++++++++----------
>> 36 files changed, 104 insertions(+), 104 deletions(-)
>>
>> diff --git a/block/backup.c b/block/backup.c
>> index b69184eac5..44cc2b22ab 100644
>> --- a/block/backup.c
>> +++ b/block/backup.c
>> @@ -639,11 +639,11 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
>> ret = bdrv_get_info(target, &bdi);
>> if (ret == -ENOTSUP && !target->backing) {
>> /* Cluster size is not defined */
>> - error_report("WARNING: The target block device doesn't provide "
>> - "information about the block size and it doesn't have a "
>> - "backing file. The default block size of %u bytes is "
>> - "used. If the actual block size of the target exceeds "
>> - "this default, the backup may be unusable",
>> + warn_report("The target block device doesn't provide "
>> + "information about the block size and it doesn't have a "
>> + "backing file. The default block size of %u bytes is "
>> + "used. If the actual block size of the target exceeds "
>> + "this default, the backup may be unusable",
>> BACKUP_CLUSTER_SIZE_DEFAULT);
>
> As Max noted, indentation's off here.
>
> Preexisting, not this patch's problem: an error or warning message
> should be a (short!) phrase, not a novel. If additional explanations
> are needed, they go on separate lines, like this:
>
> warn_report("The target block device doesn't provide "
> "information about the block size and it doesn't have a "
> "backing file");
> error_printf("The default block size of %u bytes is used."
> " If the actual block size of the target exceeds "
> "this default, the backup may be unusable.",
> BACKUP_CLUSTER_SIZE_DEFAULT);
>
> Note that the warning message is a *phrase*, not a sentence, let alone
> three, and its followed by additional hints, which *are* sentences.
>
> More of the same below. Followup patches welcome.
Yeah, there are still some things that need to be fixed. I'll keep
this in mind for the next series.
>
>> job->cluster_size = BACKUP_CLUSTER_SIZE_DEFAULT;
>> } else if (ret < 0 && !target->backing) {
>> diff --git a/block/gluster.c b/block/gluster.c
>> index addceed6eb..ba88fe116e 100644
>> --- a/block/gluster.c
>> +++ b/block/gluster.c
>> @@ -345,8 +345,8 @@ static int qemu_gluster_parse_uri(BlockdevOptionsGluster *gconf,
>> is_unix = true;
>> } else if (!strcmp(uri->scheme, "gluster+rdma")) {
>> gsconf->type = SOCKET_ADDRESS_TYPE_INET;
>> - error_report("Warning: rdma feature is not supported, falling "
>> - "back to tcp");
>> + warn_report("rdma feature is not supported, falling "
>> + "back to tcp");
>
> Further cleanup:
>
> warn_report("rdma feature is not supported, falling back to tcp");
>
> Could be done as a separate patch to keep this one as mechanical as
> possible, but I think squashing it in would be okay, too. If we do,
> then the commit message needs to updated. I think
>
> Indentation fixed up manually afterwards.
>
> instead of
>
> Then the white space changes where manually fixed afterwards.
>
> should do.
>
>> } else {
>> ret = -EINVAL;
>> goto out;
>> diff --git a/block/iscsi.c b/block/iscsi.c
>> index 54067e2620..3aa438a0b7 100644
>> --- a/block/iscsi.c
>> +++ b/block/iscsi.c
>> @@ -1761,9 +1761,9 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
>> * filename encoded options */
>> filename = qdict_get_try_str(options, "filename");
>> if (filename) {
>> - error_report("Warning: 'filename' option specified. "
>> - "This is an unsupported option, and may be deprecated "
>> - "in the future");
>> + warn_report("'filename' option specified. "
>> + "This is an unsupported option, and may be deprecated "
>> + "in the future");
>> iscsi_parse_filename(filename, options, &local_err);
>> if (local_err) {
>> ret = -EINVAL;
>> diff --git a/block/nfs.c b/block/nfs.c
>> index c3c5de0113..43929c6f23 100644
>> --- a/block/nfs.c
>> +++ b/block/nfs.c
>> @@ -558,8 +558,8 @@ static int64_t nfs_client_open(NFSClient *client, QDict *options,
>> }
>> client->readahead = qemu_opt_get_number(opts, "readahead-size", 0);
>> if (client->readahead > QEMU_NFS_MAX_READAHEAD_SIZE) {
>> - error_report("NFS Warning: Truncating NFS readahead "
>> - "size to %d", QEMU_NFS_MAX_READAHEAD_SIZE);
>> + warn_report("Truncating NFS readahead "
>> + "size to %d", QEMU_NFS_MAX_READAHEAD_SIZE);
>
> Similarly:
>
> warn_report("Truncating NFS readahead size to %d",
> QEMU_NFS_MAX_READAHEAD_SIZE);
>
>> client->readahead = QEMU_NFS_MAX_READAHEAD_SIZE;
>> }
>> nfs_set_readahead(client->context, client->readahead);
>> @@ -579,8 +579,8 @@ static int64_t nfs_client_open(NFSClient *client, QDict *options,
>> }
>> client->pagecache = qemu_opt_get_number(opts, "page-cache-size", 0);
>> if (client->pagecache > QEMU_NFS_MAX_PAGECACHE_SIZE) {
>> - error_report("NFS Warning: Truncating NFS pagecache "
>> - "size to %d pages", QEMU_NFS_MAX_PAGECACHE_SIZE);
>> + warn_report("Truncating NFS pagecache "
>> + "size to %d pages", QEMU_NFS_MAX_PAGECACHE_SIZE);
>
> Similarly:
>
> warn_report("Truncating NFS pagecache size to %d pages",
> QEMU_NFS_MAX_PAGECACHE_SIZE);
>
>> client->pagecache = QEMU_NFS_MAX_PAGECACHE_SIZE;
>> }
>> nfs_set_pagecache(client->context, client->pagecache);
>> @@ -595,8 +595,8 @@ static int64_t nfs_client_open(NFSClient *client, QDict *options,
>> /* limit the maximum debug level to avoid potential flooding
>> * of our log files. */
>> if (client->debug > QEMU_NFS_MAX_DEBUG_LEVEL) {
>> - error_report("NFS Warning: Limiting NFS debug level "
>> - "to %d", QEMU_NFS_MAX_DEBUG_LEVEL);
>> + warn_report("Limiting NFS debug level "
>> + "to %d", QEMU_NFS_MAX_DEBUG_LEVEL);
>
> warn_report("Limiting NFS debug level to %d",
> QEMU_NFS_MAX_DEBUG_LEVEL);
>
>> client->debug = QEMU_NFS_MAX_DEBUG_LEVEL;
>> }
>> nfs_set_debug(client->context, client->debug);
>> diff --git a/block/rbd.c b/block/rbd.c
>> index 9da02cdceb..d461f7dc87 100644
>> --- a/block/rbd.c
>> +++ b/block/rbd.c
>> @@ -555,9 +555,9 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,
>> * filename encoded options */
>> filename = qdict_get_try_str(options, "filename");
>> if (filename) {
>> - error_report("Warning: 'filename' option specified. "
>> - "This is an unsupported option, and may be deprecated "
>> - "in the future");
>> + warn_report("'filename' option specified. "
>> + "This is an unsupported option, and may be deprecated "
>> + "in the future");
>> qemu_rbd_parse_filename(filename, options, &local_err);
>> if (local_err) {
>> r = -EINVAL;
>> diff --git a/block/ssh.c b/block/ssh.c
>> index 52964416da..07a57eb466 100644
>> --- a/block/ssh.c
>> +++ b/block/ssh.c
>> @@ -1114,8 +1114,8 @@ static coroutine_fn int ssh_co_writev(BlockDriverState *bs,
>> static void unsafe_flush_warning(BDRVSSHState *s, const char *what)
>> {
>> if (!s->unsafe_flush_warning) {
>> - error_report("warning: ssh server %s does not support fsync",
>> - s->inet->host);
>> + warn_report("ssh server %s does not support fsync",
>> + s->inet->host);
>> if (what) {
>> error_report("to support fsync, you need %s", what);
>> }
>> diff --git a/blockdev.c b/blockdev.c
>> index e2016b6f37..a521c85956 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
>> @@ -914,7 +914,7 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
>> copy_on_read = qemu_opt_get_bool(legacy_opts, "copy-on-read", false);
>>
>> if (read_only && copy_on_read) {
>> - error_report("warning: disabling copy-on-read on read-only drive");
>> + warn_report("disabling copy-on-read on read-only drive");
>> copy_on_read = false;
>> }
>>
>> diff --git a/cpus.c b/cpus.c
>> index 14bb8d552e..9bed61eefc 100644
>> --- a/cpus.c
>> +++ b/cpus.c
>> @@ -557,7 +557,7 @@ void qemu_start_warp_timer(void)
>> if (deadline < 0) {
>> static bool notified;
>> if (!icount_sleep && !notified) {
>> - error_report("WARNING: icount sleep disabled and no active timers");
>> + warn_report("icount sleep disabled and no active timers");
>> notified = true;
>> }
>> return;
>> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
>> index 6c92bad5b3..333dbb6f8e 100644
>> --- a/hw/9pfs/9p.c
>> +++ b/hw/9pfs/9p.c
>> @@ -2376,7 +2376,7 @@ static void coroutine_fn v9fs_flush(void *opaque)
>> trace_v9fs_flush(pdu->tag, pdu->id, tag);
>>
>> if (pdu->tag == tag) {
>> - error_report("Warning: the guest sent a self-referencing 9P flush request");
>> + warn_report("the guest sent a self-referencing 9P flush request");
>> } else {
>> QLIST_FOREACH(cancel_pdu, &s->active_list, next) {
>> if (cancel_pdu->tag == tag) {
>> diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
>> index d209b97dee..750c463e2a 100644
>> --- a/hw/arm/highbank.c
>> +++ b/hw/arm/highbank.c
>> @@ -383,9 +383,9 @@ static void calxeda_init(MachineState *machine, enum cxmachines machine_id)
>> highbank_binfo.write_board_setup = hb_write_board_setup;
>> highbank_binfo.secure_board_setup = true;
>> } else {
>> - error_report("WARNING: cannot load built-in Monitor support "
>> - "if KVM is enabled. Some guests (such as Linux) "
>> - "may not boot.");
>> + warn_report("cannot load built-in Monitor support "
>> + "if KVM is enabled. Some guests (such as Linux) "
>> + "may not boot.");
>> }
>>
>> arm_load_kernel(ARM_CPU(first_cpu), &highbank_binfo);
>> diff --git a/hw/arm/imx25_pdk.c b/hw/arm/imx25_pdk.c
>> index 44e741fde3..7d42c74001 100644
>> --- a/hw/arm/imx25_pdk.c
>> +++ b/hw/arm/imx25_pdk.c
>> @@ -80,9 +80,9 @@ static void imx25_pdk_init(MachineState *machine)
>>
>> /* We need to initialize our memory */
>> if (machine->ram_size > (FSL_IMX25_SDRAM0_SIZE + FSL_IMX25_SDRAM1_SIZE)) {
>> - error_report("WARNING: RAM size " RAM_ADDR_FMT " above max supported, "
>> - "reduced to %x", machine->ram_size,
>> - FSL_IMX25_SDRAM0_SIZE + FSL_IMX25_SDRAM1_SIZE);
>> + warn_report("RAM size " RAM_ADDR_FMT " above max supported, "
>> + "reduced to %x", machine->ram_size,
>> + FSL_IMX25_SDRAM0_SIZE + FSL_IMX25_SDRAM1_SIZE);
>> machine->ram_size = FSL_IMX25_SDRAM0_SIZE + FSL_IMX25_SDRAM1_SIZE;
>> }
>>
>> diff --git a/hw/arm/kzm.c b/hw/arm/kzm.c
>> index 2c96ee33b6..3ed6577a55 100644
>> --- a/hw/arm/kzm.c
>> +++ b/hw/arm/kzm.c
>> @@ -79,9 +79,9 @@ static void kzm_init(MachineState *machine)
>>
>> /* Check the amount of memory is compatible with the SOC */
>> if (machine->ram_size > (FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE)) {
>> - error_report("WARNING: RAM size " RAM_ADDR_FMT " above max supported, "
>> - "reduced to %x", machine->ram_size,
>> - FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE);
>> + warn_report("RAM size " RAM_ADDR_FMT " above max supported, "
>> + "reduced to %x", machine->ram_size,
>> + FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE);
>> machine->ram_size = FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE;
>> }
>>
>> diff --git a/hw/core/machine.c b/hw/core/machine.c
>> index ecb55528e8..dc431fabf5 100644
>> --- a/hw/core/machine.c
>> +++ b/hw/core/machine.c
>> @@ -741,11 +741,11 @@ static void machine_numa_finish_init(MachineState *machine)
>> }
>> }
>> if (s->len && !qtest_enabled()) {
>> - error_report("warning: CPU(s) not present in any NUMA nodes: %s",
>> - s->str);
>> - error_report("warning: All CPU(s) up to maxcpus should be described "
>> - "in NUMA config, ability to start up with partial NUMA "
>> - "mappings is obsoleted and will be removed in future");
>> + warn_report("CPU(s) not present in any NUMA nodes: %s",
>> + s->str);
>> + warn_report("All CPU(s) up to maxcpus should be described "
>> + "in NUMA config, ability to start up with partial NUMA "
>> + "mappings is obsoleted and will be removed in future");
>
> Preexisting, not this patch's problem: the resulting output looks like
> two independent issues instead of one. Multiple error_report() /
> warn_report() in a row are almost always inappropriate.
>
> warn_report("CPU(s) not present in any NUMA nodes: %s", s->str);
> error_printf("All CPU(s) up to maxcpus should be described "
> "in NUMA config, ability to start up with partial NUMA "
> "mappings is obsoleted and will be removed in future");
>
> More of the same below. Followup patches welcome.
>
>> }
>> g_string_free(s, true);
>> }
>> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
>> index f11d57831b..f5983c83da 100644
>> --- a/hw/core/qdev-properties.c
>> +++ b/hw/core/qdev-properties.c
>> @@ -1132,15 +1132,15 @@ int qdev_prop_check_globals(void)
>> oc = object_class_by_name(prop->driver);
>> oc = object_class_dynamic_cast(oc, TYPE_DEVICE);
>> if (!oc) {
>> - error_report("Warning: global %s.%s has invalid class name",
>> - prop->driver, prop->property);
>> + warn_report("global %s.%s has invalid class name",
>> + prop->driver, prop->property);
>> ret = 1;
>> continue;
>> }
>> dc = DEVICE_CLASS(oc);
>> if (!dc->hotpluggable && !prop->used) {
>> - error_report("Warning: global %s.%s=%s not used",
>> - prop->driver, prop->property, prop->value);
>> + warn_report("global %s.%s=%s not used",
>> + prop->driver, prop->property, prop->value);
>> ret = 1;
>> continue;
>> }
>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> index 5464977424..6b7bade183 100644
>> --- a/hw/i386/acpi-build.c
>> +++ b/hw/i386/acpi-build.c
>> @@ -2766,17 +2766,17 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
>> ACPI_BUILD_ALIGN_SIZE);
>> if (tables_blob->len > legacy_table_size) {
>> /* Should happen only with PCI bridges and -M pc-i440fx-2.0. */
>> - error_report("Warning: migration may not work.");
>> + warn_report("migration may not work.");
>
> Could use a tree-wide sweep to drop periods from the end of error
> messages. I'm not asking you to do this now.
>
>> }
>> g_array_set_size(tables_blob, legacy_table_size);
>> } else {
>> /* Make sure we have a buffer in case we need to resize the tables. */
>> if (tables_blob->len > ACPI_BUILD_TABLE_SIZE / 2) {
>> /* As of QEMU 2.1, this fires with 160 VCPUs and 255 memory slots. */
>> - error_report("Warning: ACPI tables are larger than 64k.");
>> - error_report("Warning: migration may not work.");
>> - error_report("Warning: please remove CPUs, NUMA nodes, "
>> - "memory slots or PCI bridges.");
>> + warn_report("ACPI tables are larger than 64k.");
>> + warn_report("migration may not work.");
>> + warn_report("please remove CPUs, NUMA nodes, "
>> + "memory slots or PCI bridges.");
>> }
>> acpi_align_size(tables_blob, ACPI_BUILD_TABLE_SIZE);
>> }
>> diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
>> index 9f2615cbe0..33e20cb3e8 100644
>> --- a/hw/i386/kvm/pci-assign.c
>> +++ b/hw/i386/kvm/pci-assign.c
>> @@ -1353,9 +1353,9 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev, Error **errp)
>> PCI_CAP_ID_EXP);
>> return -EINVAL;
>> } else if (size != 0x3c) {
>> - error_report("WARNING, %s: PCIe cap-id 0x%x has "
>> - "non-standard size 0x%x; std size should be 0x3c",
>> - __func__, PCI_CAP_ID_EXP, size);
>> + warn_report("%s: PCIe cap-id 0x%x has "
>> + "non-standard size 0x%x; std size should be 0x3c",
>> + __func__, PCI_CAP_ID_EXP, size);
>> }
>> } else if (version == 0) {
>> uint16_t vid, did;
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index 224fe58fe7..465e91cc5b 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -381,8 +381,8 @@ ISADevice *pc_find_fdc0(void)
>> }
>>
>> if (state.multiple) {
>> - error_report("warning: multiple floppy disk controllers with "
>> - "iobase=0x3f0 have been found");
>> + warn_report("multiple floppy disk controllers with "
>> + "iobase=0x3f0 have been found");
>> error_printf("the one being picked for CMOS setup might not reflect "
>> "your intent\n");
>> }
>> @@ -2087,9 +2087,9 @@ static void pc_machine_set_max_ram_below_4g(Object *obj, Visitor *v,
>> }
>>
>> if (value < (1ULL << 20)) {
>> - error_report("Warning: small max_ram_below_4g(%"PRIu64
>> - ") less than 1M. BIOS may not work..",
>> - value);
>> + warn_report("small max_ram_below_4g(%"PRIu64
>
> Let's insert a space between string literal and PRIu64 while there.
>
>> + ") less than 1M. BIOS may not work..",
>> + value);
>> }
>>
>> pcms->max_ram_below_4g = value;
>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>> index 22dbef64c6..11b4336a42 100644
>> --- a/hw/i386/pc_piix.c
>> +++ b/hw/i386/pc_piix.c
>> @@ -131,10 +131,10 @@ static void pc_init1(MachineState *machine,
>> lowmem = 0xc0000000;
>> }
>> if (lowmem & ((1ULL << 30) - 1)) {
>> - error_report("Warning: Large machine and max_ram_below_4g "
>> - "(%" PRIu64 ") not a multiple of 1G; "
>> - "possible bad performance.",
>> - pcms->max_ram_below_4g);
>> + warn_report("Large machine and max_ram_below_4g "
>> + "(%" PRIu64 ") not a multiple of 1G; "
>> + "possible bad performance.",
>> + pcms->max_ram_below_4g);
>> }
>> }
>> }
>> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
>> index 8f696b7cb6..1653a47f0a 100644
>> --- a/hw/i386/pc_q35.c
>> +++ b/hw/i386/pc_q35.c
>> @@ -101,9 +101,9 @@ static void pc_q35_init(MachineState *machine)
>> lowmem = pcms->max_ram_below_4g;
>> if (machine->ram_size - lowmem > lowmem &&
>> lowmem & ((1ULL << 30) - 1)) {
>> - error_report("Warning: Large machine and max_ram_below_4g(%"PRIu64
>> - ") not a multiple of 1G; possible bad performance.",
>> - pcms->max_ram_below_4g);
>> + warn_report("Large machine and max_ram_below_4g(%"PRIu64
>> + ") not a multiple of 1G; possible bad performance.",
>> + pcms->max_ram_below_4g);
>
> Let's insert a space between string literal and PRIu64 while there.
These ones are fixed in next patch, so I'm just going to leave them as
is in this patch.
>
>> }
>> }
>>
>> diff --git a/hw/misc/aspeed_sdmc.c b/hw/misc/aspeed_sdmc.c
>> index 5f3ac0b6f6..633fa4510e 100644
>> --- a/hw/misc/aspeed_sdmc.c
>> +++ b/hw/misc/aspeed_sdmc.c
>> @@ -157,8 +157,8 @@ static int ast2400_rambits(AspeedSDMCState *s)
>> }
>>
>> /* use a common default */
>> - error_report("warning: Invalid RAM size 0x%" PRIx64
>> - ". Using default 256M", s->ram_size);
>> + warn_report("Invalid RAM size 0x%" PRIx64
>> + ". Using default 256M", s->ram_size);
>
> warn_report("Invalid RAM size 0x%" PRIx64 ". Using default 256M",
> s->ram_size);
>
>> s->ram_size = 256 << 20;
>> return ASPEED_SDMC_DRAM_256MB;
>> }
>> @@ -179,8 +179,8 @@ static int ast2500_rambits(AspeedSDMCState *s)
>> }
>>
>> /* use a common default */
>> - error_report("warning: Invalid RAM size 0x%" PRIx64
>> - ". Using default 512M", s->ram_size);
>> + warn_report("Invalid RAM size 0x%" PRIx64
>> + ". Using default 512M", s->ram_size);
>
> warn_report("Invalid RAM size 0x%" PRIx64 ". Using default 512M",
> s->ram_size);
>
>> s->ram_size = 512 << 20;
>> return ASPEED_SDMC_AST2500_512MB;
>> }
>> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
>> index 99bdbc2233..e881e3b812 100644
>> --- a/hw/nvram/fw_cfg.c
>> +++ b/hw/nvram/fw_cfg.c
>> @@ -781,7 +781,7 @@ static int get_fw_cfg_order(FWCfgState *s, const char *name)
>> }
>>
>> /* Stick unknown stuff at the end. */
>> - error_report("warning: Unknown firmware file in legacy mode: %s", name);
>> + warn_report("Unknown firmware file in legacy mode: %s", name);
>> return FW_CFG_ORDER_OVERRIDE_LAST;
>> }
>>
>> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
>> index a2c1033dbe..072a04e318 100644
>> --- a/hw/pci-host/piix.c
>> +++ b/hw/pci-host/piix.c
>> @@ -307,7 +307,7 @@ static void i440fx_realize(PCIDevice *dev, Error **errp)
>> dev->config[I440FX_SMRAM] = 0x02;
>>
>> if (object_property_get_bool(qdev_get_machine(), "iommu", NULL)) {
>> - error_report("warning: i440fx doesn't support emulated iommu");
>> + warn_report("i440fx doesn't support emulated iommu");
>> }
>> }
>>
>> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
>> index a4cd733cba..47221158d4 100644
>> --- a/hw/ppc/pnv.c
>> +++ b/hw/ppc/pnv.c
>> @@ -160,13 +160,13 @@ static void powernv_create_core_node(PnvChip *chip, PnvCore *pc, void *fdt)
>> _FDT((fdt_setprop_cell(fdt, offset, "d-cache-size",
>> pcc->l1_dcache_size)));
>> } else {
>> - error_report("Warning: Unknown L1 dcache size for cpu");
>> + warn_report("Unknown L1 dcache size for cpu");
>> }
>> if (pcc->l1_icache_size) {
>> _FDT((fdt_setprop_cell(fdt, offset, "i-cache-size",
>> pcc->l1_icache_size)));
>> } else {
>> - error_report("Warning: Unknown L1 icache size for cpu");
>> + warn_report("Unknown L1 icache size for cpu");
>> }
>>
>> _FDT((fdt_setprop_cell(fdt, offset, "timebase-frequency", tbfreq)));
>> @@ -556,7 +556,7 @@ static void ppc_powernv_init(MachineState *machine)
>>
>> /* allocate RAM */
>> if (machine->ram_size < (1 * G_BYTE)) {
>> - error_report("Warning: skiboot may not work with < 1GB of RAM");
>> + warn_report("skiboot may not work with < 1GB of RAM");
>> }
>>
>> ram = g_new(MemoryRegion, 1);
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 0ee9fac50b..fdd55d4820 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -534,13 +534,13 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
>> _FDT((fdt_setprop_cell(fdt, offset, "d-cache-size",
>> pcc->l1_dcache_size)));
>> } else {
>> - error_report("Warning: Unknown L1 dcache size for cpu");
>> + warn_report("Unknown L1 dcache size for cpu");
>> }
>> if (pcc->l1_icache_size) {
>> _FDT((fdt_setprop_cell(fdt, offset, "i-cache-size",
>> pcc->l1_icache_size)));
>> } else {
>> - error_report("Warning: Unknown L1 icache size for cpu");
>> + warn_report("Unknown L1 icache size for cpu");
>> }
>>
>> _FDT((fdt_setprop_cell(fdt, offset, "timebase-frequency", tbfreq)));
>> diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
>> index 8656a54a3e..583afc1a46 100644
>> --- a/hw/ppc/spapr_iommu.c
>> +++ b/hw/ppc/spapr_iommu.c
>> @@ -334,7 +334,7 @@ void spapr_tce_table_enable(sPAPRTCETable *tcet,
>> uint32_t nb_table)
>> {
>> if (tcet->nb_table) {
>> - error_report("Warning: trying to enable already enabled TCE table");
>> + warn_report("trying to enable already enabled TCE table");
>> return;
>> }
>>
>> diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
>> index f5574469c8..23c51de66a 100644
>> --- a/hw/scsi/scsi-bus.c
>> +++ b/hw/scsi/scsi-bus.c
>> @@ -282,9 +282,9 @@ void scsi_bus_legacy_handle_cmdline(SCSIBus *bus, bool deprecated)
>> continue; /* claimed */
>> }
>> if (!dinfo->is_default) {
>> - error_report("warning: bus=%d,unit=%d is deprecated with this"
>> - " machine type",
>> - bus->busnr, unit);
>> + warn_report("bus=%d,unit=%d is deprecated with this"
>> + " machine type",
>> + bus->busnr, unit);
>> }
>> }
>> scsi_bus_legacy_add_drive(bus, blk_by_legacy_dinfo(dinfo),
>> diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
>> index 49cb1829b5..79cd2758a5 100644
>> --- a/hw/usb/dev-smartcard-reader.c
>> +++ b/hw/usb/dev-smartcard-reader.c
>> @@ -1314,12 +1314,12 @@ static int ccid_card_init(DeviceState *qdev)
>> int ret = 0;
>>
>> if (card->slot != 0) {
>> - error_report("Warning: usb-ccid supports one slot, can't add %d",
>> + warn_report("usb-ccid supports one slot, can't add %d",
>> card->slot);
>
> Let's fix indentation while there.
>
>> return -1;
>> }
>> if (s->card != NULL) {
>> - error_report("Warning: usb-ccid card already full, not adding");
>> + warn_report("usb-ccid card already full, not adding");
>> return -1;
>> }
>> ret = ccid_card_initfn(card);
>> diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
>> index aa22d69216..5b65965cc2 100644
>> --- a/hw/usb/redirect.c
>> +++ b/hw/usb/redirect.c
>> @@ -193,7 +193,7 @@ static void usbredir_handle_status(USBRedirDevice *dev, USBPacket *p,
>> #define WARNING(...) \
>> do { \
>> if (dev->debug >= usbredirparser_warning) { \
>> - error_report("usb-redir warning: " __VA_ARGS__); \
>> + warn_report("" __VA_ARGS__); \
>> } \
>> } while (0)
>> #define INFO(...) \
>> diff --git a/net/tap-linux.c b/net/tap-linux.c
>> index a503fa9c6e..535b1ddb61 100644
>> --- a/net/tap-linux.c
>> +++ b/net/tap-linux.c
>> @@ -55,7 +55,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
>> ifr.ifr_flags = IFF_TAP | IFF_NO_PI;
>>
>> if (ioctl(fd, TUNGETFEATURES, &features) == -1) {
>> - error_report("warning: TUNGETFEATURES failed: %s", strerror(errno));
>> + warn_report("TUNGETFEATURES failed: %s", strerror(errno));
>> features = 0;
>> }
>>
>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>> index c57177278b..c078c8e7f1 100644
>> --- a/target/i386/cpu.c
>> +++ b/target/i386/cpu.c
>> @@ -2060,13 +2060,13 @@ static void x86_cpu_parse_featurestr(const char *typename, char *features,
>> name = featurestr;
>>
>> if (g_list_find_custom(plus_features, name, compare_string)) {
>> - error_report("warning: Ambiguous CPU model string. "
>> + warn_report("Ambiguous CPU model string. "
>> "Don't mix both \"+%s\" and \"%s=%s\"",
>> name, name, val);
>
> Indentation's off.
>
>> ambiguous = true;
>> }
>> if (g_list_find_custom(minus_features, name, compare_string)) {
>> - error_report("warning: Ambiguous CPU model string. "
>> + warn_report("Ambiguous CPU model string. "
>> "Don't mix both \"-%s\" and \"%s=%s\"",
>> name, name, val);
>
> Likewise.
>
>> ambiguous = true;
>> @@ -2096,7 +2096,7 @@ static void x86_cpu_parse_featurestr(const char *typename, char *features,
>> }
>>
>> if (ambiguous) {
>> - error_report("warning: Compatibility of ambiguous CPU model "
>> + warn_report("Compatibility of ambiguous CPU model "
>> "strings won't be kept on future QEMU versions");
>
> Likewise.
>
>> }
>> }
>> @@ -3547,7 +3547,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
>> */
>> if (cpu->phys_bits != host_phys_bits && cpu->phys_bits != 0 &&
>> !warned) {
>> - error_report("Warning: Host physical bits (%u)"
>> + warn_report("Host physical bits (%u)"
>> " does not match phys-bits property (%u)",
>> host_phys_bits, cpu->phys_bits);
>
> Let's fix indentation while there.
>
>> warned = true;
>> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
>> index f84a49d366..2ea1d8c728 100644
>> --- a/target/i386/kvm.c
>> +++ b/target/i386/kvm.c
>> @@ -600,7 +600,7 @@ static int kvm_arch_set_tsc_khz(CPUState *cs)
>> kvm_vcpu_ioctl(cs, KVM_GET_TSC_KHZ) :
>> -ENOTSUP;
>> if (cur_freq <= 0 || cur_freq != env->tsc_khz) {
>> - error_report("warning: TSC frequency mismatch between "
>> + warn_report("TSC frequency mismatch between "
>> "VM (%" PRId64 " kHz) and host (%d kHz), "
>> "and TSC scaling unavailable",
>> env->tsc_khz, cur_freq);
>
> Indentation's off.
>
>> @@ -919,7 +919,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
>> error_report("kvm: LMCE not supported");
>> return -ENOTSUP;
>> }
>> - error_report("warning: Unsupported MCG_CAP bits: 0x%" PRIx64,
>> + warn_report("Unsupported MCG_CAP bits: 0x%" PRIx64,
>> unsupported_caps);
>
> Likewise.
>
>> }
>>
>> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
>> index 7cb55dc7e3..8db2dfd6d4 100644
>> --- a/target/s390x/cpu_models.c
>> +++ b/target/s390x/cpu_models.c
>> @@ -677,7 +677,7 @@ static void check_consistency(const S390CPUModel *model)
>> for (i = 0; i < ARRAY_SIZE(dep); i++) {
>> if (test_bit(dep[i][0], model->features) &&
>> !test_bit(dep[i][1], model->features)) {
>> - error_report("Warning: \'%s\' requires \'%s\'.",
>> + warn_report("\'%s\' requires \'%s\'.",
>> s390_feat_def(dep[i][0])->name,
>> s390_feat_def(dep[i][1])->name);
>
> Likewise.
>
>> }
>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
>> index a3d00196f4..8e956ae39c 100644
>> --- a/target/s390x/kvm.c
>> +++ b/target/s390x/kvm.c
>> @@ -2675,7 +2675,7 @@ void kvm_s390_apply_cpu_model(const S390CPUModel *model, Error **errp)
>> /* enable CMM via CMMA - disable on hugetlbfs */
>> if (test_bit(S390_FEAT_CMM, model->features)) {
>> if (mem_path) {
>> - error_report("Warning: CMM will not be enabled because it is not "
>> + warn_report("CMM will not be enabled because it is not "
>> "compatible to hugetlbfs.");
>
> Likewise.
>
>> } else {
>> kvm_s390_enable_cmma();
>> diff --git a/tests/test-qdev-global-props.c b/tests/test-qdev-global-props.c
>> index 48e5b7315f..b25fe892ed 100644
>> --- a/tests/test-qdev-global-props.c
>> +++ b/tests/test-qdev-global-props.c
>> @@ -232,10 +232,10 @@ static void test_dynamic_globalprop(void)
>> g_test_trap_assert_passed();
>> g_test_trap_assert_stderr_unmatched("*prop1*");
>> g_test_trap_assert_stderr_unmatched("*prop2*");
>> - g_test_trap_assert_stderr("*Warning: global dynamic-prop-type-bad.prop3 has invalid class name\n*");
>> + g_test_trap_assert_stderr("*warning: global dynamic-prop-type-bad.prop3 has invalid class name\n*");
>> g_test_trap_assert_stderr_unmatched("*prop4*");
>> - g_test_trap_assert_stderr("*Warning: global nohotplug-type.prop5=105 not used\n*");
>> - g_test_trap_assert_stderr("*Warning: global nondevice-type.prop6 has invalid class name\n*");
>> + g_test_trap_assert_stderr("*warning: global nohotplug-type.prop5=105 not used\n*");
>> + g_test_trap_assert_stderr("*warning: global nondevice-type.prop6 has invalid class name\n*");
>> g_test_trap_assert_stdout("");
>> }
>>
>> diff --git a/trace/control.c b/trace/control.c
>> index 9b157b0ca7..bda1000554 100644
>> --- a/trace/control.c
>> +++ b/trace/control.c
>> @@ -171,7 +171,7 @@ static void do_trace_enable_events(const char *line_buf)
>> while ((ev = trace_event_iter_next(&iter)) != NULL) {
>> if (!trace_event_get_state_static(ev)) {
>> if (!is_pattern) {
>> - error_report("WARNING: trace event '%s' is not traceable",
>> + warn_report("trace event '%s' is not traceable",
>> line_ptr);
>
> Likewise.
>
>> return;
>> }
>> @@ -186,7 +186,7 @@ static void do_trace_enable_events(const char *line_buf)
>> }
>>
>> if (!is_pattern) {
>> - error_report("WARNING: trace event '%s' does not exist",
>> + warn_report("trace event '%s' does not exist",
>> line_ptr);
>
> Likewise.
>
>> }
>> }
>> diff --git a/vl.c b/vl.c
>> index d17c863409..d5342fe816 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -952,8 +952,8 @@ static void bt_vhci_add(int vlan_id)
>> struct bt_scatternet_s *vlan = qemu_find_bt_vlan(vlan_id);
>>
>> if (!vlan->slave)
>> - error_report("warning: adding a VHCI to an empty scatternet %i",
>> - vlan_id);
>> + warn_report("adding a VHCI to an empty scatternet %i",
>> + vlan_id);
>>
>> bt_vhci_init(bt_new_hci(vlan));
>> }
>> @@ -979,8 +979,8 @@ static struct bt_device_s *bt_device_add(const char *opt)
>> vlan = qemu_find_bt_vlan(vlan_id);
>>
>> if (!vlan->slave)
>> - error_report("warning: adding a slave device to an empty scatternet %i",
>> - vlan_id);
>> + warn_report("adding a slave device to an empty scatternet %i",
>> + vlan_id);
>>
>> if (!strcmp(devname, "keyboard"))
>> return bt_keyboard_init(vlan);
>> @@ -2302,8 +2302,8 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts, Error **errp)
>> return -1;
>> }
>> if (strncmp(name, "opt/", 4) != 0) {
>> - error_report("warning: externally provided fw_cfg item names "
>> - "should be prefixed with \"opt/\"");
>> + warn_report("externally provided fw_cfg item names "
>> + "should be prefixed with \"opt/\"");
>> }
>> if (nonempty_str(str)) {
>> size = strlen(str); /* NUL terminator NOT included in fw_cfg blob */
>> @@ -3760,7 +3760,7 @@ int main(int argc, char **argv, char **envp)
>> qemu_opts_parse_noisily(olist, "accel=tcg", false);
>> break;
>> case QEMU_OPTION_no_kvm_pit: {
>> - error_report("warning: ignoring deprecated option");
>> + warn_report("ignoring deprecated option");
>> break;
>> }
>> case QEMU_OPTION_no_kvm_pit_reinjection: {
>> @@ -3770,8 +3770,8 @@ int main(int argc, char **argv, char **envp)
>> .value = "discard",
>> };
>>
>> - error_report("warning: deprecated, replaced by "
>> - "-global kvm-pit.lost_tick_policy=discard");
>> + warn_report("deprecated, replaced by "
>> + "-global kvm-pit.lost_tick_policy=discard");
>> qdev_prop_register_global(&kvm_pit_lost_tick_policy);
>> break;
>> }
>> @@ -3896,7 +3896,7 @@ int main(int argc, char **argv, char **envp)
>> }
>> break;
>> case QEMU_OPTION_tdf:
>> - error_report("warning: ignoring deprecated option");
>> + warn_report("ignoring deprecated option");
>> break;
>> case QEMU_OPTION_name:
>> opts = qemu_opts_parse_noisily(qemu_find_opts("name"),
>
> Nothing remotely serious here, but too many little touch-ups for me to
> volunteer doing them on commit, sorry.
No worries, I'll prepare a new version.
Thanks,
Alistair
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v3 2/8] error: Functions to report warnings and informational messages
2017-07-12 7:57 ` Markus Armbruster
@ 2017-07-12 10:48 ` Alistair Francis
0 siblings, 0 replies; 23+ messages in thread
From: Alistair Francis @ 2017-07-12 10:48 UTC (permalink / raw)
To: Markus Armbruster
Cc: Alistair Francis, qemu-devel@nongnu.org Developers, philippe
On Wed, Jul 12, 2017 at 9:57 AM, Markus Armbruster <armbru@redhat.com> wrote:
> Alistair Francis <alistair.francis@xilinx.com> writes:
>
>> Add warn_report(), warn_vreport() for reporting warnings, and
>> info_report(), info_vreport() for informational messages.
>>
>> These are implemented them with a helper function factored out of
>> error_vreport(), suitably generalized. This patch makes no changes
>> to the output of the original error_report() function.
>>
>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> v2:
>> - Don't add *vreport() functions to checkpatch
>> - Maintain original comments for the reporting functions
>> - Don't change the error report output in this patch
>> v1:
>> - Don't expose the generic report and vreport() functions
>> - Prefix error messages
>> - Use vreport instead of qmsg_vreport()
>> RFC V3:
>> - Change the function and enum names to be more descriptive
>> - Add wrapper functions for *_report() and *_vreport()
>>
>> include/qemu/error-report.h | 7 +++
>> scripts/checkpatch.pl | 6 ++-
>> util/qemu-error.c | 102 +++++++++++++++++++++++++++++++++++++++++---
>> 3 files changed, 109 insertions(+), 6 deletions(-)
> [...]
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>> index 73efc927a9..60b1f320a1 100755
>> --- a/scripts/checkpatch.pl
>> +++ b/scripts/checkpatch.pl
>> @@ -2535,7 +2535,11 @@ sub process {
>> error_prepend|
>> error_reportf_err|
>> error_vreport|
>> - error_report}x;
>> + report_vreport|
>
> Should be warn_vreport, oops. I can fix this on commit.
I have to send a new version anyway so I'll just fix it.
Thanks,
Alistair
>
>> + info_vreport|
>> + error_report|
>> + warn_report|
>> + info_report}x;
>>
>> if ($rawline =~ /\b(?:$qemu_error_funcs)\s*\(.*\".*\\n/) {
>> ERROR("Error messages should not contain newlines\n" . $herecurr);
> [...]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v3 8/8] error: Add a 'error: ' prefix to error_report()
2017-07-11 17:44 ` Max Reitz
@ 2017-07-12 12:27 ` Alistair Francis
2017-07-12 12:37 ` Max Reitz
2017-07-12 13:03 ` Markus Armbruster
0 siblings, 2 replies; 23+ messages in thread
From: Alistair Francis @ 2017-07-12 12:27 UTC (permalink / raw)
To: Max Reitz
Cc: Alistair Francis, qemu-devel@nongnu.org Developers, philippe,
Markus Armbruster
On Tue, Jul 11, 2017 at 7:44 PM, Max Reitz <mreitz@redhat.com> wrote:
> On 2017-07-11 14:07, Alistair Francis wrote:
>> As we don't regard error messages as a stable API the let's add a
>> 'error: ' prefix to the original error_report() messages.
>>
>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>> ---
>>
>> util/qemu-error.c | 1 +
>> 1 file changed, 1 insertion(+)
>
> This breaks quite a few qemu-iotests.
Hmmm... Ok, I'll have to dig into that.
I'm traveling at the moment so I won't get a chance to fix this. I'm
going to send the next version of this series, but remove this patch
and I can look at adding it again when I get back. There is already a
few things that have been pointed out that need to be fixed after this
patch set.
Do people think we should add this prefix or leave the error messages
as is? I haven't heard a definitive answer if people think this is the
right path to take.
Thanks,
Alistair
>
> Max
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v3 8/8] error: Add a 'error: ' prefix to error_report()
2017-07-12 12:27 ` Alistair Francis
@ 2017-07-12 12:37 ` Max Reitz
2017-07-12 13:03 ` Markus Armbruster
1 sibling, 0 replies; 23+ messages in thread
From: Max Reitz @ 2017-07-12 12:37 UTC (permalink / raw)
To: Alistair Francis
Cc: qemu-devel@nongnu.org Developers, philippe, Markus Armbruster
[-- Attachment #1: Type: text/plain, Size: 1153 bytes --]
On 2017-07-12 14:27, Alistair Francis wrote:
> On Tue, Jul 11, 2017 at 7:44 PM, Max Reitz <mreitz@redhat.com> wrote:
>> On 2017-07-11 14:07, Alistair Francis wrote:
>>> As we don't regard error messages as a stable API the let's add a
>>> 'error: ' prefix to the original error_report() messages.
>>>
>>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>>> ---
>>>
>>> util/qemu-error.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>
>> This breaks quite a few qemu-iotests.
>
> Hmmm... Ok, I'll have to dig into that.
>
> I'm traveling at the moment so I won't get a chance to fix this. I'm
> going to send the next version of this series, but remove this patch
> and I can look at adding it again when I get back. There is already a
> few things that have been pointed out that need to be fixed after this
> patch set.
>
> Do people think we should add this prefix or leave the error messages
> as is? I haven't heard a definitive answer if people think this is the
> right path to take.
I don't really mind either way. But it probably is a good idea to
separate this one patch from this series.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v3 8/8] error: Add a 'error: ' prefix to error_report()
2017-07-12 12:27 ` Alistair Francis
2017-07-12 12:37 ` Max Reitz
@ 2017-07-12 13:03 ` Markus Armbruster
1 sibling, 0 replies; 23+ messages in thread
From: Markus Armbruster @ 2017-07-12 13:03 UTC (permalink / raw)
To: Alistair Francis; +Cc: Max Reitz, philippe, qemu-devel@nongnu.org Developers
Alistair Francis <alistair.francis@xilinx.com> writes:
> On Tue, Jul 11, 2017 at 7:44 PM, Max Reitz <mreitz@redhat.com> wrote:
>> On 2017-07-11 14:07, Alistair Francis wrote:
>>> As we don't regard error messages as a stable API the let's add a
>>> 'error: ' prefix to the original error_report() messages.
>>>
>>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>>> ---
>>>
>>> util/qemu-error.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>
>> This breaks quite a few qemu-iotests.
>
> Hmmm... Ok, I'll have to dig into that.
>
> I'm traveling at the moment so I won't get a chance to fix this. I'm
> going to send the next version of this series, but remove this patch
> and I can look at adding it again when I get back. There is already a
> few things that have been pointed out that need to be fixed after this
> patch set.
Separating this change from the rest of the series makes sense, because
it lets use get the bulk of your work in more quickly.
> Do people think we should add this prefix or leave the error messages
> as is? I haven't heard a definitive answer if people think this is the
> right path to take.
I'd leave them as is. More inertia than opposition. If you think a
prefix solves a problem people have, by all means post a patch. Please
explain in the commit message why the patch is useful. The current one
explains only why we can change error messages, not why we *want* to
change them.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v3 4/8] hw/i386: Improve some of the warning messages
2017-07-12 9:39 ` Markus Armbruster
@ 2017-07-12 19:45 ` Eduardo Habkost
2017-07-13 6:24 ` Markus Armbruster
0 siblings, 1 reply; 23+ messages in thread
From: Eduardo Habkost @ 2017-07-12 19:45 UTC (permalink / raw)
To: Markus Armbruster
Cc: Alistair Francis, qemu-devel, alistair23, philippe,
Michael S. Tsirkin, Igor Mammedov
On Wed, Jul 12, 2017 at 11:39:59AM +0200, Markus Armbruster wrote:
> Alistair Francis <alistair.francis@xilinx.com> writes:
>
> > Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> > Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
>
> You forgot to cc: Eduardo. Fixed.
>
> > ---
> >
> > hw/i386/acpi-build.c | 7 ++++---
> > hw/i386/pc.c | 9 ++++-----
> > hw/i386/pc_q35.c | 4 ++--
> > 3 files changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index 6b7bade183..f9efb6be41 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -2766,7 +2766,8 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
> > ACPI_BUILD_ALIGN_SIZE);
> > if (tables_blob->len > legacy_table_size) {
> > /* Should happen only with PCI bridges and -M pc-i440fx-2.0. */
> > - warn_report("migration may not work.");
> > + warn_report("ACPI tables are larger than legacy_table_size");
> > + warn_report("migration may not work");
>
> The user has no idea what legacy_table_size means, what its value might
> be, or what he can do to reduce it.
>
> Recommend
>
> warn_report("ACPI tables too large, migration may not work");
>
> If the user can do something to reduce the table size, printing suitable
> hints would be nice. Printing both tables_blob->len and
> legacy_table_size might also help then.
>
> > }
> > g_array_set_size(tables_blob, legacy_table_size);
> > } else {
> > @@ -2774,9 +2775,9 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
> > if (tables_blob->len > ACPI_BUILD_TABLE_SIZE / 2) {
> > /* As of QEMU 2.1, this fires with 160 VCPUs and 255 memory slots. */
> > warn_report("ACPI tables are larger than 64k.");
>
> The warning text hardcodes the value of ACPI_BUILD_TABLE_SIZE / 2. Not
> nice. Clean up while there?
>
> > - warn_report("migration may not work.");
> > + warn_report("migration may not work");
> > warn_report("please remove CPUs, NUMA nodes, "
> > - "memory slots or PCI bridges.");
> > + "memory slots or PCI bridges");
>
> Aha, here's what the user can do.
>
> What about:
>
> warn_report("ACPI tables are large, migration may not work");
> error_printf("Try removing CPUs, NUMA nodes, memory slots"
> " or PCI bridges.");
>
> If we want to show actual size and limit, then this might do instead:
>
> warn_report("ACPI table size %u exceeds %d bytes,"
> " migration may not work",
> tables_blob->len, ACPI_BUILD_TABLE_SIZE / 2);
> error_printf("Try removing CPUs, NUMA nodes, memory slots"
> " or PCI bridges.");
Yep, this suggestion is good for both cases: the check
(ACPI_BUILD_TABLE_SIZE / 2), and the check for legacy_table_size.
>
> > }
> > acpi_align_size(tables_blob, ACPI_BUILD_TABLE_SIZE);
> > }
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index 465e91cc5b..084ca796c2 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -383,8 +383,8 @@ ISADevice *pc_find_fdc0(void)
> > if (state.multiple) {
> > warn_report("multiple floppy disk controllers with "
> > "iobase=0x3f0 have been found");
> > - error_printf("the one being picked for CMOS setup might not reflect "
> > - "your intent\n");
> > + warn_report("the one being picked for CMOS setup might not reflect "
> > + "your intent");
>
> Please keep error_printf() here.
>
I think I suggested warn_report() here for consistency, because I
have seen other cases where multiple warn_report() calls were
used. We probably want to change those other cases like you
suggested above.
> > }
> >
> > return state.floppy;
> > @@ -2087,9 +2087,8 @@ static void pc_machine_set_max_ram_below_4g(Object *obj, Visitor *v,
> > }
> >
> > if (value < (1ULL << 20)) {
> > - warn_report("small max_ram_below_4g(%"PRIu64
> > - ") less than 1M. BIOS may not work..",
> > - value);
> > + warn_report("max_ram_below_4g (%" PRIu64 ") is less than 1M; "
> > + "BIOS may not work.", value);
>
> The user has no idea what max_ram_below_4g might be. Suggest:
>
> warn_report("Only %" PRIu64 " bytes of RAM below the 4GiB boundary,"
> "BIOS may not work with less than 1MiB");
Actually, the user probably knows what it is, because this setter
will be invoked only if "-M max-ram-below-4g=..." is used in the
command-line. We should fix the spelling to "max-ram-below-4g",
though.
>
> > }
> >
> > pcms->max_ram_below_4g = value;
> > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> > index 1653a47f0a..682c576cf1 100644
> > --- a/hw/i386/pc_q35.c
> > +++ b/hw/i386/pc_q35.c
> > @@ -101,8 +101,8 @@ static void pc_q35_init(MachineState *machine)
> > lowmem = pcms->max_ram_below_4g;
> > if (machine->ram_size - lowmem > lowmem &&
> > lowmem & ((1ULL << 30) - 1)) {
> > - warn_report("Large machine and max_ram_below_4g(%"PRIu64
> > - ") not a multiple of 1G; possible bad performance.",
> > + warn_report("Large machine and max_ram_below_4g (%"PRIu64") not a "
> > + "multiple of 1G; possible bad performance.",
>
> Space between string literal and PRIu64, please.
>
> The user has no idea what max_ram_below_4g might be, [...]
Same as above: the warning should appear only if the user set
"max-ram-below-4g" explicitly, so the user probably knows what it
is.
> [...] or what makes the
> machine "large".
True.
>
> > pcms->max_ram_below_4g);
> > }
> > }
--
Eduardo
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v3 4/8] hw/i386: Improve some of the warning messages
2017-07-12 19:45 ` Eduardo Habkost
@ 2017-07-13 6:24 ` Markus Armbruster
2017-07-13 7:10 ` Alistair Francis
0 siblings, 1 reply; 23+ messages in thread
From: Markus Armbruster @ 2017-07-13 6:24 UTC (permalink / raw)
To: Eduardo Habkost
Cc: philippe, Michael S. Tsirkin, qemu-devel, Alistair Francis,
Igor Mammedov, alistair23
Eduardo Habkost <ehabkost@redhat.com> writes:
> On Wed, Jul 12, 2017 at 11:39:59AM +0200, Markus Armbruster wrote:
>> Alistair Francis <alistair.francis@xilinx.com> writes:
>>
>> > Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>> > Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
>>
>> You forgot to cc: Eduardo. Fixed.
>>
>> > ---
>> >
>> > hw/i386/acpi-build.c | 7 ++++---
>> > hw/i386/pc.c | 9 ++++-----
>> > hw/i386/pc_q35.c | 4 ++--
>> > 3 files changed, 10 insertions(+), 10 deletions(-)
>> >
>> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> > index 6b7bade183..f9efb6be41 100644
>> > --- a/hw/i386/acpi-build.c
>> > +++ b/hw/i386/acpi-build.c
>> > @@ -2766,7 +2766,8 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
>> > ACPI_BUILD_ALIGN_SIZE);
>> > if (tables_blob->len > legacy_table_size) {
>> > /* Should happen only with PCI bridges and -M pc-i440fx-2.0. */
>> > - warn_report("migration may not work.");
>> > + warn_report("ACPI tables are larger than legacy_table_size");
>> > + warn_report("migration may not work");
>>
>> The user has no idea what legacy_table_size means, what its value might
>> be, or what he can do to reduce it.
>>
>> Recommend
>>
>> warn_report("ACPI tables too large, migration may not work");
>>
>> If the user can do something to reduce the table size, printing suitable
>> hints would be nice. Printing both tables_blob->len and
>> legacy_table_size might also help then.
>>
>> > }
>> > g_array_set_size(tables_blob, legacy_table_size);
>> > } else {
>> > @@ -2774,9 +2775,9 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
>> > if (tables_blob->len > ACPI_BUILD_TABLE_SIZE / 2) {
>> > /* As of QEMU 2.1, this fires with 160 VCPUs and 255 memory slots. */
>> > warn_report("ACPI tables are larger than 64k.");
>>
>> The warning text hardcodes the value of ACPI_BUILD_TABLE_SIZE / 2. Not
>> nice. Clean up while there?
>>
>> > - warn_report("migration may not work.");
>> > + warn_report("migration may not work");
>> > warn_report("please remove CPUs, NUMA nodes, "
>> > - "memory slots or PCI bridges.");
>> > + "memory slots or PCI bridges");
>>
>> Aha, here's what the user can do.
>>
>> What about:
>>
>> warn_report("ACPI tables are large, migration may not work");
>> error_printf("Try removing CPUs, NUMA nodes, memory slots"
>> " or PCI bridges.");
>>
>> If we want to show actual size and limit, then this might do instead:
>>
>> warn_report("ACPI table size %u exceeds %d bytes,"
>> " migration may not work",
>> tables_blob->len, ACPI_BUILD_TABLE_SIZE / 2);
>> error_printf("Try removing CPUs, NUMA nodes, memory slots"
>> " or PCI bridges.");
>
> Yep, this suggestion is good for both cases: the check
> (ACPI_BUILD_TABLE_SIZE / 2), and the check for legacy_table_size.
>
>>
>> > }
>> > acpi_align_size(tables_blob, ACPI_BUILD_TABLE_SIZE);
>> > }
>> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> > index 465e91cc5b..084ca796c2 100644
>> > --- a/hw/i386/pc.c
>> > +++ b/hw/i386/pc.c
>> > @@ -383,8 +383,8 @@ ISADevice *pc_find_fdc0(void)
>> > if (state.multiple) {
>> > warn_report("multiple floppy disk controllers with "
>> > "iobase=0x3f0 have been found");
>> > - error_printf("the one being picked for CMOS setup might not reflect "
>> > - "your intent\n");
>> > + warn_report("the one being picked for CMOS setup might not reflect "
>> > + "your intent");
>>
>> Please keep error_printf() here.
>>
>
> I think I suggested warn_report() here for consistency, because I
> have seen other cases where multiple warn_report() calls were
> used. We probably want to change those other cases like you
> suggested above.
>
>> > }
>> >
>> > return state.floppy;
>> > @@ -2087,9 +2087,8 @@ static void pc_machine_set_max_ram_below_4g(Object *obj, Visitor *v,
>> > }
>> >
>> > if (value < (1ULL << 20)) {
>> > - warn_report("small max_ram_below_4g(%"PRIu64
>> > - ") less than 1M. BIOS may not work..",
>> > - value);
>> > + warn_report("max_ram_below_4g (%" PRIu64 ") is less than 1M; "
>> > + "BIOS may not work.", value);
>>
>> The user has no idea what max_ram_below_4g might be. Suggest:
>>
>> warn_report("Only %" PRIu64 " bytes of RAM below the 4GiB boundary,"
>> "BIOS may not work with less than 1MiB");
>
> Actually, the user probably knows what it is, because this setter
> will be invoked only if "-M max-ram-below-4g=..." is used in the
> command-line. We should fix the spelling to "max-ram-below-4g",
> though.
>
>>
>> > }
>> >
>> > pcms->max_ram_below_4g = value;
>> > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
>> > index 1653a47f0a..682c576cf1 100644
>> > --- a/hw/i386/pc_q35.c
>> > +++ b/hw/i386/pc_q35.c
>> > @@ -101,8 +101,8 @@ static void pc_q35_init(MachineState *machine)
>> > lowmem = pcms->max_ram_below_4g;
>> > if (machine->ram_size - lowmem > lowmem &&
>> > lowmem & ((1ULL << 30) - 1)) {
>> > - warn_report("Large machine and max_ram_below_4g(%"PRIu64
>> > - ") not a multiple of 1G; possible bad performance.",
>> > + warn_report("Large machine and max_ram_below_4g (%"PRIu64") not a "
>> > + "multiple of 1G; possible bad performance.",
>>
>> Space between string literal and PRIu64, please.
>>
>> The user has no idea what max_ram_below_4g might be, [...]
>
> Same as above: the warning should appear only if the user set
> "max-ram-below-4g" explicitly, so the user probably knows what it
> is.
>
>> [...] or what makes the
>> machine "large".
>
> True.
>
>>
>> > pcms->max_ram_below_4g);
>> > }
>> > }
Alistair, I suggest I apply just the other six patches for now, and you
improve this patch without undue time pressure. What do you think?
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v3 4/8] hw/i386: Improve some of the warning messages
2017-07-13 6:24 ` Markus Armbruster
@ 2017-07-13 7:10 ` Alistair Francis
0 siblings, 0 replies; 23+ messages in thread
From: Alistair Francis @ 2017-07-13 7:10 UTC (permalink / raw)
To: Markus Armbruster
Cc: Eduardo Habkost, philippe, Michael S. Tsirkin,
qemu-devel@nongnu.org Developers, Alistair Francis,
Igor Mammedov
On Thu, Jul 13, 2017 at 8:24 AM, Markus Armbruster <armbru@redhat.com> wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
>
>> On Wed, Jul 12, 2017 at 11:39:59AM +0200, Markus Armbruster wrote:
>>> Alistair Francis <alistair.francis@xilinx.com> writes:
>>>
>>> > Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>>> > Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
>>>
>>> You forgot to cc: Eduardo. Fixed.
>>>
>>> > ---
>>> >
>>> > hw/i386/acpi-build.c | 7 ++++---
>>> > hw/i386/pc.c | 9 ++++-----
>>> > hw/i386/pc_q35.c | 4 ++--
>>> > 3 files changed, 10 insertions(+), 10 deletions(-)
>>> >
>>> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>>> > index 6b7bade183..f9efb6be41 100644
>>> > --- a/hw/i386/acpi-build.c
>>> > +++ b/hw/i386/acpi-build.c
>>> > @@ -2766,7 +2766,8 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
>>> > ACPI_BUILD_ALIGN_SIZE);
>>> > if (tables_blob->len > legacy_table_size) {
>>> > /* Should happen only with PCI bridges and -M pc-i440fx-2.0. */
>>> > - warn_report("migration may not work.");
>>> > + warn_report("ACPI tables are larger than legacy_table_size");
>>> > + warn_report("migration may not work");
>>>
>>> The user has no idea what legacy_table_size means, what its value might
>>> be, or what he can do to reduce it.
>>>
>>> Recommend
>>>
>>> warn_report("ACPI tables too large, migration may not work");
>>>
>>> If the user can do something to reduce the table size, printing suitable
>>> hints would be nice. Printing both tables_blob->len and
>>> legacy_table_size might also help then.
>>>
>>> > }
>>> > g_array_set_size(tables_blob, legacy_table_size);
>>> > } else {
>>> > @@ -2774,9 +2775,9 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
>>> > if (tables_blob->len > ACPI_BUILD_TABLE_SIZE / 2) {
>>> > /* As of QEMU 2.1, this fires with 160 VCPUs and 255 memory slots. */
>>> > warn_report("ACPI tables are larger than 64k.");
>>>
>>> The warning text hardcodes the value of ACPI_BUILD_TABLE_SIZE / 2. Not
>>> nice. Clean up while there?
>>>
>>> > - warn_report("migration may not work.");
>>> > + warn_report("migration may not work");
>>> > warn_report("please remove CPUs, NUMA nodes, "
>>> > - "memory slots or PCI bridges.");
>>> > + "memory slots or PCI bridges");
>>>
>>> Aha, here's what the user can do.
>>>
>>> What about:
>>>
>>> warn_report("ACPI tables are large, migration may not work");
>>> error_printf("Try removing CPUs, NUMA nodes, memory slots"
>>> " or PCI bridges.");
>>>
>>> If we want to show actual size and limit, then this might do instead:
>>>
>>> warn_report("ACPI table size %u exceeds %d bytes,"
>>> " migration may not work",
>>> tables_blob->len, ACPI_BUILD_TABLE_SIZE / 2);
>>> error_printf("Try removing CPUs, NUMA nodes, memory slots"
>>> " or PCI bridges.");
>>
>> Yep, this suggestion is good for both cases: the check
>> (ACPI_BUILD_TABLE_SIZE / 2), and the check for legacy_table_size.
>>
>>>
>>> > }
>>> > acpi_align_size(tables_blob, ACPI_BUILD_TABLE_SIZE);
>>> > }
>>> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>>> > index 465e91cc5b..084ca796c2 100644
>>> > --- a/hw/i386/pc.c
>>> > +++ b/hw/i386/pc.c
>>> > @@ -383,8 +383,8 @@ ISADevice *pc_find_fdc0(void)
>>> > if (state.multiple) {
>>> > warn_report("multiple floppy disk controllers with "
>>> > "iobase=0x3f0 have been found");
>>> > - error_printf("the one being picked for CMOS setup might not reflect "
>>> > - "your intent\n");
>>> > + warn_report("the one being picked for CMOS setup might not reflect "
>>> > + "your intent");
>>>
>>> Please keep error_printf() here.
>>>
>>
>> I think I suggested warn_report() here for consistency, because I
>> have seen other cases where multiple warn_report() calls were
>> used. We probably want to change those other cases like you
>> suggested above.
>>
>>> > }
>>> >
>>> > return state.floppy;
>>> > @@ -2087,9 +2087,8 @@ static void pc_machine_set_max_ram_below_4g(Object *obj, Visitor *v,
>>> > }
>>> >
>>> > if (value < (1ULL << 20)) {
>>> > - warn_report("small max_ram_below_4g(%"PRIu64
>>> > - ") less than 1M. BIOS may not work..",
>>> > - value);
>>> > + warn_report("max_ram_below_4g (%" PRIu64 ") is less than 1M; "
>>> > + "BIOS may not work.", value);
>>>
>>> The user has no idea what max_ram_below_4g might be. Suggest:
>>>
>>> warn_report("Only %" PRIu64 " bytes of RAM below the 4GiB boundary,"
>>> "BIOS may not work with less than 1MiB");
>>
>> Actually, the user probably knows what it is, because this setter
>> will be invoked only if "-M max-ram-below-4g=..." is used in the
>> command-line. We should fix the spelling to "max-ram-below-4g",
>> though.
>>
>>>
>>> > }
>>> >
>>> > pcms->max_ram_below_4g = value;
>>> > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
>>> > index 1653a47f0a..682c576cf1 100644
>>> > --- a/hw/i386/pc_q35.c
>>> > +++ b/hw/i386/pc_q35.c
>>> > @@ -101,8 +101,8 @@ static void pc_q35_init(MachineState *machine)
>>> > lowmem = pcms->max_ram_below_4g;
>>> > if (machine->ram_size - lowmem > lowmem &&
>>> > lowmem & ((1ULL << 30) - 1)) {
>>> > - warn_report("Large machine and max_ram_below_4g(%"PRIu64
>>> > - ") not a multiple of 1G; possible bad performance.",
>>> > + warn_report("Large machine and max_ram_below_4g (%"PRIu64") not a "
>>> > + "multiple of 1G; possible bad performance.",
>>>
>>> Space between string literal and PRIu64, please.
>>>
>>> The user has no idea what max_ram_below_4g might be, [...]
>>
>> Same as above: the warning should appear only if the user set
>> "max-ram-below-4g" explicitly, so the user probably knows what it
>> is.
>>
>>> [...] or what makes the
>>> machine "large".
>>
>> True.
>>
>>>
>>> > pcms->max_ram_below_4g);
>>> > }
>>> > }
>
> Alistair, I suggest I apply just the other six patches for now, and you
> improve this patch without undue time pressure. What do you think?
That sounds great! I can move this patch into a future cleanup series
(which has a growing amount of work) and work on it when I return. The
sooner this series is in the better, then people can start to use
warn_error() so we don't have more calls to convert.
Thanks,
Alistair
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2017-07-13 7:10 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-11 12:07 [Qemu-devel] [PATCH v3 0/8] Implement a warning_report function Alistair Francis
2017-07-11 12:07 ` [Qemu-devel] [PATCH v3 1/8] util/qemu-error: Rename error_print_loc() to be more generic Alistair Francis
2017-07-11 12:07 ` [Qemu-devel] [PATCH v3 2/8] error: Functions to report warnings and informational messages Alistair Francis
2017-07-12 7:57 ` Markus Armbruster
2017-07-12 10:48 ` Alistair Francis
2017-07-11 12:07 ` [Qemu-devel] [PATCH v3 3/8] Convert error_report() to warn_report() Alistair Francis
2017-07-11 16:51 ` Max Reitz
2017-07-12 8:34 ` Markus Armbruster
2017-07-12 10:46 ` Alistair Francis
2017-07-11 12:07 ` [Qemu-devel] [PATCH v3 4/8] hw/i386: Improve some of the warning messages Alistair Francis
2017-07-12 9:39 ` Markus Armbruster
2017-07-12 19:45 ` Eduardo Habkost
2017-07-13 6:24 ` Markus Armbruster
2017-07-13 7:10 ` Alistair Francis
2017-07-11 12:07 ` [Qemu-devel] [PATCH v3 5/8] char-socket: Report TCP socket waiting as information Alistair Francis
2017-07-11 12:07 ` [Qemu-devel] [PATCH v3 6/8] error: Implement the warn and free Error functions Alistair Francis
2017-07-11 12:07 ` [Qemu-devel] [PATCH v3 7/8] Convert error_report*_err() to warn_report*_err() Alistair Francis
2017-07-11 12:07 ` [Qemu-devel] [PATCH v3 8/8] error: Add a 'error: ' prefix to error_report() Alistair Francis
2017-07-11 17:44 ` Max Reitz
2017-07-12 12:27 ` Alistair Francis
2017-07-12 12:37 ` Max Reitz
2017-07-12 13:03 ` Markus Armbruster
2017-07-11 13:58 ` [Qemu-devel] [PATCH v3 0/8] Implement a warning_report function no-reply
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.