From: Peter Xu <peterx@redhat.com>
To: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Cc: qemu-devel@nongnu.org, "Kevin Wolf" <kwolf@redhat.com>,
"Hanna Reitz" <hreitz@redhat.com>,
"Philippe Mathieu-Daudé" <philmd@mailo.com>,
"Zhao Liu" <zhao1.liu@intel.com>,
"Stefano Stabellini" <sstabellini@kernel.org>,
"Anthony PERARD" <anthony@xenproject.org>,
"Edgar E. Iglesias" <edgar.iglesias@gmail.com>,
"Fabiano Rosas" <farosas@suse.de>,
"Paolo Bonzini" <pbonzini@redhat.com>,
"Reinoud Zandijk" <reinoud@netbsd.org>,
"Marcelo Tosatti" <mtosatti@redhat.com>,
"Alex Williamson" <alex@shazbot.org>,
"Cédric Le Goater" <clg@redhat.com>,
qemu-block@nongnu.org, xen-devel@lists.xenproject.org,
kvm@vger.kernel.org
Subject: Re: [PATCH 0/3] migration/ram: Abort on unsupported migratable RAM changes
Date: Tue, 23 Jun 2026 11:45:08 -0400 [thread overview]
Message-ID: <ajqqBBA3Ynxqgu_S@x1.local> (raw)
In-Reply-To: <b8154a3d-5538-42e1-995b-2aeb05daa4ab@rsg.ci.i.u-tokyo.ac.jp>
On Tue, Jun 23, 2026 at 09:05:22PM +0900, Akihiko Odaki wrote:
> On 2026/06/23 5:23, Peter Xu wrote:
> > On Thu, Jun 11, 2026 at 03:35:47PM +0900, Akihiko Odaki wrote:
> > > Supersedes: <20260604-migration-v1-1-cef4a5b1bbdd@rsg.ci.i.u-tokyo.ac.jp>
> > > ("[PATCH] system/physmem: Assert migration invariants")
> > >
> > > ram_mig_ram_block_resized() already aborts migration when migratable RAM
> > > is resized. Extend the same handling to other unsupported changes to the
> > > migratable RAMBlock set, such as removing a migratable RAMBlock or
> > > changing a RAMBlock's migratable state.
> > >
> > > Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> > > ---
> > > Akihiko Odaki (3):
> > > system/physmem: Pass RAMBlock to RAMBlockNotifier callbacks
> > > system/physmem: Notify RAMBlock migratable and idstr changes
> > > migration/ram: Abort on unsupported migratable RAM changes
> >
> > Thanks for looking at this, Akihiko.
> >
> > I understand this is a protection to the system to trap error use cases.
> > The question I have is do we have any possible way to trigger these.
> >
> > I worry we add a bunch of code and notifiers, and then there's zero way to
> > trigger, essentially add dead code.
> >
> > Logically we could already add assert() on things we don't expect to
> > happen. This case might be slightly risky, but still I think we can also
> > consider things like error_report_once() instead of introducing slightly
> > complex notifiers just to cover what we think shouldn't happen.
> >
> > Or do you have way to trigger any of these notifiers?
>
> I simply followed what's already done for resize(), expecting resize() does
> the correct thing and following it won't introduce a regression.
>
> >
> > PS: today I went back and I wanted to try how the existing resize()
> > notifier would trigger, I can't even reproduce it with David's example
> > here:
> >
> > https://lore.kernel.org/qemu-devel/20210429112708.12291-1-david@redhat.com/#t
> >
> > I can trap a qemu_ram_resize(), but that's invoked with newsize==rb->size,
> > so it didn't really notify a thing. I don't really know how to trigger
> > ram_block_notify_resize(). If you know, please share.
> I made an LLM amend the reproducer. Below is its output.
>
> Regards,
> Akihiko Odaki
>
> LLM output:
>
> A synthetic but effective variant is to add custom ACPI filler tables so the
> initial `etc/acpi/tables` blob is just under the 128 KiB alignment bucket,
> then let the normal boot-time fw_cfg ACPI rebuild push it over.
>
> I tested this shape:
>
> ```sh
> truncate -s 65000 /tmp/fill1
> truncate -s 50600 /tmp/fill2
> ```
>
> Then add to the original-ish command:
>
> ```sh
> -device pcie-root-port,id=rp0,chassis=1,slot=1 \
> -acpitable sig=FI1A,data=/tmp/fill1 \
> -acpitable sig=FI2A,data=/tmp/fill2
> ```
These lines should inject some sections into ACPI, but I don't see why the
acpi table would change: that should be appended right at QEMU boots, so I
expect the ACPI table to grow indeed comparing to when without these lines,
but not resize during VM running. I wonder if below is hallucinations from
the AI.
>
> Observed via `info ramblock`:
>
> ```text
> before cont:
> /rom@etc/acpi/tables Used 0x0000000000020000
>
> after cont:
> /rom@etc/acpi/tables Used 0x0000000000040000
> ```
>
> So this does produce a real RAMBlock used-size growth during boot in the
> current tree. With migration started before `cont` using a stalled `exec:`
> target, `info migrate` moved to `cancelling`, which is consistent with the
> current resize-during-precopy abort path.
>
> The key is not the root port itself; the key is making the ACPI table
> rebuild cross `ACPI_BUILD_TABLE_SIZE` alignment. The filler is a bit
> artificial, but it is a good stress variant for the exact class of bug.
I did have a closer look on this whole "MR size can change" thing.
We have two users: ACPI (rom_add_blob()) and other firmwares (most of them
rom_add_file() users, very little used rom_add_blob()).
AFAIU, the real resize should only happen at the 2nd user, not ACPI.
ACPI seems to be able to change ROM size (PS: this is tricky to call it ROM
in the first place: I believe it's only a data blob in fw_cfg) when e.g. it
scans the pci bus and things changed, only happen during reboot, but it
can't happen during migration because qdev_add is forbidden.
Device ROMs can really change size if dest host has newer firmware packages
than source, but that's another use case and I _think_ we support fine,
except that firmwares can only grow not shrink, guarded by
qemu_ram_resize() check on max_length.
That's a pretty niche use case and nothing I can think of that on change of
flipping migratable and so on. So IMHO we will need to understand the
problem better before having more notifiers.
PS: I wished ACPI three use cases of ROM can be part of device states
already, then it is out of question on MR resize complexity: the max size
is 128K as far as I know; it doesn't need iterability... we migrate devices
sometimes much larger than 128KB on device states. It can be a VMSD field.
Thanks,
--
Peter Xu
next prev parent reply other threads:[~2026-06-23 15:45 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-11 6:35 [PATCH 0/3] migration/ram: Abort on unsupported migratable RAM changes Akihiko Odaki
2026-06-11 6:35 ` [PATCH 1/3] system/physmem: Pass RAMBlock to RAMBlockNotifier callbacks Akihiko Odaki
2026-06-11 6:35 ` [PATCH 2/3] system/physmem: Notify RAMBlock migratable and idstr changes Akihiko Odaki
2026-06-11 6:35 ` [PATCH 3/3] migration/ram: Abort on unsupported migratable RAM changes Akihiko Odaki
2026-06-22 20:23 ` [PATCH 0/3] " Peter Xu
2026-06-23 12:05 ` Akihiko Odaki
2026-06-23 15:45 ` Peter Xu [this message]
2026-06-23 16:38 ` Akihiko Odaki
2026-06-29 18:21 ` Peter Xu
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=ajqqBBA3Ynxqgu_S@x1.local \
--to=peterx@redhat.com \
--cc=alex@shazbot.org \
--cc=anthony@xenproject.org \
--cc=clg@redhat.com \
--cc=edgar.iglesias@gmail.com \
--cc=farosas@suse.de \
--cc=hreitz@redhat.com \
--cc=kvm@vger.kernel.org \
--cc=kwolf@redhat.com \
--cc=mtosatti@redhat.com \
--cc=odaki@rsg.ci.i.u-tokyo.ac.jp \
--cc=pbonzini@redhat.com \
--cc=philmd@mailo.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=reinoud@netbsd.org \
--cc=sstabellini@kernel.org \
--cc=xen-devel@lists.xenproject.org \
--cc=zhao1.liu@intel.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.