diff mbox series

[V1,10/32] kvmclock: restore paused KVM clock

Message ID 1596122076-341293-11-git-send-email-steven.sistare@oracle.com
State New
Headers show
Series [V1,01/32] savevm: add vmstate handler iterators | expand

Commit Message

Steven Sistare July 30, 2020, 3:14 p.m. UTC
If the VM is paused when the KVM clock is serialized to a file, record
that the clock is valid, so the value will be reused rather than
overwritten after cprload with a new call to KVM_GET_CLOCK here:

kvmclock_vm_state_change()
    if (running)
        ...
    else
        if (s->clock_valid)
            return;         <-- instead, return here

        kvm_update_clock()
           kvm_vm_ioctl(kvm_state, KVM_GET_CLOCK, &data)  <-- overwritten

Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
---
 hw/i386/kvm/clock.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Steven Sistare Sept. 25, 2020, 6:07 p.m. UTC | #1
On 9/11/2020 1:50 PM, Dr. David Alan Gilbert wrote:
> * Steve Sistare (steven.sistare@oracle.com) wrote:
>> If the VM is paused when the KVM clock is serialized to a file, record
>> that the clock is valid, so the value will be reused rather than
>> overwritten after cprload with a new call to KVM_GET_CLOCK here:
>>
>> kvmclock_vm_state_change()
>>     if (running)
>>         ...
>>     else
>>         if (s->clock_valid)
>>             return;         <-- instead, return here
>>
>>         kvm_update_clock()
>>            kvm_vm_ioctl(kvm_state, KVM_GET_CLOCK, &data)  <-- overwritten
>>
>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>> ---
>>  hw/i386/kvm/clock.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/i386/kvm/clock.c b/hw/i386/kvm/clock.c
>> index 6428335..161991a 100644
>> --- a/hw/i386/kvm/clock.c
>> +++ b/hw/i386/kvm/clock.c
>> @@ -285,18 +285,22 @@ static int kvmclock_pre_save(void *opaque)
>>      if (!s->runstate_paused) {
>>          kvm_update_clock(s);
>>      }
>> +    if (!runstate_is_running()) {
>> +        s->clock_valid = true;
>> +    }
>>  
>>      return 0;
>>  }
>>  
>>  static const VMStateDescription kvmclock_vmsd = {
>>      .name = "kvmclock",
>> -    .version_id = 1,
>> +    .version_id = 2,
>>      .minimum_version_id = 1,
>>      .pre_load = kvmclock_pre_load,
>>      .pre_save = kvmclock_pre_save,
>>      .fields = (VMStateField[]) {
>>          VMSTATE_UINT64(clock, KVMClockState),
>> +        VMSTATE_BOOL_V(clock_valid, KVMClockState, 2),
>>          VMSTATE_END_OF_LIST()
> 
> We always try and avoid bumping version_id unless we're
> desperate because it breaks backwards migration.
> 
> Didn't you already know from the stored migration state
> (in the globalstate) if the loaded VM was running?
> 
> It's also not clear to me why you're avoiding reloading the state;
> have you preserved that some other way?

This patch was needed only for an early version of cprload which had some gratuitous
vmstate transitions.  I will happily drop this patch.

- Steve

>>      },
>>      .subsections = (const VMStateDescription * []) {
>> -- 
>> 1.8.3.1
>>
diff mbox series

Patch

diff --git a/hw/i386/kvm/clock.c b/hw/i386/kvm/clock.c
index 6428335..161991a 100644
--- a/hw/i386/kvm/clock.c
+++ b/hw/i386/kvm/clock.c
@@ -285,18 +285,22 @@  static int kvmclock_pre_save(void *opaque)
     if (!s->runstate_paused) {
         kvm_update_clock(s);
     }
+    if (!runstate_is_running()) {
+        s->clock_valid = true;
+    }
 
     return 0;
 }
 
 static const VMStateDescription kvmclock_vmsd = {
     .name = "kvmclock",
-    .version_id = 1,
+    .version_id = 2,
     .minimum_version_id = 1,
     .pre_load = kvmclock_pre_load,
     .pre_save = kvmclock_pre_save,
     .fields = (VMStateField[]) {
         VMSTATE_UINT64(clock, KVMClockState),
+        VMSTATE_BOOL_V(clock_valid, KVMClockState, 2),
         VMSTATE_END_OF_LIST()
     },
     .subsections = (const VMStateDescription * []) {