Define an abstraction SAVEVM_FOREACH to loop over all savevm state
handlers, and replace QTAILQ_FOREACH. Define variants for ALL so
we can loop over all handlers vs a subset of handlers in a subsequent
patch, but at this time there is no distinction between the two.
No functional change.
Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
---
migration/savevm.c | 55 +++++++++++++++++++++++++++++++-----------------------
1 file changed, 32 insertions(+), 23 deletions(-)
diff --git a/migration/savevm.c b/migration/savevm.c
index 4509482..6829ba3 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -237,6 +237,15 @@ static SaveState savevm_state = {
.global_section_id = 0,
};
+#define SAVEVM_FOREACH(se, entry) \
+ QTAILQ_FOREACH(se, &savevm_state.handlers, entry) \
+
+#define SAVEVM_FOREACH_ALL(se, entry) \
+ QTAILQ_FOREACH(se, &savevm_state.handlers, entry)
+
+#define SAVEVM_FOREACH_SAFE_ALL(se, entry, new_se) \
+ QTAILQ_FOREACH_SAFE(se, &savevm_state.handlers, entry, new_se)
+
static SaveStateEntry *find_se(const char *idstr, uint32_t instance_id);
static bool should_validate_capability(int capability)
@@ -674,7 +683,7 @@ static uint32_t calculate_new_instance_id(const char *idstr)
SaveStateEntry *se;
uint32_t instance_id = 0;
- QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
+ SAVEVM_FOREACH_ALL(se, entry) {
if (strcmp(idstr, se->idstr) == 0
&& instance_id <= se->instance_id) {
instance_id = se->instance_id + 1;
@@ -690,7 +699,7 @@ static int calculate_compat_instance_id(const char *idstr)
SaveStateEntry *se;
int instance_id = 0;
- QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
+ SAVEVM_FOREACH(se, entry) {
if (!se->compat) {
continue;
}
@@ -816,7 +825,7 @@ void unregister_savevm(VMStateIf *obj, const char *idstr, void *opaque)
}
pstrcat(id, sizeof(id), idstr);
- QTAILQ_FOREACH_SAFE(se, &savevm_state.handlers, entry, new_se) {
+ SAVEVM_FOREACH_SAFE_ALL(se, entry, new_se) {
if (strcmp(se->idstr, id) == 0 && se->opaque == opaque) {
savevm_state_handler_remove(se);
g_free(se->compat);
@@ -939,7 +948,7 @@ void vmstate_unregister(VMStateIf *obj, const VMStateDescription *vmsd,
{
SaveStateEntry *se, *new_se;
- QTAILQ_FOREACH_SAFE(se, &savevm_state.handlers, entry, new_se) {
+ SAVEVM_FOREACH_SAFE_ALL(se, entry, new_se) {
if (se->vmsd == vmsd && se->opaque == opaque) {
savevm_state_handler_remove(se);
g_free(se->compat);
@@ -1223,7 +1232,7 @@ bool qemu_savevm_state_blocked(Error **errp)
{
SaveStateEntry *se;
- QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
+ SAVEVM_FOREACH(se, entry) {
if (se->vmsd && se->vmsd->unmigratable) {
error_setg(errp, "State blocked by non-migratable device '%s'",
se->idstr);
@@ -1237,7 +1246,7 @@ void qemu_savevm_non_migratable_list(strList **reasons)
{
SaveStateEntry *se;
- QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
+ SAVEVM_FOREACH(se, entry) {
if (se->vmsd && se->vmsd->unmigratable) {
QAPI_LIST_PREPEND(*reasons,
g_strdup_printf("non-migratable device: %s",
@@ -1276,7 +1285,7 @@ bool qemu_savevm_state_guest_unplug_pending(void)
{
SaveStateEntry *se;
- QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
+ SAVEVM_FOREACH(se, entry) {
if (se->vmsd && se->vmsd->dev_unplug_pending &&
se->vmsd->dev_unplug_pending(se->opaque)) {
return true;
@@ -1291,7 +1300,7 @@ int qemu_savevm_state_prepare(Error **errp)
SaveStateEntry *se;
int ret;
- QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
+ SAVEVM_FOREACH(se, entry) {
if (!se->ops || !se->ops->save_prepare) {
continue;
}
@@ -1321,7 +1330,7 @@ int qemu_savevm_state_setup(QEMUFile *f, Error **errp)
json_writer_start_array(ms->vmdesc, "devices");
trace_savevm_state_setup();
- QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
+ SAVEVM_FOREACH(se, entry) {
if (se->vmsd && se->vmsd->early_setup) {
ret = vmstate_save(f, se, ms->vmdesc, errp);
if (ret) {
@@ -1365,7 +1374,7 @@ int qemu_savevm_state_resume_prepare(MigrationState *s)
trace_savevm_state_resume_prepare();
- QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
+ SAVEVM_FOREACH(se, entry) {
if (!se->ops || !se->ops->resume_prepare) {
continue;
}
@@ -1396,7 +1405,7 @@ int qemu_savevm_state_iterate(QEMUFile *f, bool postcopy)
int ret;
trace_savevm_state_iterate();
- QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
+ SAVEVM_FOREACH(se, entry) {
if (!se->ops || !se->ops->save_live_iterate) {
continue;
}
@@ -1461,7 +1470,7 @@ void qemu_savevm_state_complete_postcopy(QEMUFile *f)
SaveStateEntry *se;
int ret;
- QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
+ SAVEVM_FOREACH(se, entry) {
if (!se->ops || !se->ops->save_live_complete_postcopy) {
continue;
}
@@ -1495,7 +1504,7 @@ int qemu_savevm_state_complete_precopy_iterable(QEMUFile *f, bool in_postcopy)
SaveStateEntry *se;
int ret;
- QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
+ SAVEVM_FOREACH(se, entry) {
if (!se->ops ||
(in_postcopy && se->ops->has_postcopy &&
se->ops->has_postcopy(se->opaque)) ||
@@ -1543,7 +1552,7 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
Error *local_err = NULL;
int ret;
- QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
+ SAVEVM_FOREACH(se, entry) {
if (se->vmsd && se->vmsd->early_setup) {
/* Already saved during qemu_savevm_state_setup(). */
continue;
@@ -1649,7 +1658,7 @@ void qemu_savevm_state_pending_estimate(uint64_t *must_precopy,
*must_precopy = 0;
*can_postcopy = 0;
- QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
+ SAVEVM_FOREACH(se, entry) {
if (!se->ops || !se->ops->state_pending_estimate) {
continue;
}
@@ -1670,7 +1679,7 @@ void qemu_savevm_state_pending_exact(uint64_t *must_precopy,
*must_precopy = 0;
*can_postcopy = 0;
- QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
+ SAVEVM_FOREACH(se, entry) {
if (!se->ops || !se->ops->state_pending_exact) {
continue;
}
@@ -1693,7 +1702,7 @@ void qemu_savevm_state_cleanup(void)
}
trace_savevm_state_cleanup();
- QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
+ SAVEVM_FOREACH(se, entry) {
if (se->ops && se->ops->save_cleanup) {
se->ops->save_cleanup(se->opaque);
}
@@ -1778,7 +1787,7 @@ int qemu_save_device_state(QEMUFile *f)
}
cpu_synchronize_all_states();
- QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
+ SAVEVM_FOREACH(se, entry) {
int ret;
if (se->is_ram) {
@@ -1801,7 +1810,7 @@ static SaveStateEntry *find_se(const char *idstr, uint32_t instance_id)
{
SaveStateEntry *se;
- QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
+ SAVEVM_FOREACH_ALL(se, entry) {
if (!strcmp(se->idstr, idstr) &&
(instance_id == se->instance_id ||
instance_id == se->alias_id))
@@ -2680,7 +2689,7 @@ qemu_loadvm_section_part_end(QEMUFile *f, MigrationIncomingState *mis,
}
trace_qemu_loadvm_state_section_partend(section_id);
- QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
+ SAVEVM_FOREACH(se, entry) {
if (se->load_section_id == section_id) {
break;
}
@@ -2755,7 +2764,7 @@ static void qemu_loadvm_state_switchover_ack_needed(MigrationIncomingState *mis)
{
SaveStateEntry *se;
- QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
+ SAVEVM_FOREACH(se, entry) {
if (!se->ops || !se->ops->switchover_ack_needed) {
continue;
}
@@ -2775,7 +2784,7 @@ static int qemu_loadvm_state_setup(QEMUFile *f, Error **errp)
int ret;
trace_loadvm_state_setup();
- QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
+ SAVEVM_FOREACH(se, entry) {
if (!se->ops || !se->ops->load_setup) {
continue;
}
@@ -2801,7 +2810,7 @@ void qemu_loadvm_state_cleanup(void)
SaveStateEntry *se;
trace_loadvm_state_cleanup();
- QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
+ SAVEVM_FOREACH(se, entry) {
if (se->ops && se->ops->load_cleanup) {
se->ops->load_cleanup(se->opaque);
}
--
1.8.3.1Steve Sistare <steven.sistare@oracle.com> writes:
> Define an abstraction SAVEVM_FOREACH to loop over all savevm state
> handlers, and replace QTAILQ_FOREACH. Define variants for ALL so
> we can loop over all handlers vs a subset of handlers in a subsequent
> patch, but at this time there is no distinction between the two.
> No functional change.
>
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> ---
> migration/savevm.c | 55 +++++++++++++++++++++++++++++++-----------------------
> 1 file changed, 32 insertions(+), 23 deletions(-)
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 4509482..6829ba3 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -237,6 +237,15 @@ static SaveState savevm_state = {
> .global_section_id = 0,
> };
>
> +#define SAVEVM_FOREACH(se, entry) \
> + QTAILQ_FOREACH(se, &savevm_state.handlers, entry) \
> +
> +#define SAVEVM_FOREACH_ALL(se, entry) \
> + QTAILQ_FOREACH(se, &savevm_state.handlers, entry)
This feels worse than SAVEVM_FOREACH_NOT_PRECREATED. We'll have to keep
coming back to the definition to figure out which FOREACH is the real
deal.
> +
> +#define SAVEVM_FOREACH_SAFE_ALL(se, entry, new_se) \
> + QTAILQ_FOREACH_SAFE(se, &savevm_state.handlers, entry, new_se)
> +
> static SaveStateEntry *find_se(const char *idstr, uint32_t instance_id);
>
> static bool should_validate_capability(int capability)
> @@ -674,7 +683,7 @@ static uint32_t calculate_new_instance_id(const char *idstr)
> SaveStateEntry *se;
> uint32_t instance_id = 0;
>
> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
> + SAVEVM_FOREACH_ALL(se, entry) {
In this patch we can't have both instances...
> if (strcmp(idstr, se->idstr) == 0
> && instance_id <= se->instance_id) {
> instance_id = se->instance_id + 1;
> @@ -690,7 +699,7 @@ static int calculate_compat_instance_id(const char *idstr)
> SaveStateEntry *se;
> int instance_id = 0;
>
> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
> + SAVEVM_FOREACH(se, entry) {
...otherwise one of the two changes will go undocumented because the
actual reason for it will only be described in the next patch.
> if (!se->compat) {
> continue;
> }
> @@ -816,7 +825,7 @@ void unregister_savevm(VMStateIf *obj, const char *idstr, void *opaque)
> }
> pstrcat(id, sizeof(id), idstr);
>
> - QTAILQ_FOREACH_SAFE(se, &savevm_state.handlers, entry, new_se) {
> + SAVEVM_FOREACH_SAFE_ALL(se, entry, new_se) {
> if (strcmp(se->idstr, id) == 0 && se->opaque == opaque) {
> savevm_state_handler_remove(se);
> g_free(se->compat);
> @@ -939,7 +948,7 @@ void vmstate_unregister(VMStateIf *obj, const VMStateDescription *vmsd,
> {
> SaveStateEntry *se, *new_se;
>
> - QTAILQ_FOREACH_SAFE(se, &savevm_state.handlers, entry, new_se) {
> + SAVEVM_FOREACH_SAFE_ALL(se, entry, new_se) {
> if (se->vmsd == vmsd && se->opaque == opaque) {
> savevm_state_handler_remove(se);
> g_free(se->compat);
> @@ -1223,7 +1232,7 @@ bool qemu_savevm_state_blocked(Error **errp)
> {
> SaveStateEntry *se;
>
> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
> + SAVEVM_FOREACH(se, entry) {
> if (se->vmsd && se->vmsd->unmigratable) {
> error_setg(errp, "State blocked by non-migratable device '%s'",
> se->idstr);
> @@ -1237,7 +1246,7 @@ void qemu_savevm_non_migratable_list(strList **reasons)
> {
> SaveStateEntry *se;
>
> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
> + SAVEVM_FOREACH(se, entry) {
> if (se->vmsd && se->vmsd->unmigratable) {
> QAPI_LIST_PREPEND(*reasons,
> g_strdup_printf("non-migratable device: %s",
> @@ -1276,7 +1285,7 @@ bool qemu_savevm_state_guest_unplug_pending(void)
> {
> SaveStateEntry *se;
>
> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
> + SAVEVM_FOREACH(se, entry) {
> if (se->vmsd && se->vmsd->dev_unplug_pending &&
> se->vmsd->dev_unplug_pending(se->opaque)) {
> return true;
> @@ -1291,7 +1300,7 @@ int qemu_savevm_state_prepare(Error **errp)
> SaveStateEntry *se;
> int ret;
>
> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
> + SAVEVM_FOREACH(se, entry) {
> if (!se->ops || !se->ops->save_prepare) {
> continue;
> }
> @@ -1321,7 +1330,7 @@ int qemu_savevm_state_setup(QEMUFile *f, Error **errp)
> json_writer_start_array(ms->vmdesc, "devices");
>
> trace_savevm_state_setup();
> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
> + SAVEVM_FOREACH(se, entry) {
> if (se->vmsd && se->vmsd->early_setup) {
> ret = vmstate_save(f, se, ms->vmdesc, errp);
> if (ret) {
> @@ -1365,7 +1374,7 @@ int qemu_savevm_state_resume_prepare(MigrationState *s)
>
> trace_savevm_state_resume_prepare();
>
> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
> + SAVEVM_FOREACH(se, entry) {
> if (!se->ops || !se->ops->resume_prepare) {
> continue;
> }
> @@ -1396,7 +1405,7 @@ int qemu_savevm_state_iterate(QEMUFile *f, bool postcopy)
> int ret;
>
> trace_savevm_state_iterate();
> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
> + SAVEVM_FOREACH(se, entry) {
> if (!se->ops || !se->ops->save_live_iterate) {
> continue;
> }
> @@ -1461,7 +1470,7 @@ void qemu_savevm_state_complete_postcopy(QEMUFile *f)
> SaveStateEntry *se;
> int ret;
>
> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
> + SAVEVM_FOREACH(se, entry) {
> if (!se->ops || !se->ops->save_live_complete_postcopy) {
> continue;
> }
> @@ -1495,7 +1504,7 @@ int qemu_savevm_state_complete_precopy_iterable(QEMUFile *f, bool in_postcopy)
> SaveStateEntry *se;
> int ret;
>
> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
> + SAVEVM_FOREACH(se, entry) {
> if (!se->ops ||
> (in_postcopy && se->ops->has_postcopy &&
> se->ops->has_postcopy(se->opaque)) ||
> @@ -1543,7 +1552,7 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
> Error *local_err = NULL;
> int ret;
>
> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
> + SAVEVM_FOREACH(se, entry) {
> if (se->vmsd && se->vmsd->early_setup) {
> /* Already saved during qemu_savevm_state_setup(). */
> continue;
> @@ -1649,7 +1658,7 @@ void qemu_savevm_state_pending_estimate(uint64_t *must_precopy,
> *must_precopy = 0;
> *can_postcopy = 0;
>
> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
> + SAVEVM_FOREACH(se, entry) {
> if (!se->ops || !se->ops->state_pending_estimate) {
> continue;
> }
> @@ -1670,7 +1679,7 @@ void qemu_savevm_state_pending_exact(uint64_t *must_precopy,
> *must_precopy = 0;
> *can_postcopy = 0;
>
> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
> + SAVEVM_FOREACH(se, entry) {
> if (!se->ops || !se->ops->state_pending_exact) {
> continue;
> }
> @@ -1693,7 +1702,7 @@ void qemu_savevm_state_cleanup(void)
> }
>
> trace_savevm_state_cleanup();
> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
> + SAVEVM_FOREACH(se, entry) {
> if (se->ops && se->ops->save_cleanup) {
> se->ops->save_cleanup(se->opaque);
> }
> @@ -1778,7 +1787,7 @@ int qemu_save_device_state(QEMUFile *f)
> }
> cpu_synchronize_all_states();
>
> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
> + SAVEVM_FOREACH(se, entry) {
> int ret;
>
> if (se->is_ram) {
> @@ -1801,7 +1810,7 @@ static SaveStateEntry *find_se(const char *idstr, uint32_t instance_id)
> {
> SaveStateEntry *se;
>
> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
> + SAVEVM_FOREACH_ALL(se, entry) {
> if (!strcmp(se->idstr, idstr) &&
> (instance_id == se->instance_id ||
> instance_id == se->alias_id))
> @@ -2680,7 +2689,7 @@ qemu_loadvm_section_part_end(QEMUFile *f, MigrationIncomingState *mis,
> }
>
> trace_qemu_loadvm_state_section_partend(section_id);
> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
> + SAVEVM_FOREACH(se, entry) {
> if (se->load_section_id == section_id) {
> break;
> }
> @@ -2755,7 +2764,7 @@ static void qemu_loadvm_state_switchover_ack_needed(MigrationIncomingState *mis)
> {
> SaveStateEntry *se;
>
> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
> + SAVEVM_FOREACH(se, entry) {
> if (!se->ops || !se->ops->switchover_ack_needed) {
> continue;
> }
> @@ -2775,7 +2784,7 @@ static int qemu_loadvm_state_setup(QEMUFile *f, Error **errp)
> int ret;
>
> trace_loadvm_state_setup();
> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
> + SAVEVM_FOREACH(se, entry) {
> if (!se->ops || !se->ops->load_setup) {
> continue;
> }
> @@ -2801,7 +2810,7 @@ void qemu_loadvm_state_cleanup(void)
> SaveStateEntry *se;
>
> trace_loadvm_state_cleanup();
> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
> + SAVEVM_FOREACH(se, entry) {
> if (se->ops && se->ops->load_cleanup) {
> se->ops->load_cleanup(se->opaque);
> }
On 5/6/2024 7:17 PM, Fabiano Rosas wrote:
> Steve Sistare <steven.sistare@oracle.com> writes:
>
>> Define an abstraction SAVEVM_FOREACH to loop over all savevm state
>> handlers, and replace QTAILQ_FOREACH. Define variants for ALL so
>> we can loop over all handlers vs a subset of handlers in a subsequent
>> patch, but at this time there is no distinction between the two.
>> No functional change.
>>
>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>> ---
>> migration/savevm.c | 55 +++++++++++++++++++++++++++++++-----------------------
>> 1 file changed, 32 insertions(+), 23 deletions(-)
>>
>> diff --git a/migration/savevm.c b/migration/savevm.c
>> index 4509482..6829ba3 100644
>> --- a/migration/savevm.c
>> +++ b/migration/savevm.c
>> @@ -237,6 +237,15 @@ static SaveState savevm_state = {
>> .global_section_id = 0,
>> };
>>
>> +#define SAVEVM_FOREACH(se, entry) \
>> + QTAILQ_FOREACH(se, &savevm_state.handlers, entry) \
>> +
>> +#define SAVEVM_FOREACH_ALL(se, entry) \
>> + QTAILQ_FOREACH(se, &savevm_state.handlers, entry)
>
> This feels worse than SAVEVM_FOREACH_NOT_PRECREATED. We'll have to keep
> coming back to the definition to figure out which FOREACH is the real
> deal.
I take your point, but the majority of the loops do not care about precreated
objects, so it seems backwards to make them more verbose with
SAVEVM_FOREACH_NOT_PRECREATE. I can go either way, but we need
Peter's opinion also.
>> +
>> +#define SAVEVM_FOREACH_SAFE_ALL(se, entry, new_se) \
>> + QTAILQ_FOREACH_SAFE(se, &savevm_state.handlers, entry, new_se)
>> +
>> static SaveStateEntry *find_se(const char *idstr, uint32_t instance_id);
>>
>> static bool should_validate_capability(int capability)
>> @@ -674,7 +683,7 @@ static uint32_t calculate_new_instance_id(const char *idstr)
>> SaveStateEntry *se;
>> uint32_t instance_id = 0;
>>
>> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>> + SAVEVM_FOREACH_ALL(se, entry) {
>
> In this patch we can't have both instances...
>
>> if (strcmp(idstr, se->idstr) == 0
>> && instance_id <= se->instance_id) {
>> instance_id = se->instance_id + 1;
>> @@ -690,7 +699,7 @@ static int calculate_compat_instance_id(const char *idstr)
>> SaveStateEntry *se;
>> int instance_id = 0;
>>
>> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>> + SAVEVM_FOREACH(se, entry) {
>
> ...otherwise one of the two changes will go undocumented because the
> actual reason for it will only be described in the next patch.
Sure, I'll move this to the precreate patch.
- Steve
>> if (!se->compat) {
>> continue;
>> }
>> @@ -816,7 +825,7 @@ void unregister_savevm(VMStateIf *obj, const char *idstr, void *opaque)
>> }
>> pstrcat(id, sizeof(id), idstr);
>>
>> - QTAILQ_FOREACH_SAFE(se, &savevm_state.handlers, entry, new_se) {
>> + SAVEVM_FOREACH_SAFE_ALL(se, entry, new_se) {
>> if (strcmp(se->idstr, id) == 0 && se->opaque == opaque) {
>> savevm_state_handler_remove(se);
>> g_free(se->compat);
>> @@ -939,7 +948,7 @@ void vmstate_unregister(VMStateIf *obj, const VMStateDescription *vmsd,
>> {
>> SaveStateEntry *se, *new_se;
>>
>> - QTAILQ_FOREACH_SAFE(se, &savevm_state.handlers, entry, new_se) {
>> + SAVEVM_FOREACH_SAFE_ALL(se, entry, new_se) {
>> if (se->vmsd == vmsd && se->opaque == opaque) {
>> savevm_state_handler_remove(se);
>> g_free(se->compat);
>> @@ -1223,7 +1232,7 @@ bool qemu_savevm_state_blocked(Error **errp)
>> {
>> SaveStateEntry *se;
>>
>> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>> + SAVEVM_FOREACH(se, entry) {
>> if (se->vmsd && se->vmsd->unmigratable) {
>> error_setg(errp, "State blocked by non-migratable device '%s'",
>> se->idstr);
>> @@ -1237,7 +1246,7 @@ void qemu_savevm_non_migratable_list(strList **reasons)
>> {
>> SaveStateEntry *se;
>>
>> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>> + SAVEVM_FOREACH(se, entry) {
>> if (se->vmsd && se->vmsd->unmigratable) {
>> QAPI_LIST_PREPEND(*reasons,
>> g_strdup_printf("non-migratable device: %s",
>> @@ -1276,7 +1285,7 @@ bool qemu_savevm_state_guest_unplug_pending(void)
>> {
>> SaveStateEntry *se;
>>
>> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>> + SAVEVM_FOREACH(se, entry) {
>> if (se->vmsd && se->vmsd->dev_unplug_pending &&
>> se->vmsd->dev_unplug_pending(se->opaque)) {
>> return true;
>> @@ -1291,7 +1300,7 @@ int qemu_savevm_state_prepare(Error **errp)
>> SaveStateEntry *se;
>> int ret;
>>
>> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>> + SAVEVM_FOREACH(se, entry) {
>> if (!se->ops || !se->ops->save_prepare) {
>> continue;
>> }
>> @@ -1321,7 +1330,7 @@ int qemu_savevm_state_setup(QEMUFile *f, Error **errp)
>> json_writer_start_array(ms->vmdesc, "devices");
>>
>> trace_savevm_state_setup();
>> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>> + SAVEVM_FOREACH(se, entry) {
>> if (se->vmsd && se->vmsd->early_setup) {
>> ret = vmstate_save(f, se, ms->vmdesc, errp);
>> if (ret) {
>> @@ -1365,7 +1374,7 @@ int qemu_savevm_state_resume_prepare(MigrationState *s)
>>
>> trace_savevm_state_resume_prepare();
>>
>> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>> + SAVEVM_FOREACH(se, entry) {
>> if (!se->ops || !se->ops->resume_prepare) {
>> continue;
>> }
>> @@ -1396,7 +1405,7 @@ int qemu_savevm_state_iterate(QEMUFile *f, bool postcopy)
>> int ret;
>>
>> trace_savevm_state_iterate();
>> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>> + SAVEVM_FOREACH(se, entry) {
>> if (!se->ops || !se->ops->save_live_iterate) {
>> continue;
>> }
>> @@ -1461,7 +1470,7 @@ void qemu_savevm_state_complete_postcopy(QEMUFile *f)
>> SaveStateEntry *se;
>> int ret;
>>
>> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>> + SAVEVM_FOREACH(se, entry) {
>> if (!se->ops || !se->ops->save_live_complete_postcopy) {
>> continue;
>> }
>> @@ -1495,7 +1504,7 @@ int qemu_savevm_state_complete_precopy_iterable(QEMUFile *f, bool in_postcopy)
>> SaveStateEntry *se;
>> int ret;
>>
>> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>> + SAVEVM_FOREACH(se, entry) {
>> if (!se->ops ||
>> (in_postcopy && se->ops->has_postcopy &&
>> se->ops->has_postcopy(se->opaque)) ||
>> @@ -1543,7 +1552,7 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
>> Error *local_err = NULL;
>> int ret;
>>
>> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>> + SAVEVM_FOREACH(se, entry) {
>> if (se->vmsd && se->vmsd->early_setup) {
>> /* Already saved during qemu_savevm_state_setup(). */
>> continue;
>> @@ -1649,7 +1658,7 @@ void qemu_savevm_state_pending_estimate(uint64_t *must_precopy,
>> *must_precopy = 0;
>> *can_postcopy = 0;
>>
>> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>> + SAVEVM_FOREACH(se, entry) {
>> if (!se->ops || !se->ops->state_pending_estimate) {
>> continue;
>> }
>> @@ -1670,7 +1679,7 @@ void qemu_savevm_state_pending_exact(uint64_t *must_precopy,
>> *must_precopy = 0;
>> *can_postcopy = 0;
>>
>> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>> + SAVEVM_FOREACH(se, entry) {
>> if (!se->ops || !se->ops->state_pending_exact) {
>> continue;
>> }
>> @@ -1693,7 +1702,7 @@ void qemu_savevm_state_cleanup(void)
>> }
>>
>> trace_savevm_state_cleanup();
>> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>> + SAVEVM_FOREACH(se, entry) {
>> if (se->ops && se->ops->save_cleanup) {
>> se->ops->save_cleanup(se->opaque);
>> }
>> @@ -1778,7 +1787,7 @@ int qemu_save_device_state(QEMUFile *f)
>> }
>> cpu_synchronize_all_states();
>>
>> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>> + SAVEVM_FOREACH(se, entry) {
>> int ret;
>>
>> if (se->is_ram) {
>> @@ -1801,7 +1810,7 @@ static SaveStateEntry *find_se(const char *idstr, uint32_t instance_id)
>> {
>> SaveStateEntry *se;
>>
>> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>> + SAVEVM_FOREACH_ALL(se, entry) {
>> if (!strcmp(se->idstr, idstr) &&
>> (instance_id == se->instance_id ||
>> instance_id == se->alias_id))
>> @@ -2680,7 +2689,7 @@ qemu_loadvm_section_part_end(QEMUFile *f, MigrationIncomingState *mis,
>> }
>>
>> trace_qemu_loadvm_state_section_partend(section_id);
>> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>> + SAVEVM_FOREACH(se, entry) {
>> if (se->load_section_id == section_id) {
>> break;
>> }
>> @@ -2755,7 +2764,7 @@ static void qemu_loadvm_state_switchover_ack_needed(MigrationIncomingState *mis)
>> {
>> SaveStateEntry *se;
>>
>> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>> + SAVEVM_FOREACH(se, entry) {
>> if (!se->ops || !se->ops->switchover_ack_needed) {
>> continue;
>> }
>> @@ -2775,7 +2784,7 @@ static int qemu_loadvm_state_setup(QEMUFile *f, Error **errp)
>> int ret;
>>
>> trace_loadvm_state_setup();
>> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>> + SAVEVM_FOREACH(se, entry) {
>> if (!se->ops || !se->ops->load_setup) {
>> continue;
>> }
>> @@ -2801,7 +2810,7 @@ void qemu_loadvm_state_cleanup(void)
>> SaveStateEntry *se;
>>
>> trace_loadvm_state_cleanup();
>> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>> + SAVEVM_FOREACH(se, entry) {
>> if (se->ops && se->ops->load_cleanup) {
>> se->ops->load_cleanup(se->opaque);
>> }
On Mon, May 13, 2024 at 03:27:30PM -0400, Steven Sistare wrote:
> On 5/6/2024 7:17 PM, Fabiano Rosas wrote:
> > Steve Sistare <steven.sistare@oracle.com> writes:
> >
> > > Define an abstraction SAVEVM_FOREACH to loop over all savevm state
> > > handlers, and replace QTAILQ_FOREACH. Define variants for ALL so
> > > we can loop over all handlers vs a subset of handlers in a subsequent
> > > patch, but at this time there is no distinction between the two.
> > > No functional change.
> > >
> > > Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> > > ---
> > > migration/savevm.c | 55 +++++++++++++++++++++++++++++++-----------------------
> > > 1 file changed, 32 insertions(+), 23 deletions(-)
> > >
> > > diff --git a/migration/savevm.c b/migration/savevm.c
> > > index 4509482..6829ba3 100644
> > > --- a/migration/savevm.c
> > > +++ b/migration/savevm.c
> > > @@ -237,6 +237,15 @@ static SaveState savevm_state = {
> > > .global_section_id = 0,
> > > };
> > > +#define SAVEVM_FOREACH(se, entry) \
> > > + QTAILQ_FOREACH(se, &savevm_state.handlers, entry) \
> > > +
> > > +#define SAVEVM_FOREACH_ALL(se, entry) \
> > > + QTAILQ_FOREACH(se, &savevm_state.handlers, entry)
> >
> > This feels worse than SAVEVM_FOREACH_NOT_PRECREATED. We'll have to keep
> > coming back to the definition to figure out which FOREACH is the real
> > deal.
>
> I take your point, but the majority of the loops do not care about precreated
> objects, so it seems backwards to make them more verbose with
> SAVEVM_FOREACH_NOT_PRECREATE. I can go either way, but we need
> Peter's opinion also.
I don't have a strong opinion yet on the name, I think it'll be clearer
indeed when the _ALL() (or whatever other name) is used only when with the
real users.
OTOH, besides the name (which is much more trivial..) the precreated idea
in general is a bit scary to me.. if that was for a "workaround" to some
new ordering issue due to newly added dependencies on exec() support.
Maybe I'll understand better when I get to know better on the whole design.
--
Peter Xu
© 2016 - 2026 Red Hat, Inc.