qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2 00/16] vl.c: Error message rework
@ 2015-10-28 18:36 Eduardo Habkost
  2015-10-28 18:36 ` [Qemu-devel] [PATCH v2 01/16] vl.c: Replace fprintf(stderr) with error_report() Eduardo Habkost
                   ` (15 more replies)
  0 siblings, 16 replies; 26+ messages in thread
From: Eduardo Habkost @ 2015-10-28 18:36 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Andrew Jones, Markus Armbruster

Extra patches for many suggestions I got when changing vl.c to use
error_report(). I tried to implement each suggestion in a separate patch to
make it easier to pick and choose the changes that are considered good enough.

Eduardo Habkost (16):
  vl.c: Replace fprintf(stderr) with error_report()
  vl.c: Use error_report() when reporting shutdown signal
  vl.c: Remove periods from error_report() error messages
  vl.c: Use "warning:" prefix consistently on warnings
  vl.c: Use "cannot" instead of "can not" in error messages
  vl.c: Use 'quotes' instead of `quotes' in messages
  vl.c: Use "%s support disabled" error messages consistently
  vl.c: Simplify "ignoring deprecated option" warnings
  vl.c: Reword -no-kvm-pit-reinjection deprecation warning
  vl.c: Convert error sentences to simpler phrases
  vl.c: Remove unnecessary uppercase in error messages
  vl.c: trivial: Don't wrap lines unnecessarily
  vl.c: Reword -machine help error messages
  vl.c: Simplify date format error message
  vl.c: Use US spelling for "unrecognized"
  vl.c: Reword fw_cfg name prefix warning

 vl.c | 268 ++++++++++++++++++++++++++++++++-----------------------------------
 1 file changed, 129 insertions(+), 139 deletions(-)

-- 
2.1.0

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v2 01/16] vl.c: Replace fprintf(stderr) with error_report()
  2015-10-28 18:36 [Qemu-devel] [PATCH v2 00/16] vl.c: Error message rework Eduardo Habkost
@ 2015-10-28 18:36 ` Eduardo Habkost
  2015-10-29 17:34   ` Markus Armbruster
  2015-10-28 18:36 ` [Qemu-devel] [PATCH v2 02/16] vl.c: Use error_report() when reporting shutdown signal Eduardo Habkost
                   ` (14 subsequent siblings)
  15 siblings, 1 reply; 26+ messages in thread
From: Eduardo Habkost @ 2015-10-28 18:36 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Andrew Jones, Markus Armbruster

Replace most fprintf(stderr) calls on vl.c with error_report().

Minimal changes were made in the error messages. Only the trailing
newlines, "qemu:" and "error:" message prefixes were removed.

The only remaining fprintf(stderr) calls are the ones at
qemu_kill_report(), because the error mesage is split in multiple
fprintf() calls.

Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v1 -> v2:
* Indentation fix at "No machine specified" error message
---
 vl.c | 228 +++++++++++++++++++++++++++++++++----------------------------------
 1 file changed, 112 insertions(+), 116 deletions(-)

diff --git a/vl.c b/vl.c
index f5f7c3f..25b91fc 100644
--- a/vl.c
+++ b/vl.c
@@ -674,9 +674,9 @@ void runstate_set(RunState new_state)
     assert(new_state < RUN_STATE_MAX);
 
     if (!runstate_valid_transitions[current_run_state][new_state]) {
-        fprintf(stderr, "ERROR: invalid runstate transition: '%s' -> '%s'\n",
-                RunState_lookup[current_run_state],
-                RunState_lookup[new_state]);
+        error_report("invalid runstate transition: '%s' -> '%s'",
+                     RunState_lookup[current_run_state],
+                     RunState_lookup[new_state]);
         abort();
     }
     trace_runstate_set(new_state);
@@ -828,8 +828,8 @@ static void configure_rtc_date_offset(const char *startdate, int legacy)
         rtc_start_date = mktimegm(&tm);
         if (rtc_start_date == -1) {
         date_fail:
-            fprintf(stderr, "Invalid date format. Valid formats are:\n"
-                            "'2006-06-17T16:01:21' or '2006-06-17'\n");
+            error_report("Invalid date format. Valid formats are:\n"
+                         "'2006-06-17T16:01:21' or '2006-06-17'");
             exit(1);
         }
         rtc_date_offset = qemu_time() - rtc_start_date;
@@ -859,7 +859,7 @@ static void configure_rtc(QemuOpts *opts)
         } else if (!strcmp(value, "vm")) {
             rtc_clock = QEMU_CLOCK_VIRTUAL;
         } else {
-            fprintf(stderr, "qemu: invalid option value '%s'\n", value);
+            error_report("invalid option value '%s'", value);
             exit(1);
         }
     }
@@ -879,7 +879,7 @@ static void configure_rtc(QemuOpts *opts)
         } else if (!strcmp(value, "none")) {
             /* discard is default */
         } else {
-            fprintf(stderr, "qemu: invalid option value '%s'\n", value);
+            error_report("invalid option value '%s'", value);
             exit(1);
         }
     }
@@ -905,7 +905,7 @@ static int bt_hci_parse(const char *str)
     bdaddr_t bdaddr;
 
     if (nb_hcis >= MAX_NICS) {
-        fprintf(stderr, "qemu: Too many bluetooth HCIs (max %i).\n", MAX_NICS);
+        error_report("Too many bluetooth HCIs (max %i).", MAX_NICS);
         return -1;
     }
 
@@ -931,8 +931,8 @@ static void bt_vhci_add(int vlan_id)
     struct bt_scatternet_s *vlan = qemu_find_bt_vlan(vlan_id);
 
     if (!vlan->slave)
-        fprintf(stderr, "qemu: warning: adding a VHCI to "
-                        "an empty scatternet %i\n", vlan_id);
+        error_report("warning: adding a VHCI to "
+                     "an empty scatternet %i", vlan_id);
 
     bt_vhci_init(bt_new_hci(vlan));
 }
@@ -950,7 +950,7 @@ static struct bt_device_s *bt_device_add(const char *opt)
     if (endp) {
         vlan_id = strtol(endp + 6, &endp, 0);
         if (*endp) {
-            fprintf(stderr, "qemu: unrecognised bluetooth vlan Id\n");
+            error_report("unrecognised bluetooth vlan Id");
             return 0;
         }
     }
@@ -958,13 +958,13 @@ static struct bt_device_s *bt_device_add(const char *opt)
     vlan = qemu_find_bt_vlan(vlan_id);
 
     if (!vlan->slave)
-        fprintf(stderr, "qemu: warning: adding a slave device to "
-                        "an empty scatternet %i\n", vlan_id);
+        error_report("warning: adding a slave device to "
+                     "an empty scatternet %i", vlan_id);
 
     if (!strcmp(devname, "keyboard"))
         return bt_keyboard_init(vlan);
 
-    fprintf(stderr, "qemu: unsupported bluetooth device `%s'\n", devname);
+    error_report("unsupported bluetooth device `%s'", devname);
     return 0;
 }
 
@@ -987,11 +987,11 @@ static int bt_parse(const char *opt)
                 if (strstart(endp, ",vlan=", &p)) {
                     vlan = strtol(p, (char **) &endp, 0);
                     if (*endp) {
-                        fprintf(stderr, "qemu: bad scatternet '%s'\n", p);
+                        error_report("bad scatternet '%s'", p);
                         return 1;
                     }
                 } else {
-                    fprintf(stderr, "qemu: bad parameter '%s'\n", endp + 1);
+                    error_report("bad parameter '%s'", endp + 1);
                     return 1;
                 }
             } else
@@ -1003,7 +1003,7 @@ static int bt_parse(const char *opt)
     } else if (strstart(opt, "device:", &endp))
         return !bt_device_add(endp);
 
-    fprintf(stderr, "qemu: bad bluetooth parameter '%s'\n", opt);
+    error_report("bad bluetooth parameter '%s'", opt);
     return 1;
 }
 
@@ -1220,18 +1220,19 @@ static void smp_parse(QemuOpts *opts)
         } else if (threads == 0) {
             threads = cpus / (cores * sockets);
         } else if (sockets * cores * threads < cpus) {
-            fprintf(stderr, "cpu topology: error: "
-                    "sockets (%u) * cores (%u) * threads (%u) < "
-                    "smp_cpus (%u)\n",
-                    sockets, cores, threads, cpus);
+            error_report("cpu topology: "
+                         "sockets (%u) * cores (%u) * threads (%u) < "
+                         "smp_cpus (%u)",
+                         sockets, cores, threads, cpus);
             exit(1);
         }
 
         max_cpus = qemu_opt_get_number(opts, "maxcpus", cpus);
         if (sockets * cores * threads > max_cpus) {
-            fprintf(stderr, "cpu topology: error: "
-                    "sockets (%u) * cores (%u) * threads (%u) > maxcpus (%u)\n",
-                    sockets, cores, threads, max_cpus);
+            error_report("cpu topology: "
+                         "sockets (%u) * cores (%u) * threads (%u) > "
+                         "maxcpus (%u)",
+                         sockets, cores, threads, max_cpus);
             exit(1);
         }
 
@@ -1246,11 +1247,11 @@ static void smp_parse(QemuOpts *opts)
     }
 
     if (max_cpus > MAX_CPUMASK_BITS) {
-        fprintf(stderr, "Unsupported number of maxcpus\n");
+        error_report("Unsupported number of maxcpus");
         exit(1);
     }
     if (max_cpus < smp_cpus) {
-        fprintf(stderr, "maxcpus must be equal to or greater than smp\n");
+        error_report("maxcpus must be equal to or greater than smp");
         exit(1);
     }
 
@@ -1260,7 +1261,7 @@ static void realtime_init(void)
 {
     if (enable_mlock) {
         if (os_mlock() < 0) {
-            fprintf(stderr, "qemu: locking memory failed\n");
+            error_report("locking memory failed");
             exit(1);
         }
     }
@@ -1414,7 +1415,7 @@ static int usb_parse(const char *cmdline)
     int r;
     r = usb_device_add(cmdline);
     if (r < 0) {
-        fprintf(stderr, "qemu: could not add USB device '%s'\n", cmdline);
+        error_report("could not add USB device '%s'", cmdline);
     }
     return r;
 }
@@ -1980,28 +1981,28 @@ static void select_vgahw (const char *p)
         if (vga_available()) {
             vga_interface_type = VGA_STD;
         } else {
-            fprintf(stderr, "Error: standard VGA not available\n");
+            error_report("standard VGA not available");
             exit(0);
         }
     } else if (strstart(p, "cirrus", &opts)) {
         if (cirrus_vga_available()) {
             vga_interface_type = VGA_CIRRUS;
         } else {
-            fprintf(stderr, "Error: Cirrus VGA not available\n");
+            error_report("Cirrus VGA not available");
             exit(0);
         }
     } else if (strstart(p, "vmware", &opts)) {
         if (vmware_vga_available()) {
             vga_interface_type = VGA_VMWARE;
         } else {
-            fprintf(stderr, "Error: VMWare SVGA not available\n");
+            error_report("VMWare SVGA not available");
             exit(0);
         }
     } else if (strstart(p, "virtio", &opts)) {
         if (virtio_vga_available()) {
             vga_interface_type = VGA_VIRTIO;
         } else {
-            fprintf(stderr, "Error: Virtio VGA not available\n");
+            error_report("Virtio VGA not available");
             exit(0);
         }
     } else if (strstart(p, "xenfb", &opts)) {
@@ -2010,26 +2011,26 @@ static void select_vgahw (const char *p)
         if (qxl_vga_available()) {
             vga_interface_type = VGA_QXL;
         } else {
-            fprintf(stderr, "Error: QXL VGA not available\n");
+            error_report("QXL VGA not available");
             exit(0);
         }
     } else if (strstart(p, "tcx", &opts)) {
         if (tcx_vga_available()) {
             vga_interface_type = VGA_TCX;
         } else {
-            fprintf(stderr, "Error: TCX framebuffer not available\n");
+            error_report("TCX framebuffer not available");
             exit(0);
         }
     } else if (strstart(p, "cg3", &opts)) {
         if (cg3_vga_available()) {
             vga_interface_type = VGA_CG3;
         } else {
-            fprintf(stderr, "Error: CG3 framebuffer not available\n");
+            error_report("CG3 framebuffer not available");
             exit(0);
         }
     } else if (!strstart(p, "none", &opts)) {
     invalid_vga:
-        fprintf(stderr, "Unknown vga type: %s\n", p);
+        error_report("Unknown vga type: %s", p);
         exit(1);
     }
     while (*opts) {
@@ -2349,7 +2350,7 @@ static int mon_init_func(void *opaque, QemuOpts *opts, Error **errp)
     } else if (strcmp(mode, "control") == 0) {
         flags = MONITOR_USE_CONTROL;
     } else {
-        fprintf(stderr, "unknown monitor mode \"%s\"\n", mode);
+        error_report("unknown monitor mode \"%s\"", mode);
         exit(1);
     }
 
