The SaveVMHandlers structure is still in use for complex subsystems
and devices. Document the handlers since we are going to modify a few
later.
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
include/migration/register.h | 257 +++++++++++++++++++++++++++++++----
1 file changed, 231 insertions(+), 26 deletions(-)
diff --git a/include/migration/register.h b/include/migration/register.h
index 2e6a7d766e62f64940086b7b511249c9ff21fa62..2cc71ec45f65bf2884c9e7a823d2968752f15c20 100644
--- a/include/migration/register.h
+++ b/include/migration/register.h
@@ -16,30 +16,129 @@
#include "hw/vmstate-if.h"
+/**
+ * struct SaveVMHandlers: handler structure to finely control
+ * migration of complex subsystems and devices, such as RAM, block and
+ * VFIO.
+ */
typedef struct SaveVMHandlers {
- /* This runs inside the BQL. */
+
+ /* The following handlers runs inside the BQL. */
+
+ /**
+ * @save_state
+ *
+ * Saves state section on the source using the latest state format
+ * version.
+ *
+ * Legacy method. Should be deprecated when all users are ported
+ * to VMState.
+ *
+ * @f: QEMUFile where to send the data
+ * @opaque: data pointer passed to register_savevm_live()
+ */
void (*save_state)(QEMUFile *f, void *opaque);
- /*
- * save_prepare is called early, even before migration starts, and can be
- * used to perform early checks.
+ /**
+ * @save_prepare
+ *
+ * Called early, even before migration starts, and can be used to
+ * perform early checks.
+ *
+ * @opaque: data pointer passed to register_savevm_live()
+ * @errp: pointer to Error*, to store an error if it happens.
+ *
+ * Returns zero to indicate success and negative for error
*/
int (*save_prepare)(void *opaque, Error **errp);
+
+ /**
+ * @save_setup
+ *
+ * Initializes the data structures on the source and transmits
+ * first section containing information on the device
+ *
+ * @f: QEMUFile where to send the data
+ * @opaque: data pointer passed to register_savevm_live()
+ *
+ * Returns zero to indicate success and negative for error
+ */
int (*save_setup)(QEMUFile *f, void *opaque);
+
+ /**
+ * @save_cleanup
+ *
+ * Performs save related cleanup
+ *
+ * @opaque: data pointer passed to register_savevm_live()
+ *
+ * Returns zero to indicate success and negative for error
+ */
void (*save_cleanup)(void *opaque);
+
+ /**
+ * @save_live_complete_postcopy
+ *
+ * Called at the end of postcopy for all postcopyiable devices.
+ *
+ * @f: QEMUFile where to send the data
+ * @opaque: data pointer passed to register_savevm_live()
+ *
+ * Returns zero to indicate success and negative for error
+ */
int (*save_live_complete_postcopy)(QEMUFile *f, void *opaque);
+
+ /**
+ * @save_live_complete_precopy
+ *
+ * Transmits the last section for the device containing any
+ * remaining data.
+ *
+ * @f: QEMUFile where to send the data
+ * @opaque: data pointer passed to register_savevm_live()
+ *
+ * Returns zero to indicate success and negative for error
+ */
int (*save_live_complete_precopy)(QEMUFile *f, void *opaque);
/* This runs both outside and inside the BQL. */
+
+ /**
+ * @is_active
+ *
+ * Will skip a state section if not active
+ *
+ * @opaque: data pointer passed to register_savevm_live()
+ *
+ * Returns true if state section is active else false
+ */
bool (*is_active)(void *opaque);
+
+ /**
+ * @has_postcopy
+ *
+ * checks if a device supports postcopy
+ *
+ * @opaque: data pointer passed to register_savevm_live()
+ *
+ * Returns true for postcopy support else false
+ */
bool (*has_postcopy)(void *opaque);
- /* is_active_iterate
- * If it is not NULL then qemu_savevm_state_iterate will skip iteration if
- * it returns false. For example, it is needed for only-postcopy-states,
- * which needs to be handled by qemu_savevm_state_setup and
- * qemu_savevm_state_pending, but do not need iterations until not in
- * postcopy stage.
+ /**
+ * @is_active_iterate
+ *
+ * As #SaveVMHandlers.is_active(), will skip an inactive state
+ * section in qemu_savevm_state_iterate.
+ *
+ * For example, it is needed for only-postcopy-states, which needs
+ * to be handled by qemu_savevm_state_setup() and
+ * qemu_savevm_state_pending(), but do not need iterations until
+ * not in postcopy stage.
+ *
+ * @opaque: data pointer passed to register_savevm_live()
+ *
+ * Returns true if state section is active else false
*/
bool (*is_active_iterate)(void *opaque);
@@ -48,44 +147,150 @@ typedef struct SaveVMHandlers {
* use data that is local to the migration thread or protected
* by other locks.
*/
+
+ /**
+ * @save_live_iterate
+ *
+ * Should send a chunk of data until the point that stream
+ * bandwidth limits tell it to stop. Each call generates one
+ * section.
+ *
+ * @f: QEMUFile where to send the data
+ * @opaque: data pointer passed to register_savevm_live()
+ *
+ * Returns zero to indicate success and negative for error
+ */
int (*save_live_iterate)(QEMUFile *f, void *opaque);
/* This runs outside the BQL! */
- /* Note for save_live_pending:
- * must_precopy:
- * - must be migrated in precopy or in stopped state
- * - i.e. must be migrated before target start
- *
- * can_postcopy:
- * - can migrate in postcopy or in stopped state
- * - i.e. can migrate after target start
- * - some can also be migrated during precopy (RAM)
- * - some must be migrated after source stops (block-dirty-bitmap)
- *
- * Sum of can_postcopy and must_postcopy is the whole amount of
+
+ /**
+ * @state_pending_estimate
+ *
+ * This estimates the remaining data to transfer
+ *
+ * Sum of @can_postcopy and @must_postcopy is the whole amount of
* pending data.
+ *
+ * @opaque: data pointer passed to register_savevm_live()
+ * @must_precopy: amount of data that must be migrated in precopy
+ * or in stopped state, i.e. that must be migrated
+ * before target start.
+ * @can_postcopy: amount of data that can be migrated in postcopy
+ * or in stopped state, i.e. after target start.
+ * Some can also be migrated during precopy (RAM).
+ * Some must be migrated after source stops
+ * (block-dirty-bitmap)
*/
- /* This estimates the remaining data to transfer */
void (*state_pending_estimate)(void *opaque, uint64_t *must_precopy,
uint64_t *can_postcopy);
- /* This calculate the exact remaining data to transfer */
+
+ /**
+ * @state_pending_exact
+ *
+ * This calculate the exact remaining data to transfer
+ *
+ * Sum of @can_postcopy and @must_postcopy is the whole amount of
+ * pending data.
+ *
+ * @opaque: data pointer passed to register_savevm_live()
+ * @must_precopy: amount of data that must be migrated in precopy
+ * or in stopped state, i.e. that must be migrated
+ * before target start.
+ * @can_postcopy: amount of data that can be migrated in postcopy
+ * or in stopped state, i.e. after target start.
+ * Some can also be migrated during precopy (RAM).
+ * Some must be migrated after source stops
+ * (block-dirty-bitmap)
+ */
void (*state_pending_exact)(void *opaque, uint64_t *must_precopy,
uint64_t *can_postcopy);
+
+ /**
+ * @load_state
+ *
+ * Load sections generated by any of the save functions that
+ * generate sections.
+ *
+ * Legacy method. Should be deprecated when all users are ported
+ * to VMState.
+ *
+ * @f: QEMUFile where to receive the data
+ * @opaque: data pointer passed to register_savevm_live()
+ * @version_id: the maximum version_id supported
+ *
+ * Returns zero to indicate success and negative for error
+ */
int (*load_state)(QEMUFile *f, void *opaque, int version_id);
+
+ /**
+ * @load_setup
+ *
+ * Initializes the data structures on the destination.
+ *
+ * @f: QEMUFile where to receive the data
+ * @opaque: data pointer passed to register_savevm_live()
+ *
+ * Returns zero to indicate success and negative for error
+ */
int (*load_setup)(QEMUFile *f, void *opaque);
+
+ /**
+ * @load_cleanup
+ *
+ * Performs cleanup of load data structures
+ *
+ * @opaque: data pointer passed to register_savevm_live()
+ *
+ * Returns zero to indicate success and negative for error
+ */
int (*load_cleanup)(void *opaque);
- /* Called when postcopy migration wants to resume from failure */
+
+ /**
+ * @resume_prepare
+ *
+ * Called when postcopy migration wants to resume from failure
+ *
+ * @s: Current migration state
+ * @opaque: data pointer passed to register_savevm_live()
+ *
+ * Returns zero to indicate success and negative for error
+ */
int (*resume_prepare)(MigrationState *s, void *opaque);
- /* Checks if switchover ack should be used. Called only in dest */
+
+ /**
+ * @switchover_ack_needed
+ *
+ * Checks if switchover ack should be used. Called only on
+ * destination.
+ *
+ * @opaque: data pointer passed to register_savevm_live()
+ */
bool (*switchover_ack_needed)(void *opaque);
} SaveVMHandlers;
+/**
+ * register_savevm_live: Register a set of custom migration handlers
+ *
+ * @idstr: state section identifier
+ * @instance_id: instance id
+ * @version_id: version id supported
+ * @ops: SaveVMHandlers structurex
+ * @opaque: data pointer passed SaveVMHandlers handlers
+ */
int register_savevm_live(const char *idstr,
uint32_t instance_id,
int version_id,
const SaveVMHandlers *ops,
void *opaque);
+/**
+ * unregister_savevm: Unregister custom migration handlers
+ *
+ * @obj: object associated with state section
+ * @idstr: state section identifier
+ * @opaque: data pointer passed to register_savevm_live()
+ */
void unregister_savevm(VMStateIf *obj, const char *idstr, void *opaque);
#endif
--
2.43.2
Hi Cedric,
A few nits below.
On 27/02/2024 20:03, Cédric Le Goater wrote:
> External email: Use caution opening links or attachments
>
>
> The SaveVMHandlers structure is still in use for complex subsystems
> and devices. Document the handlers since we are going to modify a few
> later.
>
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> ---
> include/migration/register.h | 257 +++++++++++++++++++++++++++++++----
> 1 file changed, 231 insertions(+), 26 deletions(-)
>
> diff --git a/include/migration/register.h b/include/migration/register.h
> index 2e6a7d766e62f64940086b7b511249c9ff21fa62..2cc71ec45f65bf2884c9e7a823d2968752f15c20 100644
> --- a/include/migration/register.h
> +++ b/include/migration/register.h
> @@ -16,30 +16,129 @@
>
> #include "hw/vmstate-if.h"
>
> +/**
> + * struct SaveVMHandlers: handler structure to finely control
> + * migration of complex subsystems and devices, such as RAM, block and
> + * VFIO.
> + */
> typedef struct SaveVMHandlers {
> - /* This runs inside the BQL. */
> +
> + /* The following handlers runs inside the BQL. */
s/runs/run
> +
> + /**
> + * @save_state
> + *
> + * Saves state section on the source using the latest state format
> + * version.
> + *
> + * Legacy method. Should be deprecated when all users are ported
> + * to VMState.
> + *
> + * @f: QEMUFile where to send the data
> + * @opaque: data pointer passed to register_savevm_live()
> + */
> void (*save_state)(QEMUFile *f, void *opaque);
>
> - /*
> - * save_prepare is called early, even before migration starts, and can be
> - * used to perform early checks.
> + /**
> + * @save_prepare
> + *
> + * Called early, even before migration starts, and can be used to
> + * perform early checks.
> + *
> + * @opaque: data pointer passed to register_savevm_live()
> + * @errp: pointer to Error*, to store an error if it happens.
> + *
> + * Returns zero to indicate success and negative for error
> */
> int (*save_prepare)(void *opaque, Error **errp);
> +
> + /**
> + * @save_setup
> + *
> + * Initializes the data structures on the source and transmits
> + * first section containing information on the device
> + *
> + * @f: QEMUFile where to send the data
> + * @opaque: data pointer passed to register_savevm_live()
> + *
> + * Returns zero to indicate success and negative for error
> + */
> int (*save_setup)(QEMUFile *f, void *opaque);
> +
> + /**
> + * @save_cleanup
> + *
> + * Performs save related cleanup
Use save_setup phrasing?
Uninitializes the data structures on the source.
> + *
> + * @opaque: data pointer passed to register_savevm_live()
> + *
> + * Returns zero to indicate success and negative for error
This can be dropped as it's a void function.
> + */
> void (*save_cleanup)(void *opaque);
> +
> + /**
> + * @save_live_complete_postcopy
> + *
> + * Called at the end of postcopy for all postcopyiable devices.
s/postcopyiable/postcopyable
> + *
> + * @f: QEMUFile where to send the data
> + * @opaque: data pointer passed to register_savevm_live()
> + *
> + * Returns zero to indicate success and negative for error
> + */
> int (*save_live_complete_postcopy)(QEMUFile *f, void *opaque);
> +
> + /**
> + * @save_live_complete_precopy
> + *
> + * Transmits the last section for the device containing any
> + * remaining data.
> + *
> + * @f: QEMUFile where to send the data
> + * @opaque: data pointer passed to register_savevm_live()
> + *
> + * Returns zero to indicate success and negative for error
> + */
> int (*save_live_complete_precopy)(QEMUFile *f, void *opaque);
>
> /* This runs both outside and inside the BQL. */
> +
> + /**
> + * @is_active
> + *
> + * Will skip a state section if not active
> + *
> + * @opaque: data pointer passed to register_savevm_live()
> + *
> + * Returns true if state section is active else false
> + */
> bool (*is_active)(void *opaque);
> +
> + /**
> + * @has_postcopy
> + *
> + * checks if a device supports postcopy
s/checks/Checks
> + *
> + * @opaque: data pointer passed to register_savevm_live()
> + *
> + * Returns true for postcopy support else false
> + */
> bool (*has_postcopy)(void *opaque);
>
> - /* is_active_iterate
> - * If it is not NULL then qemu_savevm_state_iterate will skip iteration if
> - * it returns false. For example, it is needed for only-postcopy-states,
> - * which needs to be handled by qemu_savevm_state_setup and
> - * qemu_savevm_state_pending, but do not need iterations until not in
> - * postcopy stage.
> + /**
> + * @is_active_iterate
> + *
> + * As #SaveVMHandlers.is_active(), will skip an inactive state
> + * section in qemu_savevm_state_iterate.
> + *
> + * For example, it is needed for only-postcopy-states, which needs
> + * to be handled by qemu_savevm_state_setup() and
> + * qemu_savevm_state_pending(), but do not need iterations until
> + * not in postcopy stage.
> + *
> + * @opaque: data pointer passed to register_savevm_live()
> + *
> + * Returns true if state section is active else false
> */
> bool (*is_active_iterate)(void *opaque);
>
> @@ -48,44 +147,150 @@ typedef struct SaveVMHandlers {
> * use data that is local to the migration thread or protected
> * by other locks.
> */
> +
> + /**
> + * @save_live_iterate
> + *
> + * Should send a chunk of data until the point that stream
> + * bandwidth limits tell it to stop. Each call generates one
> + * section.
> + *
> + * @f: QEMUFile where to send the data
> + * @opaque: data pointer passed to register_savevm_live()
> + *
> + * Returns zero to indicate success and negative for error
0 indicates that there is still more data to send, 1 indicates that
there is no more data to send and negative indicates error.
> + */
> int (*save_live_iterate)(QEMUFile *f, void *opaque);
>
> /* This runs outside the BQL! */
> - /* Note for save_live_pending:
> - * must_precopy:
> - * - must be migrated in precopy or in stopped state
> - * - i.e. must be migrated before target start
> - *
> - * can_postcopy:
> - * - can migrate in postcopy or in stopped state
> - * - i.e. can migrate after target start
> - * - some can also be migrated during precopy (RAM)
> - * - some must be migrated after source stops (block-dirty-bitmap)
> - *
> - * Sum of can_postcopy and must_postcopy is the whole amount of
> +
> + /**
> + * @state_pending_estimate
> + *
> + * This estimates the remaining data to transfer
> + *
> + * Sum of @can_postcopy and @must_postcopy is the whole amount of
> * pending data.
> + *
> + * @opaque: data pointer passed to register_savevm_live()
> + * @must_precopy: amount of data that must be migrated in precopy
> + * or in stopped state, i.e. that must be migrated
> + * before target start.
> + * @can_postcopy: amount of data that can be migrated in postcopy
> + * or in stopped state, i.e. after target start.
> + * Some can also be migrated during precopy (RAM).
> + * Some must be migrated after source stops
> + * (block-dirty-bitmap)
> */
> - /* This estimates the remaining data to transfer */
> void (*state_pending_estimate)(void *opaque, uint64_t *must_precopy,
> uint64_t *can_postcopy);
> - /* This calculate the exact remaining data to transfer */
> +
> + /**
> + * @state_pending_exact
> + *
> + * This calculate the exact remaining data to transfer
s/calculate/calculates
> + *
> + * Sum of @can_postcopy and @must_postcopy is the whole amount of
> + * pending data.
> + *
> + * @opaque: data pointer passed to register_savevm_live()
> + * @must_precopy: amount of data that must be migrated in precopy
> + * or in stopped state, i.e. that must be migrated
> + * before target start.
> + * @can_postcopy: amount of data that can be migrated in postcopy
> + * or in stopped state, i.e. after target start.
> + * Some can also be migrated during precopy (RAM).
> + * Some must be migrated after source stops
> + * (block-dirty-bitmap)
> + */
> void (*state_pending_exact)(void *opaque, uint64_t *must_precopy,
> uint64_t *can_postcopy);
> +
> + /**
> + * @load_state
> + *
> + * Load sections generated by any of the save functions that
> + * generate sections.
> + *
> + * Legacy method. Should be deprecated when all users are ported
> + * to VMState.
> + *
> + * @f: QEMUFile where to receive the data
> + * @opaque: data pointer passed to register_savevm_live()
> + * @version_id: the maximum version_id supported
> + *
> + * Returns zero to indicate success and negative for error
> + */
> int (*load_state)(QEMUFile *f, void *opaque, int version_id);
> +
> + /**
> + * @load_setup
> + *
> + * Initializes the data structures on the destination.
> + *
> + * @f: QEMUFile where to receive the data
> + * @opaque: data pointer passed to register_savevm_live()
> + *
> + * Returns zero to indicate success and negative for error
> + */
> int (*load_setup)(QEMUFile *f, void *opaque);
> +
> + /**
> + * @load_cleanup
> + *
> + * Performs cleanup of load data structures
Use load_setup phrasing?
Uninitializes the data structures on the destination.
> + *
> + * @opaque: data pointer passed to register_savevm_live()
> + *
> + * Returns zero to indicate success and negative for error
> + */
> int (*load_cleanup)(void *opaque);
> - /* Called when postcopy migration wants to resume from failure */
> +
> + /**
> + * @resume_prepare
> + *
> + * Called when postcopy migration wants to resume from failure
> + *
> + * @s: Current migration state
> + * @opaque: data pointer passed to register_savevm_live()
> + *
> + * Returns zero to indicate success and negative for error
> + */
> int (*resume_prepare)(MigrationState *s, void *opaque);
> - /* Checks if switchover ack should be used. Called only in dest */
> +
> + /**
> + * @switchover_ack_needed
> + *
> + * Checks if switchover ack should be used. Called only on
> + * destination.
> + *
> + * @opaque: data pointer passed to register_savevm_live()
Add:
Returns true if switchover ack should be used and false otherwise
> + */
> bool (*switchover_ack_needed)(void *opaque);
> } SaveVMHandlers;
>
> +/**
> + * register_savevm_live: Register a set of custom migration handlers
> + *
> + * @idstr: state section identifier
> + * @instance_id: instance id
> + * @version_id: version id supported
> + * @ops: SaveVMHandlers structurex
s/structurex/structure
> + * @opaque: data pointer passed SaveVMHandlers handlers
s/passed SaveVMHandlers/passed to SaveVMHandlers
Thanks.
> + */
> int register_savevm_live(const char *idstr,
> uint32_t instance_id,
> int version_id,
> const SaveVMHandlers *ops,
> void *opaque);
>
> +/**
> + * unregister_savevm: Unregister custom migration handlers
> + *
> + * @obj: object associated with state section
> + * @idstr: state section identifier
> + * @opaque: data pointer passed to register_savevm_live()
> + */
> void unregister_savevm(VMStateIf *obj, const char *idstr, void *opaque);
>
> #endif
> --
> 2.43.2
>
On 3/4/24 10:05, Avihai Horon wrote:
> Hi Cedric,
>
> A few nits below.
Just in time for v3 I was about to send ! I will include these
suggestions.
Thanks,
C.
>
> On 27/02/2024 20:03, Cédric Le Goater wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> The SaveVMHandlers structure is still in use for complex subsystems
>> and devices. Document the handlers since we are going to modify a few
>> later.
>>
>> Signed-off-by: Cédric Le Goater <clg@redhat.com>
>> ---
>> include/migration/register.h | 257 +++++++++++++++++++++++++++++++----
>> 1 file changed, 231 insertions(+), 26 deletions(-)
>>
>> diff --git a/include/migration/register.h b/include/migration/register.h
>> index 2e6a7d766e62f64940086b7b511249c9ff21fa62..2cc71ec45f65bf2884c9e7a823d2968752f15c20 100644
>> --- a/include/migration/register.h
>> +++ b/include/migration/register.h
>> @@ -16,30 +16,129 @@
>>
>> #include "hw/vmstate-if.h"
>>
>> +/**
>> + * struct SaveVMHandlers: handler structure to finely control
>> + * migration of complex subsystems and devices, such as RAM, block and
>> + * VFIO.
>> + */
>> typedef struct SaveVMHandlers {
>> - /* This runs inside the BQL. */
>> +
>> + /* The following handlers runs inside the BQL. */
>
> s/runs/run
>
>> +
>> + /**
>> + * @save_state
>> + *
>> + * Saves state section on the source using the latest state format
>> + * version.
>> + *
>> + * Legacy method. Should be deprecated when all users are ported
>> + * to VMState.
>> + *
>> + * @f: QEMUFile where to send the data
>> + * @opaque: data pointer passed to register_savevm_live()
>> + */
>> void (*save_state)(QEMUFile *f, void *opaque);
>>
>> - /*
>> - * save_prepare is called early, even before migration starts, and can be
>> - * used to perform early checks.
>> + /**
>> + * @save_prepare
>> + *
>> + * Called early, even before migration starts, and can be used to
>> + * perform early checks.
>> + *
>> + * @opaque: data pointer passed to register_savevm_live()
>> + * @errp: pointer to Error*, to store an error if it happens.
>> + *
>> + * Returns zero to indicate success and negative for error
>> */
>> int (*save_prepare)(void *opaque, Error **errp);
>> +
>> + /**
>> + * @save_setup
>> + *
>> + * Initializes the data structures on the source and transmits
>> + * first section containing information on the device
>> + *
>> + * @f: QEMUFile where to send the data
>> + * @opaque: data pointer passed to register_savevm_live()
>> + *
>> + * Returns zero to indicate success and negative for error
>> + */
>> int (*save_setup)(QEMUFile *f, void *opaque);
>> +
>> + /**
>> + * @save_cleanup
>> + *
>> + * Performs save related cleanup
>
> Use save_setup phrasing?
> Uninitializes the data structures on the source.
>
>> + *
>> + * @opaque: data pointer passed to register_savevm_live()
>> + *
>> + * Returns zero to indicate success and negative for error
>
> This can be dropped as it's a void function.
>
>> + */
>> void (*save_cleanup)(void *opaque);
>> +
>> + /**
>> + * @save_live_complete_postcopy
>> + *
>> + * Called at the end of postcopy for all postcopyiable devices.
>
> s/postcopyiable/postcopyable>
>> + *
>> + * @f: QEMUFile where to send the data
>> + * @opaque: data pointer passed to register_savevm_live()
>> + *
>> + * Returns zero to indicate success and negative for error
>> + */
>> int (*save_live_complete_postcopy)(QEMUFile *f, void *opaque);
>> +
>> + /**
>> + * @save_live_complete_precopy
>> + *
>> + * Transmits the last section for the device containing any
>> + * remaining data.
>> + *
>> + * @f: QEMUFile where to send the data
>> + * @opaque: data pointer passed to register_savevm_live()
>> + *
>> + * Returns zero to indicate success and negative for error
>> + */
>> int (*save_live_complete_precopy)(QEMUFile *f, void *opaque);
>>
>> /* This runs both outside and inside the BQL. */
>> +
>> + /**
>> + * @is_active
>> + *
>> + * Will skip a state section if not active
>> + *
>> + * @opaque: data pointer passed to register_savevm_live()
>> + *
>> + * Returns true if state section is active else false
>> + */
>> bool (*is_active)(void *opaque);
>> +
>> + /**
>> + * @has_postcopy
>> + *
>> + * checks if a device supports postcopy
>
> s/checks/Checks
>
>> + *
>> + * @opaque: data pointer passed to register_savevm_live()
>> + *
>> + * Returns true for postcopy support else false
>> + */
>> bool (*has_postcopy)(void *opaque);
>>
>> - /* is_active_iterate
>> - * If it is not NULL then qemu_savevm_state_iterate will skip iteration if
>> - * it returns false. For example, it is needed for only-postcopy-states,
>> - * which needs to be handled by qemu_savevm_state_setup and
>> - * qemu_savevm_state_pending, but do not need iterations until not in
>> - * postcopy stage.
>> + /**
>> + * @is_active_iterate
>> + *
>> + * As #SaveVMHandlers.is_active(), will skip an inactive state
>> + * section in qemu_savevm_state_iterate.
>> + *
>> + * For example, it is needed for only-postcopy-states, which needs
>> + * to be handled by qemu_savevm_state_setup() and
>> + * qemu_savevm_state_pending(), but do not need iterations until
>> + * not in postcopy stage.
>> + *
>> + * @opaque: data pointer passed to register_savevm_live()
>> + *
>> + * Returns true if state section is active else false
>> */
>> bool (*is_active_iterate)(void *opaque);
>>
>> @@ -48,44 +147,150 @@ typedef struct SaveVMHandlers {
>> * use data that is local to the migration thread or protected
>> * by other locks.
>> */
>> +
>> + /**
>> + * @save_live_iterate
>> + *
>> + * Should send a chunk of data until the point that stream
>> + * bandwidth limits tell it to stop. Each call generates one
>> + * section.
>> + *
>> + * @f: QEMUFile where to send the data
>> + * @opaque: data pointer passed to register_savevm_live()
>> + *
>> + * Returns zero to indicate success and negative for error
>
> 0 indicates that there is still more data to send, 1 indicates that there is no more data to send and negative indicates error.
>
>> + */
>> int (*save_live_iterate)(QEMUFile *f, void *opaque);
>>
>> /* This runs outside the BQL! */
>> - /* Note for save_live_pending:
>> - * must_precopy:
>> - * - must be migrated in precopy or in stopped state
>> - * - i.e. must be migrated before target start
>> - *
>> - * can_postcopy:
>> - * - can migrate in postcopy or in stopped state
>> - * - i.e. can migrate after target start
>> - * - some can also be migrated during precopy (RAM)
>> - * - some must be migrated after source stops (block-dirty-bitmap)
>> - *
>> - * Sum of can_postcopy and must_postcopy is the whole amount of
>> +
>> + /**
>> + * @state_pending_estimate
>> + *
>> + * This estimates the remaining data to transfer
>> + *
>> + * Sum of @can_postcopy and @must_postcopy is the whole amount of
>> * pending data.
>> + *
>> + * @opaque: data pointer passed to register_savevm_live()
>> + * @must_precopy: amount of data that must be migrated in precopy
>> + * or in stopped state, i.e. that must be migrated
>> + * before target start.
>> + * @can_postcopy: amount of data that can be migrated in postcopy
>> + * or in stopped state, i.e. after target start.
>> + * Some can also be migrated during precopy (RAM).
>> + * Some must be migrated after source stops
>> + * (block-dirty-bitmap)
>> */
>> - /* This estimates the remaining data to transfer */
>> void (*state_pending_estimate)(void *opaque, uint64_t *must_precopy,
>> uint64_t *can_postcopy);
>> - /* This calculate the exact remaining data to transfer */
>> +
>> + /**
>> + * @state_pending_exact
>> + *
>> + * This calculate the exact remaining data to transfer
>
> s/calculate/calculates
>
>> + *
>> + * Sum of @can_postcopy and @must_postcopy is the whole amount of
>> + * pending data.
>> + *
>> + * @opaque: data pointer passed to register_savevm_live()
>> + * @must_precopy: amount of data that must be migrated in precopy
>> + * or in stopped state, i.e. that must be migrated
>> + * before target start.
>> + * @can_postcopy: amount of data that can be migrated in postcopy
>> + * or in stopped state, i.e. after target start.
>> + * Some can also be migrated during precopy (RAM).
>> + * Some must be migrated after source stops
>> + * (block-dirty-bitmap)
>> + */
>> void (*state_pending_exact)(void *opaque, uint64_t *must_precopy,
>> uint64_t *can_postcopy);
>> +
>> + /**
>> + * @load_state
>> + *
>> + * Load sections generated by any of the save functions that
>> + * generate sections.
>> + *
>> + * Legacy method. Should be deprecated when all users are ported
>> + * to VMState.
>> + *
>> + * @f: QEMUFile where to receive the data
>> + * @opaque: data pointer passed to register_savevm_live()
>> + * @version_id: the maximum version_id supported
>> + *
>> + * Returns zero to indicate success and negative for error
>> + */
>> int (*load_state)(QEMUFile *f, void *opaque, int version_id);
>> +
>> + /**
>> + * @load_setup
>> + *
>> + * Initializes the data structures on the destination.
>> + *
>> + * @f: QEMUFile where to receive the data
>> + * @opaque: data pointer passed to register_savevm_live()
>> + *
>> + * Returns zero to indicate success and negative for error
>> + */
>> int (*load_setup)(QEMUFile *f, void *opaque);
>> +
>> + /**
>> + * @load_cleanup
>> + *
>> + * Performs cleanup of load data structures
>
> Use load_setup phrasing?
> Uninitializes the data structures on the destination.
>
>> + *
>> + * @opaque: data pointer passed to register_savevm_live()
>> + *
>> + * Returns zero to indicate success and negative for error
>> + */
>> int (*load_cleanup)(void *opaque);
>> - /* Called when postcopy migration wants to resume from failure */
>> +
>> + /**
>> + * @resume_prepare
>> + *
>> + * Called when postcopy migration wants to resume from failure
>> + *
>> + * @s: Current migration state
>> + * @opaque: data pointer passed to register_savevm_live()
>> + *
>> + * Returns zero to indicate success and negative for error
>> + */
>> int (*resume_prepare)(MigrationState *s, void *opaque);
>> - /* Checks if switchover ack should be used. Called only in dest */
>> +
>> + /**
>> + * @switchover_ack_needed
>> + *
>> + * Checks if switchover ack should be used. Called only on
>> + * destination.
>> + *
>> + * @opaque: data pointer passed to register_savevm_live()
>
> Add:
> Returns true if switchover ack should be used and false otherwise
>
>> + */
>> bool (*switchover_ack_needed)(void *opaque);
>> } SaveVMHandlers;
>>
>> +/**
>> + * register_savevm_live: Register a set of custom migration handlers
>> + *
>> + * @idstr: state section identifier
>> + * @instance_id: instance id
>> + * @version_id: version id supported
>> + * @ops: SaveVMHandlers structurex
>
> s/structurex/structure
>
>> + * @opaque: data pointer passed SaveVMHandlers handlers
>
> s/passed SaveVMHandlers/passed to SaveVMHandlers
>
> Thanks.
>
>> + */
>> int register_savevm_live(const char *idstr,
>> uint32_t instance_id,
>> int version_id,
>> const SaveVMHandlers *ops,
>> void *opaque);
>>
>> +/**
>> + * unregister_savevm: Unregister custom migration handlers
>> + *
>> + * @obj: object associated with state section
>> + * @idstr: state section identifier
>> + * @opaque: data pointer passed to register_savevm_live()
>> + */
>> void unregister_savevm(VMStateIf *obj, const char *idstr, void *opaque);
>>
>> #endif
>> --
>> 2.43.2
>>
>
On Tue, Feb 27, 2024 at 07:03:27PM +0100, Cédric Le Goater wrote:
> The SaveVMHandlers structure is still in use for complex subsystems
> and devices. Document the handlers since we are going to modify a few
> later.
>
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Still a few nitpick comments below.
> ---
> include/migration/register.h | 257 +++++++++++++++++++++++++++++++----
> 1 file changed, 231 insertions(+), 26 deletions(-)
>
> diff --git a/include/migration/register.h b/include/migration/register.h
> index 2e6a7d766e62f64940086b7b511249c9ff21fa62..2cc71ec45f65bf2884c9e7a823d2968752f15c20 100644
> --- a/include/migration/register.h
> +++ b/include/migration/register.h
> @@ -16,30 +16,129 @@
>
> #include "hw/vmstate-if.h"
>
> +/**
> + * struct SaveVMHandlers: handler structure to finely control
> + * migration of complex subsystems and devices, such as RAM, block and
> + * VFIO.
> + */
> typedef struct SaveVMHandlers {
> - /* This runs inside the BQL. */
> +
> + /* The following handlers runs inside the BQL. */
> +
> + /**
> + * @save_state
> + *
> + * Saves state section on the source using the latest state format
> + * version.
> + *
> + * Legacy method. Should be deprecated when all users are ported
> + * to VMState.
VMStateDescription?
> + *
> + * @f: QEMUFile where to send the data
> + * @opaque: data pointer passed to register_savevm_live()
> + */
> void (*save_state)(QEMUFile *f, void *opaque);
>
> - /*
> - * save_prepare is called early, even before migration starts, and can be
> - * used to perform early checks.
> + /**
> + * @save_prepare
> + *
> + * Called early, even before migration starts, and can be used to
> + * perform early checks.
> + *
> + * @opaque: data pointer passed to register_savevm_live()
> + * @errp: pointer to Error*, to store an error if it happens.
> + *
> + * Returns zero to indicate success and negative for error
> */
> int (*save_prepare)(void *opaque, Error **errp);
> +
> + /**
> + * @save_setup
> + *
> + * Initializes the data structures on the source and transmits
> + * first section containing information on the device
> + *
> + * @f: QEMUFile where to send the data
> + * @opaque: data pointer passed to register_savevm_live()
> + *
> + * Returns zero to indicate success and negative for error
> + */
> int (*save_setup)(QEMUFile *f, void *opaque);
> +
> + /**
> + * @save_cleanup
> + *
> + * Performs save related cleanup
> + *
> + * @opaque: data pointer passed to register_savevm_live()
> + *
> + * Returns zero to indicate success and negative for error
> + */
> void (*save_cleanup)(void *opaque);
> +
> + /**
> + * @save_live_complete_postcopy
> + *
> + * Called at the end of postcopy for all postcopyiable devices.
> + *
> + * @f: QEMUFile where to send the data
> + * @opaque: data pointer passed to register_savevm_live()
> + *
> + * Returns zero to indicate success and negative for error
> + */
> int (*save_live_complete_postcopy)(QEMUFile *f, void *opaque);
> +
> + /**
> + * @save_live_complete_precopy
> + *
> + * Transmits the last section for the device containing any
> + * remaining data.
This doesn't flush all remaining data when postcopy is enabled and
supported on the specific device. Perhaps attach one more sentence to
describe that case?
Transmits the last section for the device containing any remaining data
at the end of a precopy phase. When postcopy is enabled, devices that
support postcopy will skip this step, where the final data will be
flushed at the end of postcopy via @save_live_complete_postcopy instead.
> + *
> + * @f: QEMUFile where to send the data
> + * @opaque: data pointer passed to register_savevm_live()
> + *
> + * Returns zero to indicate success and negative for error
> + */
> int (*save_live_complete_precopy)(QEMUFile *f, void *opaque);
>
> /* This runs both outside and inside the BQL. */
> +
> + /**
> + * @is_active
> + *
> + * Will skip a state section if not active
> + *
> + * @opaque: data pointer passed to register_savevm_live()
> + *
> + * Returns true if state section is active else false
> + */
> bool (*is_active)(void *opaque);
> +
> + /**
> + * @has_postcopy
> + *
> + * checks if a device supports postcopy
> + *
> + * @opaque: data pointer passed to register_savevm_live()
> + *
> + * Returns true for postcopy support else false
> + */
> bool (*has_postcopy)(void *opaque);
>
> - /* is_active_iterate
> - * If it is not NULL then qemu_savevm_state_iterate will skip iteration if
> - * it returns false. For example, it is needed for only-postcopy-states,
> - * which needs to be handled by qemu_savevm_state_setup and
> - * qemu_savevm_state_pending, but do not need iterations until not in
> - * postcopy stage.
> + /**
> + * @is_active_iterate
> + *
> + * As #SaveVMHandlers.is_active(), will skip an inactive state
> + * section in qemu_savevm_state_iterate.
> + *
> + * For example, it is needed for only-postcopy-states, which needs
> + * to be handled by qemu_savevm_state_setup() and
> + * qemu_savevm_state_pending(), but do not need iterations until
> + * not in postcopy stage.
> + *
> + * @opaque: data pointer passed to register_savevm_live()
> + *
> + * Returns true if state section is active else false
> */
> bool (*is_active_iterate)(void *opaque);
>
> @@ -48,44 +147,150 @@ typedef struct SaveVMHandlers {
> * use data that is local to the migration thread or protected
> * by other locks.
> */
> +
> + /**
> + * @save_live_iterate
> + *
> + * Should send a chunk of data until the point that stream
> + * bandwidth limits tell it to stop. Each call generates one
> + * section.
> + *
> + * @f: QEMUFile where to send the data
> + * @opaque: data pointer passed to register_savevm_live()
> + *
> + * Returns zero to indicate success and negative for error
> + */
> int (*save_live_iterate)(QEMUFile *f, void *opaque);
>
> /* This runs outside the BQL! */
> - /* Note for save_live_pending:
> - * must_precopy:
> - * - must be migrated in precopy or in stopped state
> - * - i.e. must be migrated before target start
> - *
> - * can_postcopy:
> - * - can migrate in postcopy or in stopped state
> - * - i.e. can migrate after target start
> - * - some can also be migrated during precopy (RAM)
> - * - some must be migrated after source stops (block-dirty-bitmap)
> - *
> - * Sum of can_postcopy and must_postcopy is the whole amount of
> +
> + /**
> + * @state_pending_estimate
> + *
> + * This estimates the remaining data to transfer
> + *
> + * Sum of @can_postcopy and @must_postcopy is the whole amount of
> * pending data.
> + *
> + * @opaque: data pointer passed to register_savevm_live()
> + * @must_precopy: amount of data that must be migrated in precopy
> + * or in stopped state, i.e. that must be migrated
> + * before target start.
> + * @can_postcopy: amount of data that can be migrated in postcopy
> + * or in stopped state, i.e. after target start.
> + * Some can also be migrated during precopy (RAM).
> + * Some must be migrated after source stops
> + * (block-dirty-bitmap)
> */
> - /* This estimates the remaining data to transfer */
> void (*state_pending_estimate)(void *opaque, uint64_t *must_precopy,
> uint64_t *can_postcopy);
> - /* This calculate the exact remaining data to transfer */
> +
> + /**
> + * @state_pending_exact
> + *
> + * This calculate the exact remaining data to transfer
> + *
> + * Sum of @can_postcopy and @must_postcopy is the whole amount of
> + * pending data.
> + *
> + * @opaque: data pointer passed to register_savevm_live()
> + * @must_precopy: amount of data that must be migrated in precopy
> + * or in stopped state, i.e. that must be migrated
> + * before target start.
> + * @can_postcopy: amount of data that can be migrated in postcopy
> + * or in stopped state, i.e. after target start.
> + * Some can also be migrated during precopy (RAM).
> + * Some must be migrated after source stops
> + * (block-dirty-bitmap)
> + */
> void (*state_pending_exact)(void *opaque, uint64_t *must_precopy,
> uint64_t *can_postcopy);
> +
> + /**
> + * @load_state
> + *
> + * Load sections generated by any of the save functions that
> + * generate sections.
> + *
> + * Legacy method. Should be deprecated when all users are ported
> + * to VMState.
VMStateDescription.
> + *
> + * @f: QEMUFile where to receive the data
> + * @opaque: data pointer passed to register_savevm_live()
> + * @version_id: the maximum version_id supported
> + *
> + * Returns zero to indicate success and negative for error
> + */
> int (*load_state)(QEMUFile *f, void *opaque, int version_id);
> +
> + /**
> + * @load_setup
> + *
> + * Initializes the data structures on the destination.
> + *
> + * @f: QEMUFile where to receive the data
> + * @opaque: data pointer passed to register_savevm_live()
> + *
> + * Returns zero to indicate success and negative for error
> + */
> int (*load_setup)(QEMUFile *f, void *opaque);
> +
> + /**
> + * @load_cleanup
> + *
> + * Performs cleanup of load data structures
> + *
> + * @opaque: data pointer passed to register_savevm_live()
> + *
> + * Returns zero to indicate success and negative for error
> + */
> int (*load_cleanup)(void *opaque);
> - /* Called when postcopy migration wants to resume from failure */
> +
> + /**
> + * @resume_prepare
> + *
> + * Called when postcopy migration wants to resume from failure
> + *
> + * @s: Current migration state
> + * @opaque: data pointer passed to register_savevm_live()
> + *
> + * Returns zero to indicate success and negative for error
> + */
> int (*resume_prepare)(MigrationState *s, void *opaque);
> - /* Checks if switchover ack should be used. Called only in dest */
> +
> + /**
> + * @switchover_ack_needed
> + *
> + * Checks if switchover ack should be used. Called only on
> + * destination.
> + *
> + * @opaque: data pointer passed to register_savevm_live()
> + */
> bool (*switchover_ack_needed)(void *opaque);
> } SaveVMHandlers;
>
> +/**
> + * register_savevm_live: Register a set of custom migration handlers
> + *
> + * @idstr: state section identifier
> + * @instance_id: instance id
> + * @version_id: version id supported
> + * @ops: SaveVMHandlers structurex
> + * @opaque: data pointer passed SaveVMHandlers handlers
> + */
> int register_savevm_live(const char *idstr,
> uint32_t instance_id,
> int version_id,
> const SaveVMHandlers *ops,
> void *opaque);
>
> +/**
> + * unregister_savevm: Unregister custom migration handlers
> + *
> + * @obj: object associated with state section
> + * @idstr: state section identifier
> + * @opaque: data pointer passed to register_savevm_live()
> + */
> void unregister_savevm(VMStateIf *obj, const char *idstr, void *opaque);
>
> #endif
> --
> 2.43.2
>
--
Peter Xu
On 2/29/24 05:10, Peter Xu wrote: > On Tue, Feb 27, 2024 at 07:03:27PM +0100, Cédric Le Goater wrote: >> The SaveVMHandlers structure is still in use for complex subsystems >> and devices. Document the handlers since we are going to modify a few >> later. >> >> Signed-off-by: Cédric Le Goater <clg@redhat.com> > > Reviewed-by: Peter Xu <peterx@redhat.com> > > Still a few nitpick comments below. I have applied your suggestions for the next spin. Thanks, C.
© 2016 - 2024 Red Hat, Inc.