Re: [Qemu-devel] [PATCH v2 3/5] hpet 'driftfix': add fields to HPETTimer and VMStateDescription

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 04/11/2011 03:24 AM, Ulrich Obergfell wrote:
   typedef struct HPETState {
@@ -248,7 +253,7 @@ static int hpet_post_load(void *opaque, int
version_id)

   static const VMStateDescription vmstate_hpet_timer = {
       .name = "hpet_timer",
- .version_id = 1,
+ .version_id = 3,
Why jump from 1 to 3?

       .minimum_version_id = 1,
       .minimum_version_id_old = 1,
       .fields = (VMStateField []) {
@@ -258,6 +263,11 @@ static const VMStateDescription
vmstate_hpet_timer = {
           VMSTATE_UINT64(fsb, HPETTimer),
           VMSTATE_UINT64(period, HPETTimer),
           VMSTATE_UINT8(wrap_flag, HPETTimer),
+ VMSTATE_UINT64_V(saved_period, HPETTimer, 3),
+ VMSTATE_UINT64_V(ticks_not_accounted, HPETTimer, 3),
+ VMSTATE_UINT32_V(irqs_to_inject, HPETTimer, 3),
+ VMSTATE_UINT32_V(irq_rate, HPETTimer, 3),
+ VMSTATE_UINT32_V(divisor, HPETTimer, 3),

Anthony,

I incremented the version ID of 'vmstate_hpet' from 2 to 3 to make sure
that migrations from a QEMU process that is capable of 'driftfix' to a
QEMU process that is _not_ capable of 'driftfix' will fail. I assigned
version ID 3 to 'vmstate_hpet_timer' and to the new fields in there too
to indicate that adding those fields was the reason why the version ID
of 'vmstate_hpet' was incremented to 3.

As far as the flow of execution in vmstate_load_state() is concerned, I
think it does not matter whether the version ID of 'vmstate_hpet_timer'
and the new fields in there is 2 or 3 (as long as they are consistent).
When the 'while(field->name)' loop in vmstate_load_state() gets to the
following field in 'vmstate_hpet' ...

     VMSTATE_STRUCT_VARRAY_UINT8(timer, HPETState, num_timers, 0,
                                 vmstate_hpet_timer, HPETTimer),

... it calls itself recursively ...

     if (field->flags&  VMS_STRUCT) {
         ret = vmstate_load_state(f, field->vmsd, addr, field->vmsd->version_id);

'field->vmsd->version_id' is the version ID of 'vmstate_hpet_timer' [1].
Hence 'vmstate_hpet_timer.version_id' is being checked against itself ...

     if (version_id>  vmsd->version_id) {
         return -EINVAL;
     }

... and the version IDs of the new fields are also being checked against
'vmstate_hpet_timer.version_id' ...

     if ((field->field_exists&&
          field->field_exists(opaque, version_id)) ||
         (!field->field_exists&&
          field->version_id<= version_id)) {

If you want me to change the version ID of 'vmstate_hpet_timer' and the
new fields in there from 3 to 2, I can do that.

It avoids surprises so I think it's a reasonable thing to do. But yes, your analysis is correct.

Regards,

Anthony Liguori


Regards,

Uli


[1] Ref.: commit fa3aad24d94a6cf894db52d83f72a399324a17bb

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux