Re: [Qemu-devel] [RFC PATCH v2 10/49] rtl8139: adding new fields to vmstate

2014-07-30 Thread Pavel Dovgaluk
 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

2014-07-30 Thread Paolo Bonzini
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

2014-07-28 Thread 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 pavel.dovga...@ispras.ru
 ---
  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()
  },
 
 
 




Re: [Qemu-devel] [RFC PATCH v2 10/49] rtl8139: adding new fields to vmstate

2014-07-28 Thread Pavel Dovgaluk
 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 pavel.dovga...@ispras.ru
  ---
   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

2014-07-28 Thread 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.

Paolo



[Qemu-devel] [RFC PATCH v2 10/49] rtl8139: adding new fields to vmstate

2014-07-17 Thread Pavel Dovgalyuk
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 pavel.dovga...@ispras.ru
---
 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),
+VMSTATE_INT64_V(TimerExpire, RTL8139State, 5),
+
 VMSTATE_UINT32_V(cplus_enabled, RTL8139State, 4),
 VMSTATE_END_OF_LIST()
 },