From: Laszlo Ersek <lersek@redhat.com>
To: Igor Mammedov <imammedo@redhat.com>
Cc: peter.maydell@linaro.org, mst@redhat.com, qemu-stable@nongnu.org,
qemu-devel@nongnu.org, pbonzini@redhat.com, dgilbert@redhat.com
Subject: Re: [Qemu-devel] [PATCH for-2.1 0/2] Fix migration failure due to ACPI tables size changes
Date: Fri, 25 Jul 2014 18:17:27 +0200 [thread overview]
Message-ID: <53D28317.9090108@redhat.com> (raw)
In-Reply-To: <1406303338-13212-1-git-send-email-imammedo@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
next prev parent reply other threads:[~2014-07-25 16:17 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-07-25 15:48 [Qemu-devel] [PATCH for-2.1 0/2] Fix migration failure due to ACPI tables size changes Igor Mammedov
2014-07-25 15:48 ` [Qemu-devel] [PATCH for-2.1 1/2] migration: load smaller RAMBlock to a bigger one if permitted Igor Mammedov
2014-07-25 17:56 ` Laszlo Ersek
2014-07-28 7:40 ` Igor Mammedov
2014-07-28 8:11 ` Laszlo Ersek
2014-07-25 15:48 ` [Qemu-devel] [PATCH for-2.1 2/2] acpi: mark ACPI tables ROM blob as extend-able on migration Igor Mammedov
2014-07-25 17:37 ` Paolo Bonzini
2014-07-28 7:56 ` Igor Mammedov
2014-07-28 8:16 ` Paolo Bonzini
2014-07-25 17:58 ` Laszlo Ersek
2014-11-03 17:36 ` Dr. David Alan Gilbert
2014-11-04 16:30 ` Paolo Bonzini
2014-11-04 16:46 ` Michael S. Tsirkin
2014-11-04 16:54 ` Paolo Bonzini
2014-11-04 17:25 ` Michael S. Tsirkin
2014-11-04 17:35 ` Paolo Bonzini
2014-11-04 17:42 ` Michael S. Tsirkin
2014-07-25 16:17 ` Laszlo Ersek [this message]
2014-07-26 7:03 ` [Qemu-devel] [PATCH for-2.1 0/2] Fix migration failure due to ACPI tables size changes Paolo Bonzini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=53D28317.9090108@redhat.com \
--to=lersek@redhat.com \
--cc=dgilbert@redhat.com \
--cc=imammedo@redhat.com \
--cc=mst@redhat.com \
--cc=pbonzini@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
--cc=qemu-stable@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).