| Message ID | 1453208789-42479-8-git-send-email-imammedo@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jan 19, 2016 at 02:06:27PM +0100, Igor Mammedov wrote: > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > include/hw/boards.h | 1 + > vl.c | 4 ++++ > 2 files changed, 5 insertions(+) > > diff --git a/include/hw/boards.h b/include/hw/boards.h > index 0f30959..d495611 100644 > --- a/include/hw/boards.h > +++ b/include/hw/boards.h > @@ -90,6 +90,7 @@ struct MachineClass { > const char *default_machine_opts; > const char *default_boot_order; > const char *default_display; > + GlobalProperty *default_props; > GlobalProperty *compat_props; Could you explain (in a comment?) the purpose of each field? They seem to do exactly the same thing, so why couldn't they become a single linked list, where the compat classes just append new items to the existing default_props list? (If we build default_props by appending instead of overwriting the parent class list, we will be able to finally eliminate PC_COMPAT_* macro nesting)
On Sat, 23 Jan 2016 12:59:56 -0200 Eduardo Habkost <ehabkost@redhat.com> wrote: > On Tue, Jan 19, 2016 at 02:06:27PM +0100, Igor Mammedov wrote: > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > --- > > include/hw/boards.h | 1 + > > vl.c | 4 ++++ > > 2 files changed, 5 insertions(+) > > > > diff --git a/include/hw/boards.h b/include/hw/boards.h > > index 0f30959..d495611 100644 > > --- a/include/hw/boards.h > > +++ b/include/hw/boards.h > > @@ -90,6 +90,7 @@ struct MachineClass { > > const char *default_machine_opts; > > const char *default_boot_order; > > const char *default_display; > > + GlobalProperty *default_props; > > GlobalProperty *compat_props; > > Could you explain (in a comment?) the purpose of each field? They > seem to do exactly the same thing, so why couldn't they become a > single linked list, where the compat classes just append new > items to the existing default_props list? > > (If we build default_props by appending instead of overwriting > the parent class list, we will be able to finally eliminate > PC_COMPAT_* macro nesting) The only reason I've added it as separate field is to keep the current way compat_props are working instead of rewriting not related to this series part. Alternatively we could add qdev_prop_prepend_global_list() API and add static defaults calling it from board's machine-init.
On Tue, Jan 26, 2016 at 11:28:01AM +0100, Igor Mammedov wrote: > On Sat, 23 Jan 2016 12:59:56 -0200 > Eduardo Habkost <ehabkost@redhat.com> wrote: > > > On Tue, Jan 19, 2016 at 02:06:27PM +0100, Igor Mammedov wrote: > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > > --- > > > include/hw/boards.h | 1 + > > > vl.c | 4 ++++ > > > 2 files changed, 5 insertions(+) > > > > > > diff --git a/include/hw/boards.h b/include/hw/boards.h > > > index 0f30959..d495611 100644 > > > --- a/include/hw/boards.h > > > +++ b/include/hw/boards.h > > > @@ -90,6 +90,7 @@ struct MachineClass { > > > const char *default_machine_opts; > > > const char *default_boot_order; > > > const char *default_display; > > > + GlobalProperty *default_props; > > > GlobalProperty *compat_props; > > > > Could you explain (in a comment?) the purpose of each field? They > > seem to do exactly the same thing, so why couldn't they become a > > single linked list, where the compat classes just append new > > items to the existing default_props list? > > > > (If we build default_props by appending instead of overwriting > > the parent class list, we will be able to finally eliminate > > PC_COMPAT_* macro nesting) > The only reason I've added it as separate field is to keep the > current way compat_props are working instead of rewriting > not related to this series part. > OK to me, but I would like to have the reason for having two very similar fields documented in struct MachineClass. > Alternatively we could add qdev_prop_prepend_global_list() API > and add static defaults calling it from board's machine-init. If we can choose between having stuff in MachineClass and adding extra machine-init code, I prefer to have it in MachineClass.
diff --git a/include/hw/boards.h b/include/hw/boards.h index 0f30959..d495611 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -90,6 +90,7 @@ struct MachineClass { const char *default_machine_opts; const char *default_boot_order; const char *default_display; + GlobalProperty *default_props; GlobalProperty *compat_props; const char *hw_version; ram_addr_t default_ram_size; diff --git a/vl.c b/vl.c index b7a083e..0db74b1 100644 --- a/vl.c +++ b/vl.c @@ -4492,6 +4492,10 @@ int main(int argc, char **argv, char **envp) exit (i == 1 ? 1 : 0); } + if (machine_class->default_props) { + qdev_prop_register_global_list(machine_class->default_props); + } + if (machine_class->compat_props) { qdev_prop_register_global_list(machine_class->compat_props); }
Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- include/hw/boards.h | 1 + vl.c | 4 ++++ 2 files changed, 5 insertions(+)