Re: [Qemu-devel] [RFC PATCH v2 10/49] rtl8139: adding new fields to vmstate
Il 30/07/2014 10:24, Pavel Dovgaluk ha scritto: > Assertions is a good idea, we added such warning message to qemu_get_timedate > function > to be sure, that it is used correctly with replay. > > Another thing, that could help for making snapshots - find a way to load > replay structures > before all other ones. Are there any priorities in migration states list? No, it is just about vmstate registration order. In general bus parents come before bus children for obvious reasons. Paolo > Priorities could also solve some other issues, because sometimes post_load > function > of one device uses other devices' functions. And the second ones could be not > loaded yet.
Re: [Qemu-devel] [RFC PATCH v2 10/49] rtl8139: adding new fields to vmstate
> From: Paolo Bonzini [mailto:paolo.bonz...@gmail.com] On Behalf Of Paolo > Bonzini > Il 28/07/2014 11:54, Pavel Dovgaluk ha scritto: > >>> > > +VMSTATE_TIMER_V(timer, RTL8139State, 5), > >> > > >> > timer need not be migrated, because it is reinstated by > >> > rtl8139_post_load. > >> > > > That's true for normal execution. > > In replay execution mode post_load can be called before cached virtual > > clock > > values are loaded. This may cause invalid setting of the timer and raising > > an IRQ which didn't happen in record mode. > > I will update this patch to fix post_load function and avoid this > > non-deterministic behavior. > > This is what worries me of this series. These invariants are not > documented anywhere, and people will break them unless you add > assertions that also hold in normal mode. Assertions is a good idea, we added such warning message to qemu_get_timedate function to be sure, that it is used correctly with replay. Another thing, that could help for making snapshots - find a way to load replay structures before all other ones. Are there any priorities in migration states list? Priorities could also solve some other issues, because sometimes post_load function of one device uses other devices' functions. And the second ones could be not loaded yet. Pavel Dovgalyuk
Re: [Qemu-devel] [RFC PATCH v2 10/49] rtl8139: adding new fields to vmstate
Il 28/07/2014 11:54, Pavel Dovgaluk ha scritto: >>> > > +VMSTATE_TIMER_V(timer, RTL8139State, 5), >> > >> > timer need not be migrated, because it is reinstated by rtl8139_post_load. >> > > That's true for normal execution. > In replay execution mode post_load can be called before cached virtual clock > values are loaded. This may cause invalid setting of the timer and raising > an IRQ which didn't happen in record mode. > I will update this patch to fix post_load function and avoid this > non-deterministic behavior. This is what worries me of this series. These invariants are not documented anywhere, and people will break them unless you add assertions that also hold in normal mode. Paolo
Re: [Qemu-devel] [RFC PATCH v2 10/49] rtl8139: adding new fields to vmstate
> From: Paolo Bonzini [mailto:paolo.bonz...@gmail.com] On Behalf Of Paolo
> Bonzini
> Il 17/07/2014 13:02, Pavel Dovgalyuk ha scritto:
> > This patch adds virtual clock-dependent timers to VMState to allow correct
> > saving and restoring the state of RTL8139 network controller.
> >
> > Signed-off-by: Pavel Dovgalyuk
> > ---
> > hw/net/rtl8139.c |5 -
> > 1 files changed, 4 insertions(+), 1 deletions(-)
> >
> > diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
> > index 90bc5ec..992caf0 100644
> > --- a/hw/net/rtl8139.c
> > +++ b/hw/net/rtl8139.c
> > @@ -3289,7 +3289,7 @@ static void rtl8139_pre_save(void *opaque)
> >
> > static const VMStateDescription vmstate_rtl8139 = {
> > .name = "rtl8139",
> > -.version_id = 4,
> > +.version_id = 5,
> > .minimum_version_id = 3,
> > .post_load = rtl8139_post_load,
> > .pre_save = rtl8139_pre_save,
> > @@ -3363,6 +3363,9 @@ static const VMStateDescription vmstate_rtl8139 = {
> > VMSTATE_STRUCT(tally_counters, RTL8139State, 0,
> > vmstate_tally_counters, RTL8139TallyCounters),
> >
> > +VMSTATE_TIMER_V(timer, RTL8139State, 5),
>
> timer need not be migrated, because it is reinstated by rtl8139_post_load.
>
That's true for normal execution.
In replay execution mode post_load can be called before cached virtual clock
values are loaded. This may cause invalid setting of the timer and raising
an IRQ which didn't happen in record mode.
I will update this patch to fix post_load function and avoid this
non-deterministic behavior.
Pavel Dovgalyuk
Re: [Qemu-devel] [RFC PATCH v2 10/49] rtl8139: adding new fields to vmstate
Il 17/07/2014 13:02, Pavel Dovgalyuk ha scritto:
> This patch adds virtual clock-dependent timers to VMState to allow correct
> saving and restoring the state of RTL8139 network controller.
>
> Signed-off-by: Pavel Dovgalyuk
> ---
> hw/net/rtl8139.c |5 -
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
> index 90bc5ec..992caf0 100644
> --- a/hw/net/rtl8139.c
> +++ b/hw/net/rtl8139.c
> @@ -3289,7 +3289,7 @@ static void rtl8139_pre_save(void *opaque)
>
> static const VMStateDescription vmstate_rtl8139 = {
> .name = "rtl8139",
> -.version_id = 4,
> +.version_id = 5,
> .minimum_version_id = 3,
> .post_load = rtl8139_post_load,
> .pre_save = rtl8139_pre_save,
> @@ -3363,6 +3363,9 @@ static const VMStateDescription vmstate_rtl8139 = {
> VMSTATE_STRUCT(tally_counters, RTL8139State, 0,
> vmstate_tally_counters, RTL8139TallyCounters),
>
> +VMSTATE_TIMER_V(timer, RTL8139State, 5),
timer need not be migrated, because it is reinstated by rtl8139_post_load.
> +VMSTATE_INT64_V(TimerExpire, RTL8139State, 5),
This can be in a subsection, migrated only if non-zero.
Paolo
> VMSTATE_UINT32_V(cplus_enabled, RTL8139State, 4),
> VMSTATE_END_OF_LIST()
> },
>
>
>
