| Message ID | 0e2ed0a9f48b8e6bbfbfa127e2e92c3d01b591fd.1375251985.git.minovotn@redhat.com |
|---|---|
| State | New |
| Headers | show |
Michal Novotny <minovotn@redhat.com> writes: > Output error message to stderr when user provides the invalid machine > type on the command line. This also saves time to find what issue is > when you downgrade from one version of qemu to another that doesn't > support required machine type yet (the version user downgraded to have > to have this patch applied too, of course). > > Signed-off-by: Michal Novotny <minovotn@redhat.com> > --- > vl.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/vl.c b/vl.c > index f422a1c..6ee1a03 100644 > --- a/vl.c > +++ b/vl.c > @@ -2671,6 +2671,11 @@ static QEMUMachine *machine_parse(const char *name) > if (machine) { > return machine; > } > + > + if (name && !is_help_option(name)) { > + fprintf(stderr, "Error: Unsupported machine type '%s'\n", name); > + } > + > printf("Supported machines are:\n"); > for (m = first_machine; m != NULL; m = m->next) { > if (m->alias) { Sorry, should've looked more closely. Please use error_report() like this: error_report("Unsupported machine type"); Improves the message from Error: Unsupported machine type 'HAL-9000' to the standard form qemu-system-x86_64: -M HAL-9000: Unsupported machine type
On 07/31/2013 08:45 AM, Markus Armbruster wrote: > Michal Novotny <minovotn@redhat.com> writes: > >> Output error message to stderr when user provides the invalid machine >> type on the command line. This also saves time to find what issue is >> when you downgrade from one version of qemu to another that doesn't >> support required machine type yet (the version user downgraded to have >> to have this patch applied too, of course). >> >> Signed-off-by: Michal Novotny <minovotn@redhat.com> >> --- >> vl.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/vl.c b/vl.c >> index f422a1c..6ee1a03 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -2671,6 +2671,11 @@ static QEMUMachine *machine_parse(const char *name) >> if (machine) { >> return machine; >> } >> + >> + if (name && !is_help_option(name)) { >> + fprintf(stderr, "Error: Unsupported machine type '%s'\n", name); >> + } >> + >> printf("Supported machines are:\n"); >> for (m = first_machine; m != NULL; m = m->next) { >> if (m->alias) { > Sorry, should've looked more closely. Please use error_report() like > this: > > error_report("Unsupported machine type"); > > Improves the message from > > Error: Unsupported machine type 'HAL-9000' > > to the standard form > > qemu-system-x86_64: -M HAL-9000: Unsupported machine type That's nice qemu has this function, I didn't know. However, there are still some code parts that are not using error_report() as well, e.g. fprintf(stderr, "Error: standard VGA not available\n"); in the select_vgahw() function (line 2168) or: fprintf(stderr, "Failed to start VNC server on `%s': %s\n", vnc_display, error_get_pretty(local_err)); on line 4385 or: fprintf(stderr, "-incoming %s: %s\n", incoming, error_get_pretty(local_err)); on line 4428 (I've been looking just to vl.c file so far). Shouldn't all this be using error_report() as well? If so, a follow-up patch would be nice. Thanks, Michal
On 07/31/2013 08:45 AM, Markus Armbruster wrote: > Michal Novotny <minovotn@redhat.com> writes: > >> Output error message to stderr when user provides the invalid machine >> type on the command line. This also saves time to find what issue is >> when you downgrade from one version of qemu to another that doesn't >> support required machine type yet (the version user downgraded to have >> to have this patch applied too, of course). >> >> Signed-off-by: Michal Novotny <minovotn@redhat.com> >> --- >> vl.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/vl.c b/vl.c >> index f422a1c..6ee1a03 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -2671,6 +2671,11 @@ static QEMUMachine *machine_parse(const char *name) >> if (machine) { >> return machine; >> } >> + >> + if (name && !is_help_option(name)) { >> + fprintf(stderr, "Error: Unsupported machine type '%s'\n", name); >> + } >> + >> printf("Supported machines are:\n"); >> for (m = first_machine; m != NULL; m = m->next) { >> if (m->alias) { > Sorry, should've looked more closely. Please use error_report() like > this: > > error_report("Unsupported machine type"); > > Improves the message from > > Error: Unsupported machine type 'HAL-9000' > > to the standard form > > qemu-system-x86_64: -M HAL-9000: Unsupported machine type FYI: v5 has been sent using the error_report() Michal >
Michal Novotny <minovotn@redhat.com> writes: > On 07/31/2013 08:45 AM, Markus Armbruster wrote: >> Michal Novotny <minovotn@redhat.com> writes: >> >>> Output error message to stderr when user provides the invalid machine >>> type on the command line. This also saves time to find what issue is >>> when you downgrade from one version of qemu to another that doesn't >>> support required machine type yet (the version user downgraded to have >>> to have this patch applied too, of course). >>> >>> Signed-off-by: Michal Novotny <minovotn@redhat.com> >>> --- >>> vl.c | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/vl.c b/vl.c >>> index f422a1c..6ee1a03 100644 >>> --- a/vl.c >>> +++ b/vl.c >>> @@ -2671,6 +2671,11 @@ static QEMUMachine *machine_parse(const char *name) >>> if (machine) { >>> return machine; >>> } >>> + >>> + if (name && !is_help_option(name)) { >>> + fprintf(stderr, "Error: Unsupported machine type '%s'\n", name); >>> + } >>> + >>> printf("Supported machines are:\n"); >>> for (m = first_machine; m != NULL; m = m->next) { >>> if (m->alias) { >> Sorry, should've looked more closely. Please use error_report() like >> this: >> >> error_report("Unsupported machine type"); >> >> Improves the message from >> >> Error: Unsupported machine type 'HAL-9000' >> >> to the standard form >> >> qemu-system-x86_64: -M HAL-9000: Unsupported machine type > > That's nice qemu has this function, I didn't know. However, there are > still some code parts that are not using error_report() as well, e.g. > > fprintf(stderr, "Error: standard VGA not available\n"); > > in the select_vgahw() function (line 2168) or: > > fprintf(stderr, "Failed to start VNC server on `%s': %s\n", > vnc_display, error_get_pretty(local_err)); > > on line 4385 or: > > fprintf(stderr, "-incoming %s: %s\n", incoming, > error_get_pretty(local_err)); > > on line 4428 (I've been looking just to vl.c file so far). Lots of them, all over the place. > Shouldn't all this be using error_report() as well? If so, a follow-up > patch would be nice. As usual, patches welcome :)
Am 31.07.2013 08:54, schrieb Michal Novotny: > there are > still some code parts that are not using error_report() as well, e.g. > > fprintf(stderr, "Error: standard VGA not available\n"); > > in the select_vgahw() function (line 2168) or: > > fprintf(stderr, "Failed to start VNC server on `%s': %s\n", > vnc_display, error_get_pretty(local_err)); > > on line 4385 or: > > fprintf(stderr, "-incoming %s: %s\n", incoming, > error_get_pretty(local_err)); > > on line 4428 (I've been looking just to vl.c file so far). > > Shouldn't all this be using error_report() as well? If so, a follow-up > patch would be nice. See http://patchwork.ozlabs.org/patch/260801/ for a start. :) Regards, Andreas
diff --git a/vl.c b/vl.c index f422a1c..6ee1a03 100644 --- a/vl.c +++ b/vl.c @@ -2671,6 +2671,11 @@ static QEMUMachine *machine_parse(const char *name) if (machine) { return machine; } + + if (name && !is_help_option(name)) { + fprintf(stderr, "Error: Unsupported machine type '%s'\n", name); + } + printf("Supported machines are:\n"); for (m = first_machine; m != NULL; m = m->next) { if (m->alias) {
Output error message to stderr when user provides the invalid machine type on the command line. This also saves time to find what issue is when you downgrade from one version of qemu to another that doesn't support required machine type yet (the version user downgraded to have to have this patch applied too, of course). Signed-off-by: Michal Novotny <minovotn@redhat.com> --- vl.c | 5 +++++ 1 file changed, 5 insertions(+)