On 5/28/2024 1:44 PM, Peter Xu wrote:
On Tue, May 28, 2024 at 11:10:03AM -0400, Steven Sistare via wrote:
On 5/27/2024 2:20 PM, Peter Xu wrote:
On Mon, Apr 29, 2024 at 08:55:16AM -0700, Steve Sistare wrote:
Define a type for the 256 byte id string to guarantee the same length is
used and enforced everywhere.
Signed-off-by: Steve Sistare <steven.sist...@oracle.com>
---
include/exec/ramblock.h | 3 ++-
include/migration/vmstate.h | 2 ++
migration/savevm.c | 8 ++++----
migration/vmstate.c | 3 ++-
4 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/include/exec/ramblock.h b/include/exec/ramblock.h
index 0babd10..61deefe 100644
--- a/include/exec/ramblock.h
+++ b/include/exec/ramblock.h
@@ -23,6 +23,7 @@
#include "cpu-common.h"
#include "qemu/rcu.h"
#include "exec/ramlist.h"
+#include "migration/vmstate.h"
struct RAMBlock {
struct rcu_head rcu;
@@ -35,7 +36,7 @@ struct RAMBlock {
void (*resized)(const char*, uint64_t length, void *host);
uint32_t flags;
/* Protected by the BQL. */
- char idstr[256];
+ VMStateId idstr;
/* RCU-enabled, writes protected by the ramlist lock */
QLIST_ENTRY(RAMBlock) next;
QLIST_HEAD(, RAMBlockNotifier) ramblock_notifiers;
Hmm.. Don't look like a good idea to include a migration header in
ramblock.h? Is this ramblock change needed for this work?
Well, entities that are migrated include migration headers, and now that
includes RAMBlock. There is precedent:
0 include/exec/ramblock.h 26 #include "migration/vmstate.h"
1 include/hw/acpi/ich9_tco. 14 #include "migration/vmstate.h"
2 include/hw/display/ramfb. 4 #include "migration/vmstate.h"
3 include/hw/hyperv/vmbus.h 16 #include "migration/vmstate.h"
4 include/hw/input/pl050.h 14 #include "migration/vmstate.h"
5 include/hw/pci/shpc.h 7 #include "migration/vmstate.h"
6 include/hw/virtio/virtio. 20 #include "migration/vmstate.h"
7 include/migration/cpu.h 8 #include "migration/vmstate.h"
Granted, only some of the C files that include ramblock.h need all of vmstate.h.
I could define VMStateId in a smaller file such as migration/misc.h, or a
new file migration/vmstateid.h, and include that in ramblock.h.
Any preference?
One issue here is currently the idstr[] of ramblock is a verbose name of
the ramblock, and logically it doesn't need to have anything to do with
vmstate.
I'll continue to read to see why we need VMStateID here for a ramblock. So
if it is necessary, maybe the name VMStateID to be used here is misleading?
It'll also be nice to separate the changes, as ramblock.h doesn't belong to
migration subsystem but memory api.
cpr requires migrating vmstate for ramblock. See the physmem patches for
why. vmstate requires a unique id, and ramblock already defines a unique
id which is used to identify the block and dirty bitmap in the migration
stream.
How about a more general name for the type:
migration/misc.h
typedef char (MigrationId)[256];
exec/ramblock.h
struct RAMBlock {
MigrationId idstr;
migration/savevm.c
typedef struct CompatEntry {
MigrationId idstr;
typedef struct SaveStateEntry {
MigrationId idstr;
- Steve
