On 10/26/2015 11:13 AM, Eduardo Habkost wrote: > This replaces most fprintf(stderr) calls on vl.c with error_report(). > > 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. > > Signed-off-by: Eduardo Habkost <ehabk...@redhat.com> > --- > Not sure if this is appropriate post soft-freeze, but if we are going to apply > the max-cpus patch from Drew before 2.5.0, we could simply change all the > fprintf() calls in a single step.
Soft freeze means no new features that didn't already have a patch
review started - but this is a cleanup, not a new feature. I think it
qualifies for 2.5 inclusion just fine.
> ---
> vl.c | 228
> +++++++++++++++++++++++++++++++++----------------------------------
> 1 file changed, 112 insertions(+), 116 deletions(-)
In addition to the things Markus pointed out,
> @@ -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;
Do we have any strong preference for US vs. UK spelling in user-visible
messages? (I don't, but someone else might)
> }
> }
> @@ -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);
Use of `' quoting is unusual these days; if it were me, I'd use this as
a chance to switch to '' quoting.
> @@ -3005,8 +3006,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);
> }
error_report_err() is nicer than manually calling
error_report(error_get_pretty(err))
> @@ -3999,8 +3995,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!");
Indentation looks off.
> @@ -4164,12 +4160,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");
s/can not/cannot/ while touching this
> 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");
and again
Whether you squash in those fixes, roll a v2, or send a followup, feel
free to add:
Reviewed-by: Eric Blake <ebl...@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
