Re: [Qemu-devel] [PATCH v2 06/31] pc: create custom generic PC machine type

[Igor Mammedov]

On Tue, 20 May 2014 18:55:32 +0300
Marcel Apfelbaum <address@hidden> wrote:

> On Tue, 2014-05-20 at 17:15 +0200, Igor Mammedov wrote:
> > it will be used for PC specific options/variables
> > 
> > Signed-off-by: Igor Mammedov <address@hidden>
> > ---
> >  hw/i386/pc.c         |   57 
> > ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  hw/i386/pc_piix.c    |   36 +++++++++++++++---------------
> >  hw/i386/pc_q35.c     |   12 +++++-----
> >  include/hw/i386/pc.h |   24 +++++++++++++++++++++
> >  4 files changed, 105 insertions(+), 24 deletions(-)
> > 
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index e6369d5..f6781d8 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -1459,3 +1459,60 @@ void ioapic_init_gsi(GSIState *gsi_state, const char 
> > *parent_name)
> >          gsi_state->ioapic_irq[i] = qdev_get_gpio_in(dev, i);
> >      }
> >  }
> > +
> > +static void pc_generic_machine_class_init(ObjectClass *oc, void *data)
> > +{
> > +    MachineClass *mc = MACHINE_CLASS(oc);
> > +    QEMUMachine *qm = data;
> > +
> > +    mc->name = qm->name;
> > +    mc->alias = qm->alias;
> > +    mc->desc = qm->desc;
> > +    mc->init = qm->init;
> > +    mc->reset = qm->reset;
> > +    mc->hot_add_cpu = qm->hot_add_cpu;
> > +    mc->kvm_type = qm->kvm_type;
> > +    mc->block_default_type = qm->block_default_type;
> > +    mc->max_cpus = qm->max_cpus;
> > +    mc->no_serial = qm->no_serial;
> > +    mc->no_parallel = qm->no_parallel;
> > +    mc->use_virtcon = qm->use_virtcon;
> > +    mc->use_sclp = qm->use_sclp;
> > +    mc->no_floppy = qm->no_floppy;
> > +    mc->no_cdrom = qm->no_cdrom;
> > +    mc->no_sdcard = qm->no_sdcard;
> > +    mc->is_default = qm->is_default;
> > +    mc->default_machine_opts = qm->default_machine_opts;
> > +    mc->default_boot_order = qm->default_boot_order;
> > +    mc->compat_props = qm->compat_props;
> > +    mc->hw_version = qm->hw_version;
> > +}
> Hi Igor,
> This conflicts with the latest "machine as QOM object" series in which
> MachineClass does not have a pointer to QEMUMachine and
> pc_generic_machine_class_init is not needed anymore.
I'm not sure with what it conflicts, this patch is rebased on top of
https://github.com/afaerber/qemu-cpu/commit/33ead66c86c96e37d141cfcfda5664b2fa0408fd
from qom-next queue, which drops QEMUMachineInitArgs but QEMUMachine is still 
exists/used.

MachineClass does not have a pointer to QEMUMachine so we have to
initialize MachineClass fields directly /that's what's changed from previous 
version/