@@ -2362,7 +2363,7 @@ static int mon_init_func(void *opaque, QemuOpts *opts, Error **errp)
     chardev = qemu_opt_get(opts, "chardev");
     chr = qemu_chr_find(chardev);
     if (chr == NULL) {
-        fprintf(stderr, "chardev \"%s\" not found\n", chardev);
+        error_report("chardev \"%s\" not found", chardev);
         exit(1);
     }
 
@@ -2390,7 +2391,7 @@ static void monitor_parse(const char *optarg, const char *mode, bool pretty)
         }
         opts = qemu_chr_parse_compat(label, optarg);
         if (!opts) {
-            fprintf(stderr, "parse error: %s\n", optarg);
+            error_report("parse error: %s", optarg);
             exit(1);
         }
     }
@@ -2464,14 +2465,14 @@ static int serial_parse(const char *devname)
     if (strcmp(devname, "none") == 0)
         return 0;
     if (index == MAX_SERIAL_PORTS) {
-        fprintf(stderr, "qemu: too many serial ports\n");
+        error_report("too many serial ports");
         exit(1);
     }
     snprintf(label, sizeof(label), "serial%d", index);
     serial_hds[index] = qemu_chr_new(label, devname, NULL);
     if (!serial_hds[index]) {
-        fprintf(stderr, "qemu: could not connect serial device"
-                " to character backend '%s'\n", devname);
+        error_report("could not connect serial device"
+                     " to character backend '%s'", devname);
         return -1;
     }
     index++;
@@ -2486,14 +2487,14 @@ static int parallel_parse(const char *devname)
     if (strcmp(devname, "none") == 0)
         return 0;
     if (index == MAX_PARALLEL_PORTS) {
-        fprintf(stderr, "qemu: too many parallel ports\n");
+        error_report("too many parallel ports");
         exit(1);
     }
     snprintf(label, sizeof(label), "parallel%d", index);
     parallel_hds[index] = qemu_chr_new(label, devname, NULL);
     if (!parallel_hds[index]) {
-        fprintf(stderr, "qemu: could not connect parallel device"
-                " to character backend '%s'\n", devname);
+        error_report("could not connect parallel device"
+                     " to character backend '%s'", devname);
         return -1;
     }
     index++;
@@ -2510,7 +2511,7 @@ static int virtcon_parse(const char *devname)
     if (strcmp(devname, "none") == 0)
         return 0;
     if (index == MAX_VIRTIO_CONSOLES) {
-        fprintf(stderr, "qemu: too many virtio consoles\n");
+        error_report("too many virtio consoles");
         exit(1);
     }
 
@@ -2527,8 +2528,8 @@ static int virtcon_parse(const char *devname)
     snprintf(label, sizeof(label), "virtcon%d", index);
     virtcon_hds[index] = qemu_chr_new(label, devname, NULL);
     if (!virtcon_hds[index]) {
-        fprintf(stderr, "qemu: could not connect virtio console"
-                " to character backend '%s'\n", devname);
+        error_report("could not connect virtio console"
+                     " to character backend '%s'", devname);
         return -1;
     }
     qemu_opt_set(dev_opts, "chardev", label, &error_abort);
@@ -2548,7 +2549,7 @@ static int sclp_parse(const char *devname)
         return 0;
     }
     if (index == MAX_SCLP_CONSOLES) {
-        fprintf(stderr, "qemu: too many sclp consoles\n");
+        error_report("too many sclp consoles");
         exit(1);
     }
 
@@ -2560,8 +2561,8 @@ static int sclp_parse(const char *devname)
     snprintf(label, sizeof(label), "sclpcon%d", index);
     sclp_hds[index] = qemu_chr_new(label, devname, NULL);
     if (!sclp_hds[index]) {
-        fprintf(stderr, "qemu: could not connect sclp console"
-                " to character backend '%s'\n", devname);
+        error_report("could not connect sclp console"
+                     " to character backend '%s'", devname);
         return -1;
     }
     qemu_opt_set(dev_opts, "chardev", label, &error_abort);
@@ -2579,7 +2580,7 @@ static int debugcon_parse(const char *devname)
     }
     opts = qemu_opts_create(qemu_find_opts("device"), "debugcon", 1, NULL);
     if (!opts) {
-        fprintf(stderr, "qemu: already have a debugcon device\n");
+        error_report("already have a debugcon device");
         exit(1);
     }
     qemu_opt_set(opts, "driver", "isa-debugcon", &error_abort);
@@ -3010,8 +3011,7 @@ int main(int argc, char **argv, char **envp)
     runstate_init();
 
     if (qcrypto_init(&err) < 0) {
-        fprintf(stderr, "Cannot initialize crypto: %s\n",
-                error_get_pretty(err));
+        error_report("Cannot initialize crypto: %s", error_get_pretty(err));
         exit(1);
     }
     rtc_clock = QEMU_CLOCK_HOST;
@@ -3169,7 +3169,7 @@ int main(int argc, char **argv, char **envp)
                         }
                     } else if (*p != '\0') {
                     chs_fail:
-                        fprintf(stderr, "qemu: invalid physical CHS format\n");
+                        error_report("invalid physical CHS format");
                         exit(1);
                     }
                     if (hda_opts != NULL) {
@@ -3212,7 +3212,7 @@ int main(int argc, char **argv, char **envp)
 #ifdef CONFIG_CURSES
                 display_type = DT_CURSES;
 #else
-                fprintf(stderr, "Curses support is disabled\n");
+                error_report("Curses support is disabled");
                 exit(1);
 #endif
                 break;
@@ -3223,8 +3223,7 @@ int main(int argc, char **argv, char **envp)
                 graphic_rotate = strtol(optarg, (char **) &optarg, 10);
                 if (graphic_rotate != 0 && graphic_rotate != 90 &&
                     graphic_rotate != 180 && graphic_rotate != 270) {
-                    fprintf(stderr,
-                        "qemu: only 90, 180, 270 deg rotation is available\n");
+                    error_report("only 90, 180, 270 deg rotation is available");
                     exit(1);
                 }
                 break;
@@ -3375,7 +3374,7 @@ int main(int argc, char **argv, char **envp)
                     w = strtol(p, (char **)&p, 10);
                     if (w <= 0) {
                     graphic_error:
-                        fprintf(stderr, "qemu: invalid resolution or depth\n");
+                        error_report("invalid resolution or depth");
                         exit(1);
                     }
                     if (*p != 'x')
@@ -3441,7 +3440,7 @@ int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_fsdev:
                 olist = qemu_find_opts("fsdev");
                 if (!olist) {
-                    fprintf(stderr, "fsdev is not supported by this qemu build.\n");
+                    error_report("fsdev is not supported by this qemu build.");
                     exit(1);
                 }
                 opts = qemu_opts_parse_noisily(olist, optarg, true);
@@ -3456,7 +3455,7 @@ int main(int argc, char **argv, char **envp)
 
                 olist = qemu_find_opts("virtfs");
                 if (!olist) {
-                    fprintf(stderr, "virtfs is not supported by this qemu build.\n");
+                    error_report("virtfs is not supported by this qemu build.");
                     exit(1);
                 }
                 opts = qemu_opts_parse_noisily(olist, optarg, true);
@@ -3466,15 +3465,15 @@ int main(int argc, char **argv, char **envp)
 
                 if (qemu_opt_get(opts, "fsdriver") == NULL ||
                     qemu_opt_get(opts, "mount_tag") == NULL) {
-                    fprintf(stderr, "Usage: -virtfs fsdriver,mount_tag=tag.\n");
+                    error_report("Usage: -virtfs fsdriver,mount_tag=tag.");
                     exit(1);
                 }
                 fsdev = qemu_opts_create(qemu_find_opts("fsdev"),
                                          qemu_opt_get(opts, "mount_tag"),
                                          1, NULL);
                 if (!fsdev) {
-                    fprintf(stderr, "duplicate fsdev id: %s\n",
-                            qemu_opt_get(opts, "mount_tag"));
+                    error_report("duplicate fsdev id: %s",
+                                 qemu_opt_get(opts, "mount_tag"));
                     exit(1);
                 }
 
@@ -3483,8 +3482,7 @@ int main(int argc, char **argv, char **envp)
 #ifdef CONFIG_SYNC_FILE_RANGE
                     qemu_opt_set(fsdev, "writeout", writeout, &error_abort);
 #else
-                    fprintf(stderr, "writeout=immediate not supported on "
-                            "this platform\n");
+                    error_report("writeout=immediate not supported on this platform");
                     exit(1);
 #endif
                 }
@@ -3523,7 +3521,7 @@ int main(int argc, char **argv, char **envp)
                 fsdev = qemu_opts_create(qemu_find_opts("fsdev"), "v_synth",
                                          1, NULL);
                 if (!fsdev) {
-                    fprintf(stderr, "duplicate option: %s\n", "virtfs_synth");
+                    error_report("duplicate option: %s", "virtfs_synth");
                     exit(1);
                 }
                 qemu_opt_set(fsdev, "fsdriver", "synth", &error_abort);
@@ -3544,15 +3542,14 @@ int main(int argc, char **argv, char **envp)
                 break;
             case QEMU_OPTION_watchdog:
                 if (watchdog) {
-                    fprintf(stderr,
-                            "qemu: only one watchdog option may be given\n");
+                    error_report("only one watchdog option may be given");
                     return 1;
                 }
                 watchdog = optarg;
                 break;
             case QEMU_OPTION_watchdog_action:
                 if (select_watchdog_action(optarg) == -1) {
-                    fprintf(stderr, "Unknown -watchdog-action parameter\n");
+                    error_report("Unknown -watchdog-action parameter");
                     exit(1);
                 }
                 break;
@@ -3596,7 +3593,7 @@ int main(int argc, char **argv, char **envp)
                 display_type = DT_SDL;
                 break;
 #else
-                fprintf(stderr, "SDL support is disabled\n");
+                error_report("SDL support is disabled");
                 exit(1);
 #endif
             case QEMU_OPTION_pidfile:
@@ -3658,8 +3655,8 @@ int main(int argc, char **argv, char **envp)
                 qemu_opts_parse_noisily(olist, "accel=tcg", false);
                 break;
             case QEMU_OPTION_no_kvm_pit: {
-                fprintf(stderr, "Warning: KVM PIT can no longer be disabled "
-                                "separately.\n");
+                error_report("Warning: KVM PIT can no longer be disabled "
+                             "separately.");
                 break;
             }
             case QEMU_OPTION_no_kvm_pit_reinjection: {
@@ -3672,8 +3669,8 @@ int main(int argc, char **argv, char **envp)
                     { /* end of list */ }
                 };
 
-                fprintf(stderr, "Warning: option deprecated, use "
-                        "lost_tick_policy property of kvm-pit instead.\n");
+                error_report("Warning: option deprecated, use "
+                             "lost_tick_policy property of kvm-pit instead.");
                 qdev_prop_register_global_list(kvm_pit_lost_tick_policy);
                 break;
             }
@@ -3708,7 +3705,7 @@ int main(int argc, char **argv, char **envp)
                     exit(1);
                 }
 #else
-                fprintf(stderr, "VNC support is disabled\n");
+                error_report("VNC support is disabled");
                 exit(1);
 #endif
                 break;
@@ -3721,7 +3718,7 @@ int main(int argc, char **argv, char **envp)
                 break;
             case QEMU_OPTION_balloon:
                 if (balloon_parse(optarg) < 0) {
-                    fprintf(stderr, "Unknown -balloon argument %s\n", optarg);
+                    error_report("Unknown -balloon argument %s", optarg);
                     exit(1);
                 }
                 break;
@@ -3736,15 +3733,15 @@ int main(int argc, char **argv, char **envp)
                 break;
             case QEMU_OPTION_uuid:
                 if(qemu_uuid_parse(optarg, qemu_uuid) < 0) {
-                    fprintf(stderr, "Fail to parse UUID string."
-                            " Wrong format.\n");
+                    error_report("Fail to parse UUID string."
+                                 " Wrong format.");
                     exit(1);
                 }
                 qemu_uuid_set = true;
                 break;
             case QEMU_OPTION_option_rom:
                 if (nb_option_roms >= MAX_OPTION_ROMS) {
-                    fprintf(stderr, "Too many option ROMs\n");
+                    error_report("Too many option ROMs");
                     exit(1);
                 }
                 opts = qemu_opts_parse_noisily(qemu_find_opts("option-rom"),
@@ -3756,7 +3753,7 @@ int main(int argc, char **argv, char **envp)
                 option_rom[nb_option_roms].bootindex =
                     qemu_opt_get_number(opts, "bootindex", -1);
                 if (!option_rom[nb_option_roms].name) {
-                    fprintf(stderr, "Option ROM file is not specified\n");
+                    error_report("Option ROM file is not specified");
                     exit(1);
                 }
                 nb_option_roms++;
@@ -3781,9 +3778,8 @@ int main(int argc, char **argv, char **envp)
                         } else  if (strcmp("auto", target) == 0) {
                             semihosting.target = SEMIHOSTING_TARGET_AUTO;
                         } else {
-                            fprintf(stderr, "Unsupported semihosting-config"
-                                    " %s\n",
-                                optarg);
+                            error_report("Unsupported semihosting-config %s",
+                                         optarg);
                             exit(1);
                         }
                     } else {
@@ -3793,14 +3789,14 @@ int main(int argc, char **argv, char **envp)
                     qemu_opt_foreach(opts, add_semihosting_arg,
                                      &semihosting, NULL);
                 } else {
-                    fprintf(stderr, "Unsupported semihosting-config %s\n",
-                            optarg);
+                    error_report("Unsupported semihosting-config %s",
+                                 optarg);
                     exit(1);
                 }
                 break;
             case QEMU_OPTION_tdf:
