From: Juan Quintela <quintela@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Stefan Berger" <stefanb@linux.vnet.ibm.com>,
"Marcel Apfelbaum" <marcel.apfelbaum@gmail.com>,
qemu-ppc@nongnu.org, "Nicholas Piggin" <npiggin@gmail.com>,
qemu-s390x@nongnu.org, "Gerd Hoffmann" <kraxel@redhat.com>,
"Corey Minyard" <cminyard@mvista.com>,
"Samuel Thibault" <samuel.thibault@ens-lyon.org>,
"Richard Henderson" <richard.henderson@linaro.org>,
"David Hildenbrand" <david@redhat.com>,
"Ilya Leoshkevich" <iii@linux.ibm.com>,
"Fabiano Rosas" <farosas@suse.de>,
"Eric Farman" <farman@linux.ibm.com>,
"Peter Xu" <peterx@redhat.com>,
"Harsh Prateek Bora" <harshpb@linux.ibm.com>,
"John Snow" <jsnow@redhat.com>,
qemu-block@nongnu.org,
"Mark Cave-Ayland" <mark.cave-ayland@ilande.co.uk>,
"Christian Borntraeger" <borntraeger@linux.ibm.com>,
"Marc-André Lureau" <marcandre.lureau@redhat.com>,
"Stefan Weil" <sw@weilnetz.de>,
qemu-arm@nongnu.org, "Juan Quintela" <quintela@redhat.com>,
"Jason Wang" <jasowang@redhat.com>,
"Corey Minyard" <minyard@acm.org>,
"Leonardo Bras" <leobras@redhat.com>,
"Thomas Huth" <thuth@redhat.com>,
"Peter Maydell" <peter.maydell@linaro.org>,
"Michael S. Tsirkin" <mst@redhat.com>,
"Cédric Le Goater" <clg@kaod.org>,
"David Gibson" <david@gibson.dropbear.id.au>,
"Halil Pasic" <pasic@linux.ibm.com>,
"Daniel Henrique Barboza" <danielhb413@gmail.com>
Subject: [PATCH 00/13] migration: Check for duplicates on vmstate_register()
Date: Thu, 19 Oct 2023 21:08:18 +0200 [thread overview]
Message-ID: <20231019190831.20363-1-quintela@redhat.com> (raw)
Hi
This series are based in a patch from Peter than check if a we try to
register the same device with the same instance_id more than once. It
was not merged when he sent it because it broke "make check". So I
fixed all devices to be able to merge it.
- I create vmstate_register_any(), its the same that
vmstate_register(VMSTATE_INSTANCE_ID_ANY)
- Later I check in vmstate_register() that they are not calling it
with VMSTATE_INSTANCE_ID_ANY
- After that I change vmstate_register() to make sure that we don't
include a duplicate.
And we get all the errors that I change in patches 3, 4, 5, 6, 7.
After those patches: make check works again.
And then I reviewed all the rest of vmstate_register() callers.
There are the cases where they pass a device_id that is generated
somehow, that ones are ok.
Then we have the ones that pass always 0. This ones are only valid
when there is a maximum of one device instantiated for a given
machine.
- audio: you can choose more than one audio output.
- eeprom93xx: you can have more than one e100 card.
- vmware_vga: I am not completely sure here, it appears that you could
have more than one. Notice that VMSTATE_INSTANCE_ID_ANY will give
us the value 0 if there is only one instance, so we are in no
trouble. We can drop it if people think that we can't have more
than one vmware_vga.
- for the rest of the devices, I can't see any that can be
instantiated more than once (testing it is easy, just starting the
machine will make it fail). Notice that again, for the same
reasoning, we could change all the calls to _any(). And only left
the vmstate_register(... 0 ...) calls for devices that we know that
we only ever want one.
What needs to be done:
- icp/server: We need to rename the old icp server name. Notice that
I doubt that anyone is migrating this, but I need help from PPC
experts. As said in the commit message, it is "abusing" the interface:
- it register a new device
- it realizes that it is instantiting an old beard
- it unregister the new device
- it registers the old device
- rest of devices:
* pxa2xx devices: I can't see how you can create more than one
device in a machine
* acpi_build: I can't see how to create more than once.
* replay: neither
* cpu timers: created in vl.c
* global_state: only once
* s390 css: not a way that I can think
* spapr: looks only one
* or1ktimer: I can only see one
* tsc*: I see only use in pxa2xx and one by board
- And now, another abuser:
vmstate_register(VMSTATE_IF(tcet), tcet->liobn, &vmstate_spapr_tce_table,
tcet->liobn is an uint32_t, and instance_id is an int. And it just happens that is value is < VMSTATE_INSTANCE_ID_ANY.
Please, review.
Juan Quintela (12):
migration: Create vmstate_register_any()
migration: Use vmstate_register_any()
migration: Use vmstate_register_any() for isa-ide
migration: Use vmstate_register_any() for ipmi-bt*
migration: Use VMSTATE_INSTANCE_ID_ANY for slirp
migration: Use VMSTATE_INSTANCE_ID_ANY for s390 devices
RFC migration: icp/server is a mess
migration: vmstate_register() check that instance_id is valid
migration: Improve example and documentation of vmstate_register()
migration: Use vmstate_register_any() for audio
migration: Use vmstate_register_any() for eeprom93xx
migration: Use vmstate_register_any() for vmware_vga
Peter Xu (1):
migration: Check in savevm_state_handler_insert for dups
docs/devel/migration.rst | 12 ++++++++----
include/migration/vmstate.h | 23 +++++++++++++++++++++++
audio/audio.c | 2 +-
backends/dbus-vmstate.c | 3 +--
backends/tpm/tpm_emulator.c | 3 +--
hw/display/vmware_vga.c | 2 +-
hw/i2c/core.c | 2 +-
hw/ide/isa.c | 2 +-
hw/input/adb.c | 2 +-
hw/input/ads7846.c | 2 +-
hw/input/stellaris_input.c | 3 +--
hw/ipmi/ipmi_bmc_extern.c | 2 +-
hw/ipmi/ipmi_bmc_sim.c | 2 +-
hw/ipmi/isa_ipmi_bt.c | 2 +-
hw/ipmi/isa_ipmi_kcs.c | 2 +-
hw/net/eepro100.c | 3 +--
hw/nvram/eeprom93xx.c | 2 +-
hw/pci/pci.c | 2 +-
hw/ppc/spapr.c | 7 ++++++-
hw/ppc/spapr_nvdimm.c | 3 +--
hw/s390x/s390-skeys.c | 3 ++-
hw/s390x/s390-stattrib.c | 3 ++-
hw/timer/arm_timer.c | 2 +-
hw/virtio/virtio-mem.c | 4 ++--
migration/savevm.c | 14 ++++++++++++++
net/slirp.c | 5 +++--
26 files changed, 78 insertions(+), 34 deletions(-)
--
2.41.0
next reply other threads:[~2023-10-19 19:09 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-19 19:08 Juan Quintela [this message]
2023-10-19 19:08 ` [PATCH 01/13] migration: Create vmstate_register_any() Juan Quintela
2023-10-19 20:18 ` Stefan Berger
2023-10-19 19:08 ` [PATCH 02/13] migration: Use vmstate_register_any() Juan Quintela
2023-10-19 20:18 ` Stefan Berger
2023-10-19 19:08 ` [PATCH 03/13] migration: Use vmstate_register_any() for isa-ide Juan Quintela
2023-10-19 20:19 ` Stefan Berger
2023-10-19 19:08 ` [PATCH 04/13] migration: Use vmstate_register_any() for ipmi-bt* Juan Quintela
2023-10-19 20:20 ` Stefan Berger
2023-10-19 19:08 ` [PATCH 05/13] migration: Use VMSTATE_INSTANCE_ID_ANY for slirp Juan Quintela
2023-10-19 20:29 ` Stefan Berger
2023-10-19 19:08 ` [PATCH 06/13] migration: Use VMSTATE_INSTANCE_ID_ANY for s390 devices Juan Quintela
2023-10-19 20:30 ` Stefan Berger
2023-10-19 19:08 ` [PATCH 07/13] RFC migration: icp/server is a mess Juan Quintela
2023-10-19 20:49 ` Greg Kurz
2023-10-19 21:15 ` Cédric Le Goater
2023-10-20 5:10 ` Thomas Huth
2023-10-20 7:39 ` Cédric Le Goater
2023-10-19 21:39 ` Greg Kurz
2023-10-20 7:30 ` Juan Quintela
2023-10-20 8:06 ` Greg Kurz
2023-10-20 8:12 ` Thomas Huth
2023-10-20 8:57 ` Juan Quintela
2023-10-20 7:49 ` Nicholas Piggin
2023-10-20 8:33 ` Juan Quintela
2023-10-20 8:33 ` Greg Kurz
2023-10-20 10:21 ` Nicholas Piggin
2023-10-19 19:08 ` [PATCH 08/13] migration: vmstate_register() check that instance_id is valid Juan Quintela
2023-10-19 19:08 ` [PATCH 09/13] migration: Check in savevm_state_handler_insert for dups Juan Quintela
2023-10-19 19:08 ` [PATCH 10/13] migration: Improve example and documentation of vmstate_register() Juan Quintela
2023-10-19 20:38 ` Stefan Berger
2023-10-20 9:03 ` Juan Quintela
2023-10-19 19:08 ` [PATCH 11/13] migration: Use vmstate_register_any() for audio Juan Quintela
2023-10-19 20:39 ` Stefan Berger
2023-10-19 19:08 ` [PATCH 12/13] migration: Use vmstate_register_any() for eeprom93xx Juan Quintela
2023-10-19 20:39 ` Stefan Berger
2023-10-19 19:08 ` [PATCH 13/13] migration: Use vmstate_register_any() for vmware_vga Juan Quintela
2023-10-19 20:42 ` Stefan Berger
2023-10-20 7:33 ` Juan Quintela
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=20231019190831.20363-1-quintela@redhat.com \
--to=quintela@redhat.com \
--cc=borntraeger@linux.ibm.com \
--cc=clg@kaod.org \
--cc=cminyard@mvista.com \
--cc=danielhb413@gmail.com \
--cc=david@gibson.dropbear.id.au \
--cc=david@redhat.com \
--cc=farman@linux.ibm.com \
--cc=farosas@suse.de \
--cc=harshpb@linux.ibm.com \
--cc=iii@linux.ibm.com \
--cc=jasowang@redhat.com \
--cc=jsnow@redhat.com \
--cc=kraxel@redhat.com \
--cc=leobras@redhat.com \
--cc=marcandre.lureau@redhat.com \
--cc=marcel.apfelbaum@gmail.com \
--cc=mark.cave-ayland@ilande.co.uk \
--cc=minyard@acm.org \
--cc=mst@redhat.com \
--cc=npiggin@gmail.com \
--cc=pasic@linux.ibm.com \
--cc=peter.maydell@linaro.org \
--cc=peterx@redhat.com \
--cc=qemu-arm@nongnu.org \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=qemu-ppc@nongnu.org \
--cc=qemu-s390x@nongnu.org \
--cc=richard.henderson@linaro.org \
--cc=samuel.thibault@ens-lyon.org \
--cc=stefanb@linux.vnet.ibm.com \
--cc=sw@weilnetz.de \
--cc=thuth@redhat.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 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).