[v3] Make MachineClass::is_default boolean, refuse multiple default machines

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

Philippe Mathieu-Daudé posted 3 patches 6 years ago

Diff against v4
Download mbox

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

There is a newer version of this series

Expand all Fold all

[PATCH v3 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é)

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

diff --git a/vl.c b/vl.c
index 7dcb0879c4..b3e0f7af25 100644
--- a/vl.c
+++ b/vl.c
@@ -2354,6 +2354,8 @@ static MachineClass *machine_parse(const char *name, GSList *machines)
     GSList *el;
 
     if (is_help_option(name)) {
+        int default_count = 0;
+
         printf("Supported machines are:\n");
         machines = g_slist_sort(machines, machine_class_cmp);
         for (el = machines; el; el = el->next) {
@@ -2364,7 +2366,9 @@ static MachineClass *machine_parse(const char *name, GSList *machines)
             printf("%-20s %s%s%s\n", mc->name, mc->desc,
                    mc->is_default ? " (default)" : "",
                    mc->deprecation_reason ? " (deprecated)" : "");
+            default_count += !!mc->is_default;
         }
+        assert(default_count <= 1);
         exit(0);
     }
 
-- 
2.21.1

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

Posted by Michael S. Tsirkin 6 years ago

On Fri, Feb 07, 2020 at 02:04:53PM +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>
> ---
> v2: Use assert() instead of human friendly message (Marc-André)
> 
> Cc: Marc-André Lureau <marcandre.lureau@gmail.com>
> Cc: Laurent Vivier <laurent@vivier.eu>

Is it really worth adding counting logic just so we can assert?
There are a bunch of other places that deal with
default, like qmp query machines and find_default_machine.

> ---
>  vl.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/vl.c b/vl.c
> index 7dcb0879c4..b3e0f7af25 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2354,6 +2354,8 @@ static MachineClass *machine_parse(const char *name, GSList *machines)
>      GSList *el;
>  
>      if (is_help_option(name)) {
> +        int default_count = 0;
> +
>          printf("Supported machines are:\n");
>          machines = g_slist_sort(machines, machine_class_cmp);
>          for (el = machines; el; el = el->next) {
> @@ -2364,7 +2366,9 @@ static MachineClass *machine_parse(const char *name, GSList *machines)
>              printf("%-20s %s%s%s\n", mc->name, mc->desc,
>                     mc->is_default ? " (default)" : "",
>                     mc->deprecation_reason ? " (deprecated)" : "");
> +            default_count += !!mc->is_default;
>          }
> +        assert(default_count <= 1);
>          exit(0);
>      }
>  
> -- 
> 2.21.1

[PATCH v3 1/3] hw: Do not initialize MachineClass::is_default to 0
[PATCH v3 2/3] hw: Make MachineClass::is_default a boolean type
[PATCH v3 3/3] vl: Abort if multiple machines are registered as default