* zhanghailiang (zhang.zhanghaili...@huawei.com) wrote:
> qemu_loadvm_state is too long, and we can simplify it by splitting up
> with three helper functions.
Yes, good idea.
> Signed-off-by: zhanghailiang <zhang.zhanghaili...@huawei.com>
> ---
> migration/savevm.c | 161
> ++++++++++++++++++++++++++++++++---------------------
> 1 file changed, 97 insertions(+), 64 deletions(-)
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index f102870..c7c26d8 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1710,90 +1710,123 @@ void loadvm_free_handlers(MigrationIncomingState
> *mis)
> }
> }
>
> +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;
> +
> + /* Read section start */
> + section_id = qemu_get_be32(f);
> + if (!qemu_get_counted_string(f, idstr)) {
> + error_report("Unable to read ID string for section %u",
> + section_id);
> + return -EINVAL;
> + }
> + instance_id = qemu_get_be32(f);
> + version_id = qemu_get_be32(f);
> +
> + trace_qemu_loadvm_state_section_startfull(section_id, idstr,
> + instance_id, version_id);
> + /* Find savevm section */
> + se = find_se(idstr, instance_id);
> + if (se == NULL) {
> + error_report("Unknown savevm section or instance '%s' %d",
> + idstr, instance_id);
> + ret = -EINVAL;
> + return ret;
Minor; you don't need 'ret' there, just return -EINVAL.
> + }
> +
> + /* Validate version */
> + if (version_id > se->version_id) {
> + error_report("savevm: unsupported version %d for '%s' v%d",
> + version_id, idstr, se->version_id);
> + ret = -EINVAL;
> + return ret;
same
> + }
> +
> + /* 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, le->se, le->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)) {
> + ret = -EINVAL;
> + return ret;
same.
> + }
> +
> + return 0;
> +}
> +
> +static int
> +qemu_loadvm_section_part_end(QEMUFile *f, MigrationIncomingState *mis)
> +{
> + uint32_t section_id;
> + LoadStateEntry *le;
> + 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) {
> + break;
> + }
> + }
> + if (le == NULL) {
> + error_report("Unknown savevm section %d", section_id);
> + ret = -EINVAL;
> + return ret;
same
> + }
> +
> + ret = vmstate_load(f, le->se, le->version_id);
> + if (ret < 0) {
> + error_report("error while loading state section id %d(%s)",
> + section_id, le->se->idstr);
> + return ret;
> + }
> + if (!check_section_footer(f, le)) {
> + ret = -EINVAL;
> + return ret;
same
> + }
> +
> + return 0;
> +}
> +
> static int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis)
> {
> uint8_t section_type;
> int ret;
>
> while ((section_type = qemu_get_byte(f)) != QEMU_VM_EOF) {
> - uint32_t instance_id, version_id, section_id;
> - SaveStateEntry *se;
> - LoadStateEntry *le;
> - char idstr[256];
>
> trace_qemu_loadvm_state_section(section_type);
> switch (section_type) {
> case QEMU_VM_SECTION_START:
> case QEMU_VM_SECTION_FULL:
> - /* Read section start */
> - section_id = qemu_get_be32(f);
> - if (!qemu_get_counted_string(f, idstr)) {
> - error_report("Unable to read ID string for section %u",
> - section_id);
> - return -EINVAL;
> - }
> - instance_id = qemu_get_be32(f);
> - version_id = qemu_get_be32(f);
> -
> - trace_qemu_loadvm_state_section_startfull(section_id, idstr,
> - instance_id,
> version_id);
> - /* Find savevm section */
> - se = find_se(idstr, instance_id);
> - if (se == NULL) {
> - error_report("Unknown savevm section or instance '%s' %d",
> - idstr, instance_id);
> - return -EINVAL;
> - }
> -
> - /* Validate version */
> - if (version_id > se->version_id) {
> - error_report("savevm: unsupported version %d for '%s' v%d",
> - version_id, idstr, se->version_id);
> - 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, le->se, le->version_id);
> + ret = qemu_loadvm_section_start_full(f, mis);
> 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)) {
> - return -EINVAL;
> - }
> break;
> case QEMU_VM_SECTION_PART:
> case QEMU_VM_SECTION_END:
> - 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) {
> - break;
> - }
> - }
> - if (le == NULL) {
> - error_report("Unknown savevm section %d", section_id);
> - return -EINVAL;
> - }
> -
> - ret = vmstate_load(f, le->se, le->version_id);
> + ret = qemu_loadvm_section_part_end(f, mis);
> if (ret < 0) {
> - error_report("error while loading state section id %d(%s)",
> - section_id, le->se->idstr);
> return ret;
> }
> - if (!check_section_footer(f, le)) {
> - return -EINVAL;
> - }
> break;
> case QEMU_VM_COMMAND:
> ret = loadvm_process_command(f);
> --
> 1.8.3.1
Other than the minor return fixups;
Reviewed-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
>
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
