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

[Andreas Färber]

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

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