[libvirt PATCH 06/20] qemu_snapshot: use virDomainDiskByName while updating domain def

Pavel Hrdina posted 20 patches 1 year, 4 months ago
There is a newer version of this series
[libvirt PATCH 06/20] qemu_snapshot: use virDomainDiskByName while updating domain def
Posted by Pavel Hrdina 1 year, 4 months ago
When creating external snapshot this function is called only when the VM
is not running so there is only one definition to care about. However,
it will be used by external snapshot revert code for active and inactive
definition and they may be different if a disk was (un)plugged only for
the active or inactive definition.

The current code would crash so use virDomainDiskByName() to get the
correct disk from the domain definition based on the disk name and make
sure it exists.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
 src/qemu/qemu_snapshot.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 924731fdf1..bbef753db6 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -152,11 +152,14 @@ qemuSnapshotDomainDefUpdateDisk(virDomainDef *domdef,
     for (i = 0; i < snapdef->ndisks; i++) {
         g_autoptr(virStorageSource) newsrc = NULL;
         virDomainSnapshotDiskDef *snapdisk = &(snapdef->disks[i]);
-        virDomainDiskDef *defdisk = domdef->disks[i];
+        virDomainDiskDef *defdisk = virDomainDiskByName(domdef, snapdisk->name, false);
 
         if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
             continue;
 
+        if (!defdisk)
+            continue;
+
         if (!(newsrc = virStorageSourceCopy(snapdisk->src, false)))
             return -1;
 
-- 
2.39.2
Re: [libvirt PATCH 06/20] qemu_snapshot: use virDomainDiskByName while updating domain def
Posted by Peter Krempa 1 year, 3 months ago
On Mon, Mar 13, 2023 at 16:42:07 +0100, Pavel Hrdina wrote:
> When creating external snapshot this function is called only when the VM
> is not running so there is only one definition to care about. However,
> it will be used by external snapshot revert code for active and inactive
> definition and they may be different if a disk was (un)plugged only for
> the active or inactive definition.
> 
> The current code would crash so use virDomainDiskByName() to get the
> correct disk from the domain definition based on the disk name and make
> sure it exists.

Historically the snapshot code expects that virDomainSnapshotAlignDisks
was ran and thus the disk and snapshot definitions are in sync.

> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
>  src/qemu/qemu_snapshot.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)

Reviewed-by: Peter Krempa <pkrempa@redhat.com>