Re: [Qemu-devel] [PATCH for-1.6 2/2] pvpanic: make pvpanic known to user

["Andreas Färber" <afaerber@suse.de>]

Am 02.08.2013 09:04, schrieb Hu Tao:
> Thus user can create pvpanic by -device.
> 
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  hw/misc/pvpanic.c    | 23 ++++++++---------------
>  include/hw/i386/pc.h |  3 ---
>  2 files changed, 8 insertions(+), 18 deletions(-)
> 
> diff --git a/hw/misc/pvpanic.c b/hw/misc/pvpanic.c
> index 7bb49a5..6e4c53e 100644
> --- a/hw/misc/pvpanic.c
> +++ b/hw/misc/pvpanic.c
> @@ -93,14 +93,6 @@ static void pvpanic_isa_initfn(Object *obj)
>      memory_region_init_io(&s->io, OBJECT(s), &pvpanic_ops, s, "pvpanic", 1);
>  }
>  
> -static void pvpanic_isa_realizefn(DeviceState *dev, Error **errp)
> -{
> -    ISADevice *d = ISA_DEVICE(dev);
> -    PVPanicState *s = ISA_PVPANIC_DEVICE(dev);
> -
> -    isa_register_ioport(d, &s->io, s->ioport);
> -}
> -
>  static void pvpanic_fw_cfg(ISADevice *dev, FWCfgState *fw_cfg)
>  {
>      PVPanicState *s = ISA_PVPANIC_DEVICE(dev);
> @@ -111,15 +103,16 @@ static void pvpanic_fw_cfg(ISADevice *dev, FWCfgState *fw_cfg)
>                      sizeof(*pvpanic_port));
>  }
>  
> -void pvpanic_init(ISABus *bus)
> +static void pvpanic_isa_realizefn(DeviceState *dev, Error **errp)
>  {
> -    ISADevice *dev;
> +    ISADevice *d = ISA_DEVICE(dev);
> +    PVPanicState *s = ISA_PVPANIC_DEVICE(dev);
>      FWCfgState *fw_cfg = fw_cfg_find();
> -    if (!fw_cfg) {
> -        return;
> +
> +    isa_register_ioport(d, &s->io, s->ioport);
> +    if (fw_cfg) {
> +        pvpanic_fw_cfg(d, fw_cfg);
>      }
> -    dev = isa_create_simple (bus, TYPE_ISA_PVPANIC_DEVICE);
> -    pvpanic_fw_cfg(dev, fw_cfg);
>  }

Doing this in-place above might've been a bit easier to read. ;)

The only thing fw_cfg does at realize time is registering its I/O ports,
the /machine/fw_cfg path is set up in fw_cfg_init() helper function
before, so there are no potential realize ordering problems here.

>  
>  static Property pvpanic_isa_properties[] = {
> @@ -132,8 +125,8 @@ static void pvpanic_isa_class_init(ObjectClass *klass, void *data)
>      DeviceClass *dc = DEVICE_CLASS(klass);
>  
>      dc->realize = pvpanic_isa_realizefn;
> -    dc->no_user = 1;
>      dc->props = pvpanic_isa_properties;

> +    dc->bus_type = TYPE_ISA_BUS;

This is already done in hw/isa/isa-bus.c:isa_device_class_init(), please
drop if we go with this.

Regards,
Andreas

>  }
>  
>  static TypeInfo pvpanic_isa_info = {
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 3a0c4e3..e54751c 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -202,9 +202,6 @@ static inline bool isa_ne2000_init(ISABus *bus, int base, int irq, NICInfo *nd)
>  /* pc_sysfw.c */
>  void pc_system_firmware_init(MemoryRegion *rom_memory);
>  
> -/* pvpanic.c */
> -void pvpanic_init(ISABus *bus);
> -
>  /* e820 types */
>  #define E820_RAM        1
>  #define E820_RESERVED   2
> 


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