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

[Marcel Apfelbaum <marcel.a@redhat.com>]

On Thu, 2014-03-20 at 16:01 +0100, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  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' ?

The patch is:
[PATCH 1/3] hw/machine: move QEMUMachine assignment into the core machine
http://lists.gnu.org/archive/html/qemu-devel/2014-03/msg02151.html

> +{
> +    TypeInfo ti = {
> +        .name       = g_strconcat(m->name, TYPE_MACHINE_SUFFIX, NULL),
This leads to a small memory leak, I missed that myself :(
Here is a fixed version:
[PATCHv2] vl.c: Fix memory leak in qemu_register_machine
https://www.mail-archive.com/qemu-devel@nongnu.org/msg222800.html

Thanks,
Marcel

> +        .parent     = TYPE_PC_MACHINE,
> +        .class_init = machine_class_init,
> +        .class_data = (void *)m,
> +    };
> +
> +    type_register(&ti);
> +}
> +
> +static const TypeInfo pc_machine_info = {
> +    .name = TYPE_PC_MACHINE,
> +    .parent = TYPE_MACHINE,
> +    .abstract = true,
> +    .instance_size = sizeof(PCMachineState),
> +};
> +
> +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 7930a26..97df43e 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -817,24 +817,24 @@ static QEMUMachine xenfv_machine = {
>  
>  static void pc_machine_init(void)
>  {
> -    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_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 c844dc2..16b4daa 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -358,11 +358,11 @@ static QEMUMachine pc_q35_machine_v1_4 = {
>  
>  static void pc_q35_machine_init(void)
>  {
> -    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_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 9010246..a01a220 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -12,9 +12,23 @@
>  #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;
> +};
> +
> +typedef struct PCMachineState PCMachineState;
> +
> +#define TYPE_PC_MACHINE "abstract-pc-machine"
> +#define PC_MACHINE(obj) \
> +     OBJECT_CHECK(PCMachineState, (obj), TYPE_PC_MACHINE)
> +
> +void qemu_register_pc_machine(QEMUMachine *m);
> +
>  /* PC-style peripherals (also used by other machines).  */
>  
>  typedef struct PcPciInfo {