Re: [Qemu-devel] [PATCH v11 1/5] vmstate: add VMSTATE_PTIMER_ARRAY

[Peter Maydell]

On 11 December 2013 08:08, liguang <address@hidden> wrote:
> +static int get_ptimer(QEMUFile *f, void *pv, size_t size)
> +{
> +    ptimer_state *v = pv;
> +    uint64_t count;
> +
> +    count = qemu_get_be64(f);
> +    if (count != -1) {
> +        ptimer_set_count(v, count);
> +    } else {
> +        ptimer_stop(v);
> +    }
> +
> +    return 0;
> +}
> +
> +static void put_ptimer(QEMUFile *f, void *pv, size_t size)
> +{
> +    ptimer_state *v = pv;
> +    uint64_t count;
> +
> +    count = ptimer_get_count(v);
> +    qemu_put_be64(f, count);
> +}
> +
> +const VMStateInfo vmstate_info_ptimer = {
> +    .name = "ptimer",
> +    .get  = get_ptimer,
> +    .put  = put_ptimer,
> +};

Sorry, I led you a bit astray with my last review comment;
this is definitely wrong because it isn't saving and
restoring each ptimer_state according to the vmstate_ptimer
definition, it's only saving a single 64 bit count.
Doing this right isn't quite as obvious as I thought
because we haven't needed to do "array of pointers to
structures" yet, so there's a missing macro.

I've written a patch which does this correctly -- I'll
send it out shortly and you can add it to your patch
series in place of this one.

thanks
-- PMM