From 7c1244f3a564243b00e12e22003bcc5425daefa1 Mon Sep 17 00:00:00 2001
From: Peter Krempa <pkrempa@redhat.com>
Date: Wed, 6 Dec 2023 14:47:01 +0100
Subject: [PATCH] qemuMigrationDstPrecreateStorage: Fix and clarify logic
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

While it's intended that qemuMigrationDstPrecreateDisk is called with
any kind of the disk, the logic in qemuMigrationDstPrecreateStorage
which checks the existence of the image wouldn't properly handle e.g.
network backed disks, where it would attempt to use virFileExists() on
the disk's 'src->path'.

Fix the logic by first skipping disks not meant for migration, then do
the existence check only when 'disk->src' is local storage.

Since qemuMigrationDstPrecreateDisk has a debug statement there's no
need to have an extra one right before calling into it.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
---
 src/qemu/qemu_migration.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 356b7c382b..f4e99087d8 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -448,7 +448,7 @@ qemuMigrationDstPrecreateStorage(virDomainObj *vm,
 
     for (i = 0; i < nbd->ndisks; i++) {
         virDomainDiskDef *disk;
-        const char *diskSrcPath;
+        const char *diskSrcPath = NULL;
         g_autofree char *nvmePath = NULL;
 
         VIR_DEBUG("Looking up disk target '%s' (capacity=%llu)",
@@ -461,19 +461,28 @@ qemuMigrationDstPrecreateStorage(virDomainObj *vm,
             return -1;
         }
 
+        /* Skip disks we don't want to migrate. */
+        if (!qemuMigrationAnyCopyDisk(disk, nmigrate_disks, migrate_disks))
+            continue;
+
         if (disk->src->type == VIR_STORAGE_TYPE_NVME) {
             virPCIDeviceAddressGetSysfsFile(&disk->src->nvme->pciAddr, &nvmePath);
             diskSrcPath = nvmePath;
-        } else {
+        } else if (virStorageSourceIsLocalStorage(disk->src)) {
             diskSrcPath = virDomainDiskGetSource(disk);
         }
 
-        /* Skip disks we don't want to migrate and already existing disks. */
-        if (!qemuMigrationAnyCopyDisk(disk, nmigrate_disks, migrate_disks) ||
-            (diskSrcPath && virFileExists(diskSrcPath))) {
-            continue;
+        if (diskSrcPath) {
+
+            /* don't pre-create existing disks */
+            if (virFileExists(diskSrcPath)) {
+                VIR_DEBUG("Skipping pre-create of existing source for disk '%s'", disk->dst);
+                continue;
+            }
         }
 
+        /* create the storage - if supported */
+
         if (incremental) {
             virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
                            _("pre-creation of storage target '%1$s' for incremental storage migration of disk '%2$s' is not supported"),
@@ -481,8 +490,6 @@ qemuMigrationDstPrecreateStorage(virDomainObj *vm,
             return -1;
         }
 
-        VIR_DEBUG("Proceeding with disk source %s", NULLSTR(diskSrcPath));
-
         if (qemuMigrationDstPrecreateDisk(&conn, disk, nbd->disks[i].capacity) < 0)
             return -1;
     }
-- 
2.39.5