Hi,

Just want to clarify your suggestions.

1) migrate=off/share=on

I'm not sure that adding new flag 'migrate=off' is a good idea. I think that
share=on as you suggested at first is enough.
* It's a new flag which has sense only with share=on
* It seems to that the meaning of this flag isn't clear. migrate=off isn't
  RAM_MIGRATABLE, it's rather RAM_IGNORABLE. I.e. it means that we don't migrate
  such block only if the capability is enabled.
If you don't mind, I'll prefer share=on and ignore-shared capability.

2) Keep RAM migration version

It's more complicated question. To validate GPAs for ignored blocks I have to
change the stream format. I can do this conditionally (if (cap_enabled) { ... })
but in this case, I want to make sure that the capability is enabled or disabled
on both source and target. Otherwise, there is an undefined behavior on the
target side (most likely some error during deserialization).
To fix that I can add a capability validation feature.

For example, add a new section to vmstate_configuration (not complete):
+static const VMStateDescription vmstate_capabilites = {
+    .name = "configuration/capabilities",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = vmstate_capabilites_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32_V(caps_count, SaveState, 1),
+        VMSTATE_VARRAY_UINT32_ALLOC(capabilities, SaveState, caps_count, 1,
+                                    vmstate_info_bool, bool),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static const VMStateDescription vmstate_configuration = {
     .name = "configuration",
     .version_id = 1,
@@ -404,6 +456,7 @@ static const VMStateDescription vmstate_configuration = {
     },
     .subsections = (const VMStateDescription*[]) {
         &vmstate_target_page_bits,
+        &vmstate_capabilites,
         NULL
     }
 };

It seems too complicated. If I should change the migration stream anyway, maybe
it's better to increment the version?

What do you think?

Regards,
Yury

Reply via email to