[libvirt] [PATCH] qemu: migration: Don't crash on access to 'current' job

Peter Krempa posted 1 patch 5 years, 11 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/af25ead039827e98be76083b73ec65a09c940280.1524839125.git.pkrempa@redhat.com
Test syntax-check passed
There is a newer version of this series
src/qemu/qemu_migration.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
[libvirt] [PATCH] qemu: migration: Don't crash on access to 'current' job
Posted by Peter Krempa 5 years, 11 months ago
When a VM is destroyed while being migrated (waiting in
qemuMigrationSrcWaitForCompletion) the private object cleanup code frees
the 'current' job info. Since the migration code attempts to setup
various aspects of the current job even on failure this results into a
crash.

Job data is cleared in qemuDomainObjPrivateDataClear since commit
888aa4b6b9db

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
I'm not certain that there isn't any other bigger root cause for this,
but if the job is not present due to the VM being destroyed, it does not
make much sense to set any data.

 src/qemu/qemu_migration.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 88b8253fa9..9f4de08d46 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -3552,6 +3552,7 @@ qemuMigrationSrcRun(virQEMUDriverPtr driver,
     orig_err = virSaveLastError();

     if (cancel &&
+        priv->job.current &&
         priv->job.current->status != QEMU_DOMAIN_JOB_STATUS_QEMU_COMPLETED &&
         virDomainObjIsActive(vm) &&
         qemuDomainObjEnterMonitorAsync(driver, vm,
@@ -3569,7 +3570,8 @@ qemuMigrationSrcRun(virQEMUDriverPtr driver,
     if (iothread)
         qemuMigrationSrcStopTunnel(iothread, true);

-    if (priv->job.current->status != QEMU_DOMAIN_JOB_STATUS_CANCELED)
+    if (priv->job.current &&
+        priv->job.current->status != QEMU_DOMAIN_JOB_STATUS_CANCELED)
         priv->job.current->status = QEMU_DOMAIN_JOB_STATUS_FAILED;

     goto cleanup;
-- 
2.16.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: migration: Don't crash on access to 'current' job
Posted by Jiri Denemark 5 years, 11 months ago
On Fri, Apr 27, 2018 at 16:26:36 +0200, Peter Krempa wrote:
> When a VM is destroyed while being migrated (waiting in
> qemuMigrationSrcWaitForCompletion) the private object cleanup code frees
> the 'current' job info. Since the migration code attempts to setup
> various aspects of the current job even on failure this results into a
> crash.
> 
> Job data is cleared in qemuDomainObjPrivateDataClear since commit
> 888aa4b6b9db
> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
> I'm not certain that there isn't any other bigger root cause for this,
> but if the job is not present due to the VM being destroyed, it does not
> make much sense to set any data.

In other words, the crash happens when the domain is destroyed during
migration. I think it would make more sense to change the code after the
"error" label to skip everything but qemuMigrationSrcStopTunnel() if the
domain is not active anymore. It doesn't make sense to cancel the
migration and drive mirrors if the QEMU process is already gone.

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list