[libvirt PATCH] qemu_snapshot: fix detection if non-leaf snapshot isn't in active chain

Pavel Hrdina posted 1 patch 3 weeks, 4 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20003eac1c479cee580d14594a2f6e3e1fe31e0b.1706616942.git.phrdina@redhat.com
src/qemu/qemu_snapshot.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[libvirt PATCH] qemu_snapshot: fix detection if non-leaf snapshot isn't in active chain
Posted by Pavel Hrdina 3 weeks, 4 days ago
The condition was completely wrong. As per the comment for function
virDomainMomentIsAncestor() it checks that the first argument is
descendant of the second argument.

Consider the following snapshot tree for VM:

  s1
    |
    +- s2
    |   |
    |   +- s3
    |
    +- s4
        |
        +- s5 (current)

When deleting s2 with the original code we checked if
virDomainMomentIsAncestor(s2, s5) which would return false basically for
any snapshot as s5 is leaf snapshot so no children.

When deleting s2 with fixed code we check if
virDomainMomentIsAncestor(s5, s2) which still returns false but when
deleting s4 it will correctly return true.

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

diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 73ff533827..af5f995b0d 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -3815,7 +3815,7 @@ qemuSnapshotDeleteValidate(virDomainObj *vm,
         }
 
         if (snap != current && snap->nchildren != 0 &&
-            virDomainMomentIsAncestor(snap, current)) {
+            !virDomainMomentIsAncestor(current, snap)) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                            _("deletion of non-leaf external snapshot that is not in active chain is not supported"));
             return -1;
-- 
2.43.0
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [libvirt PATCH] qemu_snapshot: fix detection if non-leaf snapshot isn't in active chain
Posted by Peter Krempa 3 weeks, 4 days ago
On Tue, Jan 30, 2024 at 13:15:50 +0100, Pavel Hrdina wrote:
> The condition was completely wrong. As per the comment for function
> virDomainMomentIsAncestor() it checks that the first argument is
> descendant of the second argument.
> 
> Consider the following snapshot tree for VM:
> 
>   s1
>     |
>     +- s2
>     |   |
>     |   +- s3
>     |
>     +- s4
>         |
>         +- s5 (current)
> 
> When deleting s2 with the original code we checked if
> virDomainMomentIsAncestor(s2, s5) which would return false basically for
> any snapshot as s5 is leaf snapshot so no children.
> 
> When deleting s2 with fixed code we check if
> virDomainMomentIsAncestor(s5, s2) which still returns false but when
> deleting s4 it will correctly return true.
> 
> Resolves: https://issues.redhat.com/browse/RHEL-23212
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
>  src/qemu/qemu_snapshot.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Please add the currently-wrong error message into the commit message.

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