* [Qemu-devel] [PATCH v3 1/3] migration: Use savevm_handlers instead of loadvm copy
2017-05-31 9:17 [Qemu-devel] [PATCH v3 0/3] Remove of loadvm handlers Juan Quintela
@ 2017-05-31 9:17 ` Juan Quintela
2017-05-31 9:17 ` [Qemu-devel] [PATCH v3 2/3] migration: loadvm handlers are not used Juan Quintela
2017-05-31 9:17 ` [Qemu-devel] [PATCH v3 3/3] migration: Remove section_id parameter from vmstate_load Juan Quintela
2 siblings, 0 replies; 6+ messages in thread
From: Juan Quintela @ 2017-05-31 9:17 UTC (permalink / raw)
To: qemu-devel; +Cc: dgilbert, lvivier, peterx
There is no reason for having the loadvm_handlers at all. There is
only one use, and we can use the savevm handlers.
We will remove the loadvm handlers on a following patch.
Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
--
- Added load_version_id: version_id read from the stream (laurent)
- Added load_section_id: section_id read from the stream (dave)
---
migration/savevm.c | 32 +++++++++++++++++++-------------
1 file changed, 19 insertions(+), 13 deletions(-)
diff --git a/migration/savevm.c b/migration/savevm.c
index a2d4f9c..07646f4 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -272,7 +272,11 @@ typedef struct SaveStateEntry {
int instance_id;
int alias_id;
int version_id;
+ /* version id read from the stream */
+ int load_version_id;
int section_id;
+ /* section id read from the stream */
+ int load_section_id;
SaveVMHandlers *ops;
const VMStateDescription *vmsd;
void *opaque;
@@ -1813,7 +1817,7 @@ struct LoadStateEntry {
* Returns: true if the footer was good
* false if there is a problem (and calls error_report to say why)
*/
-static bool check_section_footer(QEMUFile *f, LoadStateEntry *le)
+static bool check_section_footer(QEMUFile *f, SaveStateEntry *se)
{
uint8_t read_mark;
uint32_t read_section_id;
@@ -1826,15 +1830,15 @@ static bool check_section_footer(QEMUFile *f, LoadStateEntry *le)
read_mark = qemu_get_byte(f);
if (read_mark != QEMU_VM_SECTION_FOOTER) {
- error_report("Missing section footer for %s", le->se->idstr);
+ error_report("Missing section footer for %s", se->idstr);
return false;
}
read_section_id = qemu_get_be32(f);
- if (read_section_id != le->section_id) {
+ if (read_section_id != se->load_section_id) {
error_report("Mismatched section id in footer for %s -"
" read 0x%x expected 0x%x",
- le->se->idstr, read_section_id, le->section_id);
+ se->idstr, read_section_id, se->load_section_id);
return false;
}
@@ -1887,6 +1891,8 @@ qemu_loadvm_section_start_full(QEMUFile *f, MigrationIncomingState *mis)
version_id, idstr, se->version_id);
return -EINVAL;
}
+ se->load_version_id = version_id;
+ se->load_section_id = section_id;
/* Validate if it is a device's state */
if (xen_enabled() && se->is_ram) {
@@ -1902,13 +1908,13 @@ qemu_loadvm_section_start_full(QEMUFile *f, MigrationIncomingState *mis)
le->version_id = version_id;
QLIST_INSERT_HEAD(&mis->loadvm_handlers, le, entry);
- ret = vmstate_load(f, le->se, le->version_id);
+ ret = vmstate_load(f, se, se->load_version_id);
if (ret < 0) {
error_report("error while loading state for instance 0x%x of"
" device '%s'", instance_id, idstr);
return ret;
}
- if (!check_section_footer(f, le)) {
+ if (!check_section_footer(f, se)) {
return -EINVAL;
}
@@ -1919,29 +1925,29 @@ static int
qemu_loadvm_section_part_end(QEMUFile *f, MigrationIncomingState *mis)
{
uint32_t section_id;
- LoadStateEntry *le;
+ SaveStateEntry *se;
int ret;
section_id = qemu_get_be32(f);
trace_qemu_loadvm_state_section_partend(section_id);
- QLIST_FOREACH(le, &mis->loadvm_handlers, entry) {
- if (le->section_id == section_id) {
+ QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
+ if (se->load_section_id == section_id) {
break;
}
}
- if (le == NULL) {
+ if (se == NULL) {
error_report("Unknown savevm section %d", section_id);
return -EINVAL;
}
- ret = vmstate_load(f, le->se, le->version_id);
+ ret = vmstate_load(f, se, se->load_version_id);
if (ret < 0) {
error_report("error while loading state section id %d(%s)",
- section_id, le->se->idstr);
+ section_id, se->idstr);
return ret;
}
- if (!check_section_footer(f, le)) {
+ if (!check_section_footer(f, se)) {
return -EINVAL;
}
--
2.9.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH v3 2/3] migration: loadvm handlers are not used
2017-05-31 9:17 [Qemu-devel] [PATCH v3 0/3] Remove of loadvm handlers Juan Quintela
2017-05-31 9:17 ` [Qemu-devel] [PATCH v3 1/3] migration: Use savevm_handlers instead of loadvm copy Juan Quintela
@ 2017-05-31 9:17 ` Juan Quintela
2017-05-31 9:17 ` [Qemu-devel] [PATCH v3 3/3] migration: Remove section_id parameter from vmstate_load Juan Quintela
2 siblings, 0 replies; 6+ messages in thread
From: Juan Quintela @ 2017-05-31 9:17 UTC (permalink / raw)
To: qemu-devel; +Cc: dgilbert, lvivier, peterx
So we remove all traces of them.
Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
---
include/migration/migration.h | 5 -----
include/migration/vmstate.h | 2 --
include/qemu/typedefs.h | 1 -
migration/migration.c | 2 --
migration/savevm.c | 26 --------------------------
5 files changed, 36 deletions(-)
diff --git a/include/migration/migration.h b/include/migration/migration.h
index 0e807b6..d1a353a 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -50,8 +50,6 @@ enum mig_rp_message_type {
MIG_RP_MSG_MAX
};
-typedef QLIST_HEAD(, LoadStateEntry) LoadStateEntry_Head;
-
/* State for the incoming migration */
struct MigrationIncomingState {
QEMUFile *from_src_file;
@@ -89,9 +87,6 @@ struct MigrationIncomingState {
/* The coroutine we should enter (back) after failover */
Coroutine *migration_incoming_co;
QemuSemaphore colo_incoming_sem;
-
- /* See savevm.c */
- LoadStateEntry_Head loadvm_handlers;
};
MigrationIncomingState *migration_incoming_get_current(void);
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index f97411d..6689562 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -1020,8 +1020,6 @@ extern const VMStateInfo vmstate_info_qtailq;
#define SELF_ANNOUNCE_ROUNDS 5
-void loadvm_free_handlers(MigrationIncomingState *mis);
-
int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
void *opaque, int version_id);
void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index 33a6aa1..51958bf 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -39,7 +39,6 @@ typedef struct I2SCodec I2SCodec;
typedef struct ISABus ISABus;
typedef struct ISADevice ISADevice;
typedef struct IsaDma IsaDma;
-typedef struct LoadStateEntry LoadStateEntry;
typedef struct MACAddr MACAddr;
typedef struct MachineClass MachineClass;
typedef struct MachineState MachineState;
diff --git a/migration/migration.c b/migration/migration.c
index 7087d1a..c3218cd 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -122,7 +122,6 @@ MigrationIncomingState *migration_incoming_get_current(void)
if (!once) {
mis_current.state = MIGRATION_STATUS_NONE;
memset(&mis_current, 0, sizeof(MigrationIncomingState));
- QLIST_INIT(&mis_current.loadvm_handlers);
qemu_mutex_init(&mis_current.rp_mutex);
qemu_event_init(&mis_current.main_thread_load_event, false);
once = true;
@@ -135,7 +134,6 @@ void migration_incoming_state_destroy(void)
struct MigrationIncomingState *mis = migration_incoming_get_current();
qemu_event_destroy(&mis->main_thread_load_event);
- loadvm_free_handlers(mis);
}
diff --git a/migration/savevm.c b/migration/savevm.c
index 07646f4..d96209b 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1804,13 +1804,6 @@ static int loadvm_process_command(QEMUFile *f)
return 0;
}
-struct LoadStateEntry {
- QLIST_ENTRY(LoadStateEntry) entry;
- SaveStateEntry *se;
- int section_id;
- int version_id;
-};
-
/*
* Read a footer off the wire and check that it matches the expected section
*
@@ -1846,22 +1839,11 @@ static bool check_section_footer(QEMUFile *f, SaveStateEntry *se)
return true;
}
-void loadvm_free_handlers(MigrationIncomingState *mis)
-{
- LoadStateEntry *le, *new_le;
-
- QLIST_FOREACH_SAFE(le, &mis->loadvm_handlers, entry, new_le) {
- QLIST_REMOVE(le, entry);
- g_free(le);
- }
-}
-
static int
qemu_loadvm_section_start_full(QEMUFile *f, MigrationIncomingState *mis)
{
uint32_t instance_id, version_id, section_id;
SaveStateEntry *se;
- LoadStateEntry *le;
char idstr[256];
int ret;
@@ -1900,14 +1882,6 @@ qemu_loadvm_section_start_full(QEMUFile *f, MigrationIncomingState *mis)
return -EINVAL;
}
- /* Add entry */
- le = g_malloc0(sizeof(*le));
-
- le->se = se;
- le->section_id = section_id;
- le->version_id = version_id;
- QLIST_INSERT_HEAD(&mis->loadvm_handlers, le, entry);
-
ret = vmstate_load(f, se, se->load_version_id);
if (ret < 0) {
error_report("error while loading state for instance 0x%x of"
--
2.9.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH v3 3/3] migration: Remove section_id parameter from vmstate_load
2017-05-31 9:17 [Qemu-devel] [PATCH v3 0/3] Remove of loadvm handlers Juan Quintela
2017-05-31 9:17 ` [Qemu-devel] [PATCH v3 1/3] migration: Use savevm_handlers instead of loadvm copy Juan Quintela
2017-05-31 9:17 ` [Qemu-devel] [PATCH v3 2/3] migration: loadvm handlers are not used Juan Quintela
@ 2017-05-31 9:17 ` Juan Quintela
2017-05-31 11:39 ` Dr. David Alan Gilbert
2 siblings, 1 reply; 6+ messages in thread
From: Juan Quintela @ 2017-05-31 9:17 UTC (permalink / raw)
To: qemu-devel; +Cc: dgilbert, lvivier, peterx
Everything else assumes that we always load a device from its own
savevm handler.
Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
---
migration/savevm.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/migration/savevm.c b/migration/savevm.c
index d96209b..2d1d4bc 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -746,13 +746,13 @@ void vmstate_unregister(DeviceState *dev, const VMStateDescription *vmsd,
}
}
-static int vmstate_load(QEMUFile *f, SaveStateEntry *se, int version_id)
+static int vmstate_load(QEMUFile *f, SaveStateEntry *se)
{
trace_vmstate_load(se->idstr, se->vmsd ? se->vmsd->name : "(old)");
if (!se->vmsd) { /* Old style */
- return se->ops->load_state(f, se->opaque, version_id);
+ return se->ops->load_state(f, se->opaque, se->load_version_id);
}
- return vmstate_load_state(f, se->vmsd, se->opaque, version_id);
+ return vmstate_load_state(f, se->vmsd, se->opaque, se->load_version_id);
}
static void vmstate_save_old_style(QEMUFile *f, SaveStateEntry *se, QJSON *vmdesc)
@@ -1882,7 +1882,7 @@ qemu_loadvm_section_start_full(QEMUFile *f, MigrationIncomingState *mis)
return -EINVAL;
}
- ret = vmstate_load(f, se, se->load_version_id);
+ ret = vmstate_load(f, se);
if (ret < 0) {
error_report("error while loading state for instance 0x%x of"
" device '%s'", instance_id, idstr);
@@ -1915,7 +1915,7 @@ qemu_loadvm_section_part_end(QEMUFile *f, MigrationIncomingState *mis)
return -EINVAL;
}
- ret = vmstate_load(f, se, se->load_version_id);
+ ret = vmstate_load(f, se);
if (ret < 0) {
error_report("error while loading state section id %d(%s)",
section_id, se->idstr);
--
2.9.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v3 3/3] migration: Remove section_id parameter from vmstate_load
2017-05-31 9:17 ` [Qemu-devel] [PATCH v3 3/3] migration: Remove section_id parameter from vmstate_load Juan Quintela
@ 2017-05-31 11:39 ` Dr. David Alan Gilbert
2017-06-01 5:58 ` Peter Xu
0 siblings, 1 reply; 6+ messages in thread
From: Dr. David Alan Gilbert @ 2017-05-31 11:39 UTC (permalink / raw)
To: Juan Quintela; +Cc: qemu-devel, lvivier, peterx
* Juan Quintela (quintela@redhat.com) wrote:
> Everything else assumes that we always load a device from its own
> savevm handler.
Except for the title (it's the version_id you're removing not
section_id) but now I think this series is ok because you're always
using
the load_* stuff.
I guess in manyways the LoadStateEntry list made some sense; it was
harder to accidentally use version_id rather than load_version_id if
it was on a different pointer.
But generally now for the series:
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Dave
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> Reviewed-by: Laurent Vivier <lvivier@redhat.com>
> Reviewed-by: Peter Xu <peterx@redhat.com>
> ---
> migration/savevm.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index d96209b..2d1d4bc 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -746,13 +746,13 @@ void vmstate_unregister(DeviceState *dev, const VMStateDescription *vmsd,
> }
> }
>
> -static int vmstate_load(QEMUFile *f, SaveStateEntry *se, int version_id)
> +static int vmstate_load(QEMUFile *f, SaveStateEntry *se)
> {
> trace_vmstate_load(se->idstr, se->vmsd ? se->vmsd->name : "(old)");
> if (!se->vmsd) { /* Old style */
> - return se->ops->load_state(f, se->opaque, version_id);
> + return se->ops->load_state(f, se->opaque, se->load_version_id);
> }
> - return vmstate_load_state(f, se->vmsd, se->opaque, version_id);
> + return vmstate_load_state(f, se->vmsd, se->opaque, se->load_version_id);
> }
>
> static void vmstate_save_old_style(QEMUFile *f, SaveStateEntry *se, QJSON *vmdesc)
> @@ -1882,7 +1882,7 @@ qemu_loadvm_section_start_full(QEMUFile *f, MigrationIncomingState *mis)
> return -EINVAL;
> }
>
> - ret = vmstate_load(f, se, se->load_version_id);
> + ret = vmstate_load(f, se);
> if (ret < 0) {
> error_report("error while loading state for instance 0x%x of"
> " device '%s'", instance_id, idstr);
> @@ -1915,7 +1915,7 @@ qemu_loadvm_section_part_end(QEMUFile *f, MigrationIncomingState *mis)
> return -EINVAL;
> }
>
> - ret = vmstate_load(f, se, se->load_version_id);
> + ret = vmstate_load(f, se);
> if (ret < 0) {
> error_report("error while loading state section id %d(%s)",
> section_id, se->idstr);
> --
> 2.9.4
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v3 3/3] migration: Remove section_id parameter from vmstate_load
2017-05-31 11:39 ` Dr. David Alan Gilbert
@ 2017-06-01 5:58 ` Peter Xu
0 siblings, 0 replies; 6+ messages in thread
From: Peter Xu @ 2017-06-01 5:58 UTC (permalink / raw)
To: Dr. David Alan Gilbert; +Cc: Juan Quintela, qemu-devel, lvivier
On Wed, May 31, 2017 at 12:39:25PM +0100, Dr. David Alan Gilbert wrote:
> * Juan Quintela (quintela@redhat.com) wrote:
> > Everything else assumes that we always load a device from its own
> > savevm handler.
>
> Except for the title (it's the version_id you're removing not
> section_id) but now I think this series is ok because you're always
> using
> the load_* stuff.
>
> I guess in manyways the LoadStateEntry list made some sense; it was
> harder to accidentally use version_id rather than load_version_id if
> it was on a different pointer.
Indeed. After all it was:
struct LoadStateEntry {
QLIST_ENTRY(LoadStateEntry) entry;
SaveStateEntry *se;
int section_id;
int version_id;
};
And we moved the two variables into SaveStateEntry.
(I admit I didn't really notice the difference in v1...)
>
> But generally now for the series:
>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>
>
> Dave
>
> > Signed-off-by: Juan Quintela <quintela@redhat.com>
> > Reviewed-by: Laurent Vivier <lvivier@redhat.com>
> > Reviewed-by: Peter Xu <peterx@redhat.com>
Thanks,
--
Peter Xu
^ permalink raw reply [flat|nested] 6+ messages in thread