'qemuDomainSupportsCheckpointsBlockjobs()' should really be used only
with active VMs based on the scope of interlocking it does.

This means that the inactive snapshot code path needs to do the
interlocking based on what's supported:
 - external snapshot support was not implemented yet
    (bitmaps need to be propagated to the new overlay image)
 - internal snapshot support can be deferred to qemu

Move the check inside qemuSnapshotPrepare() which has knowledge about
the snapshot type and implement an explicit check for the inactive case.

See: https://gitlab.com/libvirt/libvirt/-/issues/739
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/qemu/qemu_snapshot.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 7ce018b026..ecf8eb3cca 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -39,6 +39,7 @@
 #include "domain_audit.h"
 #include "locking/domain_lock.h"
 #include "virdomainsnapshotobjlist.h"
+#include "virdomaincheckpointobjlist.h"
 #include "virqemu.h"
 #include "storage_source.h"

@@ -1067,6 +1068,23 @@ qemuSnapshotPrepare(virDomainObj *vm,

     }

+    /* Handle interlocking with 'checkpoints':
+     * - if the VM is online use qemuDomainSupportsCheckpointsBlockjobs
+     * - if the VM is offline disallow external snapshots as the support for
+     *   propagating bitmaps into the would-be-created overlay is not yet implemented
+     */
+    if (!active) {
+        if (external &&
+            virDomainListCheckpoints(vm->checkpoints, NULL, NULL, NULL, 0) > 0) {
+            virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+                           _("support for offline external snapshots while checkpoint exists was not yet implemented"));
+            return -1;
+        }
+    } else {
+        if (qemuDomainSupportsCheckpointsBlockjobs(vm) < 0)
+            return -1;
+    }
+
     /* Alter flags to let later users know what we learned.  */
     if (external && !active)
         *flags |= VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY;
@@ -2134,9 +2152,6 @@ qemuSnapshotCreateXML(virDomainPtr domain,
                             VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE,
                             NULL);

-    if (qemuDomainSupportsCheckpointsBlockjobs(vm) < 0)
-        return NULL;
-
     if (!vm->persistent && (flags & VIR_DOMAIN_SNAPSHOT_CREATE_HALT)) {
         virReportError(VIR_ERR_OPERATION_INVALID, "%s",
                        _("cannot halt after transient domain snapshot"));
-- 
2.48.1

On a Monday in 2025, Peter Krempa wrote:
>'qemuDomainSupportsCheckpointsBlockjobs()' should really be used only
>with active VMs based on the scope of interlocking it does.
>
>This means that the inactive snapshot code path needs to do the
>interlocking based on what's supported:
> - external snapshot support was not implemented yet
>    (bitmaps need to be propagated to the new overlay image)
> - internal snapshot support can be deferred to qemu
>
>Move the check inside qemuSnapshotPrepare() which has knowledge about
>the snapshot type and implement an explicit check for the inactive case.
>
>See: https://gitlab.com/libvirt/libvirt/-/issues/739
>Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>---
> src/qemu/qemu_snapshot.c | 21 ++++++++++++++++++---
> 1 file changed, 18 insertions(+), 3 deletions(-)
>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano