Re: [Qemu-devel] [RFC 3/8] pc: prepare PC for custom machine state

[Michael S. Tsirkin]

On Mon, Mar 24, 2014 at 11:52:45AM +0100, Andreas Färber wrote:
> Am 23.03.2014 16:13, schrieb Marcel Apfelbaum:
> > On Thu, 2014-03-20 at 16:01 +0100, Igor Mammedov wrote:
> >> Signed-off-by: Igor Mammedov <address@hidden>
> >> ---
> >>  hw/i386/pc.c         |   26 ++++++++++++++++++++++++++
> >>  hw/i386/pc_piix.c    |   34 +++++++++++++++++-----------------
> >>  hw/i386/pc_q35.c     |   10 +++++-----
> >>  include/hw/i386/pc.h |   14 ++++++++++++++
> >>  4 files changed, 62 insertions(+), 22 deletions(-)
> >>
> >> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> >> index e715a33..e0bc3a2 100644
> >> --- a/hw/i386/pc.c
> >> +++ b/hw/i386/pc.c
> >> @@ -1413,3 +1413,29 @@ void ioapic_init_gsi(GSIState *gsi_state, const 
> >> char *parent_name)
> >>          gsi_state->ioapic_irq[i] = qdev_get_gpio_in(dev, i);
> >>      }
> >>  }
> >> +
> >> +void qemu_register_pc_machine(QEMUMachine *m)
> > I am not so comfortable with this approach because
> > every subsystem (e.g pc) will have to duplicate the
> > "register machine" code until the conversion from
> > QEMUMachine to MachineClass is over. (which I hope
> > it will not take too much time)
> > 
> > I propose a patch already in the list which does that
> > automatically by moving this logic into hw/core/machine.c .
> > In this way it will be much less code "touched" during conversion. 
> > 
> > Andreas, did you have anything against the usage of 'class_base_init' ?
> 
> Yes, I do, .class_base_init is wrong for this, it would be .class_init;
> but please avoid making a base class' .class_init (or .class_base_init)
> depend on a subtype specifying .class_data.
> 
> I believe I asked you to post patches that finish your conversion so
> that MachineClass no longer needs this pointer to QEMUMachine and
> actually uses the fields you already prepared. In my mind the next
> logical step of QOM'ification is to have each machine specify what is
> now in a QEMUMachine struct in its own type's class_init, then there is
> no duplication of such a general assignment any more.
> 
> Since this patch is for one machine only, I would much prefer to have
> the PC duplicate the class_init like we did for sPAPR machine
> (hw/ppc/spapr.c) over exposing the class_init across files - if this
> series cannot wait to be ordered after the machine series.
> 
> Regards,
> Andreas

Ok IIUC this is exactly what Igor did so you ack the original patch?

> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg