[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Stable-9.2.1 23/41] migration: Dump correct JSON format for nullptr rep
From: |
Michael Tokarev |
Subject: |
[Stable-9.2.1 23/41] migration: Dump correct JSON format for nullptr replacement |
Date: |
Mon, 27 Jan 2025 17:17:37 +0300 |
From: Peter Xu <peterx@redhat.com>
QEMU plays a trick with null pointers inside an array of pointers in a VMSD
field. See 07d4e69147 ("migration/vmstate: fix array of ptr with
nullptrs") for more details on why. The idea makes sense in general, but
it may overlooked the JSON writer where it could write nothing in a
"struct" in the JSON hints section.
We hit some analyze-migration.py issues on s390 recently, showing that some
of the struct field contains nothing, like:
{"name": "css", "array_len": 256, "type": "struct", "struct": {}, "size": 1}
As described in details by Fabiano:
87pll37cin.fsf@suse.de">https://lore.kernel.org/r/87pll37cin.fsf@suse.de
It could be that we hit some null pointers there, and JSON was gone when
they're null pointers.
To fix it, instead of hacking around only at VMStateInfo level, do that
from VMStateField level, so that JSON writer can also be involved. In this
case, JSON writer will replace the pointer array (which used to be a
"struct") to be the real representation of the nullptr field.
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20250109185249.23952-6-farosas@suse.de>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
(cherry picked from commit 9867c3a7ced12dd7519155c047eb2c0098a11c5f)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/migration/vmstate.c b/migration/vmstate.c
index aa2821dec6..52704c822c 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -51,6 +51,36 @@ vmstate_field_exists(const VMStateDescription *vmsd, const
VMStateField *field,
return result;
}
+/*
+ * Create a fake nullptr field when there's a NULL pointer detected in the
+ * array of a VMS_ARRAY_OF_POINTER VMSD field. It's needed because we
+ * can't dereference the NULL pointer.
+ */
+static const VMStateField *
+vmsd_create_fake_nullptr_field(const VMStateField *field)
+{
+ VMStateField *fake = g_new0(VMStateField, 1);
+
+ /* It can only happen on an array of pointers! */
+ assert(field->flags & VMS_ARRAY_OF_POINTER);
+
+ /* Some of fake's properties should match the original's */
+ fake->name = field->name;
+ fake->version_id = field->version_id;
+
+ /* Do not need "field_exists" check as it always exists (which is null) */
+ fake->field_exists = NULL;
+
+ /* See vmstate_info_nullptr - use 1 byte to represent nullptr */
+ fake->size = 1;
+ fake->info = &vmstate_info_nullptr;
+ fake->flags = VMS_SINGLE;
+
+ /* All the rest fields shouldn't matter.. */
+
+ return (const VMStateField *)fake;
+}
+
static int vmstate_n_elems(void *opaque, const VMStateField *field)
{
int n_elems = 1;
@@ -143,23 +173,39 @@ int vmstate_load_state(QEMUFile *f, const
VMStateDescription *vmsd,
}
for (i = 0; i < n_elems; i++) {
void *curr_elem = first_elem + size * i;
+ const VMStateField *inner_field;
if (field->flags & VMS_ARRAY_OF_POINTER) {
curr_elem = *(void **)curr_elem;
}
+
if (!curr_elem && size) {
- /* if null pointer check placeholder and do not follow */
- assert(field->flags & VMS_ARRAY_OF_POINTER);
- ret = vmstate_info_nullptr.get(f, curr_elem, size, NULL);
- } else if (field->flags & VMS_STRUCT) {
- ret = vmstate_load_state(f, field->vmsd, curr_elem,
- field->vmsd->version_id);
- } else if (field->flags & VMS_VSTRUCT) {
- ret = vmstate_load_state(f, field->vmsd, curr_elem,
- field->struct_version_id);
+ /*
+ * If null pointer found (which should only happen in
+ * an array of pointers), use null placeholder and do
+ * not follow.
+ */
+ inner_field = vmsd_create_fake_nullptr_field(field);
+ } else {
+ inner_field = field;
+ }
+
+ if (inner_field->flags & VMS_STRUCT) {
+ ret = vmstate_load_state(f, inner_field->vmsd, curr_elem,
+ inner_field->vmsd->version_id);
+ } else if (inner_field->flags & VMS_VSTRUCT) {
+ ret = vmstate_load_state(f, inner_field->vmsd, curr_elem,
+ inner_field->struct_version_id);
} else {
- ret = field->info->get(f, curr_elem, size, field);
+ ret = inner_field->info->get(f, curr_elem, size,
+ inner_field);
}
+
+ /* If we used a fake temp field.. free it now */
+ if (inner_field != field) {
+ g_clear_pointer((gpointer *)&inner_field, g_free);
+ }
+
if (ret >= 0) {
ret = qemu_file_get_error(f);
}
@@ -387,29 +433,50 @@ int vmstate_save_state_v(QEMUFile *f, const
VMStateDescription *vmsd,
}
for (i = 0; i < n_elems; i++) {
void *curr_elem = first_elem + size * i;
+ const VMStateField *inner_field;
- vmsd_desc_field_start(vmsd, vmdesc_loop, field, i, n_elems);
old_offset = qemu_file_transferred(f);
if (field->flags & VMS_ARRAY_OF_POINTER) {
assert(curr_elem);
curr_elem = *(void **)curr_elem;
}
+
if (!curr_elem && size) {
- /* if null pointer write placeholder and do not follow */
- assert(field->flags & VMS_ARRAY_OF_POINTER);
- ret = vmstate_info_nullptr.put(f, curr_elem, size, NULL,
- NULL);
- } else if (field->flags & VMS_STRUCT) {
- ret = vmstate_save_state(f, field->vmsd, curr_elem,
- vmdesc_loop);
- } else if (field->flags & VMS_VSTRUCT) {
- ret = vmstate_save_state_v(f, field->vmsd, curr_elem,
- vmdesc_loop,
- field->struct_version_id, errp);
+ /*
+ * If null pointer found (which should only happen in
+ * an array of pointers), use null placeholder and do
+ * not follow.
+ */
+ inner_field = vmsd_create_fake_nullptr_field(field);
+ } else {
+ inner_field = field;
+ }
+
+ vmsd_desc_field_start(vmsd, vmdesc_loop, inner_field,
+ i, n_elems);
+
+ if (inner_field->flags & VMS_STRUCT) {
+ ret = vmstate_save_state(f, inner_field->vmsd,
+ curr_elem, vmdesc_loop);
+ } else if (inner_field->flags & VMS_VSTRUCT) {
+ ret = vmstate_save_state_v(f, inner_field->vmsd,
+ curr_elem, vmdesc_loop,
+ inner_field->struct_version_id,
+ errp);
} else {
- ret = field->info->put(f, curr_elem, size, field,
- vmdesc_loop);
+ ret = inner_field->info->put(f, curr_elem, size,
+ inner_field, vmdesc_loop);
}
+
+ written_bytes = qemu_file_transferred(f) - old_offset;
+ vmsd_desc_field_end(vmsd, vmdesc_loop, inner_field,
+ written_bytes);
+
+ /* If we used a fake temp field.. free it now */
+ if (inner_field != field) {
+ g_clear_pointer((gpointer *)&inner_field, g_free);
+ }
+
if (ret) {
error_setg(errp, "Save of field %s/%s failed",
vmsd->name, field->name);
@@ -419,9 +486,6 @@ int vmstate_save_state_v(QEMUFile *f, const
VMStateDescription *vmsd,
return ret;
}
- written_bytes = qemu_file_transferred(f) - old_offset;
- vmsd_desc_field_end(vmsd, vmdesc_loop, field, written_bytes);
-
/* Compressed arrays only care about the first element */
if (vmdesc_loop && vmsd_can_compress(field)) {
vmdesc_loop = NULL;
--
2.39.5
- [Stable-9.2.1 12/41] target/loongarch: Use actual operand size with vbsrl check, (continued)
- [Stable-9.2.1 12/41] target/loongarch: Use actual operand size with vbsrl check, Michael Tokarev, 2025/01/27
- [Stable-9.2.1 17/41] migration/multifd: Fix compat with QEMU < 9.0, Michael Tokarev, 2025/01/27
- [Stable-9.2.1 22/41] migration: Rename vmstate_info_nullptr, Michael Tokarev, 2025/01/27
- [Stable-9.2.1 16/41] migration/multifd: Fix compile error caused by page_size usage, Michael Tokarev, 2025/01/27
- [Stable-9.2.1 20/41] migration: Fix parsing of s390 stream, Michael Tokarev, 2025/01/27
- [Stable-9.2.1 19/41] migration: Remove unused argument in vmsd_desc_field_end, Michael Tokarev, 2025/01/27
- [Stable-9.2.1 18/41] migration: Add more error handling to analyze-migration.py, Michael Tokarev, 2025/01/27
- [Stable-9.2.1 21/41] s390x: Fix CSS migration, Michael Tokarev, 2025/01/27
- [Stable-9.2.1 24/41] migration: Fix arrays of pointers in JSON writer, Michael Tokarev, 2025/01/27
- [Stable-9.2.1 02/41] tcg: Reset free_temps before tcg_optimize, Michael Tokarev, 2025/01/27
- [Stable-9.2.1 23/41] migration: Dump correct JSON format for nullptr replacement,
Michael Tokarev <=
- [Stable-9.2.1 25/41] multifd: bugfix for migration using compression methods, Michael Tokarev, 2025/01/27
- [Stable-9.2.1 39/41] make-release: only leave tarball of wrap-file subprojects, Michael Tokarev, 2025/01/27
- [Stable-9.2.1 27/41] multifd: bugfix for incorrect migration data with qatzip compression, Michael Tokarev, 2025/01/27
- [Stable-9.2.1 28/41] tests/functional/test_rx_gdbsim: Use stable URL for test_linux_sash, Michael Tokarev, 2025/01/27
- [Stable-9.2.1 26/41] multifd: bugfix for incorrect migration data with QPL compression, Michael Tokarev, 2025/01/27
- [Stable-9.2.1 38/41] hw/cxl: Fix msix_notify: Assertion `vector < dev->msix_entries_nr`, Michael Tokarev, 2025/01/27
- [Stable-9.2.1 36/41] pci: acpi: Windows 'PCI Label Id' bug workaround, Michael Tokarev, 2025/01/27
- [Stable-9.2.1 40/41] i386/cpu: Mark avx10_version filtered when prefix is NULL, Michael Tokarev, 2025/01/27
- [Stable-9.2.1 37/41] tests: acpi: update expected blobs, Michael Tokarev, 2025/01/27
- [Stable-9.2.1 41/41] crypto: fix bogus error benchmarking pbkdf on fast machines, Michael Tokarev, 2025/01/27