[RESEND][PATCH] migration: fix xml file residual during vm crash with migration

zhengchuan posted 1 patch 1 year ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/0E8924944E28FD4A97A19E5A680D8A823F53C0D9@DGGEML521-MBX.china.huawei.com
src/qemu/qemu_migration.c | 3 +++
1 file changed, 3 insertions(+)

[RESEND][PATCH] migration: fix xml file residual during vm crash with migration

Posted by zhengchuan 1 year ago
>From 935ec812b822ca978631e72bb9b9a5d00df24a42 Mon Sep 17 00:00:00 2001
From: Zheng Chuan <zhengchuan@huawei.com>
Date: Mon, 27 Jul 2020 14:39:05 +0800
Subject: [PATCH] migration: fix xml file residual during vm crash with
 migration

when migration is cancelled (such as kill -9 vmpid in Src, etc), it could
do virDomainSaveStatus() to save xml file after qemuProcessStop(), which results
in xml residulal.

Fix it by that do not do virDomainSaveStatus() if vm is not active.

Signed-off-by: Zheng Chuan <zhengchuan@huawei.com>
---
 src/qemu/qemu_migration.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 2c7bf34..d2804ab 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -3073,6 +3073,9 @@ qemuMigrationSrcConfirmPhase(virQEMUDriverPtr driver,
         qemuMigrationParamsReset(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT,
                                  jobPriv->migParams, priv->job.apiFlags);
 
+        if (!virDomainObjIsActive(vm))
+            goto done;
+
         if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0)
             VIR_WARN("Failed to save status on vm %s", vm->def->name);
     }
-- 
1.8.3.1


Re: [RESEND][PATCH] migration: fix xml file residual during vm crash with migration

Posted by Michal Privoznik 11 months ago
On 7/27/20 10:28 AM, zhengchuan wrote:
>>From 935ec812b822ca978631e72bb9b9a5d00df24a42 Mon Sep 17 00:00:00 2001
> From: Zheng Chuan <zhengchuan@huawei.com>
> Date: Mon, 27 Jul 2020 14:39:05 +0800
> Subject: [PATCH] migration: fix xml file residual during vm crash with
>   migration
> 
> when migration is cancelled (such as kill -9 vmpid in Src, etc), it could
> do virDomainSaveStatus() to save xml file after qemuProcessStop(), which results
> in xml residulal.
> 
> Fix it by that do not do virDomainSaveStatus() if vm is not active.
> 
> Signed-off-by: Zheng Chuan <zhengchuan@huawei.com>
> ---
>   src/qemu/qemu_migration.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 2c7bf34..d2804ab 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -3073,6 +3073,9 @@ qemuMigrationSrcConfirmPhase(virQEMUDriverPtr driver,
>           qemuMigrationParamsReset(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT,
>                                    jobPriv->migParams, priv->job.apiFlags);
>   
> +        if (!virDomainObjIsActive(vm))
> +            goto done;
> +
>           if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0)
>               VIR_WARN("Failed to save status on vm %s", vm->def->name);
>       }
> 

This is exactly what qemuDomainObjSaveStatus() would do.

Michal

Re: [RESEND][PATCH] migration: fix xml file residual during vm crash with migration

Posted by Daniel Henrique Barboza 11 months ago

On 7/27/20 5:28 AM, zhengchuan wrote:
>>From 935ec812b822ca978631e72bb9b9a5d00df24a42 Mon Sep 17 00:00:00 2001
> From: Zheng Chuan <zhengchuan@huawei.com>
> Date: Mon, 27 Jul 2020 14:39:05 +0800
> Subject: [PATCH] migration: fix xml file residual during vm crash with
>   migration

I think the wording here would be 'fix residual xml file during ...'

> 
> when migration is cancelled (such as kill -9 vmpid in Src, etc), it could

Capital W in 'When'


> do virDomainSaveStatus() to save xml file after qemuProcessStop(), which results
> in xml residulal.

residulal -> residual

> 
> Fix it by that do not do virDomainSaveStatus() if vm is not active.

'Fix that by not doing virDomainSaveStatus() if vm is not active' is clearer.


> 
> Signed-off-by: Zheng Chuan <zhengchuan@huawei.com>
> ---
>   src/qemu/qemu_migration.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 2c7bf34..d2804ab 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -3073,6 +3073,9 @@ qemuMigrationSrcConfirmPhase(virQEMUDriverPtr driver,
>           qemuMigrationParamsReset(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT,
>                                    jobPriv->migParams, priv->job.apiFlags);
>   
> +        if (!virDomainObjIsActive(vm))
> +            goto done;
> +

This patch breaks build as is because this function does not have a 'done' label
any more. I changed it here to 'return 0' and it passed the tests. Not sure if
returning 0 here is what you want though.


Thanks,


DHB


>           if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0)
>               VIR_WARN("Failed to save status on vm %s", vm->def->name);
>       }
>