Re: [Qemu-devel] [RFC v4 03/12] isa: Provide set_state callback

[Markus Armbruster]

Andreas Färber <address@hidden> writes:

> To allow enabling/disabling present ISA devices without hotplug,
> keep track of state and add a helper to avoid enabling twice.
> Since the properties to be configured are defined at device level,
> delegate the actual work to a callback function.
>
> If no callback is supplied, the device can't be disabled.
>
> Prepare VMSTATE_ISA_DEVICE for devices that support disabling.
> Legacy devices never change their state and won't need this yet.
>
> Cc: Gerd Hoffmann <address@hidden>
> Signed-off-by: Andreas Färber <address@hidden>
> ---
>  hw/hw.h      |   15 +++++++++++++++
>  hw/isa-bus.c |   29 +++++++++++++++++++++++++++++
>  hw/isa.h     |    4 ++++
>  3 files changed, 48 insertions(+), 0 deletions(-)
>
> diff --git a/hw/hw.h b/hw/hw.h
> index 56447a7..07b1e2e 100644
> --- a/hw/hw.h
> +++ b/hw/hw.h
> @@ -628,6 +628,21 @@ extern const VMStateInfo vmstate_info_unused_buffer;
>      .info         = &vmstate_info_unused_buffer,                     \
>      .flags        = VMS_BUFFER,                                      \
>  }
> +
> +extern const VMStateDescription vmstate_isa_device;
> +
> +#define VMSTATE_ISA_DEVICE_V(_field, _state, _version) {             \
> +    .name       = (stringify(_field)),                               \
> +    .version_id   = (_version),                                      \
> +    .size       = sizeof(ISADevice),                                 \
> +    .vmsd       = &vmstate_isa_device,                               \
> +    .flags      = VMS_STRUCT,                                        \
> +    .offset     = vmstate_offset_value(_state, _field, ISADevice),   \
> +}
> +
> +#define VMSTATE_ISA_DEVICE(_field, _state) {                         \
> +    VMSTATE_ISA_DEVICE_V(_field, _state, 0)
> +
>  extern const VMStateDescription vmstate_pci_device;
>  
>  #define VMSTATE_PCI_DEVICE(_field, _state) {                         \
> diff --git a/hw/isa-bus.c b/hw/isa-bus.c
> index 2765543..d258932 100644
> --- a/hw/isa-bus.c
> +++ b/hw/isa-bus.c
> @@ -112,6 +112,7 @@ static int isa_qdev_init(DeviceState *qdev, DeviceInfo 
> *base)
>  
>      dev->isairq[0] = -1;
>      dev->isairq[1] = -1;
> +    dev->enabled = true;
>  
>      return info->init(dev);
>  }
> @@ -156,6 +157,34 @@ ISADevice *isa_create_simple(const char *name)
>      return dev;
>  }
>  
> +const VMStateDescription vmstate_isa_device = {
> +    .name = "ISADevice",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField []) {
> +        VMSTATE_BOOL(enabled, ISADevice),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +int isa_set_state(ISADevice *dev, bool enabled)
> +{
> +    ISADeviceInfo *info = DO_UPCAST(ISADeviceInfo, qdev, dev->qdev.info);
> +    int err;
> +
> +    if (dev->enabled == enabled) {
> +        return 42;

Sorry, too clever.  Make it return 0 for the benefit of future
maintenance programmers.

> +    } else if (info->set_state == NULL) {
> +        return -1;
> +    }
> +    err = info->set_state(dev, enabled);
> +    if (err < 0) {
> +        return err;
> +    }
> +    dev->enabled = enabled;
> +    return err;
> +}
> +
[...]