Re: [Qemu-devel] [PATCH] Save/load PC speaker internal state

[Peter Maydell]

On 27 August 2012 13:21, Pavel Dovgaluk <address@hidden> wrote:
> Save PC speaker state to remove differences between system
> states after saving the snapshot and after loading it again.
> This patch is needed for deterministic replay of the execution.
>
> Signed-off-by: Pavel Dovgalyuk<address@hidden>

Hi Pavel; thanks for this patch. Couple of minor issues:

> +static const VMStateDescription vmstate_spk = {
> +    .name = "pcspk",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .minimum_version_id_old = 1,
> +    .fields      = (VMStateField[]) {
> +        VMSTATE_UINT8_ARRAY(sample_buf, PCSpkState, PCSPK_BUF_LEN),
> +        VMSTATE_UINT32(pit_count, PCSpkState),
> +        VMSTATE_UINT32(samples, PCSpkState),
> +        VMSTATE_UINT32(play_pos, PCSpkState),
> +        VMSTATE_INT32(data_on, PCSpkState),
> +        VMSTATE_INT32(dummy_refresh_clock, PCSpkState),

I think that you need also to update the types in the PCSpkState
struct from int/unsigned int to int32_t/uint32_t, otherwise this
won't compile on a 64 bit system.

> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  static int pcspk_initfn(ISADevice *dev)
>  {
>      PCSpkState *s = DO_UPCAST(PCSpkState, dev, dev);
>
> +    vmstate_register(NULL, 0, &vmstate_spk, s);
> +

It's nicer to register the vmstate by setting
    dc->vmsd = &vmstate_spk;
in pcspk_class_initfn(); then you don't need to explicitly call
vmstate_register().

>      memory_region_init_io(&s->ioport, &pcspk_io_ops, s, "elcr", 1);
>      isa_register_ioport(dev, &s->ioport, s->iobase);

-- PMM