Re: [Qemu-devel] [PATCH v2] hpet: recover timer offset correctly

[Juan Quintela <quintela@redhat.com>]

"Pavel Dovgalyuk" <dovgaluk@ispras.ru> wrote:
>> From: Juan Quintela [mailto:quintela@redhat.com]
>> "Pavel Dovgalyuk" <dovgaluk@ispras.ru> wrote:
>> >> From: Juan Quintela [mailto:quintela@redhat.com]
>> If you *don't* use a needed function then please just increase the
>> version.  You are just breaking compatibility anyways.  The whole point
>> of subsections is that they are optional.  If they are mandatory (this
>> case), then they bring no advantage at all.
>
> Thanks, I thought that the sections are skipped automatically when
> there is no code for loading
> them.
>
>> What dave is asked for your previous version is that you disable the
>> section for old machine types.  Look at how to use DEFINE_PROP_* for
>> this use case.
>
> How do you like this one?

Much better, thanks.

> +static bool hpet_offset_needed(void *opaque)
> +{
> +    HPETState *s = opaque;
> +
> +    return s->hpet_offset_saved;
> +}
> +

If this is only one optimization, this test is ok.  If it makes things
go worse, you can add something there like && hpet_enabled() or
whatever.  Remember that I don't understand HPET.


> diff --git a/include/hw/compat.h b/include/hw/compat.h
> index 263de97..8897302 100644
> --- a/include/hw/compat.h
> +++ b/include/hw/compat.h
> @@ -13,6 +13,10 @@
>          .driver   = "virtio-tablet-device",\
>          .property = "wheel-axis",\
>          .value    = "false",\
> +    },{\
> +        .driver   = "hpet",\
> +        .property = "hpet-offset-saved",\
> +        .value    = "off",\
>      },
>  
>  #define HW_COMPAT_2_9 \

This should be on 2_11 not 2_10 O:-)

But for the vmstate bits:

Reviewed-by: Juan Quintela <quintela@redhat.com>