From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44705) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XaSD6-0003PV-U7 for qemu-devel@nongnu.org; Sat, 04 Oct 2014 12:29:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XaSD4-0001up-FS for qemu-devel@nongnu.org; Sat, 04 Oct 2014 12:29:32 -0400 Received: from mail.avalus.com ([2001:41c8:10:1dd::10]:54194) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XaSD4-0001tG-9m for qemu-devel@nongnu.org; Sat, 04 Oct 2014 12:29:30 -0400 Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 7.3 \(1878.6\)) From: Alex Bligh In-Reply-To: <20140928153058.GA4994@redhat.com> Date: Sat, 4 Oct 2014 17:29:15 +0100 Content-Transfer-Encoding: quoted-printable Message-Id: References: <1411414496-46245-1-git-send-email-alex@alex.org.uk> <1411414496-46245-2-git-send-email-alex@alex.org.uk> <20140928153058.GA4994@redhat.com> Subject: Re: [Qemu-devel] [PATCH v4] Add machine parameter qemu-kvm-migration for live migrate compatibility with qemu-kvm List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: Ryan Harper , Serge Hallyn , "quintela@redhat.com" , Libvirt , Serge Hallyn , "qemu-devel@nongnu.org" , Alexander Graf , Alex Bligh , Cole Robinson , Amit Shah , Bruce Rogers , =?iso-8859-1?Q?Andreas_F=E4rber?= , "Serge E. Hallyn" Michael, I'm about to post a nice simple v5 of this, but there are a couple of your comments I am NOT addressing: >>=20 >> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c >> index b72b34e..3c9da23 100644 >> --- a/hw/acpi/piix4.c >> +++ b/hw/acpi/piix4.c >> @@ -200,12 +200,26 @@ static const VMStateDescription = vmstate_pci_status =3D { >> } >> }; >>=20 >> +static const VMStateDescription vmstate_acpi_compat; >> + >=20 > don't forward declare things, put them right here. This is not addressable in v5; in v5 this is a forward declaration of vmstate_acpi (not _compat). vmstate_acpi references acpi_load_old, and acpi_load_old references vmstate_acpi, so we need a forward reference for one of them in any case. >> + qemu_opt_get_bool(qemu_get_machine_opts(), = "qemu-kvm-migration", >> + DEFAULT_QEMU_KVM_MIGRATION)) { >> + return vmstate_load_state(f, &vmstate_acpi_compat, >> + opaque, version_id); >> + } >=20 > else if version_id =3D=3D 2 return -EINVAL? This is incorrect I think. A version_id of 2 with qemu-kvm_migration off is permissible, and indicates an inbound migration from qemu-git 1.2, i.e. the old manual loading should be run. >> /* qemu-kvm 1.2 uses version 3 but advertised as 2 >> - * To support incoming qemu-kvm 1.2 migration, change version_id >> - * and minimum_version_id to 2 below (which breaks migration from >> + * To support incoming qemu-kvm 1.2 migration, we support >> + * via a command line option a change to minimum_version_id >> + * of 2 in a _compat structure (which breaks migration from >> * qemu 1.2). >=20 > Actually it's version 3 that breaks migration right? > Pls say this explicitly: s/which/version 3 breaks migration from qemu = 1.2/ It's changing the minimum version ID to 2 (from 3) which would break migration from qemu (git) 1.2, because that uses a version ID of 2, and we want the old loader to run in that case. I've made this clearer. --=20 Alex Bligh