[PATCH v4 3/3] vl: Abort if multiple machines are registered as default

Philippe Mathieu-Daudé posted 3 patches 6 years ago
Maintainers: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Chris Wulff <crwulff@gmail.com>, Alistair Francis <Alistair.Francis@wdc.com>, Marek Vasut <marex@denx.de>, Anthony Green <green@moxielogic.com>, David Hildenbrand <david@redhat.com>, Aleksandar Markovic <amarkovic@wavecomp.com>, Magnus Damm <magnus.damm@gmail.com>, Bastian Koppelmann <kbastian@mail.uni-paderborn.de>, Guan Xuetao <gxt@mprc.pku.edu.cn>, Aleksandar Rikalo <aleksandar.rikalo@rt-rk.com>, Laurent Vivier <laurent@vivier.eu>, Helge Deller <deller@gmx.de>, Stafford Horne <shorne@gmail.com>, "Philippe Mathieu-Daudé" <philmd@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Michael Walle <michael@walle.cc>, Christian Borntraeger <borntraeger@de.ibm.com>, Sagar Karandikar <sagark@eecs.berkeley.edu>, Thomas Huth <huth@tuxfamily.org>, Aurelien Jarno <aurelien@aurel32.net>, Cornelia Huck <cohuck@redhat.com>, Richard Henderson <rth@twiddle.net>, Eduardo Habkost <ehabkost@redhat.com>, David Gibson <david@gibson.dropbear.id.au>, "Michael S. Tsirkin" <mst@redhat.com>, Halil Pasic <pasic@linux.ibm.com>, Artyom Tarasenko <atar4qemu@gmail.com>, Palmer Dabbelt <palmer@dabbelt.com>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Jia Liu <proljc@gmail.com>
[PATCH v4 3/3] vl: Abort if multiple machines are registered as default
Posted by Philippe Mathieu-Daudé 6 years ago
It would be confusing to have multiple default machines.
Abort if this ever occurs.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
v2: Use assert() instead of human friendly message (Marc-André)
v3: Move the check to find_machine() (Michael)

Cc: Marc-André Lureau <marcandre.lureau@gmail.com>
Cc: Laurent Vivier <laurent@vivier.eu>
Cc: Michael S. Tsirkin <mst@redhat.com>
---
 vl.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/vl.c b/vl.c
index 7dcb0879c4..ebc203af0d 100644
--- a/vl.c
+++ b/vl.c
@@ -1184,16 +1184,18 @@ static MachineClass *find_machine(const char *name, GSList *machines)
 static MachineClass *find_default_machine(GSList *machines)
 {
     GSList *el;
+    MachineClass *default_machineclass = NULL;
 
     for (el = machines; el; el = el->next) {
         MachineClass *mc = el->data;
 
         if (mc->is_default) {
-            return mc;
+            assert(default_machineclass == NULL && "Multiple default machines");
+            default_machineclass = mc;
         }
     }
 
-    return NULL;
+    return default_machineclass;
 }
 
 static int machine_help_func(QemuOpts *opts, MachineState *machine)
-- 
2.21.1


Re: [PATCH v4 3/3] vl: Abort if multiple machines are registered as default
Posted by Marc-André Lureau 6 years ago
On Fri, Feb 7, 2020 at 5:21 PM Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> It would be confusing to have multiple default machines.
> Abort if this ever occurs.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> v2: Use assert() instead of human friendly message (Marc-André)
> v3: Move the check to find_machine() (Michael)
>
> Cc: Marc-André Lureau <marcandre.lureau@gmail.com>
> Cc: Laurent Vivier <laurent@vivier.eu>
> Cc: Michael S. Tsirkin <mst@redhat.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  vl.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index 7dcb0879c4..ebc203af0d 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1184,16 +1184,18 @@ static MachineClass *find_machine(const char *name, GSList *machines)
>  static MachineClass *find_default_machine(GSList *machines)
>  {
>      GSList *el;
> +    MachineClass *default_machineclass = NULL;
>
>      for (el = machines; el; el = el->next) {
>          MachineClass *mc = el->data;
>
>          if (mc->is_default) {
> -            return mc;
> +            assert(default_machineclass == NULL && "Multiple default machines");
> +            default_machineclass = mc;
>          }
>      }
>
> -    return NULL;
> +    return default_machineclass;
>  }
>
>  static int machine_help_func(QemuOpts *opts, MachineState *machine)
> --
> 2.21.1
>


-- 
Marc-André Lureau

Re: [PATCH v4 3/3] vl: Abort if multiple machines are registered as default
Posted by Laurent Vivier 6 years ago
Le 07/02/2020 à 17:19, Philippe Mathieu-Daudé a écrit :
> It would be confusing to have multiple default machines.
> Abort if this ever occurs.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> v2: Use assert() instead of human friendly message (Marc-André)
> v3: Move the check to find_machine() (Michael)
> 
> Cc: Marc-André Lureau <marcandre.lureau@gmail.com>
> Cc: Laurent Vivier <laurent@vivier.eu>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> ---
>  vl.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/vl.c b/vl.c
> index 7dcb0879c4..ebc203af0d 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1184,16 +1184,18 @@ static MachineClass *find_machine(const char *name, GSList *machines)
>  static MachineClass *find_default_machine(GSList *machines)
>  {
>      GSList *el;
> +    MachineClass *default_machineclass = NULL;
>  
>      for (el = machines; el; el = el->next) {
>          MachineClass *mc = el->data;
>  
>          if (mc->is_default) {
> -            return mc;
> +            assert(default_machineclass == NULL && "Multiple default machines");
> +            default_machineclass = mc;
>          }
>      }
>  
> -    return NULL;
> +    return default_machineclass;
>  }
>  
>  static int machine_help_func(QemuOpts *opts, MachineState *machine)
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Tested-by: Laurent Vivier <laurent@vivier.eu>

I've tested "make check" actually fails when we have several default
machines.

Thanks,
Laurent

Re: [PATCH v4 3/3] vl: Abort if multiple machines are registered as default
Posted by David Gibson 6 years ago
On Fri, Feb 07, 2020 at 05:19:48PM +0100, Philippe Mathieu-Daudé wrote:
> It would be confusing to have multiple default machines.
> Abort if this ever occurs.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
> v2: Use assert() instead of human friendly message (Marc-André)
> v3: Move the check to find_machine() (Michael)
> 
> Cc: Marc-André Lureau <marcandre.lureau@gmail.com>
> Cc: Laurent Vivier <laurent@vivier.eu>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> ---
>  vl.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/vl.c b/vl.c
> index 7dcb0879c4..ebc203af0d 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1184,16 +1184,18 @@ static MachineClass *find_machine(const char *name, GSList *machines)
>  static MachineClass *find_default_machine(GSList *machines)
>  {
>      GSList *el;
> +    MachineClass *default_machineclass = NULL;
>  
>      for (el = machines; el; el = el->next) {
>          MachineClass *mc = el->data;
>  
>          if (mc->is_default) {
> -            return mc;
> +            assert(default_machineclass == NULL && "Multiple default machines");
> +            default_machineclass = mc;
>          }
>      }
>  
> -    return NULL;
> +    return default_machineclass;
>  }
>  
>  static int machine_help_func(QemuOpts *opts, MachineState *machine)

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson