From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58639) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bLSuL-0005rZ-Uw for qemu-devel@nongnu.org; Fri, 08 Jul 2016 06:21:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bLSuH-0003i9-P6 for qemu-devel@nongnu.org; Fri, 08 Jul 2016 06:21:16 -0400 Received: from mx1.redhat.com ([209.132.183.28]:59879) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bLSuH-0003hw-JM for qemu-devel@nongnu.org; Fri, 08 Jul 2016 06:21:13 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id F20497F088 for ; Fri, 8 Jul 2016 10:21:12 +0000 (UTC) Date: Fri, 8 Jul 2016 15:51:10 +0530 From: Amit Shah Message-ID: <20160708102110.GC10869@grmbl.mre> References: <1466433206-29241-1-git-send-email-pbonzini@redhat.com> <20160627072050.GD1389@grmbl.mre> <87oa6kdx2m.fsf@emacs.mitica> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87oa6kdx2m.fsf@emacs.mitica> Subject: Re: [Qemu-devel] [PATCH] scsi: esp: fix migration List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Juan Quintela Cc: Paolo Bonzini , qemu-devel@nongnu.org, "Dr. David Alan Gilbert" On (Wed) 29 Jun 2016 [14:00:17], Juan Quintela wrote: > Paolo Bonzini wrote: > > On 27/06/2016 09:20, Amit Shah wrote: > >> On (Mon) 20 Jun 2016 [16:33:26], Paolo Bonzini wrote: > >>> Commit 926cde5 ("scsi: esp: make cmdbuf big enough for maximum CDB size", > >>> 2016-06-16) changed the size of a migrated field. Split it in two > >>> parts, and only migrate the second part in a new vmstate version. > >> > >> With this patch, the static checker fails in this way: > >> > >> Section "esp", Description "esp": expected field "cmdlen", got > >> "cmdbuf"; skipping rest > >> Section "dc390", Description "esp": expected field "cmdlen", got > >> "cmdbuf"; skipping rest > >> Section "am53c974", Description "esp": expected field "cmdlen", got > >> "cmdbuf"; skipping rest > >> > >> Note it doesn't complain about the version numbers. That's because: > >> > >>> const VMStateDescription vmstate_esp = { > >>> .name ="esp", > >>> - .version_id = 3, > >>> + .version_id = 4, > >>> .minimum_version_id = 3, > >> > >> this suggests older versions can still be accepted for incoming > >> migration, which isn't true. > > > > Sure they can: > > > > - VMSTATE_BUFFER(cmdbuf, ESPState), > > + VMSTATE_PARTIAL_BUFFER(cmdbuf, ESPState, 16), > > + VMSTATE_BUFFER_START_MIDDLE_V(cmdbuf, ESPState, 16, 4), > > Amit, would it help the checker if we do something like: > > - VMSTATE_BUFFER(cmdbuf, ESPState), > + VMSTATE_PARTIAL_BUFFER_TEST(cmdbuf, ESPState, 16, v_is_3), > + VMSTATE_BUFFER_TEST(cmdbuf, ESPState, from_4), > > Yes, VMSTATE_PARTIAL_BUFFER_TEST don't exist, but it is trivial to > define. Yea, I think it'll help. But I'm also thinking what else we do in code that we can offload to vmstate. Or if there's something that we just don't express in vmstate yet. Amit