-                fprintf(stderr, "Warning: user space PIT time drift fix "
-                                "is no longer supported.\n");
+                error_report("Warning: user space PIT time drift fix "
+                             "is no longer supported.");
                 break;
             case QEMU_OPTION_name:
                 opts = qemu_opts_parse_noisily(qemu_find_opts("name"),
@@ -3811,7 +3807,7 @@ int main(int argc, char **argv, char **envp)
                 break;
             case QEMU_OPTION_prom_env:
                 if (nb_prom_envs >= MAX_PROM_ENVS) {
-                    fprintf(stderr, "Too many prom variables\n");
+                    error_report("Too many prom variables");
                     exit(1);
                 }
                 prom_envs[nb_prom_envs] = optarg;
@@ -3894,8 +3890,8 @@ int main(int argc, char **argv, char **envp)
                 {
                     int ret = qemu_read_config_file(optarg);
                     if (ret < 0) {
-                        fprintf(stderr, "read config %s: %s\n", optarg,
-                            strerror(-ret));
+                        error_report("read config %s: %s", optarg,
+                                     strerror(-ret));
                         exit(1);
                     }
                     break;
@@ -3903,7 +3899,7 @@ int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_spice:
                 olist = qemu_find_opts("spice");
                 if (!olist) {
-                    fprintf(stderr, "spice is not supported by this qemu build.\n");
+                    error_report("spice is not supported by this qemu build.");
                     exit(1);
                 }
                 opts = qemu_opts_parse_noisily(olist, optarg, false);
@@ -3920,7 +3916,7 @@ int main(int argc, char **argv, char **envp)
                     } else {
                         fp = fopen(optarg, "w");
                         if (fp == NULL) {
-                            fprintf(stderr, "open %s: %s\n", optarg, strerror(errno));
+                            error_report("open %s: %s", optarg, strerror(errno));
                             exit(1);
                         }
                     }
@@ -3981,13 +3977,13 @@ int main(int argc, char **argv, char **envp)
                 break;
             case QEMU_OPTION_dump_vmstate:
                 if (vmstate_dump_file) {
-                    fprintf(stderr, "qemu: only one '-dump-vmstate' "
-                            "option may be given\n");
+                    error_report("only one '-dump-vmstate' "
+                                 "option may be given");
                     exit(1);
                 }
                 vmstate_dump_file = fopen(optarg, "w");
                 if (vmstate_dump_file == NULL) {
-                    fprintf(stderr, "open %s: %s\n", optarg, strerror(errno));
+                    error_report("open %s: %s", optarg, strerror(errno));
                     exit(1);
                 }
                 break;
@@ -4004,8 +4000,8 @@ int main(int argc, char **argv, char **envp)
     }
 
     if (machine_class == NULL) {
-        fprintf(stderr, "No machine specified, and there is no default.\n"
-                "Use -machine help to list supported machines!\n");
+        error_report("No machine specified, and there is no default.\n"
+                     "Use -machine help to list supported machines!");
         exit(1);
     }
 
@@ -4106,9 +4102,9 @@ int main(int argc, char **argv, char **envp)
 
     machine_class->max_cpus = machine_class->max_cpus ?: 1; /* Default to UP */
     if (max_cpus > machine_class->max_cpus) {
-        fprintf(stderr, "Number of SMP CPUs requested (%d) exceeds max CPUs "
-                "supported by machine '%s' (%d)\n", max_cpus,
-                machine_class->name, machine_class->max_cpus);
+        error_report("Number of SMP CPUs requested (%d) exceeds max CPUs "
+                     "supported by machine '%s' (%d)", max_cpus,
+                     machine_class->name, machine_class->max_cpus);
         exit(1);
     }
 
@@ -4169,12 +4165,12 @@ int main(int argc, char **argv, char **envp)
         if (display_type == DT_NOGRAPHIC
             && (default_parallel || default_serial
                 || default_monitor || default_virtcon)) {
-            fprintf(stderr, "-nographic can not be used with -daemonize\n");
+            error_report("-nographic can not be used with -daemonize");
             exit(1);
         }
 #ifdef CONFIG_CURSES
         if (display_type == DT_CURSES) {
-            fprintf(stderr, "curses display can not be used with -daemonize\n");
+            error_report("curses display can not be used with -daemonize");
             exit(1);
         }
 #endif
@@ -4233,12 +4229,12 @@ int main(int argc, char **argv, char **envp)
     }
 
     if ((no_frame || alt_grab || ctrl_grab) && display_type != DT_SDL) {
-        fprintf(stderr, "-no-frame, -alt-grab and -ctrl-grab are only valid "
-                        "for SDL, ignoring option\n");
+        error_report("-no-frame, -alt-grab and -ctrl-grab are only valid "
+                     "for SDL, ignoring option");
     }
     if (no_quit && (display_type != DT_GTK && display_type != DT_SDL)) {
-        fprintf(stderr, "-no-quit is only valid for GTK and SDL, "
-                        "ignoring option\n");
+        error_report("-no-quit is only valid for GTK and SDL, "
+                     "ignoring option");
     }
 
 #if defined(CONFIG_GTK)
@@ -4253,9 +4249,9 @@ int main(int argc, char **argv, char **envp)
 #endif
     if (request_opengl == 1 && display_opengl == 0) {
 #if defined(CONFIG_OPENGL)
-        fprintf(stderr, "OpenGL is not supported by the display.\n");
+        error_report("OpenGL is not supported by the display.");
 #else
-        fprintf(stderr, "QEMU was built without opengl support.\n");
+        error_report("QEMU was built without opengl support.");
 #endif
         exit(1);
     }
@@ -4281,7 +4277,7 @@ int main(int argc, char **argv, char **envp)
 #endif
 
     if (pid_file && qemu_create_pidfile(pid_file) != 0) {
-        fprintf(stderr, "Could not acquire pid file: %s\n", strerror(errno));
+        error_report("Could not acquire pid file: %s", strerror(errno));
         exit(1);
     }
 
@@ -4352,17 +4348,17 @@ int main(int argc, char **argv, char **envp)
     linux_boot = (kernel_filename != NULL);
 
     if (!linux_boot && *kernel_cmdline != '\0') {
-        fprintf(stderr, "-append only allowed with -kernel option\n");
+        error_report("-append only allowed with -kernel option");
         exit(1);
     }
 
     if (!linux_boot && initrd_filename != NULL) {
-        fprintf(stderr, "-initrd only allowed with -kernel option\n");
+        error_report("-initrd only allowed with -kernel option");
         exit(1);
     }
 
     if (!linux_boot && qemu_opt_get(machine_opts, "dtb")) {
-        fprintf(stderr, "-dtb only allowed with -kernel option\n");
+        error_report("-dtb only allowed with -kernel option");
         exit(1);
     }
 
@@ -4381,7 +4377,7 @@ int main(int argc, char **argv, char **envp)
     cpu_ticks_init();
     if (icount_opts) {
         if (kvm_enabled() || xen_enabled()) {
-            fprintf(stderr, "-icount is not allowed with kvm or xen\n");
+            error_report("-icount is not allowed with kvm or xen");
             exit(1);
         }
         configure_icount(icount_opts, &error_abort);
@@ -4414,7 +4410,7 @@ int main(int argc, char **argv, char **envp)
     if (!xen_enabled()) {
         /* On 32-bit hosts, QEMU is limited by virtual address space */
         if (ram_size > (2047 << 20) && HOST_LONG_BITS == 32) {
-            fprintf(stderr, "qemu: at most 2047 MB RAM can be simulated\n");
+            error_report("at most 2047 MB RAM can be simulated");
             exit(1);
         }
     }
@@ -4601,7 +4597,7 @@ int main(int argc, char **argv, char **envp)
     qemu_run_machine_init_done_notifiers();
 
     if (rom_check_and_register_reset() != 0) {
-        fprintf(stderr, "rom check and register reset failed\n");
+        error_report("rom check and register reset failed");
         exit(1);
     }
 
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v2 02/16] vl.c: Use error_report() when reporting shutdown signal
  2015-10-28 18:36 [Qemu-devel] [PATCH v2 00/16] vl.c: Error message rework Eduardo Habkost
  2015-10-28 18:36 ` [Qemu-devel] [PATCH v2 01/16] vl.c: Replace fprintf(stderr) with error_report() Eduardo Habkost
@ 2015-10-28 18:36 ` Eduardo Habkost
  2015-10-28 18:36 ` [Qemu-devel] [PATCH v2 03/16] vl.c: Remove periods from error_report() error messages Eduardo Habkost
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 26+ messages in thread
From: Eduardo Habkost @ 2015-10-28 18:36 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Andrew Jones, Markus Armbruster

This usage of fprintf(stderr) can't be directly converted to
error_report() like the others, because a single error message is split
into multiple fprintf() calls. Make separate error_report() calls for
each case.

Suggested-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 vl.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/vl.c b/vl.c
index 25b91fc..71104ae 100644
--- a/vl.c
+++ b/vl.c
@@ -1625,14 +1625,14 @@ static int qemu_shutdown_requested(void)
 static void qemu_kill_report(void)
 {
     if (!qtest_driver() && shutdown_signal != -1) {
-        fprintf(stderr, "qemu: terminating on signal %d", shutdown_signal);
         if (shutdown_pid == 0) {
             /* This happens for eg ^C at the terminal, so it's worth
              * avoiding printing an odd message in that case.
              */
-            fputc('\n', stderr);
+            error_report("terminating on signal %d", shutdown_signal);
         } else {
-            fprintf(stderr, " from pid " FMT_pid "\n", shutdown_pid);
+            error_report("terminating on signal %d from pid " FMT_pid,
+                         shutdown_signal, shutdown_pid);
         }
         shutdown_signal = -1;
     }
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v2 03/16] vl.c: Remove periods from error_report() error messages
  2015-10-28 18:36 [Qemu-devel] [PATCH v2 00/16] vl.c: Error message rework Eduardo Habkost
  2015-10-28 18:36 ` [Qemu-devel] [PATCH v2 01/16] vl.c: Replace fprintf(stderr) with error_report() Eduardo Habkost
  2015-10-28 18:36 ` [Qemu-devel] [PATCH v2 02/16] vl.c: Use error_report() when reporting shutdown signal Eduardo Habkost
@ 2015-10-28 18:36 ` Eduardo Habkost
  2015-10-28 18:36 ` [Qemu-devel] [PATCH v2 04/16] vl.c: Use "warning:" prefix consistently on warnings Eduardo Habkost
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 26+ messages in thread
From: Eduardo Habkost @ 2015-10-28 18:36 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Andrew Jones, Markus Armbruster

Except for removing periods, no other changes were made to the error
messages (yet).

Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 vl.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/vl.c b/vl.c
index 71104ae..e744c75 100644
--- a/vl.c
+++ b/vl.c
@@ -905,7 +905,7 @@ static int bt_hci_parse(const char *str)
     bdaddr_t bdaddr;
 
     if (nb_hcis >= MAX_NICS) {
-        error_report("Too many bluetooth HCIs (max %i).", MAX_NICS);
+        error_report("Too many bluetooth HCIs (max %i)", MAX_NICS);
         return -1;
     }
 
@@ -3440,7 +3440,7 @@ int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_fsdev:
                 olist = qemu_find_opts("fsdev");
                 if (!olist) {
-                    error_report("fsdev is not supported by this qemu build.");
+                    error_report("fsdev is not supported by this qemu build");
                     exit(1);
                 }
                 opts = qemu_opts_parse_noisily(olist, optarg, true);
@@ -3455,7 +3455,7 @@ int main(int argc, char **argv, char **envp)
 
                 olist = qemu_find_opts("virtfs");
                 if (!olist) {
-                    error_report("virtfs is not supported by this qemu build.");
+                    error_report("virtfs is not supported by this qemu build");
                     exit(1);
                 }
                 opts = qemu_opts_parse_noisily(olist, optarg, true);
@@ -3465,7 +3465,7 @@ int main(int argc, char **argv, char **envp)
 
                 if (qemu_opt_get(opts, "fsdriver") == NULL ||
                     qemu_opt_get(opts, "mount_tag") == NULL) {
-                    error_report("Usage: -virtfs fsdriver,mount_tag=tag.");
+                    error_report("Usage: -virtfs fsdriver,mount_tag=tag");
                     exit(1);
                 }
                 fsdev = qemu_opts_create(qemu_find_opts("fsdev"),
@@ -3656,7 +3656,7 @@ int main(int argc, char **argv, char **envp)
                 break;
             case QEMU_OPTION_no_kvm_pit: {
                 error_report("Warning: KVM PIT can no longer be disabled "
-                             "separately.");
+                             "separately");
                 break;
             }
             case QEMU_OPTION_no_kvm_pit_reinjection: {
@@ -3670,7 +3670,7 @@ int main(int argc, char **argv, char **envp)
                 };
 
                 error_report("Warning: option deprecated, use "
-                             "lost_tick_policy property of kvm-pit instead.");
+                             "lost_tick_policy property of kvm-pit instead");
                 qdev_prop_register_global_list(kvm_pit_lost_tick_policy);
                 break;
             }
@@ -3734,7 +3734,7 @@ int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_uuid:
                 if(qemu_uuid_parse(optarg, qemu_uuid) < 0) {
                     error_report("Fail to parse UUID string."
-                                 " Wrong format.");
+                                 " Wrong format");
                     exit(1);
                 }
                 qemu_uuid_set = true;
@@ -3796,7 +3796,7 @@ int main(int argc, char **argv, char **envp)
                 break;
             case QEMU_OPTION_tdf:
                 error_report("Warning: user space PIT time drift fix "
-                             "is no longer supported.");
+                             "is no longer supported");
                 break;
             case QEMU_OPTION_name:
                 opts = qemu_opts_parse_noisily(qemu_find_opts("name"),
@@ -3899,7 +3899,7 @@ int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_spice:
                 olist = qemu_find_opts("spice");
                 if (!olist) {
-                    error_report("spice is not supported by this qemu build.");
+                    error_report("spice is not supported by this qemu build");
                     exit(1);
                 }
                 opts = qemu_opts_parse_noisily(olist, optarg, false);
@@ -4249,9 +4249,9 @@ int main(int argc, char **argv, char **envp)
 #endif
     if (request_opengl == 1 && display_opengl == 0) {
 #if defined(CONFIG_OPENGL)
-        error_report("OpenGL is not supported by the display.");
+        error_report("OpenGL is not supported by the display");
 #else
-        error_report("QEMU was built without opengl support.");
+        error_report("QEMU was built without opengl support");
 #endif
         exit(1);
     }
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v2 04/16] vl.c: Use "warning:" prefix consistently on warnings
  2015-10-28 18:36 [Qemu-devel] [PATCH v2 00/16] vl.c: Error message rework Eduardo Habkost
                   ` (2 preceding siblings ...)
  2015-10-28 18:36 ` [Qemu-devel] [PATCH v2 03/16] vl.c: Remove periods from error_report() error messages Eduardo Habkost
@ 2015-10-28 18:36 ` Eduardo Habkost
  2015-10-28 18:36 ` [Qemu-devel] [PATCH v2 05/16] vl.c: Use "cannot" instead of "can not" in error messages Eduardo Habkost
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 26+ messages in thread
From: Eduardo Habkost @ 2015-10-28 18:36 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Andrew Jones, Markus Armbruster

Suggested-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 vl.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/vl.c b/vl.c
index e744c75..24343f7 100644
--- a/vl.c
+++ b/vl.c
@@ -2277,7 +2277,7 @@ 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 "
+        error_report("warning: externally provided fw_cfg item names "
                      "should be prefixed with \"opt/\"!");
     }
     if (nonempty_str(str)) {
@@ -3655,7 +3655,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: KVM PIT can no longer be disabled "
+                error_report("warning: KVM PIT can no longer be disabled "
                              "separately");
                 break;
             }
@@ -3669,7 +3669,7 @@ int main(int argc, char **argv, char **envp)
                     { /* end of list */ }
                 };
 
-                error_report("Warning: option deprecated, use "
+                error_report("warning: option deprecated, use "
                              "lost_tick_policy property of kvm-pit instead");
                 qdev_prop_register_global_list(kvm_pit_lost_tick_policy);
                 break;
@@ -3795,7 +3795,7 @@ int main(int argc, char **argv, char **envp)
                 }
                 break;
             case QEMU_OPTION_tdf:
-                error_report("Warning: user space PIT time drift fix "
+                error_report("warning: user space PIT time drift fix "
                              "is no longer supported");
                 break;
             case QEMU_OPTION_name:
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v2 05/16] vl.c: Use "cannot" instead of "can not" in error messages
  2015-10-28 18:36 [Qemu-devel] [PATCH v2 00/16] vl.c: Error message rework Eduardo Habkost
                   ` (3 preceding siblings ...)
  2015-10-28 18:36 ` [Qemu-devel] [PATCH v2 04/16] vl.c: Use "warning:" prefix consistently on warnings Eduardo Habkost
@ 2015-10-28 18:36 ` Eduardo Habkost
  2015-10-28 18:36 ` [Qemu-devel] [PATCH v2 06/16] vl.c: Use 'quotes' instead of `quotes' in messages Eduardo Habkost
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 26+ messages in thread
From: Eduardo Habkost @ 2015-10-28 18:36 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Andrew Jones, Markus Armbruster

