From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:58521) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1THu1E-0008W4-4X for qemu-devel@nongnu.org; Sat, 29 Sep 2012 06:11:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1THu1D-0002cr-5z for qemu-devel@nongnu.org; Sat, 29 Sep 2012 06:11:32 -0400 Received: from mx1.redhat.com ([209.132.183.28]:45735) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1THu1C-0002aG-TQ for qemu-devel@nongnu.org; Sat, 29 Sep 2012 06:11:31 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q8TABTf7026089 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Sat, 29 Sep 2012 06:11:29 -0400 Received: from localhost.localdomain (vpn1-4-215.ams2.redhat.com [10.36.4.215]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q8TABRUf019694 (version=TLSv1/SSLv3 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO) for ; Sat, 29 Sep 2012 06:11:28 -0400 Message-ID: <5066C95D.9080900@redhat.com> Date: Sat, 29 Sep 2012 12:11:41 +0200 From: Hans de Goede MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] vmstate within vmstate (VMS_STRUCT) versioning is broken ? List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel Hi All, While working on a bugfix for USB migration I noticed that the versioning of VMS_STRUCT does not work as I would expect. Currently when loading VMS_STRUCT state, the vmsd version gets passed: vmstate_load_state(f, field->vmsd, addr, field->vmsd->version_id); This means that any later added fields always get loaded even if there not present. Or they would if the field->version_id was not bumped in sync with the vmsd describing the struct. If the field version_id was properly bumped, like for example is done with vmstate_ide_drive, see the VMSTATE_IDE_DRIVES macro in hw/ide/internal.h, then the following check in vmstate_load_state fails: (!field->field_exists && field->version_id <= version_id)) { And the struct will not be loaded at all, rather then only the new fields in the struct not being loaded! An obvious fix for this would be to: 1) Not do the version check for structs, iow not do: (!field->field_exists && field->version_id <= version_id)) { For structs 2) Change the loading of the struct from: vmstate_load_state(f, field->vmsd, addr, field->vmsd->version_id); to: vmstate_load_state(f, field->vmsd, addr, field->version_id); But I'm afraid that if we do that now, while having the old code for a long time, we may hit some unpleasant surprises. Note that the current behavior means atleast for the ide driver vmstate, that instead of not loading the new cdrom_changed attribute when loading an old vmstate, the entire drive state is not loaded !! Which is clearly not what the ide code expects, and my suggested changes would make the code behave as expected, and would make things work how I myself expected them to work while working on the USB migration. I guess the best thing todo would be to audit for all uses of VMS_STRUCT, and see if that making the changes I suggest is feasible. But before doing such an audit I would first like some input from others on this. Regards, Hans