[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 1/2] pvpanic: initialization cleanup
From: |
Laszlo Ersek |
Subject: |
Re: [Qemu-devel] [PATCH v3 1/2] pvpanic: initialization cleanup |
Date: |
Thu, 20 Jun 2013 10:04:02 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130513 Thunderbird/17.0.6 |
On 06/19/13 17:02, Michael S. Tsirkin wrote:
> Avoid use of static variables: PC systems
> initialize pvpanic device through pvpanic_init,
> so we can simply create the fw_cfg file at that point.
> This also makes it possible to skip device
> creation completely if fw_cfg is not there, e.g. for xen -
> so the ports it reserves are not discoverable by guests.
>
> Also, make pvpanic_init void since callers ignore return
> status anyway.
>
> Cc: Stefano Stabellini <address@hidden>
> Cc: Laszlo Ersek <address@hidden>
> Cc: Paul Durrant <address@hidden>
> Signed-off-by: Michael S. Tsirkin <address@hidden>
> ---
> Chanes from v2:
> skip device creation completely if !fw_cfg
> make pvpanic_init void
> Changes from v1:
> don't assert if !fw_cfg
>
>
> hw/misc/pvpanic.c | 30 ++++++++++++++++--------------
> include/hw/i386/pc.h | 2 +-
> 2 files changed, 17 insertions(+), 15 deletions(-)
>
> diff --git a/hw/misc/pvpanic.c b/hw/misc/pvpanic.c
> index 060099b..83ed226 100644
> --- a/hw/misc/pvpanic.c
> +++ b/hw/misc/pvpanic.c
> @@ -97,26 +97,28 @@ static void pvpanic_isa_realizefn(DeviceState *dev, Error
> **errp)
> {
> ISADevice *d = ISA_DEVICE(dev);
> PVPanicState *s = ISA_PVPANIC_DEVICE(dev);
> - static bool port_configured;
> - FWCfgState *fw_cfg;
>
> isa_register_ioport(d, &s->io, s->ioport);
> +}
>
> - if (!port_configured) {
> - fw_cfg = fw_cfg_find();
> - if (fw_cfg) {
> - fw_cfg_add_file(fw_cfg, "etc/pvpanic-port",
> - g_memdup(&s->ioport, sizeof(s->ioport)),
> - sizeof(s->ioport));
> - port_configured = true;
> - }
> - }
> +static void pvpanic_fw_cfg(ISADevice *dev, FWCfgState *fw_cfg)
> +{
> + PVPanicState *s = ISA_PVPANIC_DEVICE(dev);
> +
> + fw_cfg_add_file(fw_cfg, "etc/pvpanic-port",
> + g_memdup(&s->ioport, sizeof(s->ioport)),
> + sizeof(s->ioport));
> }
>
> -int pvpanic_init(ISABus *bus)
> +void pvpanic_init(ISABus *bus)
> {
> - isa_create_simple(bus, TYPE_ISA_PVPANIC_DEVICE);
> - return 0;
> + ISADevice *dev;
> + FWCfgState *fw_cfg = fw_cfg_find();
> + if (!fw_cfg) {
> + return;
> + }
> + dev = isa_create_simple (bus, TYPE_ISA_PVPANIC_DEVICE);
> + pvpanic_fw_cfg(dev, fw_cfg);
> }
>
> static Property pvpanic_isa_properties[] = {
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index ba9ba1a..458eded 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -196,7 +196,7 @@ static inline bool isa_ne2000_init(ISABus *bus, int base,
> int irq, NICInfo *nd)
> void pc_system_firmware_init(MemoryRegion *rom_memory);
>
> /* pvpanic.c */
> -int pvpanic_init(ISABus *bus);
> +void pvpanic_init(ISABus *bus);
>
> /* e820 types */
> #define E820_RAM 1
>
series
Reviewed-by: Laszlo Ersek <address@hidden>