qemu: backup: Use 'async' monitor in 'qemuBackupDiskDataCleanupOne'

[PATCH] qemu: backup: Use 'async' monitor in 'qemuBackupDiskDataCleanupOne'

Posted by Peter Krempa 1 year, 5 months ago

'qemuBackupDiskDataCleanupOne()' is entering the monitor while we're in
the async backup job inside 'qemuBackupBegin()' which is semantically
wrong and per upstream report causes crashes if some monitoring commands
are run in parallel.

Use qemuDomainObjEnterMonitorAsync() instead.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/qemu/qemu_backup.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c
index 81391c29f7..5eb2cbe306 100644
--- a/src/qemu/qemu_backup.c
+++ b/src/qemu/qemu_backup.c
@@ -125,7 +125,8 @@ qemuBackupDiskDataCleanupOne(virDomainObj *vm,

     if (!dd->started) {
         if (dd->added) {
-            qemuDomainObjEnterMonitor(vm);
+            if (qemuDomainObjEnterMonitorAsync(vm, VIR_ASYNC_JOB_BACKUP) < 0)
+                return;
             qemuBlockStorageSourceAttachRollback(priv->mon, dd->crdata->srcdata[0]);
             qemuDomainObjExitMonitor(vm);
         }
-- 
2.46.0

Re: [PATCH] qemu: backup: Use 'async' monitor in 'qemuBackupDiskDataCleanupOne'

Posted by Jiri Denemark 1 year, 5 months ago

On Thu, Sep 05, 2024 at 14:58:45 +0200, Peter Krempa wrote:
> 'qemuBackupDiskDataCleanupOne()' is entering the monitor while we're in
> the async backup job inside 'qemuBackupBegin()' which is semantically
> wrong and per upstream report causes crashes if some monitoring commands
> are run in parallel.
> 
> Use qemuDomainObjEnterMonitorAsync() instead.
> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>

Fixes: https://gitlab.com/libvirt/libvirt/-/issues/668

> ---
>  src/qemu/qemu_backup.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c
> index 81391c29f7..5eb2cbe306 100644
> --- a/src/qemu/qemu_backup.c
> +++ b/src/qemu/qemu_backup.c
> @@ -125,7 +125,8 @@ qemuBackupDiskDataCleanupOne(virDomainObj *vm,
> 
>      if (!dd->started) {
>          if (dd->added) {
> -            qemuDomainObjEnterMonitor(vm);
> +            if (qemuDomainObjEnterMonitorAsync(vm, VIR_ASYNC_JOB_BACKUP) < 0)
> +                return;
>              qemuBlockStorageSourceAttachRollback(priv->mon, dd->crdata->srcdata[0]);
>              qemuDomainObjExitMonitor(vm);
>          }

Reviewed-by: Jiri Denemark <jdenemar@redhat.com>