From patchwork Sun Oct 28 19:40:43 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marcelo Tosatti X-Patchwork-Id: 194746 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id B5FCA2C0093 for ; Mon, 29 Oct 2012 06:55:26 +1100 (EST) Received: from localhost ([::1]:60894 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TSYxA-0001s9-9R for incoming@patchwork.ozlabs.org; Sun, 28 Oct 2012 15:55:24 -0400 Received: from eggs.gnu.org ([208.118.235.92]:54869) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TSYx3-0001rB-G7 for qemu-devel@nongnu.org; Sun, 28 Oct 2012 15:55:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TSYx2-00058H-2V for qemu-devel@nongnu.org; Sun, 28 Oct 2012 15:55:17 -0400 Received: from mx1.redhat.com ([209.132.183.28]:48242) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TSYx1-00058B-QZ for qemu-devel@nongnu.org; Sun, 28 Oct 2012 15:55:15 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q9SJtD4A029909 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Sun, 28 Oct 2012 15:55:13 -0400 Received: from amt.cnet (vpn1-4-119.gru2.redhat.com [10.97.4.119]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q9SJtCSc011551; Sun, 28 Oct 2012 15:55:12 -0400 Received: from amt.cnet (amt.cnet [127.0.0.1]) by amt.cnet (Postfix) with ESMTP id 5289968A2D5; Sun, 28 Oct 2012 17:40:47 -0200 (BRST) Received: (from marcelo@localhost) by amt.cnet (8.14.5/8.14.5/Submit) id q9SJehhX025606; Sun, 28 Oct 2012 17:40:43 -0200 Date: Sun, 28 Oct 2012 17:40:43 -0200 From: Marcelo Tosatti To: kvm , qemu-devel@nongnu.org Message-ID: <20121028194042.GA21683@amt.cnet> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 209.132.183.28 Cc: Isaku Yamahata , Anthony Liguori , Juan Quintela , Avi Kivity , Igor Mammedov Subject: [Qemu-devel] acpi_piix4 migration issue X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org qemu-kvm 1.2 -> qemu-1.3 migration fails with Unknown savevm section type 48 load of migration failed Due to a fix in acpi_piix4 in qemu-kvm (attached at the end of the message). The problem is that qemu-kvm correctly uses 2 bytes for sts and 2 bytes for en fields (which is their allocated size), while qemu uses 4*2 bytes for each. The fix present in qemu-kvm is correct, but, having it in qemu 1.3 would break qemu 1.2 -> qemu 1.3 migration (while allowing qemu-kvm 1.2 -> qemu 1.3 migration). Any opinions on what to do? commit b2e4a396f71ace63df6fa7e853f8835c4db6c920 Author: Isaku Yamahata Date: Sun Apr 17 22:50:45 2011 +0900 acpi, acpi_piix: factor out GPE logic On Sun, Apr 17, 2011 at 04:17:51PM +0300, Avi Kivity wrote: > On 03/16/2011 11:29 AM, Isaku Yamahata wrote: >> factor out ACPI GPE logic. Later it will be used by ICH9 ACPI. >> > > I think this patch is causing qemu-kvm failures on migration: > (gdb) bt > #0 0x000000000049aff4 in qemu_put_be16s (f=0x1a74490, pv=0x2c02580, > size=2) at hw/hw.h:108 > #1 put_uint16 (f=0x1a74490, pv=0x2c02580, size=2) at savevm.c:855 > #2 0x000000000049c3e4 in vmstate_save_state (f=0x1a74490, > vmsd=0x6f0b00, opaque=0x1842ef0) at savevm.c:1436 > #3 0x000000000049c3b6 in vmstate_save_state (f=0x1a74490, > vmsd=0x6f0aa0, opaque=0x1842b90) at savevm.c:1434 > #4 0x000000000049c6f1 in vmstate_save (mon=, > f=0x1a74490) at savevm.c:1459 > #5 qemu_savevm_state_complete (mon=, f=0x1a74490) > at savevm.c:1600 > #6 0x000000000049455a in migrate_fd_put_ready (opaque=0x1847890) at > migration.c:383 > #7 0x00000000004ce2eb in qemu_run_timers (clock=) > at qemu-timer.c:505 > #8 0x00000000004ce806 in qemu_run_all_timers () at qemu-timer.c:619 > #9 0x0000000000419463 in main_loop_wait (nonblocking= out>) at /build/home/tlv/akivity/qemu-kvm/vl.c:1339 > #10 0x0000000000433927 in kvm_main_loop () at > /build/home/tlv/akivity/qemu-kvm/qemu-kvm.c:1590 > #11 0x000000000041a3a6 in main_loop (argc=, > argv=, envp=) > at /build/home/tlv/akivity/qemu-kvm/vl.c:1369 > #12 main (argc=, argv=, > envp=) at /build/home/tlv/akivity/qemu-kvm/vl.c:3257 > > The vmstate being migrated is "gpe". > > > >> >> +#define VMSTATE_GPE_ARRAY(_field, _state) \ >> + { \ >> + .name = (stringify(_field)), \ >> + .version_id = 0, \ >> + .num = GPE_LEN, \ >> + .info =&vmstate_info_uint16, \ >> + .size = sizeof(uint16_t), \ >> + .flags = VMS_ARRAY | VMS_POINTER, \ >> + .offset = vmstate_offset_pointer(_state, _field, uint8_t), \ >> + } >> + >> static const VMStateDescription vmstate_gpe = { >> .name = "gpe", >> .version_id = 1, >> .minimum_version_id = 1, >> .minimum_version_id_old = 1, >> .fields = (VMStateField []) { >> - VMSTATE_UINT16(sts, struct gpe_regs), >> - VMSTATE_UINT16(en, struct gpe_regs), >> + VMSTATE_GPE_ARRAY(sts, ACPIGPE), >> + VMSTATE_GPE_ARRAY(en, ACPIGPE), >> VMSTATE_END_OF_LIST() >> } >> }; > > I'm no vmstate expert, but this does look odd. Why both VMS_ARRAY and > VMS_POINTER? aren't we trying to save/restore a simple 16-bit value? Or > at least we did before this patch. That's right. the difference is, the new member type became uint8_t*. Does the following help? Signed-off-by: Avi Kivity diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c index d65a7e9..9dc6f43 100644 --- a/hw/acpi_piix4.c +++ b/hw/acpi_piix4.c @@ -221,10 +221,9 @@ static int vmstate_acpi_post_load(void *opaque, int version_id) { \ .name = (stringify(_field)), \ .version_id = 0, \ - .num = GPE_LEN, \ .info = &vmstate_info_uint16, \ .size = sizeof(uint16_t), \ - .flags = VMS_ARRAY | VMS_POINTER, \ + .flags = VMS_SINGLE | VMS_POINTER, \ .offset = vmstate_offset_pointer(_state, _field, uint8_t), \ }