* [Qemu-devel] [PATCH 0/2] cleanup select_machine
@ 2019-03-11 6:08 Wei Yang
2019-03-11 6:08 ` [Qemu-devel] [PATCH 1/2] vl.c: make find_default_machine() local Wei Yang
` (3 more replies)
0 siblings, 4 replies; 20+ messages in thread
From: Wei Yang @ 2019-03-11 6:08 UTC (permalink / raw)
To: qemu-devel; +Cc: ehabkost, marcel.apfelbaum, pbonzini, Wei Yang
Here is two simple change related to select_machine()
[1]: make find_default_machine() local since no one outside file use it
[2]: allocate TYPE_MACHINE list only once to select machine type
Wei Yang (2):
vl.c: make find_default_machine() local
vl.c: allocate TYPE_MACHINE list once during bootup
include/hw/boards.h | 1 -
vl.c | 24 +++++++++++-------------
2 files changed, 11 insertions(+), 14 deletions(-)
--
2.19.1
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 1/2] vl.c: make find_default_machine() local
2019-03-11 6:08 [Qemu-devel] [PATCH 0/2] cleanup select_machine Wei Yang
@ 2019-03-11 6:08 ` Wei Yang
2019-04-02 6:33 ` Markus Armbruster
2019-03-11 6:08 ` [Qemu-devel] [PATCH 2/2] vl.c: allocate TYPE_MACHINE list once during bootup Wei Yang
` (2 subsequent siblings)
3 siblings, 1 reply; 20+ messages in thread
From: Wei Yang @ 2019-03-11 6:08 UTC (permalink / raw)
To: qemu-devel; +Cc: ehabkost, marcel.apfelbaum, pbonzini, Wei Yang
Function find_default_machine() is introduced by commit 2c8cffa599b7
"vl: make find_default_machine externally visible", while it seems no
one outside use it.
This patch make it local again.
Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
---
include/hw/boards.h | 1 -
vl.c | 4 ++--
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 21212f0859..e911d56c28 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -57,7 +57,6 @@ void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
#define MACHINE_CLASS(klass) \
OBJECT_CLASS_CHECK(MachineClass, (klass), TYPE_MACHINE)
-MachineClass *find_default_machine(void);
extern MachineState *current_machine;
void machine_run_board_init(MachineState *machine);
diff --git a/vl.c b/vl.c
index 502857a176..3688e2bc98 100644
--- a/vl.c
+++ b/vl.c
@@ -1441,7 +1441,7 @@ static MachineClass *find_machine(const char *name)
return mc;
}
-MachineClass *find_default_machine(void)
+static MachineClass *find_default_machine(void)
{
GSList *el, *machines = object_class_get_list(TYPE_MACHINE, false);
MachineClass *mc = NULL;
@@ -2538,7 +2538,7 @@ static gint machine_class_cmp(gconstpointer a, gconstpointer b)
object_class_get_name(OBJECT_CLASS(mc1)));
}
- static MachineClass *machine_parse(const char *name)
+static MachineClass *machine_parse(const char *name)
{
MachineClass *mc = NULL;
GSList *el, *machines = object_class_get_list(TYPE_MACHINE, false);
--
2.19.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 2/2] vl.c: allocate TYPE_MACHINE list once during bootup
2019-03-11 6:08 [Qemu-devel] [PATCH 0/2] cleanup select_machine Wei Yang
2019-03-11 6:08 ` [Qemu-devel] [PATCH 1/2] vl.c: make find_default_machine() local Wei Yang
@ 2019-03-11 6:08 ` Wei Yang
2019-04-02 13:28 ` Markus Armbruster
2019-04-02 13:26 ` [Qemu-devel] [PATCH 3/4] vl: Clean up after previous commit Markus Armbruster
2019-04-02 13:26 ` [Qemu-devel] [PATCH 4/4] vl: Simplify machine_parse() Markus Armbruster
3 siblings, 1 reply; 20+ messages in thread
From: Wei Yang @ 2019-03-11 6:08 UTC (permalink / raw)
To: qemu-devel; +Cc: ehabkost, marcel.apfelbaum, pbonzini, Wei Yang
Now all the functions used to select machine is local and the call flow
looks like below:
select_machine()
find_default_machine()
machine_parse()
find_machine()
All these related function will need a GSList for TYPE_MACHINE.
Currently we allocate this list each time we use it, while this is not
necessary to do so because we don't need to modify this.
This patch make the TYPE_MACHINE list allocation in select_machine and
pass this to its child for use.
Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
---
vl.c | 24 +++++++++++-------------
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/vl.c b/vl.c
index 3688e2bc98..cf08d96ce4 100644
--- a/vl.c
+++ b/vl.c
@@ -1418,9 +1418,9 @@ static int usb_parse(const char *cmdline)
MachineState *current_machine;
-static MachineClass *find_machine(const char *name)
+static MachineClass *find_machine(const char *name, GSList *machines)
{
- GSList *el, *machines = object_class_get_list(TYPE_MACHINE, false);
+ GSList *el;
MachineClass *mc = NULL;
for (el = machines; el; el = el->next) {
@@ -1437,13 +1437,12 @@ static MachineClass *find_machine(const char *name)
}
}
- g_slist_free(machines);
return mc;
}
-static MachineClass *find_default_machine(void)
+static MachineClass *find_default_machine(GSList *machines)
{
- GSList *el, *machines = object_class_get_list(TYPE_MACHINE, false);
+ GSList *el;
MachineClass *mc = NULL;
for (el = machines; el; el = el->next) {
@@ -1455,7 +1454,6 @@ static MachineClass *find_default_machine(void)
}
}
- g_slist_free(machines);
return mc;
}
@@ -2538,16 +2536,15 @@ static gint machine_class_cmp(gconstpointer a, gconstpointer b)
object_class_get_name(OBJECT_CLASS(mc1)));
}
-static MachineClass *machine_parse(const char *name)
+static MachineClass *machine_parse(const char *name, GSList *machines)
{
MachineClass *mc = NULL;
- GSList *el, *machines = object_class_get_list(TYPE_MACHINE, false);
+ GSList *el;
if (name) {
- mc = find_machine(name);
+ mc = find_machine(name, machines);
}
if (mc) {
- g_slist_free(machines);
return mc;
}
if (name && !is_help_option(name)) {
@@ -2567,7 +2564,6 @@ static MachineClass *machine_parse(const char *name)
}
}
- g_slist_free(machines);
exit(!name || !is_help_option(name));
}
@@ -2659,7 +2655,8 @@ static const QEMUOption *lookup_opt(int argc, char **argv,
static MachineClass *select_machine(void)
{
- MachineClass *machine_class = find_default_machine();
+ GSList *machines = object_class_get_list(TYPE_MACHINE, false);
+ MachineClass *machine_class = find_default_machine(machines);
const char *optarg;
QemuOpts *opts;
Location loc;
@@ -2671,7 +2668,7 @@ static MachineClass *select_machine(void)
optarg = qemu_opt_get(opts, "type");
if (optarg) {
- machine_class = machine_parse(optarg);
+ machine_class = machine_parse(optarg, machines);
}
if (!machine_class) {
@@ -2681,6 +2678,7 @@ static MachineClass *select_machine(void)
}
loc_pop(&loc);
+ g_slist_free(machines);
return machine_class;
}
--
2.19.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] vl.c: make find_default_machine() local
2019-03-11 6:08 ` [Qemu-devel] [PATCH 1/2] vl.c: make find_default_machine() local Wei Yang
@ 2019-04-02 6:33 ` Markus Armbruster
2019-04-02 15:07 ` Wei Yang
0 siblings, 1 reply; 20+ messages in thread
From: Markus Armbruster @ 2019-04-02 6:33 UTC (permalink / raw)
To: Wei Yang; +Cc: qemu-devel, pbonzini, ehabkost
Wei Yang <richardw.yang@linux.intel.com> writes:
> Function find_default_machine() is introduced by commit 2c8cffa599b7
> "vl: make find_default_machine externally visible", while it seems no
> one outside use it.
It was used outside of vl.c until commit a904410af5f.
> This patch make it local again.
Suggest:
Commit a904410af5f removed the only user of find_default_machine()
outside vl.c, but neglected to make it static. Do that now.
> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
> ---
> include/hw/boards.h | 1 -
> vl.c | 4 ++--
> 2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 21212f0859..e911d56c28 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -57,7 +57,6 @@ void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
> #define MACHINE_CLASS(klass) \
> OBJECT_CLASS_CHECK(MachineClass, (klass), TYPE_MACHINE)
>
> -MachineClass *find_default_machine(void);
> extern MachineState *current_machine;
>
> void machine_run_board_init(MachineState *machine);
> diff --git a/vl.c b/vl.c
> index 502857a176..3688e2bc98 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1441,7 +1441,7 @@ static MachineClass *find_machine(const char *name)
> return mc;
> }
>
> -MachineClass *find_default_machine(void)
> +static MachineClass *find_default_machine(void)
> {
> GSList *el, *machines = object_class_get_list(TYPE_MACHINE, false);
> MachineClass *mc = NULL;
> @@ -2538,7 +2538,7 @@ static gint machine_class_cmp(gconstpointer a, gconstpointer b)
> object_class_get_name(OBJECT_CLASS(mc1)));
> }
>
> - static MachineClass *machine_parse(const char *name)
> +static MachineClass *machine_parse(const char *name)
> {
> MachineClass *mc = NULL;
> GSList *el, *machines = object_class_get_list(TYPE_MACHINE, false);
Not related to this patch's stated purpose. Should go into PATCH 2,
where you're changing this line anyway.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 3/4] vl: Clean up after previous commit
2019-03-11 6:08 [Qemu-devel] [PATCH 0/2] cleanup select_machine Wei Yang
2019-03-11 6:08 ` [Qemu-devel] [PATCH 1/2] vl.c: make find_default_machine() local Wei Yang
2019-03-11 6:08 ` [Qemu-devel] [PATCH 2/2] vl.c: allocate TYPE_MACHINE list once during bootup Wei Yang
@ 2019-04-02 13:26 ` Markus Armbruster
2019-04-03 22:10 ` Wei Yang
2019-04-02 13:26 ` [Qemu-devel] [PATCH 4/4] vl: Simplify machine_parse() Markus Armbruster
3 siblings, 1 reply; 20+ messages in thread
From: Markus Armbruster @ 2019-04-02 13:26 UTC (permalink / raw)
To: qemu-devel; +Cc: richardw.yang, pbonzini, ehabkost
Since the previous commit, find_machine() and find_default_machine()
don't have to deallocate on return. This permits further
simplifications.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
vl.c | 25 +++++++------------------
1 file changed, 7 insertions(+), 18 deletions(-)
diff --git a/vl.c b/vl.c
index 126081f595..6a31e5bfac 100644
--- a/vl.c
+++ b/vl.c
@@ -1467,40 +1467,29 @@ MachineState *current_machine;
static MachineClass *find_machine(const char *name, GSList *machines)
{
GSList *el;
- MachineClass *mc = NULL;
for (el = machines; el; el = el->next) {
- MachineClass *temp = el->data;
+ MachineClass *mc = el->data;
- if (!strcmp(temp->name, name)) {
- mc = temp;
- break;
- }
- if (temp->alias &&
- !strcmp(temp->alias, name)) {
- mc = temp;
- break;
+ if (!strcmp(mc->name, name) || !g_strcmp0(mc->alias, name)) {
+ return mc;
}
}
- return mc;
+ return NULL;
}
static MachineClass *find_default_machine(GSList *machines)
{
GSList *el;
- MachineClass *mc = NULL;
for (el = machines; el; el = el->next) {
- MachineClass *temp = el->data;
-
- if (temp->is_default) {
- mc = temp;
- break;
+ if (((MachineClass *)el->data)->is_default) {
+ return el->data;
}
}
- return mc;
+ return NULL;
}
MachineInfoList *qmp_query_machines(Error **errp)
--
2.17.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 4/4] vl: Simplify machine_parse()
2019-03-11 6:08 [Qemu-devel] [PATCH 0/2] cleanup select_machine Wei Yang
` (2 preceding siblings ...)
2019-04-02 13:26 ` [Qemu-devel] [PATCH 3/4] vl: Clean up after previous commit Markus Armbruster
@ 2019-04-02 13:26 ` Markus Armbruster
2019-04-03 22:32 ` Wei Yang
3 siblings, 1 reply; 20+ messages in thread
From: Markus Armbruster @ 2019-04-02 13:26 UTC (permalink / raw)
To: qemu-devel; +Cc: richardw.yang, pbonzini, ehabkost
Exploit that argument @name is nerver null. Check is_help_option()
first, because that's what we do elsewhere.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
vl.c | 24 +++++++++++-------------
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/vl.c b/vl.c
index 6a31e5bfac..da1af3e10d 100644
--- a/vl.c
+++ b/vl.c
@@ -2573,19 +2573,10 @@ static gint machine_class_cmp(gconstpointer a, gconstpointer b)
static MachineClass *machine_parse(const char *name, GSList *machines)
{
- MachineClass *mc = NULL;
+ MachineClass *mc;
GSList *el;
- if (name) {
- mc = find_machine(name, machines);
- }
- if (mc) {
- return mc;
- }
- if (name && !is_help_option(name)) {
- error_report("unsupported machine type");
- error_printf("Use -machine help to list supported machines\n");
- } else {
+ if (is_help_option(name)) {
printf("Supported machines are:\n");
machines = g_slist_sort(machines, machine_class_cmp);
for (el = machines; el; el = el->next) {
@@ -2597,9 +2588,16 @@ static MachineClass *machine_parse(const char *name, GSList *machines)
mc->is_default ? " (default)" : "",
mc->deprecation_reason ? " (deprecated)" : "");
}
+ exit(0);
}
-
- exit(!name || !is_help_option(name));
+
+ mc = find_machine(name, machines);
+ if (!mc) {
+ error_report("unsupported machine type");
+ error_printf("Use -machine help to list supported machines\n");
+ exit(1);
+ }
+ return mc;
}
void qemu_add_exit_notifier(Notifier *notify)
--
2.17.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] vl.c: allocate TYPE_MACHINE list once during bootup
2019-03-11 6:08 ` [Qemu-devel] [PATCH 2/2] vl.c: allocate TYPE_MACHINE list once during bootup Wei Yang
@ 2019-04-02 13:28 ` Markus Armbruster
2019-04-02 15:16 ` Wei Yang
0 siblings, 1 reply; 20+ messages in thread
From: Markus Armbruster @ 2019-04-02 13:28 UTC (permalink / raw)
To: Wei Yang; +Cc: qemu-devel, pbonzini, ehabkost
Wei Yang <richardw.yang@linux.intel.com> writes:
> Now all the functions used to select machine is local and the call flow
> looks like below:
>
> select_machine()
> find_default_machine()
> machine_parse()
> find_machine()
>
> All these related function will need a GSList for TYPE_MACHINE.
> Currently we allocate this list each time we use it, while this is not
> necessary to do so because we don't need to modify this.
>
> This patch make the TYPE_MACHINE list allocation in select_machine and
> pass this to its child for use.
>
> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
> ---
> vl.c | 24 +++++++++++-------------
> 1 file changed, 11 insertions(+), 13 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index 3688e2bc98..cf08d96ce4 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1418,9 +1418,9 @@ static int usb_parse(const char *cmdline)
>
> MachineState *current_machine;
>
> -static MachineClass *find_machine(const char *name)
> +static MachineClass *find_machine(const char *name, GSList *machines)
> {
> - GSList *el, *machines = object_class_get_list(TYPE_MACHINE, false);
> + GSList *el;
> MachineClass *mc = NULL;
>
> for (el = machines; el; el = el->next) {
> @@ -1437,13 +1437,12 @@ static MachineClass *find_machine(const char *name)
> }
> }
>
> - g_slist_free(machines);
> return mc;
> }
Can be simplified further. I'll post it as PATCH 3.
>
> -static MachineClass *find_default_machine(void)
> +static MachineClass *find_default_machine(GSList *machines)
> {
> - GSList *el, *machines = object_class_get_list(TYPE_MACHINE, false);
> + GSList *el;
> MachineClass *mc = NULL;
>
> for (el = machines; el; el = el->next) {
> @@ -1455,7 +1454,6 @@ static MachineClass *find_default_machine(void)
> }
> }
>
> - g_slist_free(machines);
> return mc;
> }
Likewise.
>
> @@ -2538,16 +2536,15 @@ static gint machine_class_cmp(gconstpointer a, gconstpointer b)
> object_class_get_name(OBJECT_CLASS(mc1)));
> }
>
> -static MachineClass *machine_parse(const char *name)
> +static MachineClass *machine_parse(const char *name, GSList *machines)
> {
> MachineClass *mc = NULL;
> - GSList *el, *machines = object_class_get_list(TYPE_MACHINE, false);
> + GSList *el;
>
> if (name) {
> - mc = find_machine(name);
> + mc = find_machine(name, machines);
> }
> if (mc) {
> - g_slist_free(machines);
> return mc;
> }
> if (name && !is_help_option(name)) {
> @@ -2567,7 +2564,6 @@ static MachineClass *machine_parse(const char *name)
> }
> }
>
> - g_slist_free(machines);
> exit(!name || !is_help_option(name));
> }
Additional cleanup is possible: argument @name is never null.
While there, I'd check is_help_option() first rather than only after
find_machine() returns null, because "first" is what we commonly do.
I'll post this as PATCH 4.
>
> @@ -2659,7 +2655,8 @@ static const QEMUOption *lookup_opt(int argc, char **argv,
>
> static MachineClass *select_machine(void)
> {
> - MachineClass *machine_class = find_default_machine();
> + GSList *machines = object_class_get_list(TYPE_MACHINE, false);
> + MachineClass *machine_class = find_default_machine(machines);
> const char *optarg;
> QemuOpts *opts;
> Location loc;
> @@ -2671,7 +2668,7 @@ static MachineClass *select_machine(void)
>
> optarg = qemu_opt_get(opts, "type");
> if (optarg) {
> - machine_class = machine_parse(optarg);
> + machine_class = machine_parse(optarg, machines);
Could create and destroy @machines here:
- machine_class = machine_parse(optarg);
+ GSList *machines = object_class_get_list(TYPE_MACHINE, false);
+ machine_class = machine_parse(optarg, machines);
+ g_slist_free(machines);
Matter of taste.
> }
>
> if (!machine_class) {
> @@ -2681,6 +2678,7 @@ static MachineClass *select_machine(void)
> }
>
> loc_pop(&loc);
> + g_slist_free(machines);
> return machine_class;
> }
Reviewed-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] vl.c: make find_default_machine() local
2019-04-02 6:33 ` Markus Armbruster
@ 2019-04-02 15:07 ` Wei Yang
2019-04-02 15:17 ` Markus Armbruster
0 siblings, 1 reply; 20+ messages in thread
From: Wei Yang @ 2019-04-02 15:07 UTC (permalink / raw)
To: Markus Armbruster; +Cc: Wei Yang, pbonzini, qemu-devel, ehabkost
On Tue, Apr 02, 2019 at 08:33:08AM +0200, Markus Armbruster wrote:
>Wei Yang <richardw.yang@linux.intel.com> writes:
>
>> Function find_default_machine() is introduced by commit 2c8cffa599b7
>> "vl: make find_default_machine externally visible", while it seems no
>> one outside use it.
>It was used outside of vl.c until commit a904410af5f.
>
>> This patch make it local again.
>
>Suggest:
>
> Commit a904410af5f removed the only user of find_default_machine()
> outside vl.c, but neglected to make it static. Do that now.
>
Markus
Thanks for your comments.
BTW, I think I need to spin a v2, right?
>> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
>> ---
>> include/hw/boards.h | 1 -
>> vl.c | 4 ++--
>> 2 files changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/hw/boards.h b/include/hw/boards.h
>> index 21212f0859..e911d56c28 100644
>> --- a/include/hw/boards.h
>> +++ b/include/hw/boards.h
>> @@ -57,7 +57,6 @@ void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
>> #define MACHINE_CLASS(klass) \
>> OBJECT_CLASS_CHECK(MachineClass, (klass), TYPE_MACHINE)
>>
>> -MachineClass *find_default_machine(void);
>> extern MachineState *current_machine;
>>
>> void machine_run_board_init(MachineState *machine);
>> diff --git a/vl.c b/vl.c
>> index 502857a176..3688e2bc98 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -1441,7 +1441,7 @@ static MachineClass *find_machine(const char *name)
>> return mc;
>> }
>>
>> -MachineClass *find_default_machine(void)
>> +static MachineClass *find_default_machine(void)
>> {
>> GSList *el, *machines = object_class_get_list(TYPE_MACHINE, false);
>> MachineClass *mc = NULL;
>> @@ -2538,7 +2538,7 @@ static gint machine_class_cmp(gconstpointer a, gconstpointer b)
>> object_class_get_name(OBJECT_CLASS(mc1)));
>> }
>>
>> - static MachineClass *machine_parse(const char *name)
>> +static MachineClass *machine_parse(const char *name)
>> {
>> MachineClass *mc = NULL;
>> GSList *el, *machines = object_class_get_list(TYPE_MACHINE, false);
>
>Not related to this patch's stated purpose. Should go into PATCH 2,
>where you're changing this line anyway.
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] vl.c: allocate TYPE_MACHINE list once during bootup
2019-04-02 13:28 ` Markus Armbruster
@ 2019-04-02 15:16 ` Wei Yang
2019-04-02 16:10 ` Markus Armbruster
0 siblings, 1 reply; 20+ messages in thread
From: Wei Yang @ 2019-04-02 15:16 UTC (permalink / raw)
To: Markus Armbruster; +Cc: Wei Yang, pbonzini, qemu-devel, ehabkost
On Tue, Apr 02, 2019 at 03:28:48PM +0200, Markus Armbruster wrote:
>Wei Yang <richardw.yang@linux.intel.com> writes:
>
>> Now all the functions used to select machine is local and the call flow
>> looks like below:
>>
>> select_machine()
>> find_default_machine()
>> machine_parse()
>> find_machine()
>>
>> All these related function will need a GSList for TYPE_MACHINE.
>> Currently we allocate this list each time we use it, while this is not
>> necessary to do so because we don't need to modify this.
>>
>> This patch make the TYPE_MACHINE list allocation in select_machine and
>> pass this to its child for use.
>>
>> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
>> ---
>> vl.c | 24 +++++++++++-------------
>> 1 file changed, 11 insertions(+), 13 deletions(-)
>>
>> diff --git a/vl.c b/vl.c
>> index 3688e2bc98..cf08d96ce4 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -1418,9 +1418,9 @@ static int usb_parse(const char *cmdline)
>>
>> MachineState *current_machine;
>>
>> -static MachineClass *find_machine(const char *name)
>> +static MachineClass *find_machine(const char *name, GSList *machines)
>> {
>> - GSList *el, *machines = object_class_get_list(TYPE_MACHINE, false);
>> + GSList *el;
>> MachineClass *mc = NULL;
>>
>> for (el = machines; el; el = el->next) {
>> @@ -1437,13 +1437,12 @@ static MachineClass *find_machine(const char *name)
>> }
>> }
>>
>> - g_slist_free(machines);
>> return mc;
>> }
>
>Can be simplified further. I'll post it as PATCH 3.
>
Markus
Thanks for your comment. Do you sugest to simplify it in this patch? I
am confused here.
>>
>> -static MachineClass *find_default_machine(void)
>> +static MachineClass *find_default_machine(GSList *machines)
>> {
>> - GSList *el, *machines = object_class_get_list(TYPE_MACHINE, false);
>> + GSList *el;
>> MachineClass *mc = NULL;
>>
>> for (el = machines; el; el = el->next) {
>> @@ -1455,7 +1454,6 @@ static MachineClass *find_default_machine(void)
>> }
>> }
>>
>> - g_slist_free(machines);
>> return mc;
>> }
>
>Likewise.
>
>>
>> @@ -2538,16 +2536,15 @@ static gint machine_class_cmp(gconstpointer a, gconstpointer b)
>> object_class_get_name(OBJECT_CLASS(mc1)));
>> }
>>
>> -static MachineClass *machine_parse(const char *name)
>> +static MachineClass *machine_parse(const char *name, GSList *machines)
>> {
>> MachineClass *mc = NULL;
>> - GSList *el, *machines = object_class_get_list(TYPE_MACHINE, false);
>> + GSList *el;
>>
>> if (name) {
>> - mc = find_machine(name);
>> + mc = find_machine(name, machines);
>> }
>> if (mc) {
>> - g_slist_free(machines);
>> return mc;
>> }
>> if (name && !is_help_option(name)) {
>> @@ -2567,7 +2564,6 @@ static MachineClass *machine_parse(const char *name)
>> }
>> }
>>
>> - g_slist_free(machines);
>> exit(!name || !is_help_option(name));
>> }
>
>Additional cleanup is possible: argument @name is never null.
>
>While there, I'd check is_help_option() first rather than only after
>find_machine() returns null, because "first" is what we commonly do.
>
>I'll post this as PATCH 4.
>
>>
>> @@ -2659,7 +2655,8 @@ static const QEMUOption *lookup_opt(int argc, char **argv,
>>
>> static MachineClass *select_machine(void)
>> {
>> - MachineClass *machine_class = find_default_machine();
>> + GSList *machines = object_class_get_list(TYPE_MACHINE, false);
>> + MachineClass *machine_class = find_default_machine(machines);
>> const char *optarg;
>> QemuOpts *opts;
>> Location loc;
>> @@ -2671,7 +2668,7 @@ static MachineClass *select_machine(void)
>>
>> optarg = qemu_opt_get(opts, "type");
>> if (optarg) {
>> - machine_class = machine_parse(optarg);
>> + machine_class = machine_parse(optarg, machines);
>
>Could create and destroy @machines here:
>
> - machine_class = machine_parse(optarg);
> + GSList *machines = object_class_get_list(TYPE_MACHINE, false);
> + machine_class = machine_parse(optarg, machines);
> + g_slist_free(machines);
>
>Matter of taste.
>
>> }
>>
>> if (!machine_class) {
>> @@ -2681,6 +2678,7 @@ static MachineClass *select_machine(void)
>> }
>>
>> loc_pop(&loc);
>> + g_slist_free(machines);
>> return machine_class;
>> }
>
>Reviewed-by: Markus Armbruster <armbru@redhat.com>
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] vl.c: make find_default_machine() local
2019-04-02 15:07 ` Wei Yang
@ 2019-04-02 15:17 ` Markus Armbruster
0 siblings, 0 replies; 20+ messages in thread
From: Markus Armbruster @ 2019-04-02 15:17 UTC (permalink / raw)
To: Wei Yang; +Cc: pbonzini, Wei Yang, ehabkost, qemu-devel
Wei Yang <richard.weiyang@gmail.com> writes:
> On Tue, Apr 02, 2019 at 08:33:08AM +0200, Markus Armbruster wrote:
>>Wei Yang <richardw.yang@linux.intel.com> writes:
>>
>>> Function find_default_machine() is introduced by commit 2c8cffa599b7
>>> "vl: make find_default_machine externally visible", while it seems no
>>> one outside use it.
>>It was used outside of vl.c until commit a904410af5f.
>>
>>> This patch make it local again.
>>
>>Suggest:
>>
>> Commit a904410af5f removed the only user of find_default_machine()
>> outside vl.c, but neglected to make it static. Do that now.
>>
>
> Markus
>
> Thanks for your comments.
>
> BTW, I think I need to spin a v2, right?
A v2 with the review comments addressed and my two patches included
would help.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] vl.c: allocate TYPE_MACHINE list once during bootup
2019-04-02 15:16 ` Wei Yang
@ 2019-04-02 16:10 ` Markus Armbruster
2019-04-03 0:49 ` Wei Yang
0 siblings, 1 reply; 20+ messages in thread
From: Markus Armbruster @ 2019-04-02 16:10 UTC (permalink / raw)
To: Wei Yang; +Cc: pbonzini, Wei Yang, ehabkost, qemu-devel
Wei Yang <richard.weiyang@gmail.com> writes:
> On Tue, Apr 02, 2019 at 03:28:48PM +0200, Markus Armbruster wrote:
>>Wei Yang <richardw.yang@linux.intel.com> writes:
>>
>>> Now all the functions used to select machine is local and the call flow
>>> looks like below:
>>>
>>> select_machine()
>>> find_default_machine()
>>> machine_parse()
>>> find_machine()
>>>
>>> All these related function will need a GSList for TYPE_MACHINE.
>>> Currently we allocate this list each time we use it, while this is not
>>> necessary to do so because we don't need to modify this.
>>>
>>> This patch make the TYPE_MACHINE list allocation in select_machine and
>>> pass this to its child for use.
>>>
>>> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
>>> ---
>>> vl.c | 24 +++++++++++-------------
>>> 1 file changed, 11 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/vl.c b/vl.c
>>> index 3688e2bc98..cf08d96ce4 100644
>>> --- a/vl.c
>>> +++ b/vl.c
>>> @@ -1418,9 +1418,9 @@ static int usb_parse(const char *cmdline)
>>>
>>> MachineState *current_machine;
>>>
>>> -static MachineClass *find_machine(const char *name)
>>> +static MachineClass *find_machine(const char *name, GSList *machines)
>>> {
>>> - GSList *el, *machines = object_class_get_list(TYPE_MACHINE, false);
>>> + GSList *el;
>>> MachineClass *mc = NULL;
>>>
>>> for (el = machines; el; el = el->next) {
>>> @@ -1437,13 +1437,12 @@ static MachineClass *find_machine(const char *name)
>>> }
>>> }
>>>
>>> - g_slist_free(machines);
>>> return mc;
>>> }
>>
>>Can be simplified further. I'll post it as PATCH 3.
>>
>
> Markus
>
> Thanks for your comment. Do you sugest to simplify it in this patch? I
> am confused here.
I feel the additional simplifcations I posted as PATCH 3 and 4 are best
kept as separate patches.
Remark [*] below is the only one that's not addressed by my PATCH 3+4.
>>> -static MachineClass *find_default_machine(void)
>>> +static MachineClass *find_default_machine(GSList *machines)
>>> {
>>> - GSList *el, *machines = object_class_get_list(TYPE_MACHINE, false);
>>> + GSList *el;
>>> MachineClass *mc = NULL;
>>>
>>> for (el = machines; el; el = el->next) {
>>> @@ -1455,7 +1454,6 @@ static MachineClass *find_default_machine(void)
>>> }
>>> }
>>>
>>> - g_slist_free(machines);
>>> return mc;
>>> }
>>
>>Likewise.
>>
>>>
>>> @@ -2538,16 +2536,15 @@ static gint machine_class_cmp(gconstpointer a, gconstpointer b)
>>> object_class_get_name(OBJECT_CLASS(mc1)));
>>> }
>>>
>>> -static MachineClass *machine_parse(const char *name)
>>> +static MachineClass *machine_parse(const char *name, GSList *machines)
>>> {
>>> MachineClass *mc = NULL;
>>> - GSList *el, *machines = object_class_get_list(TYPE_MACHINE, false);
>>> + GSList *el;
>>>
>>> if (name) {
>>> - mc = find_machine(name);
>>> + mc = find_machine(name, machines);
>>> }
>>> if (mc) {
>>> - g_slist_free(machines);
>>> return mc;
>>> }
>>> if (name && !is_help_option(name)) {
>>> @@ -2567,7 +2564,6 @@ static MachineClass *machine_parse(const char *name)
>>> }
>>> }
>>>
>>> - g_slist_free(machines);
>>> exit(!name || !is_help_option(name));
>>> }
>>
>>Additional cleanup is possible: argument @name is never null.
>>
>>While there, I'd check is_help_option() first rather than only after
>>find_machine() returns null, because "first" is what we commonly do.
>>
>>I'll post this as PATCH 4.
>>
>>>
>>> @@ -2659,7 +2655,8 @@ static const QEMUOption *lookup_opt(int argc, char **argv,
>>>
>>> static MachineClass *select_machine(void)
>>> {
>>> - MachineClass *machine_class = find_default_machine();
>>> + GSList *machines = object_class_get_list(TYPE_MACHINE, false);
>>> + MachineClass *machine_class = find_default_machine(machines);
>>> const char *optarg;
>>> QemuOpts *opts;
>>> Location loc;
>>> @@ -2671,7 +2668,7 @@ static MachineClass *select_machine(void)
>>>
>>> optarg = qemu_opt_get(opts, "type");
>>> if (optarg) {
>>> - machine_class = machine_parse(optarg);
>>> + machine_class = machine_parse(optarg, machines);
>>
>>Could create and destroy @machines here:
>>
>> - machine_class = machine_parse(optarg);
>> + GSList *machines = object_class_get_list(TYPE_MACHINE, false);
>> + machine_class = machine_parse(optarg, machines);
>> + g_slist_free(machines);
>>
>>Matter of taste.
[*]
Matter of taste means the choice between your version and mine is up to
the maintainer, or if the maintainer remains silent, up to you.
>>> }
>>>
>>> if (!machine_class) {
>>> @@ -2681,6 +2678,7 @@ static MachineClass *select_machine(void)
>>> }
>>>
>>> loc_pop(&loc);
>>> + g_slist_free(machines);
>>> return machine_class;
>>> }
>>
>>Reviewed-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] vl.c: allocate TYPE_MACHINE list once during bootup
2019-04-02 16:10 ` Markus Armbruster
@ 2019-04-03 0:49 ` Wei Yang
2019-04-03 6:15 ` Markus Armbruster
0 siblings, 1 reply; 20+ messages in thread
From: Wei Yang @ 2019-04-03 0:49 UTC (permalink / raw)
To: Markus Armbruster; +Cc: Wei Yang, pbonzini, Wei Yang, ehabkost, qemu-devel
On Tue, Apr 02, 2019 at 06:10:23PM +0200, Markus Armbruster wrote:
>Wei Yang <richard.weiyang@gmail.com> writes:
[...]
>>>>
>>>> @@ -2659,7 +2655,8 @@ static const QEMUOption *lookup_opt(int argc, char **argv,
>>>>
>>>> static MachineClass *select_machine(void)
>>>> {
>>>> - MachineClass *machine_class = find_default_machine();
>>>> + GSList *machines = object_class_get_list(TYPE_MACHINE, false);
>>>> + MachineClass *machine_class = find_default_machine(machines);
>>>> const char *optarg;
>>>> QemuOpts *opts;
>>>> Location loc;
>>>> @@ -2671,7 +2668,7 @@ static MachineClass *select_machine(void)
>>>>
>>>> optarg = qemu_opt_get(opts, "type");
>>>> if (optarg) {
>>>> - machine_class = machine_parse(optarg);
>>>> + machine_class = machine_parse(optarg, machines);
>>>
>>>Could create and destroy @machines here:
>>>
>>> - machine_class = machine_parse(optarg);
>>> + GSList *machines = object_class_get_list(TYPE_MACHINE, false);
>>> + machine_class = machine_parse(optarg, machines);
>>> + g_slist_free(machines);
>>>
>>>Matter of taste.
>
>[*]
>
>Matter of taste means the choice between your version and mine is up to
>the maintainer, or if the maintainer remains silent, up to you.
>
Ok, I get your meaning.
But machines should be used in find_default_machine(), after move the
allocation in "if", would there be a problem?
I may not understand your point here.
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] vl.c: allocate TYPE_MACHINE list once during bootup
2019-04-03 0:49 ` Wei Yang
@ 2019-04-03 6:15 ` Markus Armbruster
0 siblings, 0 replies; 20+ messages in thread
From: Markus Armbruster @ 2019-04-03 6:15 UTC (permalink / raw)
To: Wei Yang; +Cc: qemu-devel, pbonzini, ehabkost, Wei Yang
Wei Yang <richardw.yang@linux.intel.com> writes:
> On Tue, Apr 02, 2019 at 06:10:23PM +0200, Markus Armbruster wrote:
>>Wei Yang <richard.weiyang@gmail.com> writes:
>
> [...]
>
>>>>>
>>>>> @@ -2659,7 +2655,8 @@ static const QEMUOption *lookup_opt(int argc, char **argv,
>>>>>
>>>>> static MachineClass *select_machine(void)
>>>>> {
>>>>> - MachineClass *machine_class = find_default_machine();
>>>>> + GSList *machines = object_class_get_list(TYPE_MACHINE, false);
>>>>> + MachineClass *machine_class = find_default_machine(machines);
>>>>> const char *optarg;
>>>>> QemuOpts *opts;
>>>>> Location loc;
>>>>> @@ -2671,7 +2668,7 @@ static MachineClass *select_machine(void)
>>>>>
>>>>> optarg = qemu_opt_get(opts, "type");
>>>>> if (optarg) {
>>>>> - machine_class = machine_parse(optarg);
>>>>> + machine_class = machine_parse(optarg, machines);
>>>>
>>>>Could create and destroy @machines here:
>>>>
>>>> - machine_class = machine_parse(optarg);
>>>> + GSList *machines = object_class_get_list(TYPE_MACHINE, false);
>>>> + machine_class = machine_parse(optarg, machines);
>>>> + g_slist_free(machines);
>>>>
>>>>Matter of taste.
>>
>>[*]
>>
>>Matter of taste means the choice between your version and mine is up to
>>the maintainer, or if the maintainer remains silent, up to you.
>>
>
> Ok, I get your meaning.
>
> But machines should be used in find_default_machine(), after move the
> allocation in "if", would there be a problem?
>
> I may not understand your point here.
You're right, I overlooked that use of @machines. Keep your patch as it
is.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] vl: Clean up after previous commit
2019-04-02 13:26 ` [Qemu-devel] [PATCH 3/4] vl: Clean up after previous commit Markus Armbruster
@ 2019-04-03 22:10 ` Wei Yang
2019-04-04 15:49 ` Markus Armbruster
0 siblings, 1 reply; 20+ messages in thread
From: Wei Yang @ 2019-04-03 22:10 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, richardw.yang, pbonzini, ehabkost
On Tue, Apr 02, 2019 at 03:26:49PM +0200, Markus Armbruster wrote:
>Since the previous commit, find_machine() and find_default_machine()
>don't have to deallocate on return. This permits further
>simplifications.
>
>Signed-off-by: Markus Armbruster <armbru@redhat.com>
>---
> vl.c | 25 +++++++------------------
> 1 file changed, 7 insertions(+), 18 deletions(-)
>
>diff --git a/vl.c b/vl.c
>index 126081f595..6a31e5bfac 100644
>--- a/vl.c
>+++ b/vl.c
>@@ -1467,40 +1467,29 @@ MachineState *current_machine;
> static MachineClass *find_machine(const char *name, GSList *machines)
> {
> GSList *el;
>- MachineClass *mc = NULL;
>
> for (el = machines; el; el = el->next) {
>- MachineClass *temp = el->data;
>+ MachineClass *mc = el->data;
>
>- if (!strcmp(temp->name, name)) {
>- mc = temp;
>- break;
>- }
>- if (temp->alias &&
>- !strcmp(temp->alias, name)) {
>- mc = temp;
>- break;
>+ if (!strcmp(mc->name, name) || !g_strcmp0(mc->alias, name)) {
>+ return mc;
> }
> }
>
>- return mc;
>+ return NULL;
> }
>
> static MachineClass *find_default_machine(GSList *machines)
> {
> GSList *el;
>- MachineClass *mc = NULL;
>
> for (el = machines; el; el = el->next) {
>- MachineClass *temp = el->data;
>-
>- if (temp->is_default) {
>- mc = temp;
>- break;
>+ if (((MachineClass *)el->data)->is_default) {
>+ return el->data;
> }
> }
Generally it looks good to me.
One tiny suggestion here is to define
MachineClass *mc = el->data;
just as it does in find_machin() and return mc instead of raw el->data.
If you agree with that I will modify this at my place.
>
>- return mc;
>+ return NULL;
> }
>
> MachineInfoList *qmp_query_machines(Error **errp)
>--
>2.17.2
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] vl: Simplify machine_parse()
2019-04-02 13:26 ` [Qemu-devel] [PATCH 4/4] vl: Simplify machine_parse() Markus Armbruster
@ 2019-04-03 22:32 ` Wei Yang
2019-04-04 16:05 ` Markus Armbruster
0 siblings, 1 reply; 20+ messages in thread
From: Wei Yang @ 2019-04-03 22:32 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, richardw.yang, pbonzini, ehabkost
On Tue, Apr 02, 2019 at 03:26:50PM +0200, Markus Armbruster wrote:
>Exploit that argument @name is nerver null. Check is_help_option()
>first, because that's what we do elsewhere.
>
>Signed-off-by: Markus Armbruster <armbru@redhat.com>
>---
> vl.c | 24 +++++++++++-------------
> 1 file changed, 11 insertions(+), 13 deletions(-)
>
>diff --git a/vl.c b/vl.c
>index 6a31e5bfac..da1af3e10d 100644
>--- a/vl.c
>+++ b/vl.c
>@@ -2573,19 +2573,10 @@ static gint machine_class_cmp(gconstpointer a, gconstpointer b)
>
> static MachineClass *machine_parse(const char *name, GSList *machines)
> {
>- MachineClass *mc = NULL;
>+ MachineClass *mc;
> GSList *el;
>
>- if (name) {
>- mc = find_machine(name, machines);
>- }
>- if (mc) {
>- return mc;
>- }
>- if (name && !is_help_option(name)) {
>- error_report("unsupported machine type");
>- error_printf("Use -machine help to list supported machines\n");
>- } else {
>+ if (is_help_option(name)) {
> printf("Supported machines are:\n");
> machines = g_slist_sort(machines, machine_class_cmp);
> for (el = machines; el; el = el->next) {
>@@ -2597,9 +2588,16 @@ static MachineClass *machine_parse(const char *name, GSList *machines)
> mc->is_default ? " (default)" : "",
> mc->deprecation_reason ? " (deprecated)" : "");
> }
>+ exit(0);
> }
>-
>- exit(!name || !is_help_option(name));
>+
>+ mc = find_machine(name, machines);
>+ if (!mc) {
>+ error_report("unsupported machine type");
>+ error_printf("Use -machine help to list supported machines\n");
>+ exit(1);
>+ }
>+ return mc;
This change looks changed the original behavior.
In original logic, if mc is not NULL, there is no message printed. While now
it rely on is_help_option(). And no it exit when !is_help_option(), while
before this change it exit when is_help_option().
I don't understand the reason behind this. My suggestion is you may split this
patch into two:
1. remove check on name
2. refine the logic with explanations.
> }
>
> void qemu_add_exit_notifier(Notifier *notify)
>--
>2.17.2
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] vl: Clean up after previous commit
2019-04-03 22:10 ` Wei Yang
@ 2019-04-04 15:49 ` Markus Armbruster
0 siblings, 0 replies; 20+ messages in thread
From: Markus Armbruster @ 2019-04-04 15:49 UTC (permalink / raw)
To: Wei Yang; +Cc: pbonzini, qemu-devel, ehabkost
Wei Yang <richardw.yang@linux.intel.com> writes:
> On Tue, Apr 02, 2019 at 03:26:49PM +0200, Markus Armbruster wrote:
>>Since the previous commit, find_machine() and find_default_machine()
>>don't have to deallocate on return. This permits further
>>simplifications.
>>
>>Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>---
>> vl.c | 25 +++++++------------------
>> 1 file changed, 7 insertions(+), 18 deletions(-)
>>
>>diff --git a/vl.c b/vl.c
>>index 126081f595..6a31e5bfac 100644
>>--- a/vl.c
>>+++ b/vl.c
>>@@ -1467,40 +1467,29 @@ MachineState *current_machine;
>> static MachineClass *find_machine(const char *name, GSList *machines)
>> {
>> GSList *el;
>>- MachineClass *mc = NULL;
>>
>> for (el = machines; el; el = el->next) {
>>- MachineClass *temp = el->data;
>>+ MachineClass *mc = el->data;
>>
>>- if (!strcmp(temp->name, name)) {
>>- mc = temp;
>>- break;
>>- }
>>- if (temp->alias &&
>>- !strcmp(temp->alias, name)) {
>>- mc = temp;
>>- break;
>>+ if (!strcmp(mc->name, name) || !g_strcmp0(mc->alias, name)) {
>>+ return mc;
>> }
>> }
>>
>>- return mc;
>>+ return NULL;
>> }
>>
>> static MachineClass *find_default_machine(GSList *machines)
>> {
>> GSList *el;
>>- MachineClass *mc = NULL;
>>
>> for (el = machines; el; el = el->next) {
>>- MachineClass *temp = el->data;
>>-
>>- if (temp->is_default) {
>>- mc = temp;
>>- break;
>>+ if (((MachineClass *)el->data)->is_default) {
>>+ return el->data;
>> }
>> }
>
> Generally it looks good to me.
>
> One tiny suggestion here is to define
>
> MachineClass *mc = el->data;
>
> just as it does in find_machin() and return mc instead of raw el->data.
>
> If you agree with that I will modify this at my place.
No objection.
>>
>>- return mc;
>>+ return NULL;
>> }
>>
>> MachineInfoList *qmp_query_machines(Error **errp)
>>--
>>2.17.2
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] vl: Simplify machine_parse()
2019-04-03 22:32 ` Wei Yang
@ 2019-04-04 16:05 ` Markus Armbruster
2019-04-04 23:15 ` Wei Yang
0 siblings, 1 reply; 20+ messages in thread
From: Markus Armbruster @ 2019-04-04 16:05 UTC (permalink / raw)
To: Wei Yang; +Cc: pbonzini, qemu-devel, ehabkost
Wei Yang <richardw.yang@linux.intel.com> writes:
> On Tue, Apr 02, 2019 at 03:26:50PM +0200, Markus Armbruster wrote:
>>Exploit that argument @name is nerver null. Check is_help_option()
>>first, because that's what we do elsewhere.
>>
>>Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>---
>> vl.c | 24 +++++++++++-------------
>> 1 file changed, 11 insertions(+), 13 deletions(-)
>>
>>diff --git a/vl.c b/vl.c
>>index 6a31e5bfac..da1af3e10d 100644
>>--- a/vl.c
>>+++ b/vl.c
>>@@ -2573,19 +2573,10 @@ static gint machine_class_cmp(gconstpointer a, gconstpointer b)
>>
>> static MachineClass *machine_parse(const char *name, GSList *machines)
>> {
>>- MachineClass *mc = NULL;
>>+ MachineClass *mc;
>> GSList *el;
>>
>>- if (name) {
>>- mc = find_machine(name, machines);
>>- }
>>- if (mc) {
>>- return mc;
>>- }
>>- if (name && !is_help_option(name)) {
>>- error_report("unsupported machine type");
>>- error_printf("Use -machine help to list supported machines\n");
>>- } else {
>>+ if (is_help_option(name)) {
>> printf("Supported machines are:\n");
>> machines = g_slist_sort(machines, machine_class_cmp);
>> for (el = machines; el; el = el->next) {
>>@@ -2597,9 +2588,16 @@ static MachineClass *machine_parse(const char *name, GSList *machines)
>> mc->is_default ? " (default)" : "",
>> mc->deprecation_reason ? " (deprecated)" : "");
>> }
>>+ exit(0);
>> }
>>-
>>- exit(!name || !is_help_option(name));
>>+
>>+ mc = find_machine(name, machines);
>>+ if (!mc) {
>>+ error_report("unsupported machine type");
>>+ error_printf("Use -machine help to list supported machines\n");
>>+ exit(1);
>>+ }
>>+ return mc;
>
> This change looks changed the original behavior.
>
> In original logic, if mc is not NULL, there is no message printed. While now
> it rely on is_help_option(). And no it exit when !is_help_option(), while
> before this change it exit when is_help_option().
>
> I don't understand the reason behind this. My suggestion is you may split this
> patch into two:
>
> 1. remove check on name
> 2. refine the logic with explanations.
Cases:
(1) User asks for help, i.e. is_help_option(name)
(1a) and no machine named @name exists, i.e.
is_help_option(name) && !find_machine(name, machines)
(1b) and a machine named @name exists
is_help_option(name) && find_machine(name, machines)
(2) User asks for a machine that doesn't exist, i.e.
!is_help_option(name) && !find_machine(name, machines)
(3) User asks for a machine that exists, i.e.
!is_help_option(name) && find_machine(name, machines)
Since no machines are called "help" or "?", case (1b) is not actually
possible.
Old code:
static MachineClass *machine_parse(const char *name, GSList *machines)
{
MachineClass *mc = NULL;
GSList *el;
if (name) {
mc = find_machine(name, machines);
}
if (mc) {
return mc;
}
if (name && !is_help_option(name)) {
error_report("unsupported machine type");
error_printf("Use -machine help to list supported machines\n");
} else {
printf("Supported machines are:\n");
machines = g_slist_sort(machines, machine_class_cmp);
for (el = machines; el; el = el->next) {
MachineClass *mc = el->data;
if (mc->alias) {
printf("%-20s %s (alias of %s)\n", mc->alias, mc->desc, mc->name);
}
printf("%-20s %s%s%s\n", mc->name, mc->desc,
mc->is_default ? " (default)" : "",
mc->deprecation_reason ? " (deprecated)" : "");
}
}
exit(!name || !is_help_option(name));
}
Case (1a): print help, exit(0)
Case (1b): return find_machine()
Case (2): report error, exit(1)
Case (3): return find_machine()
New code:
static MachineClass *machine_parse(const char *name, GSList *machines)
{
MachineClass *mc;
GSList *el;
if (is_help_option(name)) {
printf("Supported machines are:\n");
machines = g_slist_sort(machines, machine_class_cmp);
for (el = machines; el; el = el->next) {
MachineClass *mc = el->data;
if (mc->alias) {
printf("%-20s %s (alias of %s)\n", mc->alias, mc->desc, mc->name);
}
printf("%-20s %s%s%s\n", mc->name, mc->desc,
mc->is_default ? " (default)" : "",
mc->deprecation_reason ? " (deprecated)" : "");
}
exit(0);
}
mc = find_machine(name, machines);
if (!mc) {
error_report("unsupported machine type");
error_printf("Use -machine help to list supported machines\n");
exit(1);
}
return mc;
}
Case (1a): print help, exit(0)
Case (1b): print help, exit(0)
Case (2): report error, exit(1)
Case (3): return find_machine()
The patch changes "impossible" case (1b). That's intentional (but my
commit message could explain it better).
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] vl: Simplify machine_parse()
@ 2019-04-04 23:15 ` Wei Yang
0 siblings, 0 replies; 20+ messages in thread
From: Wei Yang @ 2019-04-04 23:15 UTC (permalink / raw)
To: Markus Armbruster; +Cc: Wei Yang, pbonzini, qemu-devel, ehabkost
On Thu, Apr 04, 2019 at 06:05:25PM +0200, Markus Armbruster wrote:
>Wei Yang <richardw.yang@linux.intel.com> writes:
>
>> On Tue, Apr 02, 2019 at 03:26:50PM +0200, Markus Armbruster wrote:
>>>Exploit that argument @name is nerver null. Check is_help_option()
>>>first, because that's what we do elsewhere.
>>>
>>>Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>>---
>>> vl.c | 24 +++++++++++-------------
>>> 1 file changed, 11 insertions(+), 13 deletions(-)
>>>
>>>diff --git a/vl.c b/vl.c
>>>index 6a31e5bfac..da1af3e10d 100644
>>>--- a/vl.c
>>>+++ b/vl.c
>>>@@ -2573,19 +2573,10 @@ static gint machine_class_cmp(gconstpointer a, gconstpointer b)
>>>
>>> static MachineClass *machine_parse(const char *name, GSList *machines)
>>> {
>>>- MachineClass *mc = NULL;
>>>+ MachineClass *mc;
>>> GSList *el;
>>>
>>>- if (name) {
>>>- mc = find_machine(name, machines);
>>>- }
>>>- if (mc) {
>>>- return mc;
>>>- }
>>>- if (name && !is_help_option(name)) {
>>>- error_report("unsupported machine type");
>>>- error_printf("Use -machine help to list supported machines\n");
>>>- } else {
>>>+ if (is_help_option(name)) {
>>> printf("Supported machines are:\n");
>>> machines = g_slist_sort(machines, machine_class_cmp);
>>> for (el = machines; el; el = el->next) {
>>>@@ -2597,9 +2588,16 @@ static MachineClass *machine_parse(const char *name, GSList *machines)
>>> mc->is_default ? " (default)" : "",
>>> mc->deprecation_reason ? " (deprecated)" : "");
>>> }
>>>+ exit(0);
>>> }
>>>-
>>>- exit(!name || !is_help_option(name));
>>>+
>>>+ mc = find_machine(name, machines);
>>>+ if (!mc) {
>>>+ error_report("unsupported machine type");
>>>+ error_printf("Use -machine help to list supported machines\n");
>>>+ exit(1);
>>>+ }
>>>+ return mc;
>>
>> This change looks changed the original behavior.
>>
>> In original logic, if mc is not NULL, there is no message printed. While now
>> it rely on is_help_option(). And no it exit when !is_help_option(), while
>> before this change it exit when is_help_option().
>>
>> I don't understand the reason behind this. My suggestion is you may split this
>> patch into two:
>>
>> 1. remove check on name
>> 2. refine the logic with explanations.
>
>Cases:
>
>(1) User asks for help, i.e. is_help_option(name)
>
>(1a) and no machine named @name exists, i.e.
> is_help_option(name) && !find_machine(name, machines)
>
>(1b) and a machine named @name exists
> is_help_option(name) && find_machine(name, machines)
>
>(2) User asks for a machine that doesn't exist, i.e.
> !is_help_option(name) && !find_machine(name, machines)
>
>(3) User asks for a machine that exists, i.e.
> !is_help_option(name) && find_machine(name, machines)
>
>Since no machines are called "help" or "?", case (1b) is not actually
>possible.
>
>Old code:
>
> static MachineClass *machine_parse(const char *name, GSList *machines)
> {
> MachineClass *mc = NULL;
> GSList *el;
>
> if (name) {
> mc = find_machine(name, machines);
> }
> if (mc) {
> return mc;
> }
> if (name && !is_help_option(name)) {
> error_report("unsupported machine type");
> error_printf("Use -machine help to list supported machines\n");
> } else {
> printf("Supported machines are:\n");
> machines = g_slist_sort(machines, machine_class_cmp);
> for (el = machines; el; el = el->next) {
> MachineClass *mc = el->data;
> if (mc->alias) {
> printf("%-20s %s (alias of %s)\n", mc->alias, mc->desc, mc->name);
> }
> printf("%-20s %s%s%s\n", mc->name, mc->desc,
> mc->is_default ? " (default)" : "",
> mc->deprecation_reason ? " (deprecated)" : "");
> }
> }
>
> exit(!name || !is_help_option(name));
> }
>
>Case (1a): print help, exit(0)
>
>Case (1b): return find_machine()
>
>Case (2): report error, exit(1)
>
>Case (3): return find_machine()
>
>New code:
>
> static MachineClass *machine_parse(const char *name, GSList *machines)
> {
> MachineClass *mc;
> GSList *el;
>
> if (is_help_option(name)) {
> printf("Supported machines are:\n");
> machines = g_slist_sort(machines, machine_class_cmp);
> for (el = machines; el; el = el->next) {
> MachineClass *mc = el->data;
> if (mc->alias) {
> printf("%-20s %s (alias of %s)\n", mc->alias, mc->desc, mc->name);
> }
> printf("%-20s %s%s%s\n", mc->name, mc->desc,
> mc->is_default ? " (default)" : "",
> mc->deprecation_reason ? " (deprecated)" : "");
> }
> exit(0);
> }
>
> mc = find_machine(name, machines);
> if (!mc) {
> error_report("unsupported machine type");
> error_printf("Use -machine help to list supported machines\n");
> exit(1);
> }
> return mc;
> }
>
>Case (1a): print help, exit(0)
>
>Case (1b): print help, exit(0)
>
>Case (2): report error, exit(1)
>
>Case (3): return find_machine()
>
>The patch changes "impossible" case (1b). That's intentional (but my
>commit message could explain it better).
This looks better. Would you mind refine it so that I could send all these
patches in v2.
Or you prefer send it out by our self?
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] vl: Simplify machine_parse()
@ 2019-04-04 23:15 ` Wei Yang
0 siblings, 0 replies; 20+ messages in thread
From: Wei Yang @ 2019-04-04 23:15 UTC (permalink / raw)
To: Markus Armbruster; +Cc: pbonzini, Wei Yang, ehabkost, qemu-devel
On Thu, Apr 04, 2019 at 06:05:25PM +0200, Markus Armbruster wrote:
>Wei Yang <richardw.yang@linux.intel.com> writes:
>
>> On Tue, Apr 02, 2019 at 03:26:50PM +0200, Markus Armbruster wrote:
>>>Exploit that argument @name is nerver null. Check is_help_option()
>>>first, because that's what we do elsewhere.
>>>
>>>Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>>---
>>> vl.c | 24 +++++++++++-------------
>>> 1 file changed, 11 insertions(+), 13 deletions(-)
>>>
>>>diff --git a/vl.c b/vl.c
>>>index 6a31e5bfac..da1af3e10d 100644
>>>--- a/vl.c
>>>+++ b/vl.c
>>>@@ -2573,19 +2573,10 @@ static gint machine_class_cmp(gconstpointer a, gconstpointer b)
>>>
>>> static MachineClass *machine_parse(const char *name, GSList *machines)
>>> {
>>>- MachineClass *mc = NULL;
>>>+ MachineClass *mc;
>>> GSList *el;
>>>
>>>- if (name) {
>>>- mc = find_machine(name, machines);
>>>- }
>>>- if (mc) {
>>>- return mc;
>>>- }
>>>- if (name && !is_help_option(name)) {
>>>- error_report("unsupported machine type");
>>>- error_printf("Use -machine help to list supported machines\n");
>>>- } else {
>>>+ if (is_help_option(name)) {
>>> printf("Supported machines are:\n");
>>> machines = g_slist_sort(machines, machine_class_cmp);
>>> for (el = machines; el; el = el->next) {
>>>@@ -2597,9 +2588,16 @@ static MachineClass *machine_parse(const char *name, GSList *machines)
>>> mc->is_default ? " (default)" : "",
>>> mc->deprecation_reason ? " (deprecated)" : "");
>>> }
>>>+ exit(0);
>>> }
>>>-
>>>- exit(!name || !is_help_option(name));
>>>+
>>>+ mc = find_machine(name, machines);
>>>+ if (!mc) {
>>>+ error_report("unsupported machine type");
>>>+ error_printf("Use -machine help to list supported machines\n");
>>>+ exit(1);
>>>+ }
>>>+ return mc;
>>
>> This change looks changed the original behavior.
>>
>> In original logic, if mc is not NULL, there is no message printed. While now
>> it rely on is_help_option(). And no it exit when !is_help_option(), while
>> before this change it exit when is_help_option().
>>
>> I don't understand the reason behind this. My suggestion is you may split this
>> patch into two:
>>
>> 1. remove check on name
>> 2. refine the logic with explanations.
>
>Cases:
>
>(1) User asks for help, i.e. is_help_option(name)
>
>(1a) and no machine named @name exists, i.e.
> is_help_option(name) && !find_machine(name, machines)
>
>(1b) and a machine named @name exists
> is_help_option(name) && find_machine(name, machines)
>
>(2) User asks for a machine that doesn't exist, i.e.
> !is_help_option(name) && !find_machine(name, machines)
>
>(3) User asks for a machine that exists, i.e.
> !is_help_option(name) && find_machine(name, machines)
>
>Since no machines are called "help" or "?", case (1b) is not actually
>possible.
>
>Old code:
>
> static MachineClass *machine_parse(const char *name, GSList *machines)
> {
> MachineClass *mc = NULL;
> GSList *el;
>
> if (name) {
> mc = find_machine(name, machines);
> }
> if (mc) {
> return mc;
> }
> if (name && !is_help_option(name)) {
> error_report("unsupported machine type");
> error_printf("Use -machine help to list supported machines\n");
> } else {
> printf("Supported machines are:\n");
> machines = g_slist_sort(machines, machine_class_cmp);
> for (el = machines; el; el = el->next) {
> MachineClass *mc = el->data;
> if (mc->alias) {
> printf("%-20s %s (alias of %s)\n", mc->alias, mc->desc, mc->name);
> }
> printf("%-20s %s%s%s\n", mc->name, mc->desc,
> mc->is_default ? " (default)" : "",
> mc->deprecation_reason ? " (deprecated)" : "");
> }
> }
>
> exit(!name || !is_help_option(name));
> }
>
>Case (1a): print help, exit(0)
>
>Case (1b): return find_machine()
>
>Case (2): report error, exit(1)
>
>Case (3): return find_machine()
>
>New code:
>
> static MachineClass *machine_parse(const char *name, GSList *machines)
> {
> MachineClass *mc;
> GSList *el;
>
> if (is_help_option(name)) {
> printf("Supported machines are:\n");
> machines = g_slist_sort(machines, machine_class_cmp);
> for (el = machines; el; el = el->next) {
> MachineClass *mc = el->data;
> if (mc->alias) {
> printf("%-20s %s (alias of %s)\n", mc->alias, mc->desc, mc->name);
> }
> printf("%-20s %s%s%s\n", mc->name, mc->desc,
> mc->is_default ? " (default)" : "",
> mc->deprecation_reason ? " (deprecated)" : "");
> }
> exit(0);
> }
>
> mc = find_machine(name, machines);
> if (!mc) {
> error_report("unsupported machine type");
> error_printf("Use -machine help to list supported machines\n");
> exit(1);
> }
> return mc;
> }
>
>Case (1a): print help, exit(0)
>
>Case (1b): print help, exit(0)
>
>Case (2): report error, exit(1)
>
>Case (3): return find_machine()
>
>The patch changes "impossible" case (1b). That's intentional (but my
>commit message could explain it better).
This looks better. Would you mind refine it so that I could send all these
patches in v2.
Or you prefer send it out by our self?
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] vl: Simplify machine_parse()
2019-04-04 23:15 ` Wei Yang
(?)
@ 2019-04-05 5:39 ` Markus Armbruster
-1 siblings, 0 replies; 20+ messages in thread
From: Markus Armbruster @ 2019-04-05 5:39 UTC (permalink / raw)
To: Wei Yang; +Cc: pbonzini, ehabkost, qemu-devel
Wei Yang <richardw.yang@linux.intel.com> writes:
> On Thu, Apr 04, 2019 at 06:05:25PM +0200, Markus Armbruster wrote:
>>Wei Yang <richardw.yang@linux.intel.com> writes:
>>
>>> On Tue, Apr 02, 2019 at 03:26:50PM +0200, Markus Armbruster wrote:
>>>>Exploit that argument @name is nerver null. Check is_help_option()
>>>>first, because that's what we do elsewhere.
>>>>
>>>>Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>>>---
>>>> vl.c | 24 +++++++++++-------------
>>>> 1 file changed, 11 insertions(+), 13 deletions(-)
>>>>
>>>>diff --git a/vl.c b/vl.c
>>>>index 6a31e5bfac..da1af3e10d 100644
>>>>--- a/vl.c
>>>>+++ b/vl.c
>>>>@@ -2573,19 +2573,10 @@ static gint machine_class_cmp(gconstpointer a, gconstpointer b)
>>>>
>>>> static MachineClass *machine_parse(const char *name, GSList *machines)
>>>> {
>>>>- MachineClass *mc = NULL;
>>>>+ MachineClass *mc;
>>>> GSList *el;
>>>>
>>>>- if (name) {
>>>>- mc = find_machine(name, machines);
>>>>- }
>>>>- if (mc) {
>>>>- return mc;
>>>>- }
>>>>- if (name && !is_help_option(name)) {
>>>>- error_report("unsupported machine type");
>>>>- error_printf("Use -machine help to list supported machines\n");
>>>>- } else {
>>>>+ if (is_help_option(name)) {
>>>> printf("Supported machines are:\n");
>>>> machines = g_slist_sort(machines, machine_class_cmp);
>>>> for (el = machines; el; el = el->next) {
>>>>@@ -2597,9 +2588,16 @@ static MachineClass *machine_parse(const char *name, GSList *machines)
>>>> mc->is_default ? " (default)" : "",
>>>> mc->deprecation_reason ? " (deprecated)" : "");
>>>> }
>>>>+ exit(0);
>>>> }
>>>>-
>>>>- exit(!name || !is_help_option(name));
>>>>+
>>>>+ mc = find_machine(name, machines);
>>>>+ if (!mc) {
>>>>+ error_report("unsupported machine type");
>>>>+ error_printf("Use -machine help to list supported machines\n");
>>>>+ exit(1);
>>>>+ }
>>>>+ return mc;
>>>
>>> This change looks changed the original behavior.
>>>
>>> In original logic, if mc is not NULL, there is no message printed. While now
>>> it rely on is_help_option(). And no it exit when !is_help_option(), while
>>> before this change it exit when is_help_option().
>>>
>>> I don't understand the reason behind this. My suggestion is you may split this
>>> patch into two:
>>>
>>> 1. remove check on name
>>> 2. refine the logic with explanations.
>>
>>Cases:
>>
>>(1) User asks for help, i.e. is_help_option(name)
>>
>>(1a) and no machine named @name exists, i.e.
>> is_help_option(name) && !find_machine(name, machines)
>>
>>(1b) and a machine named @name exists
>> is_help_option(name) && find_machine(name, machines)
>>
>>(2) User asks for a machine that doesn't exist, i.e.
>> !is_help_option(name) && !find_machine(name, machines)
>>
>>(3) User asks for a machine that exists, i.e.
>> !is_help_option(name) && find_machine(name, machines)
>>
>>Since no machines are called "help" or "?", case (1b) is not actually
>>possible.
>>
>>Old code:
>>
>> static MachineClass *machine_parse(const char *name, GSList *machines)
>> {
>> MachineClass *mc = NULL;
>> GSList *el;
>>
>> if (name) {
>> mc = find_machine(name, machines);
>> }
>> if (mc) {
>> return mc;
>> }
>> if (name && !is_help_option(name)) {
>> error_report("unsupported machine type");
>> error_printf("Use -machine help to list supported machines\n");
>> } else {
>> printf("Supported machines are:\n");
>> machines = g_slist_sort(machines, machine_class_cmp);
>> for (el = machines; el; el = el->next) {
>> MachineClass *mc = el->data;
>> if (mc->alias) {
>> printf("%-20s %s (alias of %s)\n", mc->alias, mc->desc, mc->name);
>> }
>> printf("%-20s %s%s%s\n", mc->name, mc->desc,
>> mc->is_default ? " (default)" : "",
>> mc->deprecation_reason ? " (deprecated)" : "");
>> }
>> }
>>
>> exit(!name || !is_help_option(name));
>> }
>>
>>Case (1a): print help, exit(0)
>>
>>Case (1b): return find_machine()
>>
>>Case (2): report error, exit(1)
>>
>>Case (3): return find_machine()
>>
>>New code:
>>
>> static MachineClass *machine_parse(const char *name, GSList *machines)
>> {
>> MachineClass *mc;
>> GSList *el;
>>
>> if (is_help_option(name)) {
>> printf("Supported machines are:\n");
>> machines = g_slist_sort(machines, machine_class_cmp);
>> for (el = machines; el; el = el->next) {
>> MachineClass *mc = el->data;
>> if (mc->alias) {
>> printf("%-20s %s (alias of %s)\n", mc->alias, mc->desc, mc->name);
>> }
>> printf("%-20s %s%s%s\n", mc->name, mc->desc,
>> mc->is_default ? " (default)" : "",
>> mc->deprecation_reason ? " (deprecated)" : "");
>> }
>> exit(0);
>> }
>>
>> mc = find_machine(name, machines);
>> if (!mc) {
>> error_report("unsupported machine type");
>> error_printf("Use -machine help to list supported machines\n");
>> exit(1);
>> }
>> return mc;
>> }
>>
>>Case (1a): print help, exit(0)
>>
>>Case (1b): print help, exit(0)
>>
>>Case (2): report error, exit(1)
>>
>>Case (3): return find_machine()
>>
>>The patch changes "impossible" case (1b). That's intentional (but my
>>commit message could explain it better).
>
> This looks better.
Amending my commit message to explain things better:
vl: Simplify machine_parse()
Exploit that argument @name is nerver null. Check is_help_option()
first, because that's what we do elsewhere. If we (foolishly!)
defined a machine named "help", -machine help would now print help
instead of selecting the machine named "help".
> Would you mind refine it so that I could send all these
> patches in v2.
>
> Or you prefer send it out by our self?
Please send v2.
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2019-04-05 5:39 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-11 6:08 [Qemu-devel] [PATCH 0/2] cleanup select_machine Wei Yang
2019-03-11 6:08 ` [Qemu-devel] [PATCH 1/2] vl.c: make find_default_machine() local Wei Yang
2019-04-02 6:33 ` Markus Armbruster
2019-04-02 15:07 ` Wei Yang
2019-04-02 15:17 ` Markus Armbruster
2019-03-11 6:08 ` [Qemu-devel] [PATCH 2/2] vl.c: allocate TYPE_MACHINE list once during bootup Wei Yang
2019-04-02 13:28 ` Markus Armbruster
2019-04-02 15:16 ` Wei Yang
2019-04-02 16:10 ` Markus Armbruster
2019-04-03 0:49 ` Wei Yang
2019-04-03 6:15 ` Markus Armbruster
2019-04-02 13:26 ` [Qemu-devel] [PATCH 3/4] vl: Clean up after previous commit Markus Armbruster
2019-04-03 22:10 ` Wei Yang
2019-04-04 15:49 ` Markus Armbruster
2019-04-02 13:26 ` [Qemu-devel] [PATCH 4/4] vl: Simplify machine_parse() Markus Armbruster
2019-04-03 22:32 ` Wei Yang
2019-04-04 16:05 ` Markus Armbruster
2019-04-04 23:15 ` Wei Yang
2019-04-04 23:15 ` Wei Yang
2019-04-05 5:39 ` Markus Armbruster
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.