From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39174) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XAiBi-0001YA-Q5 for qemu-devel@nongnu.org; Fri, 25 Jul 2014 12:17:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XAiBb-0004g3-Ek for qemu-devel@nongnu.org; Fri, 25 Jul 2014 12:17:42 -0400 Message-ID: <53D28317.9090108@redhat.com> Date: Fri, 25 Jul 2014 18:17:27 +0200 From: Laszlo Ersek MIME-Version: 1.0 References: <1406303338-13212-1-git-send-email-imammedo@redhat.com> In-Reply-To: <1406303338-13212-1-git-send-email-imammedo@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH for-2.1 0/2] Fix migration failure due to ACPI tables size changes List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: peter.maydell@linaro.org, mst@redhat.com, qemu-stable@nongnu.org, qemu-devel@nongnu.org, pbonzini@redhat.com, dgilbert@redhat.com On 07/25/14 17:48, Igor Mammedov wrote: > Changing the ACPI table size causes migration to break, and the memory > hotplug work opened our eyes on how horribly we were breaking things in > 2.0 already. > > To trigger issue start > QEMU-1.7 with -M pc-i440fx-1.7 -device pci-bridge,chassis_nr=1 > and try to migrate to QEMU-2.1 or QEMU-2.0 as result target will fail with: > qemu-system-x86_64: Length mismatch: /rom@etc/acpi/tables: 2000 in != 3000 > > This fix allows target QEMU to load smaller RAMBlock into a bigger one > and fixes regression which was introduced since 2.0, allowing > forward migration from 1.7/2.0 to 2.1 > Fix is also suitable for stable-2.0. > > Igor Mammedov (2): > migration: load smaller RAMBlock to a bigger one if permitted > acpi: mark ACPI tables ROM blob as extend-able on migration > > arch_init.c | 19 ++++++++++++++----- > exec.c | 15 +++++++++------ > hw/core/loader.c | 6 +++++- > hw/i386/acpi-build.c | 2 +- > include/exec/cpu-all.h | 15 ++++++++++++++- > include/exec/memory.h | 10 ++++++++++ > include/exec/ram_addr.h | 1 + > include/hw/loader.h | 5 +++-- > memory.c | 5 +++++ > 9 files changed, 62 insertions(+), 16 deletions(-) I can name things in favor of both approaches, Paolo's and yours. Paolo's approach keeps the hack where the problem was introduced, in the ACPI generator. The local nature of the migration-related pig sty is very attractive for me. As I said on IRC, if you fix the migration problem in the APCI generator by elegant compat flags, or such an emergency hack as Paolo's, that's an implementation detail; it's important that it stay local to the ACPI generator. Your patchset leaks the problem into RAMBlocks. In favor of your idea OTOH, as you explained, it would work for all possible ACPI configurations, and not break for a random few ones like Paolo's approach (talking about patch #2 of that set, the procedural _PRT is great in any case). You could argue that by customizing the RAMBlocks as extensible (for ACPI / fw_cfg blob backing only) the leakage is contained. I don't know how to decide between these two approaches. The optimal one would be to reimplement the 2.0 --> 2.1 feature additions from scratch, introducing compat flags afresh. Won't happen I guess. In any case I'll try to review this set for the technical things. Thanks Laszlo