Suggested-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 vl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/vl.c b/vl.c
index 24343f7..b68b891 100644
--- a/vl.c
+++ b/vl.c
@@ -4165,12 +4165,12 @@ int main(int argc, char **argv, char **envp)
         if (display_type == DT_NOGRAPHIC
             && (default_parallel || default_serial
                 || default_monitor || default_virtcon)) {
-            error_report("-nographic can not be used with -daemonize");
+            error_report("-nographic cannot be used with -daemonize");
             exit(1);
         }
 #ifdef CONFIG_CURSES
         if (display_type == DT_CURSES) {
-            error_report("curses display can not be used with -daemonize");
+            error_report("curses display cannot be used with -daemonize");
             exit(1);
         }
 #endif
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v2 06/16] vl.c: Use 'quotes' instead of `quotes' in messages
  2015-10-28 18:36 [Qemu-devel] [PATCH v2 00/16] vl.c: Error message rework Eduardo Habkost
                   ` (4 preceding siblings ...)
  2015-10-28 18:36 ` [Qemu-devel] [PATCH v2 05/16] vl.c: Use "cannot" instead of "can not" in error messages Eduardo Habkost
@ 2015-10-28 18:36 ` Eduardo Habkost
  2015-10-28 18:36 ` [Qemu-devel] [PATCH v2 07/16] vl.c: Use "%s support disabled" error messages consistently Eduardo Habkost
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 26+ messages in thread
From: Eduardo Habkost @ 2015-10-28 18:36 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Andrew Jones, Markus Armbruster

Suggested-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 vl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/vl.c b/vl.c
index b68b891..d417dd9 100644
--- a/vl.c
+++ b/vl.c
@@ -964,7 +964,7 @@ static struct bt_device_s *bt_device_add(const char *opt)
     if (!strcmp(devname, "keyboard"))
         return bt_keyboard_init(vlan);
 
-    error_report("unsupported bluetooth device `%s'", devname);
+    error_report("unsupported bluetooth device '%s'", devname);
     return 0;
 }
 
