* [PATCH] vl: Abort if multiple machines are registered as default
@ 2020-02-07 11:02 Philippe Mathieu-Daudé
2020-02-07 11:08 ` Laurent Vivier
2020-02-07 11:10 ` Marc-André Lureau
0 siblings, 2 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-02-07 11:02 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-trivial, Paolo Bonzini, Philippe Mathieu-Daudé
It would be confusing to have multiple default machines.
Abort if this ever occurs.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
vl.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/vl.c b/vl.c
index 7dcb0879c4..da828188eb 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,6 +2366,11 @@ 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;
+ }
+ if (default_count > 1) {
+ error_printf("Multiple default machines available\n");
+ abort();
}
exit(0);
}
--
2.21.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] vl: Abort if multiple machines are registered as default
2020-02-07 11:02 [PATCH] vl: Abort if multiple machines are registered as default Philippe Mathieu-Daudé
@ 2020-02-07 11:08 ` Laurent Vivier
2020-02-07 11:25 ` Philippe Mathieu-Daudé
2020-02-07 11:10 ` Marc-André Lureau
1 sibling, 1 reply; 6+ messages in thread
From: Laurent Vivier @ 2020-02-07 11:08 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel; +Cc: qemu-trivial, Paolo Bonzini
Le 07/02/2020 à 12:02, 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>
> ---
> vl.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/vl.c b/vl.c
> index 7dcb0879c4..da828188eb 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,6 +2366,11 @@ 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;
> + }
> + if (default_count > 1) {
> + error_printf("Multiple default machines available\n");
> + abort();
> }
> exit(0);
> }
>
Does it really deserve an abort?
Ideal solution would be to be able to check this at build or in the unit
tests.
Thanks,
Laurent
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] vl: Abort if multiple machines are registered as default
2020-02-07 11:08 ` Laurent Vivier
@ 2020-02-07 11:25 ` Philippe Mathieu-Daudé
2020-02-07 11:28 ` Laurent Vivier
0 siblings, 1 reply; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-02-07 11:25 UTC (permalink / raw)
To: Laurent Vivier, qemu-devel; +Cc: qemu-trivial, Paolo Bonzini
On 2/7/20 12:08 PM, Laurent Vivier wrote:
> Le 07/02/2020 à 12:02, 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>
>> ---
>> vl.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/vl.c b/vl.c
>> index 7dcb0879c4..da828188eb 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,6 +2366,11 @@ 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;
>> + }
>> + if (default_count > 1) {
>> + error_printf("Multiple default machines available\n");
>> + abort();
>> }
>> exit(0);
>> }
>>
>
> Does it really deserve an abort?
> Ideal solution would be to be able to check this at build or in the unit
> tests.
This is for developers, not for users. I'll use Marc-André suggestion
and use an assertion.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] vl: Abort if multiple machines are registered as default
2020-02-07 11:25 ` Philippe Mathieu-Daudé
@ 2020-02-07 11:28 ` Laurent Vivier
2020-02-07 11:29 ` Laurent Vivier
0 siblings, 1 reply; 6+ messages in thread
From: Laurent Vivier @ 2020-02-07 11:28 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel; +Cc: qemu-trivial, Paolo Bonzini
Le 07/02/2020 à 12:25, Philippe Mathieu-Daudé a écrit :
> On 2/7/20 12:08 PM, Laurent Vivier wrote:
>> Le 07/02/2020 à 12:02, 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>
>>> ---
>>> vl.c | 7 +++++++
>>> 1 file changed, 7 insertions(+)
>>>
>>> diff --git a/vl.c b/vl.c
>>> index 7dcb0879c4..da828188eb 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,6 +2366,11 @@ 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;
>>> + }
>>> + if (default_count > 1) {
>>> + error_printf("Multiple default machines available\n");
>>> + abort();
>>> }
>>> exit(0);
>>> }
>>>
>>
>> Does it really deserve an abort?
>> Ideal solution would be to be able to check this at build or in the unit
>> tests.
>
> This is for developers, not for users. I'll use Marc-André suggestion
> and use an assertion.
I agree, but it's why it would be better if it is detected before it
comes to the user.
Perhaps you can add a test to run "qemu -h" to trigger the abort() early
in the develompment process (if it doesn't already exist).
Thanks,
Laurent
Thanks,
LAurent
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] vl: Abort if multiple machines are registered as default
2020-02-07 11:28 ` Laurent Vivier
@ 2020-02-07 11:29 ` Laurent Vivier
0 siblings, 0 replies; 6+ messages in thread
From: Laurent Vivier @ 2020-02-07 11:29 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel; +Cc: qemu-trivial, Paolo Bonzini
Le 07/02/2020 à 12:28, Laurent Vivier a écrit :
> Le 07/02/2020 à 12:25, Philippe Mathieu-Daudé a écrit :
>> On 2/7/20 12:08 PM, Laurent Vivier wrote:
>>> Le 07/02/2020 à 12:02, 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>
>>>> ---
>>>> vl.c | 7 +++++++
>>>> 1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/vl.c b/vl.c
>>>> index 7dcb0879c4..da828188eb 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,6 +2366,11 @@ 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;
>>>> + }
>>>> + if (default_count > 1) {
>>>> + error_printf("Multiple default machines available\n");
>>>> + abort();
>>>> }
>>>> exit(0);
>>>> }
>>>>
>>>
>>> Does it really deserve an abort?
>>> Ideal solution would be to be able to check this at build or in the unit
>>> tests.
>>
>> This is for developers, not for users. I'll use Marc-André suggestion
>> and use an assertion.
>
> I agree, but it's why it would be better if it is detected before it
> comes to the user.
>
> Perhaps you can add a test to run "qemu -h" to trigger the abort() early
> in the develompment process (if it doesn't already exist).
I mean "-M help".
Thanks,
Laurent
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] vl: Abort if multiple machines are registered as default
2020-02-07 11:02 [PATCH] vl: Abort if multiple machines are registered as default Philippe Mathieu-Daudé
2020-02-07 11:08 ` Laurent Vivier
@ 2020-02-07 11:10 ` Marc-André Lureau
1 sibling, 0 replies; 6+ messages in thread
From: Marc-André Lureau @ 2020-02-07 11:10 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: qemu trival, Paolo Bonzini, QEMU
Hi
On Fri, Feb 7, 2020 at 12:03 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>
> ---
> vl.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/vl.c b/vl.c
> index 7dcb0879c4..da828188eb 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,6 +2366,11 @@ 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;
> + }
> + if (default_count > 1) {
> + error_printf("Multiple default machines available\n");
> + abort();
looks ok
It's a build-time issue? If the user can't do anything about it, you
may simply have an assert(default_count <= 1) rather than a
human-friendly string, I think.
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-02-07 11:31 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-02-07 11:02 [PATCH] vl: Abort if multiple machines are registered as default Philippe Mathieu-Daudé
2020-02-07 11:08 ` Laurent Vivier
2020-02-07 11:25 ` Philippe Mathieu-Daudé
2020-02-07 11:28 ` Laurent Vivier
2020-02-07 11:29 ` Laurent Vivier
2020-02-07 11:10 ` Marc-André Lureau
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).