Re: [Qemu-devel] [PATCH v2 1/3] migration: Use savevm_handlers instead of loadvm copy
"Dr. David Alan Gilbert" wrote:
> * Juan Quintela (quint...@redhat.com) wrote:
>> 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.
>
>
>
>> 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->section_id == section_id) {
>
> Isn't this the problem? What guarantees that the se->section_id
> is the same as the source's section_id - I don't think anything.
> It's just dynamically allocated in register_savevm_live so the
> initialisation order on source/dest could be different and you'd
> get different ID. You can't update se->section_id
> unless you guaranteed to updated all of them.
Ok. It worked for me because I was using the same command line in both
sides.
Changed to add load_section_id and adjust all around.
Thanks, Juan.
Re: [Qemu-devel] [PATCH v2 1/3] migration: Use savevm_handlers instead of loadvm copy
* Juan Quintela (quint...@redhat.com) wrote:
> 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.
> 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->section_id == section_id) {
Isn't this the problem? What guarantees that the se->section_id
is the same as the source's section_id - I don't think anything.
It's just dynamically allocated in register_savevm_live so the
initialisation order on source/dest could be different and you'd
get different ID. You can't update se->section_id
unless you guaranteed to updated all of them.
Dave
> 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
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH v2 1/3] migration: Use savevm_handlers instead of loadvm copy
On 30/05/2017 10:37, Juan Quintela wrote:
> 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
Reviewed-by: Laurent Vivier
> ---
> migration/savevm.c | 29 -
> 1 file changed, 16 insertions(+), 13 deletions(-)
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index d971e5e..d7c08c3 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -251,6 +251,8 @@ 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;
> SaveVMHandlers *ops;
> const VMStateDescription *vmsd;
> @@ -1792,7 +1794,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;
> @@ -1805,15 +1807,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->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->section_id);
> return false;
> }
>
> @@ -1866,6 +1868,7 @@ qemu_loadvm_section_start_full(QEMUFile *f,
> MigrationIncomingState *mis)
> version_id, idstr, se->version_id);
> return -EINVAL;
> }
> +se->load_version_id = version_id;
>
> /* Validate if it is a device's state */
> if (xen_enabled() && se->is_ram) {
> @@ -1881,13 +1884,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;
> }
>
> @@ -1898,29 +1901,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->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;
> }
>
>
[Qemu-devel] [PATCH v2 1/3] migration: Use savevm_handlers instead of loadvm copy
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
---
migration/savevm.c | 29 -
1 file changed, 16 insertions(+), 13 deletions(-)
diff --git a/migration/savevm.c b/migration/savevm.c
index d971e5e..d7c08c3 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -251,6 +251,8 @@ 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;
SaveVMHandlers *ops;
const VMStateDescription *vmsd;
@@ -1792,7 +1794,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;
@@ -1805,15 +1807,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->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->section_id);
return false;
}
@@ -1866,6 +1868,7 @@ qemu_loadvm_section_start_full(QEMUFile *f,
MigrationIncomingState *mis)
version_id, idstr, se->version_id);
return -EINVAL;
}
+se->load_version_id = version_id;
/* Validate if it is a device's state */
if (xen_enabled() && se->is_ram) {
@@ -1881,13 +1884,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;
}
@@ -1898,29 +1901,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->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
