On 10/10/24 07:06, Daniel P. Berrangé wrote:
On Thu, Aug 08, 2024 at 05:38:00PM -0600, Jim Fehlig via Devel wrote:
QEMU's new mapped-ram stream format [1] is incompatible with the existing
sequential stream format. An older libvirt+QEMU that does not support
mapped-ram must not attempt to restore a mapped-ram saved image. Currently
the only way to achieve this is to bump QEMU_SAVE_VERSION.
To avoid future version bumps, add a new 'features' element to the saved
image header. The element is used now to indicate the use of mapped-ram
feature, and provides a mechanism to support future save image features.
[1] https://gitlab.com/qemu-project/qemu/-/blob/master/docs/devel/migration/mapped-ram.rst?ref_type=heads
Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx>
---
src/qemu/qemu_saveimage.c | 7 +++++++
src/qemu/qemu_saveimage.h | 9 +++++++--
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c
index 018ab5a222..50fec33f54 100644
--- a/src/qemu/qemu_saveimage.c
+++ b/src/qemu/qemu_saveimage.c
@@ -74,6 +74,7 @@ qemuSaveImageBswapHeader(virQEMUSaveHeader *hdr)
hdr->was_running = GUINT32_SWAP_LE_BE(hdr->was_running);
hdr->compressed = GUINT32_SWAP_LE_BE(hdr->compressed);
hdr->cookieOffset = GUINT32_SWAP_LE_BE(hdr->cookieOffset);
+ hdr->features = GUINT32_SWAP_LE_BE(hdr->features);
}
@@ -637,6 +638,12 @@ qemuSaveImageOpen(virQEMUDriver *driver,
return -1;
}
+ if (header->features && header->features != QEMU_SAVE_FEATURE_MAPPED_RAM) {
Can simplify:
if ((header & ~(QEMU_SAVE_FEATURE_MAPPED_RAM)) != 0)
and make the code patter future proof by ancitipating:
if ((header & ~(QEMU_SAVE_FEATURE_MAPPED_RAM |
QEMU_SAVE_FEATURE_BLAH)) != 0)
+ virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("image contains unsupported features)"));
+ return -1;
+ }
+
if (header->cookieOffset)
xml_len = header->cookieOffset;
else
diff --git a/src/qemu/qemu_saveimage.h b/src/qemu/qemu_saveimage.h
index e541792153..9dd7de292d 100644
--- a/src/qemu/qemu_saveimage.h
+++ b/src/qemu/qemu_saveimage.h
@@ -28,10 +28,14 @@
*/
#define QEMU_SAVE_MAGIC "LibvirtQemudSave"
#define QEMU_SAVE_PARTIAL "LibvirtQemudPart"
-#define QEMU_SAVE_VERSION 2
+#define QEMU_SAVE_VERSION 3
This will make *all* save images incompatible with old libvirt,
even if they're not using mapped ram. This feels sub-optimal
to me. I figure we should use v2, unless the new header
files are non-zero
G_STATIC_ASSERT(sizeof(QEMU_SAVE_MAGIC) == sizeof(QEMU_SAVE_PARTIAL));
+typedef enum {
+ QEMU_SAVE_FEATURE_MAPPED_RAM = 1 << 0,
+} qemuSaveFeatures;
+
typedef struct _virQEMUSaveHeader virQEMUSaveHeader;
struct _virQEMUSaveHeader {
char magic[sizeof(QEMU_SAVE_MAGIC)-1];
@@ -40,7 +44,8 @@ struct _virQEMUSaveHeader {
uint32_t was_running;
uint32_t compressed;
uint32_t cookieOffset;
- uint32_t unused[14];
+ uint32_t features;
Some features might be possible to restore from, even if the current
impl doesn't know about it.
In qcow2 they added "compatible features" and "incompatible featurs"
fields to reflect that some are backwards compatible. So how about
we do the same here with two 32-bit uints.
Unfortunately we'll still need the version bump to prevent older libvirt from
restoring a mapped-ram saved image, but we can avoid adding the 'features' field
if we use the existing 'compressed' field (renamed to 'format', see commit
bd6d7ebf62 and related b0dc8a923d) to control mapped-ram. And if we use the
existing field for mapped-ram, do we even need a version setting in qemu.conf?
Martin seemed receptive to using *_image_format for mapped-ram. What are your
opinions on that?
Regards,
Jim
