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

[Igor Mammedov]

On Thu, 12 Jun 2014 11:20:05 +0300
"Michael S. Tsirkin" <address@hidden> wrote:

> On Thu, Jun 12, 2014 at 11:12:01AM +0300, Marcel Apfelbaum 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.
> 
> If we add code as an intermediate step that's fine,
> but I'd like to see the whole thing before applying.
> There should be net reduction in amount of
> boilerplate code at the end of the day.
+1

> 
> 
> 
> 
> > > 
> > > > 
> > > > 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
> >