@@ -4575,7 +4575,7 @@ int main(int argc, char **argv, char **envp)
                       vnc_init_func, NULL, NULL);
     if (show_vnc_port) {
         char *ret = vnc_display_local_addr("default");
-        printf("VNC server running on `%s'\n", ret);
+        printf("VNC server running on '%s'\n", ret);
         g_free(ret);
     }
 #endif
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v2 07/16] vl.c: Use "%s support disabled" error messages consistently
  2015-10-28 18:36 [Qemu-devel] [PATCH v2 00/16] vl.c: Error message rework Eduardo Habkost
                   ` (5 preceding siblings ...)
  2015-10-28 18:36 ` [Qemu-devel] [PATCH v2 06/16] vl.c: Use 'quotes' instead of `quotes' in messages Eduardo Habkost
@ 2015-10-28 18:36 ` Eduardo Habkost
  2015-10-29 16:59   ` Markus Armbruster
  2015-10-28 18:36 ` [Qemu-devel] [PATCH v2 08/16] vl.c: Simplify "ignoring deprecated option" warnings Eduardo Habkost
                   ` (8 subsequent siblings)
  15 siblings, 1 reply; 26+ messages in thread
From: Eduardo Habkost @ 2015-10-28 18:36 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Andrew Jones, Markus Armbruster

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 vl.c | 34 ++++++++++++++++------------------
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/vl.c b/vl.c
index d417dd9..f37e3a9 100644
--- a/vl.c
+++ b/vl.c
@@ -1018,8 +1018,7 @@ static int parse_sandbox(void *opaque, QemuOpts *opts, Error **errp)
             return -1;
         }
 #else
-        error_report("sandboxing request but seccomp is not compiled "
-                     "into this build");
+        error_report("seccomp support disabled");
         return -1;
 #endif
     }
@@ -2112,7 +2111,7 @@ static DisplayType select_display(const char *p)
             opts = nextopt;
         }
 #else
-        error_report("SDL support is disabled");
+        error_report("SDL support disabled");
         exit(1);
 #endif
     } else if (strstart(p, "vnc", &opts)) {
@@ -2128,14 +2127,14 @@ static DisplayType select_display(const char *p)
             exit(1);
         }
 #else
-        error_report("VNC support is disabled");
+        error_report("VNC support disabled");
         exit(1);
 #endif
     } else if (strstart(p, "curses", &opts)) {
 #ifdef CONFIG_CURSES
         display = DT_CURSES;
 #else
-        error_report("Curses support is disabled");
+        error_report("curses support disabled");
         exit(1);
 #endif
     } else if (strstart(p, "gtk", &opts)) {
@@ -2170,7 +2169,7 @@ static DisplayType select_display(const char *p)
             opts = nextopt;
         }
 #else
-        error_report("GTK support is disabled");
+        error_report("GTK support disabled");
         exit(1);
 #endif
     } else if (strstart(p, "none", &opts)) {
@@ -2635,7 +2634,7 @@ static gint machine_class_cmp(gconstpointer a, gconstpointer b)
         return mc;
     }
     if (name && !is_help_option(name)) {
-        error_report("Unsupported machine type");
+        error_report("unsupported machine type");
         error_printf("Use -machine help to list supported machines!\n");
     } else {
         printf("Supported machines are:\n");
@@ -3011,7 +3010,7 @@ int main(int argc, char **argv, char **envp)
     runstate_init();
 
     if (qcrypto_init(&err) < 0) {
-        error_report("Cannot initialize crypto: %s", error_get_pretty(err));
+        error_report("cannot initialize crypto: %s", error_get_pretty(err));
         exit(1);
     }
     rtc_clock = QEMU_CLOCK_HOST;
@@ -3212,7 +3211,7 @@ int main(int argc, char **argv, char **envp)
 #ifdef CONFIG_CURSES
                 display_type = DT_CURSES;
 #else
-                error_report("Curses support is disabled");
+                error_report("curses support disabled");
                 exit(1);
 #endif
                 break;
@@ -3440,7 +3439,7 @@ int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_fsdev:
                 olist = qemu_find_opts("fsdev");
                 if (!olist) {
-                    error_report("fsdev is not supported by this qemu build");
+                    error_report("fsdev support disabled");
                     exit(1);
                 }
                 opts = qemu_opts_parse_noisily(olist, optarg, true);
@@ -3455,7 +3454,7 @@ int main(int argc, char **argv, char **envp)
 
                 olist = qemu_find_opts("virtfs");
                 if (!olist) {
-                    error_report("virtfs is not supported by this qemu build");
+                    error_report("virtfs support disabled");
                     exit(1);
                 }
                 opts = qemu_opts_parse_noisily(olist, optarg, true);
@@ -3593,7 +3592,7 @@ int main(int argc, char **argv, char **envp)
                 display_type = DT_SDL;
                 break;
 #else
-                error_report("SDL support is disabled");
+                error_report("SDL support disabled");
                 exit(1);
 #endif
             case QEMU_OPTION_pidfile:
@@ -3705,7 +3704,7 @@ int main(int argc, char **argv, char **envp)
                     exit(1);
                 }
 #else
-                error_report("VNC support is disabled");
+                error_report("VNC support disabled");
                 exit(1);
 #endif
                 break;
@@ -3899,7 +3898,7 @@ int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_spice:
                 olist = qemu_find_opts("spice");
                 if (!olist) {
-                    error_report("spice is not supported by this qemu build");
+                    error_report("spice support disabled");
                     exit(1);
                 }
                 opts = qemu_opts_parse_noisily(olist, optarg, false);
@@ -3947,8 +3946,7 @@ int main(int argc, char **argv, char **envp)
                     exit(1);
                 }
 #else
-                error_report("File descriptor passing is disabled on this "
-                             "platform");
+                error_report("fd passing disabled on this platform");
                 exit(1);
 #endif
                 break;
@@ -4249,9 +4247,9 @@ int main(int argc, char **argv, char **envp)
 #endif
     if (request_opengl == 1 && display_opengl == 0) {
 #if defined(CONFIG_OPENGL)
-        error_report("OpenGL is not supported by the display");
+        error_report("OpenGL not supported by the display");
 #else
-        error_report("QEMU was built without opengl support");
+        error_report("OpenGL support disabled");
 #endif
         exit(1);
     }
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v2 08/16] vl.c: Simplify "ignoring deprecated option" warnings
  2015-10-28 18:36 [Qemu-devel] [PATCH v2 00/16] vl.c: Error message rework Eduardo Habkost
                   ` (6 preceding siblings ...)
  2015-10-28 18:36 ` [Qemu-devel] [PATCH v2 07/16] vl.c: Use "%s support disabled" error messages consistently Eduardo Habkost
@ 2015-10-28 18:36 ` Eduardo Habkost
  2015-10-28 18:36 ` [Qemu-devel] [PATCH v2 09/16] vl.c: Reword -no-kvm-pit-reinjection deprecation warning Eduardo Habkost
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 26+ messages in thread
From: Eduardo Habkost @ 2015-10-28 18:36 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Andrew Jones, Markus Armbruster

Simplify warnings about deprecated options by rewriting them as
"warning: ignoring deprecated option -<option>".

Suggested-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 vl.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/vl.c b/vl.c
index f37e3a9..af8d09c 100644
--- a/vl.c
+++ b/vl.c
@@ -3654,8 +3654,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: KVM PIT can no longer be disabled "
-                             "separately");
+                error_report("warning: ignoring deprecated option -no-kvm-pit");
                 break;
             }
             case QEMU_OPTION_no_kvm_pit_reinjection: {
@@ -3794,8 +3793,7 @@ int main(int argc, char **argv, char **envp)
                 }
                 break;
             case QEMU_OPTION_tdf:
-                error_report("warning: user space PIT time drift fix "
-                             "is no longer supported");
+                error_report("warning: ignoring deprecated option -tdf");
                 break;
             case QEMU_OPTION_name:
                 opts = qemu_opts_parse_noisily(qemu_find_opts("name"),
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v2 09/16] vl.c: Reword -no-kvm-pit-reinjection deprecation warning
  2015-10-28 18:36 [Qemu-devel] [PATCH v2 00/16] vl.c: Error message rework Eduardo Habkost
                   ` (7 preceding siblings ...)
  2015-10-28 18:36 ` [Qemu-devel] [PATCH v2 08/16] vl.c: Simplify "ignoring deprecated option" warnings Eduardo Habkost
@ 2015-10-28 18:36 ` Eduardo Habkost
  2015-10-29 17:10   ` Markus Armbruster
  2015-10-28 18:36 ` [Qemu-devel] [PATCH v2 10/16] vl.c: Convert error sentences to simpler phrases Eduardo Habkost
                   ` (6 subsequent siblings)
  15 siblings, 1 reply; 26+ messages in thread
From: Eduardo Habkost @ 2015-10-28 18:36 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Andrew Jones, Markus Armbruster

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 vl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/vl.c b/vl.c
index af8d09c..67147e0 100644
--- a/vl.c
+++ b/vl.c
@@ -3667,8 +3667,8 @@ int main(int argc, char **argv, char **envp)
                     { /* end of list */ }
                 };
 
-                error_report("warning: option deprecated, use "
-                             "lost_tick_policy property of kvm-pit instead");
+                error_report("warning: -no-kvm-pit-reinjection deprecated by "
+                             "-global kvm-pit.lost_tick_policy=discard");
                 qdev_prop_register_global_list(kvm_pit_lost_tick_policy);
                 break;
             }
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v2 10/16] vl.c: Convert error sentences to simpler phrases
  2015-10-28 18:36 [Qemu-devel] [PATCH v2 00/16] vl.c: Error message rework Eduardo Habkost
                   ` (8 preceding siblings ...)
  2015-10-28 18:36 ` [Qemu-devel] [PATCH v2 09/16] vl.c: Reword -no-kvm-pit-reinjection deprecation warning Eduardo Habkost
@ 2015-10-28 18:36 ` Eduardo Habkost
  2015-10-29 17:25   ` Markus Armbruster
  2015-10-28 18:37 ` [Qemu-devel] [PATCH v2 11/16] vl.c: Remove unnecessary uppercase in error messages Eduardo Habkost
                   ` (5 subsequent siblings)
  15 siblings, 1 reply; 26+ messages in thread
From: Eduardo Habkost @ 2015-10-28 18:36 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Andrew Jones, Markus Armbruster

Simplify some error messages by making them simple phrases instead of
full sentences.

Suggested-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 vl.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/vl.c b/vl.c
index 67147e0..b8c6c3c 100644
--- a/vl.c
+++ b/vl.c
@@ -1066,7 +1066,7 @@ static int parse_add_fd(void *opaque, QemuOpts *opts, Error **errp)
     fd_opaque = qemu_opt_get(opts, "opaque");
 
     if (fd < 0) {
-        error_report("fd option is required and must be non-negative");
+        error_report("non-negative 'fd' option required");
         return -1;
     }
 
@@ -1081,12 +1081,12 @@ static int parse_add_fd(void *opaque, QemuOpts *opts, Error **errp)
      */
     flags = fcntl(fd, F_GETFD);
     if (flags == -1 || (flags & FD_CLOEXEC)) {
-        error_report("fd is not valid or already in use");
+        error_report("fd not valid or already in use");
         return -1;
     }
 
     if (fdset_id < 0) {
-        error_report("set option is required and must be non-negative");
+        error_report("non-negative 'set' option required");
         return -1;
     }
 
@@ -3751,7 +3751,7 @@ int main(int argc, char **argv, char **envp)
                 option_rom[nb_option_roms].bootindex =
                     qemu_opt_get_number(opts, "bootindex", -1);
                 if (!option_rom[nb_option_roms].name) {
-                    error_report("Option ROM file is not specified");
+                    error_report("option ROM file not specified");
                     exit(1);
                 }
                 nb_option_roms++;
@@ -4225,11 +4225,11 @@ int main(int argc, char **argv, char **envp)
     }
 
     if ((no_frame || alt_grab || ctrl_grab) && display_type != DT_SDL) {
-        error_report("-no-frame, -alt-grab and -ctrl-grab are only valid "
+        error_report("-no-frame, -alt-grab and -ctrl-grab only valid "
                      "for SDL, ignoring option");
     }
     if (no_quit && (display_type != DT_GTK && display_type != DT_SDL)) {
-        error_report("-no-quit is only valid for GTK and SDL, "
+        error_report("-no-quit only valid for GTK and SDL, "
                      "ignoring option");
     }
 
@@ -4373,7 +4373,7 @@ int main(int argc, char **argv, char **envp)
     cpu_ticks_init();
     if (icount_opts) {
         if (kvm_enabled() || xen_enabled()) {
-            error_report("-icount is not allowed with kvm or xen");
+            error_report("-icount not allowed with kvm or xen");
             exit(1);
         }
         configure_icount(icount_opts, &error_abort);
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v2 11/16] vl.c: Remove unnecessary uppercase in error messages
  2015-10-28 18:36 [Qemu-devel] [PATCH v2 00/16] vl.c: Error message rework Eduardo Habkost
                   ` (9 preceding siblings ...)
  2015-10-28 18:36 ` [Qemu-devel] [PATCH v2 10/16] vl.c: Convert error sentences to simpler phrases Eduardo Habkost
@ 2015-10-28 18:37 ` Eduardo Habkost
  2015-10-28 18:37 ` [Qemu-devel] [PATCH v2 12/16] vl.c: trivial: Don't wrap lines unnecessarily Eduardo Habkost
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 26+ messages in thread
From: Eduardo Habkost @ 2015-10-28 18:37 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Andrew Jones, Markus Armbruster

Suggested-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 vl.c | 32 +++++++++++++++-----------------
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/vl.c b/vl.c
index b8c6c3c..cd76ff4 100644
--- a/vl.c
+++ b/vl.c
@@ -905,7 +905,7 @@ static int bt_hci_parse(const char *str)
     bdaddr_t bdaddr;
 
     if (nb_hcis >= MAX_NICS) {
-        error_report("Too many bluetooth HCIs (max %i)", MAX_NICS);
+        error_report("too many bluetooth HCIs (max %i)", MAX_NICS);
         return -1;
     }
 
@@ -1099,7 +1099,7 @@ static int parse_add_fd(void *opaque, QemuOpts *opts, Error **errp)
     }
 #endif
     if (dupfd == -1) {
-        error_report("Error duplicating fd: %s", strerror(errno));
+        error_report("error duplicating fd: %s", strerror(errno));
         return -1;
     }
 
@@ -1246,7 +1246,7 @@ static void smp_parse(QemuOpts *opts)
     }
 
     if (max_cpus > MAX_CPUMASK_BITS) {
-        error_report("Unsupported number of maxcpus");
+        error_report("unsupported number of maxcpus");
         exit(1);
     }
     if (max_cpus < smp_cpus) {
@@ -2029,7 +2029,7 @@ static void select_vgahw (const char *p)
         }
     } else if (!strstart(p, "none", &opts)) {
     invalid_vga:
-        error_report("Unknown vga type: %s", p);
+        error_report("unknown vga type: %s", p);
         exit(1);
     }
     while (*opts) {
@@ -2105,7 +2105,7 @@ static DisplayType select_display(const char *p)
                 }
             } else {
             invalid_sdl_args:
-                error_report("Invalid SDL option string");
+                error_report("invalid SDL option string");
                 exit(1);
             }
             opts = nextopt;
@@ -2163,7 +2163,7 @@ static DisplayType select_display(const char *p)
                 }
             } else {
             invalid_gtk_args:
-                error_report("Invalid GTK option string");
+                error_report("invalid GTK option string");
                 exit(1);
             }
             opts = nextopt;
@@ -2175,7 +2175,7 @@ static DisplayType select_display(const char *p)
     } else if (strstart(p, "none", &opts)) {
         display = DT_NONE;
     } else {
-        error_report("Unknown display type");
+        error_report("unknown display type");
         exit(1);
     }
 
@@ -3548,7 +3548,7 @@ int main(int argc, char **argv, char **envp)
                 break;
             case QEMU_OPTION_watchdog_action:
                 if (select_watchdog_action(optarg) == -1) {
-                    error_report("Unknown -watchdog-action parameter");
+                    error_report("unknown -watchdog-action parameter");
                     exit(1);
                 }
                 break;
@@ -3716,7 +3716,7 @@ int main(int argc, char **argv, char **envp)
                 break;
             case QEMU_OPTION_balloon:
                 if (balloon_parse(optarg) < 0) {
-                    error_report("Unknown -balloon argument %s", optarg);
+                    error_report("unknown -balloon argument %s", optarg);
                     exit(1);
                 }
                 break;
@@ -3731,15 +3731,14 @@ int main(int argc, char **argv, char **envp)
                 break;
             case QEMU_OPTION_uuid:
                 if(qemu_uuid_parse(optarg, qemu_uuid) < 0) {
-                    error_report("Fail to parse UUID string."
-                                 " Wrong format");
+                    error_report("failed to parse UUID string: wrong format");
                     exit(1);
                 }
                 qemu_uuid_set = true;
                 break;
             case QEMU_OPTION_option_rom:
                 if (nb_option_roms >= MAX_OPTION_ROMS) {
-                    error_report("Too many option ROMs");
+                    error_report("too many option ROMs");
                     exit(1);
                 }
                 opts = qemu_opts_parse_noisily(qemu_find_opts("option-rom"),
@@ -3776,7 +3775,7 @@ int main(int argc, char **argv, char **envp)
                         } else  if (strcmp("auto", target) == 0) {
                             semihosting.target = SEMIHOSTING_TARGET_AUTO;
                         } else {
-                            error_report("Unsupported semihosting-config %s",
+                            error_report("unsupported semihosting-config %s",
                                          optarg);
                             exit(1);
                         }
@@ -3787,8 +3786,7 @@ int main(int argc, char **argv, char **envp)
                     qemu_opt_foreach(opts, add_semihosting_arg,
                                      &semihosting, NULL);
                 } else {
-                    error_report("Unsupported semihosting-config %s",
-                                 optarg);
+                    error_report("unsupported semihosting-config %s", optarg);
                     exit(1);
                 }
                 break;
@@ -3804,7 +3802,7 @@ int main(int argc, char **argv, char **envp)
                 break;
             case QEMU_OPTION_prom_env:
                 if (nb_prom_envs >= MAX_PROM_ENVS) {
-                    error_report("Too many prom variables");
+                    error_report("too many prom variables");
                     exit(1);
                 }
                 prom_envs[nb_prom_envs] = optarg;
@@ -4273,7 +4271,7 @@ int main(int argc, char **argv, char **envp)
 #endif
 
     if (pid_file && qemu_create_pidfile(pid_file) != 0) {
-        error_report("Could not acquire pid file: %s", strerror(errno));
+        error_report("could not acquire pid file: %s", strerror(errno));
         exit(1);
     }
 
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v2 12/16] vl.c: trivial: Don't wrap lines unnecessarily
  2015-10-28 18:36 [Qemu-devel] [PATCH v2 00/16] vl.c: Error message rework Eduardo Habkost
                   ` (10 preceding siblings ...)
  2015-10-28 18:37 ` [Qemu-devel] [PATCH v2 11/16] vl.c: Remove unnecessary uppercase in error messages Eduardo Habkost
@ 2015-10-28 18:37 ` Eduardo Habkost
  2015-10-28 18:37 ` [Qemu-devel] [PATCH v2 13/16] vl.c: Reword -machine help error messages Eduardo Habkost
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 26+ messages in thread
From: Eduardo Habkost @ 2015-10-28 18:37 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Andrew Jones, Markus Armbruster

Suggested-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 vl.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/vl.c b/vl.c
index cd76ff4..61e474b 100644
--- a/vl.c
+++ b/vl.c
@@ -931,8 +931,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);
+        error_report("warning: adding a VHCI to an empty scatternet %i",
+                     vlan_id);
 
     bt_vhci_init(bt_new_hci(vlan));
 }
@@ -958,8 +958,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);
+        error_report("warning: adding a slave device to an empty scatternet %i",
+                     vlan_id);
 
     if (!strcmp(devname, "keyboard"))
         return bt_keyboard_init(vlan);
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v2 13/16] vl.c: Reword -machine help error messages
  2015-10-28 18:36 [Qemu-devel] [PATCH v2 00/16] vl.c: Error message rework Eduardo Habkost
                   ` (11 preceding siblings ...)
  2015-10-28 18:37 ` [Qemu-devel] [PATCH v2 12/16] vl.c: trivial: Don't wrap lines unnecessarily Eduardo Habkost
@ 2015-10-28 18:37 ` Eduardo Habkost
  2015-10-29 17:27   ` Markus Armbruster
  2015-10-28 18:37 ` [Qemu-devel] [PATCH v2 14/16] vl.c: Simplify date format error message Eduardo Habkost
                   ` (2 subsequent siblings)
  15 siblings, 1 reply; 26+ messages in thread
From: Eduardo Habkost @ 2015-10-28 18:37 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Andrew Jones, Markus Armbruster

* Use "\n" consistently in both error messages.
* Simplify error message.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 vl.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/vl.c b/vl.c
index 61e474b..55521a0 100644
--- a/vl.c
+++ b/vl.c
@@ -2634,8 +2634,8 @@ static gint machine_class_cmp(gconstpointer a, gconstpointer b)
         return mc;
     }
     if (name && !is_help_option(name)) {
-        error_report("unsupported machine type");
-        error_printf("Use -machine help to list supported machines!\n");
+        error_report("unsupported machine type\n"
+                     "Use -machine help to list supported machines");
     } else {
         printf("Supported machines are:\n");
         machines = g_slist_sort(machines, machine_class_cmp);
@@ -3994,8 +3994,8 @@ int main(int argc, char **argv, char **envp)
     }
 
     if (machine_class == NULL) {
-        error_report("No machine specified, and there is no default.\n"
-                     "Use -machine help to list supported machines!");
+        error_report("no machine specified\n"
+                     "Use -machine help to list supported machines");
         exit(1);
     }
 
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v2 14/16] vl.c: Simplify date format error message
  2015-10-28 18:36 [Qemu-devel] [PATCH v2 00/16] vl.c: Error message rework Eduardo Habkost
                   ` (12 preceding siblings ...)
  2015-10-28 18:37 ` [Qemu-devel] [PATCH v2 13/16] vl.c: Reword -machine help error messages Eduardo Habkost
@ 2015-10-28 18:37 ` Eduardo Habkost
  2015-10-28 18:37 ` [Qemu-devel] [PATCH v2 15/16] vl.c: Use US spelling for "unrecognized" Eduardo Habkost
  2015-10-28 18:37 ` [Qemu-devel] [PATCH v2 16/16] vl.c: Reword fw_cfg name prefix warning Eduardo Habkost
  15 siblings, 0 replies; 26+ messages in thread
