From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anthony Liguori Subject: Re: [Qemu-devel] [PATCH v2 3/5] hpet 'driftfix': add fields to HPETTimer and VMStateDescription Date: Mon, 11 Apr 2011 08:10:52 -0500 Message-ID: <4DA2FDDC.2020409@us.ibm.com> References: <1473344612.11808.1302510244411.JavaMail.root@zmail07.collab.prod.int.phx2.redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org, jan kiszka , gcosta@redhat.com, avi@redhat.com To: Ulrich Obergfell Return-path: Received: from e4.ny.us.ibm.com ([32.97.182.144]:45797 "EHLO e4.ny.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751101Ab1DKNK4 (ORCPT ); Mon, 11 Apr 2011 09:10:56 -0400 Received: from d01dlp02.pok.ibm.com (d01dlp02.pok.ibm.com [9.56.224.85]) by e4.ny.us.ibm.com (8.14.4/8.13.1) with ESMTP id p3BCoqK2013952 for ; Mon, 11 Apr 2011 08:50:55 -0400 Received: from d01relay05.pok.ibm.com (d01relay05.pok.ibm.com [9.56.227.237]) by d01dlp02.pok.ibm.com (Postfix) with ESMTP id 9A64B6E803C for ; Mon, 11 Apr 2011 09:10:55 -0400 (EDT) Received: from d01av02.pok.ibm.com (d01av02.pok.ibm.com [9.56.224.216]) by d01relay05.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p3BDAtQd149218 for ; Mon, 11 Apr 2011 09:10:55 -0400 Received: from d01av02.pok.ibm.com (loopback [127.0.0.1]) by d01av02.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p3BDAsip000966 for ; Mon, 11 Apr 2011 10:10:54 -0300 In-Reply-To: <1473344612.11808.1302510244411.JavaMail.root@zmail07.collab.prod.int.phx2.redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: 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