[PATCH 0/2] backup: Don't try to update stats if VM isn't alive
Doing so results in a crash. Peter Krempa (2): qemuBackupJobTerminate: Move cleanup of temp files earlier qemuBackupJobTerminate: Don't calculate backup job stats if VM isn't active src/qemu/qemu_backup.c | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) -- 2.29.2
Upcoming patch will remove unnecessary actions if the VM crashed. The cleanup needs to be performed always, thus needs to be moved earlier. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_backup.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c index 423de9c719..d3d98c1d6a 100644 --- a/src/qemu/qemu_backup.c +++ b/src/qemu/qemu_backup.c @@ -560,21 +560,6 @@ qemuBackupJobTerminate(virDomainObjPtr vm, qemuDomainObjPrivatePtr priv = vm->privateData; size_t i; - qemuDomainJobInfoUpdateTime(priv->job.current); - - g_clear_pointer(&priv->job.completed, qemuDomainJobInfoFree); - priv->job.completed = qemuDomainJobInfoCopy(priv->job.current); - - priv->job.completed->stats.backup.total = priv->backup->push_total; - priv->job.completed->stats.backup.transferred = priv->backup->push_transferred; - priv->job.completed->stats.backup.tmp_used = priv->backup->pull_tmp_used; - priv->job.completed->stats.backup.tmp_total = priv->backup->pull_tmp_total; - - priv->job.completed->status = jobstatus; - priv->job.completed->errmsg = g_strdup(priv->backup->errmsg); - - qemuDomainEventEmitJobCompleted(priv->driver, vm); - if (!(priv->job.apiFlags & VIR_DOMAIN_BACKUP_BEGIN_REUSE_EXTERNAL) && (priv->backup->type == VIR_DOMAIN_BACKUP_TYPE_PULL || (priv->backup->type == VIR_DOMAIN_BACKUP_TYPE_PUSH && @@ -598,6 +583,21 @@ qemuBackupJobTerminate(virDomainObjPtr vm, } } + qemuDomainJobInfoUpdateTime(priv->job.current); + + g_clear_pointer(&priv->job.completed, qemuDomainJobInfoFree); + priv->job.completed = qemuDomainJobInfoCopy(priv->job.current); + + priv->job.completed->stats.backup.total = priv->backup->push_total; + priv->job.completed->stats.backup.transferred = priv->backup->push_transferred; + priv->job.completed->stats.backup.tmp_used = priv->backup->pull_tmp_used; + priv->job.completed->stats.backup.tmp_total = priv->backup->pull_tmp_total; + + priv->job.completed->status = jobstatus; + priv->job.completed->errmsg = g_strdup(priv->backup->errmsg); + + qemuDomainEventEmitJobCompleted(priv->driver, vm); + virDomainBackupDefFree(priv->backup); priv->backup = NULL; qemuDomainObjEndAsyncJob(priv->driver, vm); -- 2.29.2
If the VM isn't active calculating the job stats doesn't make sense. Additionally this prevents a crash of libvirtd if qemu terminates while libvirt wasn't running: Thread 28 "init-backup-tes" received signal SIGSEGV, Segmentation fault. [Switching to Thread 0x7fffb9310640 (LWP 3201116)] qemuDomainJobInfoUpdateTime (jobInfo=0x0) at ../../../libvirt/src/qemu/qemu_domainjob.c:275 275 if (!jobInfo->started) (gdb) bt #0 qemuDomainJobInfoUpdateTime (jobInfo=0x0) at ../../../libvirt/src/qemu/qemu_domainjob.c:275 #1 0x00007fffcba1a12d in qemuBackupJobTerminate (vm=0x7fff9c1bc840, jobstatus=QEMU_DOMAIN_JOB_STATUS_CANCELED) at ../../../libvirt/src/qemu/qemu_backup.c:563 #2 0x00007fffcbaefcae in qemuProcessStop (driver=0x7fff9c144ff0, vm=0x7fff9c1bc840, reason=VIR_DOMAIN_SHUTOFF_DAEMON, asyncJob=QEMU_ASYNC_JOB_NONE, flags=<optimized out>) at ../../../libvirt/src/qemu/qemu_process.c:7812 #3 0x00007fffcbaf2a10 in qemuProcessReconnect (opaque=<optimized out>) at ../../../libvirt/src/qemu/qemu_process.c:8578 #4 0x00007ffff7c46bb5 in virThreadHelper (data=<optimized out>) at ../../../libvirt/src/util/virthread.c:233 #5 0x00007ffff6e453f9 in start_thread () at /lib64/libpthread.so.0 #6 0x00007ffff766fb53 in clone () at /lib64/libc.so.6 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_backup.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c index d3d98c1d6a..6ce29c28e1 100644 --- a/src/qemu/qemu_backup.c +++ b/src/qemu/qemu_backup.c @@ -583,6 +583,9 @@ qemuBackupJobTerminate(virDomainObjPtr vm, } } + if (!virDomainObjIsActive(vm)) + return; + qemuDomainJobInfoUpdateTime(priv->job.current); g_clear_pointer(&priv->job.completed, qemuDomainJobInfoFree); -- 2.29.2
On 2/25/21 2:29 PM, Peter Krempa wrote:
Doing so results in a crash.
Peter Krempa (2): qemuBackupJobTerminate: Move cleanup of temp files earlier qemuBackupJobTerminate: Don't calculate backup job stats if VM isn't active
src/qemu/qemu_backup.c | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Michal Privoznik -
Peter Krempa