diff mbox

accel: cleanup error output

Message ID 20170717132110.29962-1-lvivier@redhat.com
State New
Headers show

Commit Message

Laurent Vivier July 17, 2017, 1:21 p.m. UTC
Only emit "XXX accelerator not found", if there are not
further accelerators listed. eg

   accel=kvm:tcg

doesn't print a "KVM accelerator not found" warning
when it falls back to tcg, but a

   accel=kvm

prints a warning, since no fallback is given.

Suggested-by: Daniel P. Berrange <berrange@redhat.com>
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 accel/accel.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

Comments

Eric Blake July 17, 2017, 1:33 p.m. UTC | #1
On 07/17/2017 08:21 AM, Laurent Vivier wrote:
> Only emit "XXX accelerator not found", if there are not
> further accelerators listed. eg
> 
>    accel=kvm:tcg
> 
> doesn't print a "KVM accelerator not found" warning
> when it falls back to tcg, but a
> 
>    accel=kvm
> 
> prints a warning, since no fallback is given.
> 
> Suggested-by: Daniel P. Berrange <berrange@redhat.com>
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  accel/accel.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)


> @@ -110,7 +110,8 @@ void configure_accelerator(MachineState *ms)
>  
>      if (!accel_initialised) {
>          if (!init_failed) {
> -            fprintf(stderr, "No accelerator found!\n");
> +            fprintf(stderr, "-machine accel=%s: No accelerator found!\n",

It may be worth dropping the '!', as we tend not to shout at the user;
but whether or not you tweak that,

Reviewed-by: Eric Blake <eblake@redhat.com>
Thomas Huth July 17, 2017, 1:54 p.m. UTC | #2
On 17.07.2017 15:21, Laurent Vivier wrote:
> Only emit "XXX accelerator not found", if there are not
> further accelerators listed. eg
> 
>    accel=kvm:tcg
> 
> doesn't print a "KVM accelerator not found" warning
> when it falls back to tcg, but a
> 
>    accel=kvm
> 
> prints a warning, since no fallback is given.
> 
> Suggested-by: Daniel P. Berrange <berrange@redhat.com>
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  accel/accel.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/accel/accel.c b/accel/accel.c
> index fa85844..ababeb8 100644
> --- a/accel/accel.c
> +++ b/accel/accel.c
> @@ -69,19 +69,20 @@ static int accel_init_machine(AccelClass *acc, MachineState *ms)
>  
>  void configure_accelerator(MachineState *ms)
>  {
> -    const char *p;
> +    const char *accel, *p;
>      char buf[10];
>      int ret;
>      bool accel_initialised = false;
>      bool init_failed = false;
>      AccelClass *acc = NULL;
>  
> -    p = qemu_opt_get(qemu_get_machine_opts(), "accel");
> -    if (p == NULL) {
> +    accel = qemu_opt_get(qemu_get_machine_opts(), "accel");
> +    if (accel == NULL) {
>          /* Use the default "accelerator", tcg */
> -        p = "tcg";
> +        accel = "tcg";
>      }
>  
> +    p = accel;
>      while (!accel_initialised && *p != '\0') {
>          if (*p == ':') {
>              p++;
> @@ -89,7 +90,6 @@ void configure_accelerator(MachineState *ms)
>          p = get_opt_name(buf, sizeof(buf), p, ':');
>          acc = accel_find(buf);
>          if (!acc) {
> -            fprintf(stderr, "\"%s\" accelerator not found.\n", buf);
>              continue;
>          }
>          if (acc->available && !acc->available()) {
> @@ -110,7 +110,8 @@ void configure_accelerator(MachineState *ms)
>  
>      if (!accel_initialised) {
>          if (!init_failed) {
> -            fprintf(stderr, "No accelerator found!\n");
> +            fprintf(stderr, "-machine accel=%s: No accelerator found!\n",
> +                    accel);
>          }
>          exit(1);
>      }

Maybe use error_report() instead of fprintf(stderr, ...) ?

Apart from that, looks fine to me, so feel free to add my:

Reviewed-by: Thomas Huth <thuth@redhat.com>
diff mbox

Patch

diff --git a/accel/accel.c b/accel/accel.c
index fa85844..ababeb8 100644
--- a/accel/accel.c
+++ b/accel/accel.c
@@ -69,19 +69,20 @@  static int accel_init_machine(AccelClass *acc, MachineState *ms)
 
 void configure_accelerator(MachineState *ms)
 {
-    const char *p;
+    const char *accel, *p;
     char buf[10];
     int ret;
     bool accel_initialised = false;
     bool init_failed = false;
     AccelClass *acc = NULL;
 
-    p = qemu_opt_get(qemu_get_machine_opts(), "accel");
-    if (p == NULL) {
+    accel = qemu_opt_get(qemu_get_machine_opts(), "accel");
+    if (accel == NULL) {
         /* Use the default "accelerator", tcg */
-        p = "tcg";
+        accel = "tcg";
     }
 
+    p = accel;
     while (!accel_initialised && *p != '\0') {
         if (*p == ':') {
             p++;
@@ -89,7 +90,6 @@  void configure_accelerator(MachineState *ms)
         p = get_opt_name(buf, sizeof(buf), p, ':');
         acc = accel_find(buf);
         if (!acc) {
-            fprintf(stderr, "\"%s\" accelerator not found.\n", buf);
             continue;
         }
         if (acc->available && !acc->available()) {
@@ -110,7 +110,8 @@  void configure_accelerator(MachineState *ms)
 
     if (!accel_initialised) {
         if (!init_failed) {
-            fprintf(stderr, "No accelerator found!\n");
+            fprintf(stderr, "-machine accel=%s: No accelerator found!\n",
+                    accel);
         }
         exit(1);
     }