* Jay Zhou (jianjay.z...@huawei.com) wrote:
> Qemu_savevm_state_cleanup() takes about 300ms in my ram migration tests
> with a 8U24G vm(20G is really occupied), the main cost comes from
> KVM_SET_USER_MEMORY_REGION ioctl when mem.memory_size = 0 in
> kvm_set_user_memory_region(). In kmod, the main cost is
> kvm_zap_obsolete_pages(), which traverses the active_mmu_pages list to
> zap the unsync sptes.
Hi Jay,
Is this actually increasing the real downtime when the guest isn't
running, or is it just the reported time? I see that the s->downtime
value is calculated right after where we currently call
qemu_savevm_state_cleanup.
I guess the biggest problem is that 300ms happens before we restart
the guest on the source if a migration fails.
> I think it can be optimized:
> (1) source vm will be destroyed if the migration is successfully done,
> so the resources will be cleanuped automatically by the system
> (2) delay the cleanup if the migration failed
I don't like putting it in qmp_cont; that shouldn't have migration magic
in it.
I guess we could put it in migrate_fd_cleanup perhaps? It gets called on
a bh near the end - or could we just move it closer to the end of
migration_thread?
However, we would need to be a bit careful of anything that needs
cleaning up before the source restarts on failure; I'm not sure of
the semantics of all the current things wired into save_cleanup.
Dave
> Signed-off-by: Jay Zhou <jianjay.z...@huawei.com>
> ---
> migration/migration.c | 16 +++++++++-------
> qmp.c | 10 ++++++++++
> 2 files changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index a0db40d..72832be 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1877,6 +1877,15 @@ static void *migration_thread(void *opaque)
> if (qemu_file_get_error(s->to_dst_file)) {
> migrate_set_state(&s->state, current_active_state,
> MIGRATION_STATUS_FAILED);
> + /*
> + * The resource has been allocated by migration will be reused in
> + * COLO process, so don't release them.
> + */
> + if (!enable_colo) {
> + qemu_mutex_lock_iothread();
> + qemu_savevm_state_cleanup();
> + qemu_mutex_unlock_iothread();
> + }
> trace_migration_thread_file_err();
> break;
> }
> @@ -1916,13 +1925,6 @@ static void *migration_thread(void *opaque)
> end_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
>
> qemu_mutex_lock_iothread();
> - /*
> - * The resource has been allocated by migration will be reused in COLO
> - * process, so don't release them.
> - */
> - if (!enable_colo) {
> - qemu_savevm_state_cleanup();
> - }
> if (s->state == MIGRATION_STATUS_COMPLETED) {
> uint64_t transferred_bytes = qemu_ftell(s->to_dst_file);
> s->total_time = end_time - s->total_time;
> diff --git a/qmp.c b/qmp.c
> index b86201e..0e68eaa 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -37,6 +37,8 @@
> #include "qom/object_interfaces.h"
> #include "hw/mem/pc-dimm.h"
> #include "hw/acpi/acpi_dev_interface.h"
> +#include "migration/migration.h"
> +#include "migration/savevm.h"
>
> NameInfo *qmp_query_name(Error **errp)
> {
> @@ -200,6 +202,14 @@ void qmp_cont(Error **errp)
> if (runstate_check(RUN_STATE_INMIGRATE)) {
> autostart = 1;
> } else {
> + /*
> + * Delay the cleanup to reduce the downtime of migration.
> + * The resource has been allocated by migration will be reused
> + * in COLO process, so don't release them.
> + */
> + if (runstate_check(RUN_STATE_POSTMIGRATE) &&
> !migrate_colo_enabled()) {
> + qemu_savevm_state_cleanup();
> + }
> vm_start();
> }
> }
> --
> 1.8.3.1
>
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
