Re: [Qemu-devel] [RFC] pc: Register machine classes directly instead of using QEMUMachine

[Igor Mammedov]

On Thu, 12 Jun 2014 11:12:01 +0300
Marcel Apfelbaum <address@hidden> wrote:

> On Thu, 2014-06-12 at 10:02 +0200, Igor Mammedov wrote:
> > On Thu, 12 Jun 2014 03:22:24 -0300
> > Eduardo Habkost <address@hidden> wrote:
> > 
> > > This is a (mostly) blind and mechanical conversion of the PC QEMUMachine
> > > definitions to corresponding class registration code.
> > > 
> > > Having the PC code converted to pure QOM registration code will help us
> > > move PC-specific machine state that is currently held in static
> > > variables inside PC machine objects, and reduce duplication between
> > > pc_piix.c and pc_q35.c.
> > 
> > Getting rid of *_machine_options() functions and doing nested
> > inheritance with necessary overrides in respective *_class_init()
> > functions, would make code more readable/understandable.
> While I agree with you that this is the right direction, Eduardo's
> patch is a good step forward getting rid of QEMUMachine and those
> defines. I say we implement the hierarchy on top of this patch.
it's fine if there is the 2nd patch in the same series that does
it right. Otherwise result looks worse than it was before.

> > 
> > > 
> > > Signed-off-by: Eduardo Habkost <address@hidden>
> > > ---
> > >  hw/i386/pc.c         |  13 ++
> > >  hw/i386/pc_piix.c    | 473 
> > > +++++++++++++++++++++++++++++++++++----------------
> > >  hw/i386/pc_q35.c     | 171 +++++++++++++------
> > >  include/hw/i386/pc.h |  20 ++-
> > >  4 files changed, 465 insertions(+), 212 deletions(-)
> > > 
> > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > > index 32d1632..56720cd 100644
> > > --- a/hw/i386/pc.c
> > > +++ b/hw/i386/pc.c
> > > @@ -1456,3 +1456,16 @@ void ioapic_init_gsi(GSIState *gsi_state, const 
> > > char *parent_name)
> > >          gsi_state->ioapic_irq[i] = qdev_get_gpio_in(dev, i);
> > >      }
> > >  }
> > > +
> > > +static TypeInfo pc_machine_type_info = {
> > > +        .name       = TYPE_PC_MACHINE,
> > > +        .parent     = TYPE_MACHINE,
> > > +        .abstract   = true,
> > > +};
> Igor's series already introduces TYPE_PC_MACHINE, you may want to rebase on 
> it:
> http://lists.gnu.org/archive/html/qemu-devel/2014-06/msg00133.html
> 
> Thanks,
> Marcel
> 
> 
>