From: Eduardo Habkost @ 2015-10-28 18:37 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Andrew Jones, Markus Armbruster

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 vl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/vl.c b/vl.c
index 55521a0..686a7d3 100644
--- a/vl.c
+++ b/vl.c
@@ -828,8 +828,8 @@ static void configure_rtc_date_offset(const char *startdate, int legacy)
         rtc_start_date = mktimegm(&tm);
         if (rtc_start_date == -1) {
         date_fail:
-            error_report("Invalid date format. Valid formats are:\n"
-                         "'2006-06-17T16:01:21' or '2006-06-17'");
+            error_report("invalid date format\n"
+                         "valid formats: '2006-06-17T16:01:21' or '2006-06-17'");
             exit(1);
         }
         rtc_date_offset = qemu_time() - rtc_start_date;
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v2 15/16] vl.c: Use US spelling for "unrecognized"
  2015-10-28 18:36 [Qemu-devel] [PATCH v2 00/16] vl.c: Error message rework Eduardo Habkost
                   ` (13 preceding siblings ...)
  2015-10-28 18:37 ` [Qemu-devel] [PATCH v2 14/16] vl.c: Simplify date format error message Eduardo Habkost
@ 2015-10-28 18:37 ` Eduardo Habkost
  2015-10-28 18:37 ` [Qemu-devel] [PATCH v2 16/16] vl.c: Reword fw_cfg name prefix warning Eduardo Habkost
  15 siblings, 0 replies; 26+ messages in thread
From: Eduardo Habkost @ 2015-10-28 18:37 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Andrew Jones, Markus Armbruster

All other error_report() and error_setg() cases use "unrecognized", use
the same spelling here for consistency.

Reported-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 vl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/vl.c b/vl.c
index 686a7d3..6b831eb 100644
--- a/vl.c
+++ b/vl.c
@@ -950,7 +950,7 @@ static struct bt_device_s *bt_device_add(const char *opt)
     if (endp) {
         vlan_id = strtol(endp + 6, &endp, 0);
         if (*endp) {
-            error_report("unrecognised bluetooth vlan Id");
+            error_report("unrecognized bluetooth vlan Id");
             return 0;
         }
     }
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v2 16/16] vl.c: Reword fw_cfg name prefix warning
  2015-10-28 18:36 [Qemu-devel] [PATCH v2 00/16] vl.c: Error message rework Eduardo Habkost
                   ` (14 preceding siblings ...)
  2015-10-28 18:37 ` [Qemu-devel] [PATCH v2 15/16] vl.c: Use US spelling for "unrecognized" Eduardo Habkost
@ 2015-10-28 18:37 ` Eduardo Habkost
  2015-10-29 17:34   ` Markus Armbruster
  15 siblings, 1 reply; 26+ messages in thread
From: Eduardo Habkost @ 2015-10-28 18:37 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Andrew Jones, Markus Armbruster

Make it a shorter and simpler phrase.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 vl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/vl.c b/vl.c
index 6b831eb..67f75da 100644
--- a/vl.c
+++ b/vl.c
@@ -2276,8 +2276,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/\"!");
+        error_report("warning: no \"opt/\" prefix on fw_cfg item name '%s'",
+                     name);
     }
     if (nonempty_str(str)) {
         size = strlen(str); /* NUL terminator NOT included in fw_cfg blob */
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v2 07/16] vl.c: Use "%s support disabled" error messages consistently
  2015-10-28 18:36 ` [Qemu-devel] [PATCH v2 07/16] vl.c: Use "%s support disabled" error messages consistently Eduardo Habkost
@ 2015-10-29 16:59   ` Markus Armbruster
  2015-10-30 13:28     ` Andrew Jones
  0 siblings, 1 reply; 26+ messages in thread
From: Markus Armbruster @ 2015-10-29 16:59 UTC (permalink / raw)
  To: Eduardo Habkost; +Cc: Paolo Bonzini, Andrew Jones, qemu-devel

Eduardo Habkost <ehabkost@redhat.com> writes:

> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  vl.c | 34 ++++++++++++++++------------------
>  1 file changed, 16 insertions(+), 18 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index d417dd9..f37e3a9 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1018,8 +1018,7 @@ static int parse_sandbox(void *opaque, QemuOpts *opts, Error **errp)
>              return -1;
>          }
>  #else
> -        error_report("sandboxing request but seccomp is not compiled "
> -                     "into this build");
> +        error_report("seccomp support disabled");
>          return -1;
>  #endif
>      }

Yes, please.

> @@ -2112,7 +2111,7 @@ static DisplayType select_display(const char *p)
>              opts = nextopt;
>          }
>  #else
> -        error_report("SDL support is disabled");
> +        error_report("SDL support disabled");
>          exit(1);
>  #endif
>      } else if (strstart(p, "vnc", &opts)) {

I'd prefer the old wording.  Drew?

> @@ -2128,14 +2127,14 @@ static DisplayType select_display(const char *p)
>              exit(1);
>          }
>  #else
> -        error_report("VNC support is disabled");
> +        error_report("VNC support disabled");
>          exit(1);
>  #endif
>      } else if (strstart(p, "curses", &opts)) {
>  #ifdef CONFIG_CURSES
>          display = DT_CURSES;
>  #else
> -        error_report("Curses support is disabled");
> +        error_report("curses support disabled");
>          exit(1);
>  #endif
>      } else if (strstart(p, "gtk", &opts)) {
> @@ -2170,7 +2169,7 @@ static DisplayType select_display(const char *p)
>              opts = nextopt;
>          }
>  #else
> -        error_report("GTK support is disabled");
> +        error_report("GTK support disabled");
>          exit(1);
>  #endif
>      } else if (strstart(p, "none", &opts)) {
> @@ -2635,7 +2634,7 @@ static gint machine_class_cmp(gconstpointer a, gconstpointer b)
>          return mc;
>      }
>      if (name && !is_help_option(name)) {
> -        error_report("Unsupported machine type");
> +        error_report("unsupported machine type");
>          error_printf("Use -machine help to list supported machines!\n");
>      } else {
>          printf("Supported machines are:\n");

This belongs to PATCH 11.

> @@ -3011,7 +3010,7 @@ int main(int argc, char **argv, char **envp)
>      runstate_init();
>  
>      if (qcrypto_init(&err) < 0) {
> -        error_report("Cannot initialize crypto: %s", error_get_pretty(err));
> +        error_report("cannot initialize crypto: %s", error_get_pretty(err));
>          exit(1);
>      }
>      rtc_clock = QEMU_CLOCK_HOST;

Likewise.

> @@ -3212,7 +3211,7 @@ int main(int argc, char **argv, char **envp)
>  #ifdef CONFIG_CURSES
>                  display_type = DT_CURSES;
>  #else
> -                error_report("Curses support is disabled");
> +                error_report("curses support disabled");
>                  exit(1);
>  #endif
>                  break;

Likewise.

> @@ -3440,7 +3439,7 @@ int main(int argc, char **argv, char **envp)
>              case QEMU_OPTION_fsdev:
>                  olist = qemu_find_opts("fsdev");
>                  if (!olist) {
> -                    error_report("fsdev is not supported by this qemu build");
> +                    error_report("fsdev support disabled");
>                      exit(1);
>                  }
>                  opts = qemu_opts_parse_noisily(olist, optarg, true);
> @@ -3455,7 +3454,7 @@ int main(int argc, char **argv, char **envp)
>  
>                  olist = qemu_find_opts("virtfs");
>                  if (!olist) {
> -                    error_report("virtfs is not supported by this qemu build");
> +                    error_report("virtfs support disabled");
>                      exit(1);
>                  }
>                  opts = qemu_opts_parse_noisily(olist, optarg, true);
> @@ -3593,7 +3592,7 @@ int main(int argc, char **argv, char **envp)
>                  display_type = DT_SDL;
>                  break;
>  #else
> -                error_report("SDL support is disabled");
> +                error_report("SDL support disabled");
>                  exit(1);
>  #endif
>              case QEMU_OPTION_pidfile:
> @@ -3705,7 +3704,7 @@ int main(int argc, char **argv, char **envp)
>                      exit(1);
>                  }
>  #else
> -                error_report("VNC support is disabled");
> +                error_report("VNC support disabled");
>                  exit(1);
>  #endif
>                  break;
> @@ -3899,7 +3898,7 @@ int main(int argc, char **argv, char **envp)
>              case QEMU_OPTION_spice:
>                  olist = qemu_find_opts("spice");
>                  if (!olist) {
> -                    error_report("spice is not supported by this qemu build");
> +                    error_report("spice support disabled");
>                      exit(1);
>                  }
>                  opts = qemu_opts_parse_noisily(olist, optarg, false);
> @@ -3947,8 +3946,7 @@ int main(int argc, char **argv, char **envp)
>                      exit(1);
>                  }
>  #else
> -                error_report("File descriptor passing is disabled on this "
> -                             "platform");
> +                error_report("fd passing disabled on this platform");
>                  exit(1);
>  #endif
>                  break;

It's stick to "file descriptor".

> @@ -4249,9 +4247,9 @@ int main(int argc, char **argv, char **envp)
>  #endif
>      if (request_opengl == 1 && display_opengl == 0) {
>  #if defined(CONFIG_OPENGL)
> -        error_report("OpenGL is not supported by the display");
> +        error_report("OpenGL not supported by the display");
>  #else
> -        error_report("QEMU was built without opengl support");
> +        error_report("OpenGL support disabled");
>  #endif
>          exit(1);
>      }

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v2 09/16] vl.c: Reword -no-kvm-pit-reinjection deprecation warning
  2015-10-28 18:36 ` [Qemu-devel] [PATCH v2 09/16] vl.c: Reword -no-kvm-pit-reinjection deprecation warning Eduardo Habkost
@ 2015-10-29 17:10   ` Markus Armbruster
  2015-10-30 13:33     ` Andrew Jones
  0 siblings, 1 reply; 26+ messages in thread
From: Markus Armbruster @ 2015-10-29 17:10 UTC (permalink / raw)
  To: Eduardo Habkost; +Cc: Paolo Bonzini, Andrew Jones, qemu-devel