If there will be more patches about Machine conversion in qom-next before
next series respin, I'll rebase it on top them.
> 
> If your series will come first we will have a re-base problem on
> Andreas's QOM's queue.
> Andreas, what's the best practice here?
> 
> > +
> > +void qemu_register_pc_machine(QEMUMachine *m)
> > +{
> > +    char *name = g_strconcat(m->name, TYPE_MACHINE_SUFFIX, NULL);
> > +    TypeInfo ti = {
> > +        .name       = name,
> > +        .parent     = TYPE_PC_MACHINE,
> > +        .class_init = pc_generic_machine_class_init,
> > +        .class_data = (void *)m,
> > +    };
> > +
> > +    type_register(&ti);
> > +    g_free(name);
> > +}
> > +
> > +static const TypeInfo pc_machine_info = {
> > +    .name = TYPE_PC_MACHINE,
> > +    .parent = TYPE_MACHINE,
> > +    .abstract = true,
> > +    .instance_size = sizeof(PCMachineState),
> > +    .class_size = sizeof(PCMachineClass),
> > +};
> > +
> > +static void pc_machine_register_types(void)
> > +{
> > +    type_register_static(&pc_machine_info);
> > +}
> > +
> > +type_init(pc_machine_register_types)
> > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > index a48e263..abb599b 100644
> > --- a/hw/i386/pc_piix.c
> > +++ b/hw/i386/pc_piix.c
> > @@ -843,25 +843,25 @@ static QEMUMachine xenfv_machine = {
> >  
> >  static void pc_machine_init(void)
> >  {
> > -    qemu_register_machine(&pc_i440fx_machine_v2_1);
> > -    qemu_register_machine(&pc_i440fx_machine_v2_0);
> > -    qemu_register_machine(&pc_i440fx_machine_v1_7);
> > -    qemu_register_machine(&pc_i440fx_machine_v1_6);
> > -    qemu_register_machine(&pc_i440fx_machine_v1_5);
> > -    qemu_register_machine(&pc_i440fx_machine_v1_4);
> > -    qemu_register_machine(&pc_machine_v1_3);
> > -    qemu_register_machine(&pc_machine_v1_2);
> > -    qemu_register_machine(&pc_machine_v1_1);
> > -    qemu_register_machine(&pc_machine_v1_0);
> > -    qemu_register_machine(&pc_machine_v0_15);
> > -    qemu_register_machine(&pc_machine_v0_14);
> > -    qemu_register_machine(&pc_machine_v0_13);
> > -    qemu_register_machine(&pc_machine_v0_12);
> > -    qemu_register_machine(&pc_machine_v0_11);
> > -    qemu_register_machine(&pc_machine_v0_10);
> > -    qemu_register_machine(&isapc_machine);
> > +    qemu_register_pc_machine(&pc_i440fx_machine_v2_1);
> > +    qemu_register_pc_machine(&pc_i440fx_machine_v2_0);
> > +    qemu_register_pc_machine(&pc_i440fx_machine_v1_7);
> > +    qemu_register_pc_machine(&pc_i440fx_machine_v1_6);
> > +    qemu_register_pc_machine(&pc_i440fx_machine_v1_5);
> > +    qemu_register_pc_machine(&pc_i440fx_machine_v1_4);
> > +    qemu_register_pc_machine(&pc_machine_v1_3);
> > +    qemu_register_pc_machine(&pc_machine_v1_2);
> > +    qemu_register_pc_machine(&pc_machine_v1_1);
> > +    qemu_register_pc_machine(&pc_machine_v1_0);
> > +    qemu_register_pc_machine(&pc_machine_v0_15);
> > +    qemu_register_pc_machine(&pc_machine_v0_14);
> > +    qemu_register_pc_machine(&pc_machine_v0_13);
> > +    qemu_register_pc_machine(&pc_machine_v0_12);
> > +    qemu_register_pc_machine(&pc_machine_v0_11);
> > +    qemu_register_pc_machine(&pc_machine_v0_10);
> > +    qemu_register_pc_machine(&isapc_machine);
> >  #ifdef CONFIG_XEN
> > -    qemu_register_machine(&xenfv_machine);
> > +    qemu_register_pc_machine(&xenfv_machine);
> >  #endif
> >  }
> >  
> > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> > index b3c02c1..d211393 100644
> > --- a/hw/i386/pc_q35.c
> > +++ b/hw/i386/pc_q35.c
> > @@ -384,12 +384,12 @@ static QEMUMachine pc_q35_machine_v1_4 = {
> >  
> >  static void pc_q35_machine_init(void)
> >  {
> > -    qemu_register_machine(&pc_q35_machine_v2_1);
> > -    qemu_register_machine(&pc_q35_machine_v2_0);
> > -    qemu_register_machine(&pc_q35_machine_v1_7);
> > -    qemu_register_machine(&pc_q35_machine_v1_6);
> > -    qemu_register_machine(&pc_q35_machine_v1_5);
> > -    qemu_register_machine(&pc_q35_machine_v1_4);
> > +    qemu_register_pc_machine(&pc_q35_machine_v2_1);
> > +    qemu_register_pc_machine(&pc_q35_machine_v2_0);
> > +    qemu_register_pc_machine(&pc_q35_machine_v1_7);
> > +    qemu_register_pc_machine(&pc_q35_machine_v1_6);
> > +    qemu_register_pc_machine(&pc_q35_machine_v1_5);
> > +    qemu_register_pc_machine(&pc_q35_machine_v1_4);
> >  }
> >  
> >  machine_init(pc_q35_machine_init);
> > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> > index 32a7687..c7b053c 100644
> > --- a/include/hw/i386/pc.h
> > +++ b/include/hw/i386/pc.h
> > @@ -12,9 +12,33 @@
> >  #include "qemu/bitmap.h"
> >  #include "sysemu/sysemu.h"
> >  #include "hw/pci/pci.h"
> > +#include "hw/boards.h"
> >  
> >  #define HPET_INTCAP "hpet-intcap"
> >  
> > +struct PCMachineState {
> > +    /*< private >*/
> > +    MachineState parent_obj;
> > +};
> > +
> > +struct PCMachineClass {
> > +    /*< private >*/
> > +    MachineClass parent_class;
> > +};
> > +
> > +typedef struct PCMachineState PCMachineState;
> > +typedef struct PCMachineClass PCMachineClass;
> > +
> > +#define TYPE_PC_MACHINE "generic-pc-machine"
> I'll name it pc-machine, without introducing the "generic"
> code word, but maybe is just me.
> 
> Thanks,
> Marcel
> 
> > +#define PC_MACHINE(obj) \
> > +    OBJECT_CHECK(PCMachineState, (obj), TYPE_PC_MACHINE)
> > +#define PC_MACHINE_GET_CLASS(obj) \
> > +    OBJECT_GET_CLASS(PCMachineClass, (obj), TYPE_PC_MACHINE)
> > +#define PC_MACHINE_CLASS(klass) \
> > +    OBJECT_CLASS_CHECK(PCMachineClass, (klass), TYPE_PC_MACHINE)
> > +
> > +void qemu_register_pc_machine(QEMUMachine *m);
> > +
> >  /* PC-style peripherals (also used by other machines).  */
> >  
> >  typedef struct PcPciInfo {
> 
> 
>