Historically creating offline external snapshot required disk-only flag
as well. Now when user requests new snapshot for offline VM and at least
one disk is specified to use external snapshot we will no longer require
disk-only flag as all other not specified disk will use external
snapshots as well.

Resolves: https://issues.redhat.com/browse/RHEL-22797
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
 src/qemu/qemu_snapshot.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index ab7b47319b..d089f70d4e 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -1577,15 +1577,25 @@ qemuSnapshotCreateXMLValidateDef(virDomainObj *vm,
 
 
 static bool
-qemuSnapshotCreateUseExternal(virDomainSnapshotDef *def,
+qemuSnapshotCreateUseExternal(virDomainObj *vm,
+                              virDomainSnapshotDef *def,
                               unsigned int flags)
 {
+    ssize_t i;
+
     if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY)
         return true;
 
     if (def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
         return true;
 
+    if (!virDomainObjIsActive(vm)) {
+        for (i = 0; i < def->ndisks; i++) {
+            if (def->disks[i].snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
+                return true;
+        }
+    }
+
     return false;
 }
 
@@ -1618,7 +1628,7 @@ qemuSnapshotCreateAlignDisks(virDomainObj *vm,
             return -1;
     }
 
-    if (qemuSnapshotCreateUseExternal(def, flags)) {
+    if (qemuSnapshotCreateUseExternal(vm, def, flags)) {
         align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
         def->state = virDomainObjGetState(vm, NULL);
 
-- 
2.43.0
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org

On Tue, Jan 30, 2024 at 12:33:51 +0100, Pavel Hrdina wrote:
> Historically creating offline external snapshot required disk-only flag
> as well. Now when user requests new snapshot for offline VM and at least
> one disk is specified to use external snapshot we will no longer require
> disk-only flag as all other not specified disk will use external
> snapshots as well.
> 
> Resolves: https://issues.redhat.com/browse/RHEL-22797
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
>  src/qemu/qemu_snapshot.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
> index ab7b47319b..d089f70d4e 100644
> --- a/src/qemu/qemu_snapshot.c
> +++ b/src/qemu/qemu_snapshot.c
> @@ -1577,15 +1577,25 @@ qemuSnapshotCreateXMLValidateDef(virDomainObj *vm,
>  
>  
>  static bool
> -qemuSnapshotCreateUseExternal(virDomainSnapshotDef *def,
> +qemuSnapshotCreateUseExternal(virDomainObj *vm,
> +                              virDomainSnapshotDef *def,
>                                unsigned int flags)
>  {
> +    ssize_t i;

size_t, def->ndisks is also size_t

> +
>      if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY)
>          return true;
>  
>      if (def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
>          return true;
>  
> +    if (!virDomainObjIsActive(vm)) {
> +        for (i = 0; i < def->ndisks; i++) {

I'd mention that $OTHER_FUNCTION guarantees that we don't have a
combination of _LOCATION_INTERNAL and _LOCATION_EXTERNAL here.

Reviewed-by: Peter Krempa <pkrempa@redhat.com>
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org