Eduardo Habkost <ehabkost@redhat.com> writes:

> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  vl.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index af8d09c..67147e0 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3667,8 +3667,8 @@ int main(int argc, char **argv, char **envp)
>                      { /* end of list */ }
>                  };
>  
> -                error_report("warning: option deprecated, use "
> -                             "lost_tick_policy property of kvm-pit instead");
> +                error_report("warning: -no-kvm-pit-reinjection deprecated by "
> +                             "-global kvm-pit.lost_tick_policy=discard");
>                  qdev_prop_register_global_list(kvm_pit_lost_tick_policy);
>                  break;
>              }

Repeating the option in the error message rarely improves the message,
because error_report() already shows it.  It's actually misleading when
the option comes from a configuration file read with -readconfig.

Fortunately, -readconfig doesn't support the option, so that's not an
issue here.

For the command line, the message changes from

    qemu-system-x86_64: -no-kvm-pit-reinjection: warning: option deprecated, use lost_tick_policy property of kvm-pit instead

to

    qemu-system-x86_64: -no-kvm-pit-reinjection: warning: -no-kvm-pit-reinjection deprecated by -global kvm-pit.lost_tick_policy=discard

Perhaps:

    qemu-system-x86_64: -no-kvm-pit-reinjection: warning: deprecated, replaced by -global kvm-pit.lost_tick_policy=discard

Similar argument for PATCH 08.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v2 10/16] vl.c: Convert error sentences to simpler phrases
  2015-10-28 18:36 ` [Qemu-devel] [PATCH v2 10/16] vl.c: Convert error sentences to simpler phrases Eduardo Habkost
@ 2015-10-29 17:25   ` Markus Armbruster
  0 siblings, 0 replies; 26+ messages in thread
From: Markus Armbruster @ 2015-10-29 17:25 UTC (permalink / raw)
  To: Eduardo Habkost; +Cc: Paolo Bonzini, Andrew Jones, qemu-devel

Eduardo Habkost <ehabkost@redhat.com> writes:

> Simplify some error messages by making them simple phrases instead of
> full sentences.
>
> Suggested-by: Andrew Jones <drjones@redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  vl.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index 67147e0..b8c6c3c 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1066,7 +1066,7 @@ static int parse_add_fd(void *opaque, QemuOpts *opts, Error **errp)
>      fd_opaque = qemu_opt_get(opts, "opaque");
>  
>      if (fd < 0) {
> -        error_report("fd option is required and must be non-negative");
> +        error_report("non-negative 'fd' option required");
>          return -1;
>      }
>  

We need to make up our mind how to call a QemuOpts option's key=value
part.  qemu-options.c (actually qerror.h, which I still haven't killed
off) calls it "parameter", to avoid confusion with the entire option,
i.e. the -opt key=value,...

The error reporting lazy here: you get the same message for a missing fd
as for a negative fd, because the programmer couldn't be bothered to
detect the difference.  Not this patch's problem.

> @@ -1081,12 +1081,12 @@ static int parse_add_fd(void *opaque, QemuOpts *opts, Error **errp)
>       */
>      flags = fcntl(fd, F_GETFD);
>      if (flags == -1 || (flags & FD_CLOEXEC)) {
> -        error_report("fd is not valid or already in use");
> +        error_report("fd not valid or already in use");
>          return -1;
>      }

As in PATCH 07, I'd prefer the phrasing with the verb.  More of the same
below.

Of course, the error message is crap either way.  "The order you gave is
invalid (but I'm not telling you why), or it clashes with some other
order I got (but I'm not telling you which one).  Pffft.  Not this
patch's problem.

>  
>      if (fdset_id < 0) {
> -        error_report("set option is required and must be non-negative");
> +        error_report("non-negative 'set' option required");
>          return -1;
>      }
>  

Lazy again.

> @@ -3751,7 +3751,7 @@ int main(int argc, char **argv, char **envp)
>                  option_rom[nb_option_roms].bootindex =
>                      qemu_opt_get_number(opts, "bootindex", -1);
>                  if (!option_rom[nb_option_roms].name) {
> -                    error_report("Option ROM file is not specified");
> +                    error_report("option ROM file not specified");
>                      exit(1);
>                  }
>                  nb_option_roms++;
> @@ -4225,11 +4225,11 @@ int main(int argc, char **argv, char **envp)
>      }
>  
>      if ((no_frame || alt_grab || ctrl_grab) && display_type != DT_SDL) {
> -        error_report("-no-frame, -alt-grab and -ctrl-grab are only valid "
> +        error_report("-no-frame, -alt-grab and -ctrl-grab only valid "
>                       "for SDL, ignoring option");
>      }
>      if (no_quit && (display_type != DT_GTK && display_type != DT_SDL)) {
> -        error_report("-no-quit is only valid for GTK and SDL, "
> +        error_report("-no-quit only valid for GTK and SDL, "
>                       "ignoring option");
>      }
>  
> @@ -4373,7 +4373,7 @@ int main(int argc, char **argv, char **envp)
>      cpu_ticks_init();
>      if (icount_opts) {
>          if (kvm_enabled() || xen_enabled()) {
> -            error_report("-icount is not allowed with kvm or xen");
> +            error_report("-icount not allowed with kvm or xen");
>              exit(1);
>          }
>          configure_icount(icount_opts, &error_abort);

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v2 13/16] vl.c: Reword -machine help error messages
  2015-10-28 18:37 ` [Qemu-devel] [PATCH v2 13/16] vl.c: Reword -machine help error messages Eduardo Habkost
@ 2015-10-29 17:27   ` Markus Armbruster
  0 siblings, 0 replies; 26+ messages in thread
From: Markus Armbruster @ 2015-10-29 17:27 UTC (permalink / raw)
  To: Eduardo Habkost; +Cc: Paolo Bonzini, Andrew Jones, qemu-devel

Eduardo Habkost <ehabkost@redhat.com> writes:

> * Use "\n" consistently in both error messages.
> * Simplify error message.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  vl.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index 61e474b..55521a0 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2634,8 +2634,8 @@ static gint machine_class_cmp(gconstpointer a, gconstpointer b)
>          return mc;
>      }
>      if (name && !is_help_option(name)) {
> -        error_report("unsupported machine type");
> -        error_printf("Use -machine help to list supported machines!\n");
> +        error_report("unsupported machine type\n"
> +                     "Use -machine help to list supported machines");

error_report()'s contract:

/*
 * Print an error message to current monitor if we have one, else to stderr.
 * Format arguments like sprintf().  The result should not contain
 * newlines.
 * Prepend the current location and append a newline.
 * It's wrong to call this in a QMP monitor.  Use error_setg() there.
 */

The old code gets it right.

You could squash the removal of '!' into PATCH 03.

>      } else {
>          printf("Supported machines are:\n");
>          machines = g_slist_sort(machines, machine_class_cmp);
> @@ -3994,8 +3994,8 @@ int main(int argc, char **argv, char **envp)
>      }
>  
>      if (machine_class == NULL) {
> -        error_report("No machine specified, and there is no default.\n"
> -                     "Use -machine help to list supported machines!");
> +        error_report("no machine specified\n"
> +                     "Use -machine help to list supported machines");
>          exit(1);
>      }

Likewise.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v2 16/16] vl.c: Reword fw_cfg name prefix warning
  2015-10-28 18:37 ` [Qemu-devel] [PATCH v2 16/16] vl.c: Reword fw_cfg name prefix warning Eduardo Habkost
@ 2015-10-29 17:34   ` Markus Armbruster
  0 siblings, 0 replies; 26+ messages in thread
From: Markus Armbruster @ 2015-10-29 17:34 UTC (permalink / raw)
  To: Eduardo Habkost; +Cc: Paolo Bonzini, Andrew Jones, qemu-devel

Eduardo Habkost <ehabkost@redhat.com> writes:

> Make it a shorter and simpler phrase.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  vl.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index 6b831eb..67f75da 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2276,8 +2276,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/\"!");
> +        error_report("warning: no \"opt/\" prefix on fw_cfg item name '%s'",
> +                     name);
>      }
>      if (nonempty_str(str)) {
>          size = strlen(str); /* NUL terminator NOT included in fw_cfg blob */

The loss of the '!' is an improvement.  The rest perhaps not so much.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v2 01/16] vl.c: Replace fprintf(stderr) with error_report()
  2015-10-28 18:36 ` [Qemu-devel] [PATCH v2 01/16] vl.c: Replace fprintf(stderr) with error_report() Eduardo Habkost
@ 2015-10-29 17:34   ` Markus Armbruster
  2015-10-29 17:52     ` Eduardo Habkost
  0 siblings, 1 reply; 26+ messages in thread
From: Markus Armbruster @ 2015-10-29 17:34 UTC (permalink / raw)
  To: Eduardo Habkost; +Cc: Paolo Bonzini, Andrew Jones, qemu-devel

Eduardo Habkost <ehabkost@redhat.com> writes:

> Replace most fprintf(stderr) calls on vl.c with error_report().
>
> Minimal changes were made in the error messages. Only the trailing
> newlines, "qemu:" and "error:" message prefixes were removed.
>
> The only remaining fprintf(stderr) calls are the ones at
> qemu_kill_report(), because the error mesage is split in multiple
> fprintf() calls.
>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> Changes v1 -> v2:
> * Indentation fix at "No machine specified" error message
> ---
>  vl.c | 228 +++++++++++++++++++++++++++++++++----------------------------------
>  1 file changed, 112 insertions(+), 116 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index f5f7c3f..25b91fc 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -674,9 +674,9 @@ void runstate_set(RunState new_state)
>      assert(new_state < RUN_STATE_MAX);
>  
>      if (!runstate_valid_transitions[current_run_state][new_state]) {
> -        fprintf(stderr, "ERROR: invalid runstate transition: '%s' -> '%s'\n",
> -                RunState_lookup[current_run_state],
> -                RunState_lookup[new_state]);
> +        error_report("invalid runstate transition: '%s' -> '%s'",
> +                     RunState_lookup[current_run_state],
> +                     RunState_lookup[new_state]);
>          abort();
>      }
>      trace_runstate_set(new_state);
> @@ -828,8 +828,8 @@ static void configure_rtc_date_offset(const char *startdate, int legacy)
>          rtc_start_date = mktimegm(&tm);
>          if (rtc_start_date == -1) {
>          date_fail:
> -            fprintf(stderr, "Invalid date format. Valid formats are:\n"
> -                            "'2006-06-17T16:01:21' or '2006-06-17'\n");
> +            error_report("Invalid date format. Valid formats are:\n"
> +                         "'2006-06-17T16:01:21' or '2006-06-17'");

I'm afraid you violate error_report()'s contract:

/*
 * Print an error message to current monitor if we have one, else to stderr.
 * Format arguments like sprintf().  The result should not contain
 * newlines.
 * Prepend the current location and append a newline.
 * It's wrong to call this in a QMP monitor.  Use error_setg() there.
 */

Make it something like

            error_report("invalid date format");
            error_printf("valid formats: '2006-06-17T16:01:21'"
                        " or '2006-06-17'\n");

>              exit(1);
>          }
>          rtc_date_offset = qemu_time() - rtc_start_date;
[...]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v2 01/16] vl.c: Replace fprintf(stderr) with error_report()
  2015-10-29 17:34   ` Markus Armbruster
@ 2015-10-29 17:52     ` Eduardo Habkost
  0 siblings, 0 replies; 26+ messages in thread
From: Eduardo Habkost @ 2015-10-29 17:52 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: Paolo Bonzini, Andrew Jones, qemu-devel

On Thu, Oct 29, 2015 at 06:34:48PM +0100, Markus Armbruster wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
> 
> > Replace most fprintf(stderr) calls on vl.c with error_report().
> >
> > Minimal changes were made in the error messages. Only the trailing
> > newlines, "qemu:" and "error:" message prefixes were removed.
> >
> > The only remaining fprintf(stderr) calls are the ones at
> > qemu_kill_report(), because the error mesage is split in multiple
> > fprintf() calls.
> >
> > Reviewed-by: Markus Armbruster <armbru@redhat.com>
> > Reviewed-by: Eric Blake <eblake@redhat.com>
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> > Changes v1 -> v2:
> > * Indentation fix at "No machine specified" error message
> > ---
> >  vl.c | 228 +++++++++++++++++++++++++++++++++----------------------------------
> >  1 file changed, 112 insertions(+), 116 deletions(-)
> >
> > diff --git a/vl.c b/vl.c
> > index f5f7c3f..25b91fc 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -674,9 +674,9 @@ void runstate_set(RunState new_state)
> >      assert(new_state < RUN_STATE_MAX);
> >  
> >      if (!runstate_valid_transitions[current_run_state][new_state]) {
> > -        fprintf(stderr, "ERROR: invalid runstate transition: '%s' -> '%s'\n",
> > -                RunState_lookup[current_run_state],
> > -                RunState_lookup[new_state]);
> > +        error_report("invalid runstate transition: '%s' -> '%s'",
> > +                     RunState_lookup[current_run_state],
> > +                     RunState_lookup[new_state]);
> >          abort();
> >      }
> >      trace_runstate_set(new_state);
> > @@ -828,8 +828,8 @@ static void configure_rtc_date_offset(const char *startdate, int legacy)
> >          rtc_start_date = mktimegm(&tm);
> >          if (rtc_start_date == -1) {
> >          date_fail:
> > -            fprintf(stderr, "Invalid date format. Valid formats are:\n"
> > -                            "'2006-06-17T16:01:21' or '2006-06-17'\n");
> > +            error_report("Invalid date format. Valid formats are:\n"
> > +                         "'2006-06-17T16:01:21' or '2006-06-17'");
> 
> I'm afraid you violate error_report()'s contract:
> 
> /*
>  * Print an error message to current monitor if we have one, else to stderr.
>  * Format arguments like sprintf().  The result should not contain
>  * newlines.
>  * Prepend the current location and append a newline.
>  * It's wrong to call this in a QMP monitor.  Use error_setg() there.
>  */
> 
> Make it something like
> 
>             error_report("invalid date format");
>             error_printf("valid formats: '2006-06-17T16:01:21'"
>                         " or '2006-06-17'\n");

Oops, that's what happens when I use existing code as reference, instead
of reading the documentation. :)

I will fix it. Thanks.

-- 
Eduardo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v2 07/16] vl.c: Use "%s support disabled" error messages consistently
  2015-10-29 16:59   ` Markus Armbruster
@ 2015-10-30 13:28     ` Andrew Jones
  0 siblings, 0 replies; 26+ messages in thread
From: Andrew Jones @ 2015-10-30 13:28 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: Paolo Bonzini, Eduardo Habkost, qemu-devel

On Thu, Oct 29, 2015 at 05:59:13PM +0100, Markus Armbruster wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
> 
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> >  vl.c | 34 ++++++++++++++++------------------
> >  1 file changed, 16 insertions(+), 18 deletions(-)
> >
> > diff --git a/vl.c b/vl.c
> > index d417dd9..f37e3a9 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -1018,8 +1018,7 @@ static int parse_sandbox(void *opaque, QemuOpts *opts, Error **errp)
> >              return -1;
> >          }
> >  #else
> > -        error_report("sandboxing request but seccomp is not compiled "
> > -                     "into this build");
> > +        error_report("seccomp support disabled");
> >          return -1;
> >  #endif
> >      }
> 
> Yes, please.
> 
> > @@ -2112,7 +2111,7 @@ static DisplayType select_display(const char *p)
> >              opts = nextopt;
> >          }
> >  #else
> > -        error_report("SDL support is disabled");
> > +        error_report("SDL support disabled");
> >          exit(1);
> >  #endif
> >      } else if (strstart(p, "vnc", &opts)) {
> 
> I'd prefer the old wording.  Drew?

I like the 'is' version better too. I'm OK with both though.

> 
> > @@ -2128,14 +2127,14 @@ static DisplayType select_display(const char *p)
> >              exit(1);
> >          }
> >  #else
> > -        error_report("VNC support is disabled");
> > +        error_report("VNC support disabled");
> >          exit(1);
> >  #endif
> >      } else if (strstart(p, "curses", &opts)) {
> >  #ifdef CONFIG_CURSES
> >          display = DT_CURSES;
> >  #else
> > -        error_report("Curses support is disabled");
> > +        error_report("curses support disabled");
> >          exit(1);
> >  #endif
> >      } else if (strstart(p, "gtk", &opts)) {
> > @@ -2170,7 +2169,7 @@ static DisplayType select_display(const char *p)
> >              opts = nextopt;
> >          }
> >  #else
> > -        error_report("GTK support is disabled");
> > +        error_report("GTK support disabled");
> >          exit(1);
> >  #endif
> >      } else if (strstart(p, "none", &opts)) {
> > @@ -2635,7 +2634,7 @@ static gint machine_class_cmp(gconstpointer a, gconstpointer b)
> >          return mc;
> >      }
> >      if (name && !is_help_option(name)) {
> > -        error_report("Unsupported machine type");
> > +        error_report("unsupported machine type");
> >          error_printf("Use -machine help to list supported machines!\n");
> >      } else {
> >          printf("Supported machines are:\n");
> 
> This belongs to PATCH 11.
> 
> > @@ -3011,7 +3010,7 @@ int main(int argc, char **argv, char **envp)
> >      runstate_init();
> >  
> >      if (qcrypto_init(&err) < 0) {
> > -        error_report("Cannot initialize crypto: %s", error_get_pretty(err));
> > +        error_report("cannot initialize crypto: %s", error_get_pretty(err));
> >          exit(1);
> >      }
> >      rtc_clock = QEMU_CLOCK_HOST;
> 
> Likewise.
> 
> > @@ -3212,7 +3211,7 @@ int main(int argc, char **argv, char **envp)
> >  #ifdef CONFIG_CURSES
> >                  display_type = DT_CURSES;
> >  #else
> > -                error_report("Curses support is disabled");
> > +                error_report("curses support disabled");
> >                  exit(1);
> >  #endif
> >                  break;
> 
> Likewise.
> 
> > @@ -3440,7 +3439,7 @@ int main(int argc, char **argv, char **envp)
> >              case QEMU_OPTION_fsdev:
> >                  olist = qemu_find_opts("fsdev");
> >                  if (!olist) {
> > -                    error_report("fsdev is not supported by this qemu build");
> > +                    error_report("fsdev support disabled");
> >                      exit(1);
> >                  }
> >                  opts = qemu_opts_parse_noisily(olist, optarg, true);
> > @@ -3455,7 +3454,7 @@ int main(int argc, char **argv, char **envp)
> >  
> >                  olist = qemu_find_opts("virtfs");
> >                  if (!olist) {
> > -                    error_report("virtfs is not supported by this qemu build");
> > +                    error_report("virtfs support disabled");
> >                      exit(1);
> >                  }
> >                  opts = qemu_opts_parse_noisily(olist, optarg, true);
> > @@ -3593,7 +3592,7 @@ int main(int argc, char **argv, char **envp)
> >                  display_type = DT_SDL;
> >                  break;
> >  #else
> > -                error_report("SDL support is disabled");
> > +                error_report("SDL support disabled");
> >                  exit(1);
> >  #endif
> >              case QEMU_OPTION_pidfile:
> > @@ -3705,7 +3704,7 @@ int main(int argc, char **argv, char **envp)
> >                      exit(1);
> >                  }
> >  #else
> > -                error_report("VNC support is disabled");
> > +                error_report("VNC support disabled");
> >                  exit(1);
> >  #endif
> >                  break;
> > @@ -3899,7 +3898,7 @@ int main(int argc, char **argv, char **envp)
> >              case QEMU_OPTION_spice:
> >                  olist = qemu_find_opts("spice");
> >                  if (!olist) {
> > -                    error_report("spice is not supported by this qemu build");
> > +                    error_report("spice support disabled");
> >                      exit(1);
> >                  }
> >                  opts = qemu_opts_parse_noisily(olist, optarg, false);
> > @@ -3947,8 +3946,7 @@ int main(int argc, char **argv, char **envp)
> >                      exit(1);
> >                  }
> >  #else
> > -                error_report("File descriptor passing is disabled on this "
> > -                             "platform");
> > +                error_report("fd passing disabled on this platform");
> >                  exit(1);
> >  #endif
> >                  break;
> 
> It's stick to "file descriptor".
> 
> > @@ -4249,9 +4247,9 @@ int main(int argc, char **argv, char **envp)
> >  #endif
> >      if (request_opengl == 1 && display_opengl == 0) {
> >  #if defined(CONFIG_OPENGL)
> > -        error_report("OpenGL is not supported by the display");
> > +        error_report("OpenGL not supported by the display");
> >  #else
> > -        error_report("QEMU was built without opengl support");
> > +        error_report("OpenGL support disabled");
> >  #endif
> >          exit(1);
> >      }
> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v2 09/16] vl.c: Reword -no-kvm-pit-reinjection deprecation warning
  2015-10-29 17:10   ` Markus Armbruster
@ 2015-10-30 13:33     ` Andrew Jones
  0 siblings, 0 replies; 26+ messages in thread
From: Andrew Jones @ 2015-10-30 13:33 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: Paolo Bonzini, Eduardo Habkost, qemu-devel

On Thu, Oct 29, 2015 at 06:10:17PM +0100, Markus Armbruster wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
> 
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> >  vl.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/vl.c b/vl.c
> > index af8d09c..67147e0 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -3667,8 +3667,8 @@ int main(int argc, char **argv, char **envp)
> >                      { /* end of list */ }
> >                  };
> >  
> > -                error_report("warning: option deprecated, use "
> > -                             "lost_tick_policy property of kvm-pit instead");
> > +                error_report("warning: -no-kvm-pit-reinjection deprecated by "
> > +                             "-global kvm-pit.lost_tick_policy=discard");
> >                  qdev_prop_register_global_list(kvm_pit_lost_tick_policy);
> >                  break;
> >              }
> 
> Repeating the option in the error message rarely improves the message,
> because error_report() already shows it.  It's actually misleading when
> the option comes from a configuration file read with -readconfig.
> 
> Fortunately, -readconfig doesn't support the option, so that's not an
> issue here.
> 
> For the command line, the message changes from
> 
>     qemu-system-x86_64: -no-kvm-pit-reinjection: warning: option deprecated, use lost_tick_policy property of kvm-pit instead
> 
> to
> 
>     qemu-system-x86_64: -no-kvm-pit-reinjection: warning: -no-kvm-pit-reinjection deprecated by -global kvm-pit.lost_tick_policy=discard
> 
> Perhaps:
> 
>     qemu-system-x86_64: -no-kvm-pit-reinjection: warning: deprecated, replaced by -global kvm-pit.lost_tick_policy=discard
> 
> Similar argument for PATCH 08.
> 

Ah, I hadn't looked closely enough at error_report to know that the option
would be output already with my "ignoring" phrase suggestion. I definitely
agree that repeating it should be avoided.

"warning: deprecated, ignoring"

Thanks,
drew

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2015-10-30 13:33 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-28 18:36 [Qemu-devel] [PATCH v2 00/16] vl.c: Error message rework Eduardo Habkost
2015-10-28 18:36 ` [Qemu-devel] [PATCH v2 01/16] vl.c: Replace fprintf(stderr) with error_report() Eduardo Habkost
2015-10-29 17:34   ` Markus Armbruster
2015-10-29 17:52     ` Eduardo Habkost
2015-10-28 18:36 ` [Qemu-devel] [PATCH v2 02/16] vl.c: Use error_report() when reporting shutdown signal Eduardo Habkost
2015-10-28 18:36 ` [Qemu-devel] [PATCH v2 03/16] vl.c: Remove periods from error_report() error messages Eduardo Habkost
2015-10-28 18:36 ` [Qemu-devel] [PATCH v2 04/16] vl.c: Use "warning:" prefix consistently on warnings Eduardo Habkost
2015-10-28 18:36 ` [Qemu-devel] [PATCH v2 05/16] vl.c: Use "cannot" instead of "can not" in error messages Eduardo Habkost
2015-10-28 18:36 ` [Qemu-devel] [PATCH v2 06/16] vl.c: Use 'quotes' instead of `quotes' in messages Eduardo Habkost
2015-10-28 18:36 ` [Qemu-devel] [PATCH v2 07/16] vl.c: Use "%s support disabled" error messages consistently Eduardo Habkost
2015-10-29 16:59   ` Markus Armbruster
2015-10-30 13:28     ` Andrew Jones
2015-10-28 18:36 ` [Qemu-devel] [PATCH v2 08/16] vl.c: Simplify "ignoring deprecated option" warnings Eduardo Habkost
2015-10-28 18:36 ` [Qemu-devel] [PATCH v2 09/16] vl.c: Reword -no-kvm-pit-reinjection deprecation warning Eduardo Habkost
2015-10-29 17:10   ` Markus Armbruster
2015-10-30 13:33     ` Andrew Jones
2015-10-28 18:36 ` [Qemu-devel] [PATCH v2 10/16] vl.c: Convert error sentences to simpler phrases Eduardo Habkost
2015-10-29 17:25   ` Markus Armbruster
2015-10-28 18:37 ` [Qemu-devel] [PATCH v2 11/16] vl.c: Remove unnecessary uppercase in error messages Eduardo Habkost
2015-10-28 18:37 ` [Qemu-devel] [PATCH v2 12/16] vl.c: trivial: Don't wrap lines unnecessarily Eduardo Habkost
2015-10-28 18:37 ` [Qemu-devel] [PATCH v2 13/16] vl.c: Reword -machine help error messages Eduardo Habkost
2015-10-29 17:27   ` Markus Armbruster
2015-10-28 18:37 ` [Qemu-devel] [PATCH v2 14/16] vl.c: Simplify date format error message Eduardo Habkost
2015-10-28 18:37 ` [Qemu-devel] [PATCH v2 15/16] vl.c: Use US spelling for "unrecognized" Eduardo Habkost
2015-10-28 18:37 ` [Qemu-devel] [PATCH v2 16/16] vl.c: Reword fw_cfg name prefix warning Eduardo Habkost
2015-10-29 17:34   ` Markus Armbruster

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).