7:49 a.m.
Peter Krempa (8):
qemuMigrationDstPrepareStorage: Use 'switch' statement to include all
storage types
qemuMigrationDstPrepareStorage: Properly consider path for 'vdpa'
devices
qemuMigrationDstPrecreateDisk: Refactor cleanup
qemuMigrationDstPrepareStorage: Move block device specific logic
qemuMigrationDstPrepareStorage: Rework storage existence check
qemuMigrationDstPrepareStorage: Reject migration into 'dir' and
'vhost-user' types
qemuMigrationDstPrepareStorage: Move assumption that 'network' disks
always exist
qemuMigrationDstPrepareStorage: Annotate that existance of 'volume'
disks is checked elswhere
src/qemu/qemu_migration.c | 117 +++++++++++++++++++++-----------------
1 file changed, 66 insertions(+), 51 deletions(-)
--
2.43.0
7:49 a.m.
New subject: [PATCH 1/8] qemuMigrationDstPrepareStorage: Use 'switch' statement to include all storage types
Decrease the likelyhood that addition of a new storage type will be
forgotten.
This patch also unifies the type check to consult the 'actual' type of
the storage in both cases as the NVMe check looked for the XML declared
type while virStorageSourceIsLocalStorage() looks for the
actual/translated type.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_migration.c | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 01ab803842..3e0aae4e7c 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -465,11 +465,27 @@ qemuMigrationDstPrepareStorage(virDomainObj *vm,
if (!qemuMigrationAnyCopyDisk(disk, nmigrate_disks, migrate_disks))
continue;
- if (disk->src->type == VIR_STORAGE_TYPE_NVME) {
+ switch (virStorageSourceGetActualType(disk->src)) {
+ case VIR_STORAGE_TYPE_FILE:
+ case VIR_STORAGE_TYPE_BLOCK:
+ case VIR_STORAGE_TYPE_DIR:
+ diskSrcPath = virDomainDiskGetSource(disk);
+ break;
+
+ case VIR_STORAGE_TYPE_NVME:
+ /* While NVMe disks are local, they are not accessible via src->path.
+ * Therefore, we have to return false here. */
virPCIDeviceAddressGetSysfsFile(&disk->src->nvme->pciAddr,
&nvmePath);
diskSrcPath = nvmePath;
- } else if (virStorageSourceIsLocalStorage(disk->src)) {
- diskSrcPath = virDomainDiskGetSource(disk);
+ break;
+
+ case VIR_STORAGE_TYPE_NETWORK:
+ case VIR_STORAGE_TYPE_VOLUME:
+ case VIR_STORAGE_TYPE_VHOST_USER:
+ case VIR_STORAGE_TYPE_VHOST_VDPA:
+ case VIR_STORAGE_TYPE_LAST:
+ case VIR_STORAGE_TYPE_NONE:
+ break;
}
if (diskSrcPath) {
--
2.43.0
7:49 a.m.
New subject: [PATCH 2/8] qemuMigrationDstPrepareStorage: Properly consider path for 'vdpa' devices
Allow storage migration of VDPA devices by properly checking that they
exist on the destionation. Pre-creation is not supported but if the
device exists the migration should be able to succeed.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_migration.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 3e0aae4e7c..5e27cd5dbe 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -479,10 +479,13 @@ qemuMigrationDstPrepareStorage(virDomainObj *vm,
diskSrcPath = nvmePath;
break;
+ case VIR_STORAGE_TYPE_VHOST_VDPA:
+ diskSrcPath = disk->src->vdpadev;
+ break;
+
case VIR_STORAGE_TYPE_NETWORK:
case VIR_STORAGE_TYPE_VOLUME:
case VIR_STORAGE_TYPE_VHOST_USER:
- case VIR_STORAGE_TYPE_VHOST_VDPA:
case VIR_STORAGE_TYPE_LAST:
case VIR_STORAGE_TYPE_NONE:
break;
--
2.43.0
7:49 a.m.
New subject: [PATCH 3/8] qemuMigrationDstPrecreateDisk: Refactor cleanup
Automatically free helper variables, remove the 'cleanup' label and
use virBufferCurrentContent() to take the XML from the buffer rather
than extracting it to a separate variable.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_migration.c | 42 +++++++++++++--------------------------
1 file changed, 14 insertions(+), 28 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 5e27cd5dbe..32569ecbb4 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -277,12 +277,11 @@ qemuMigrationDstPrecreateDisk(virConnectPtr *conn,
virDomainDiskDef *disk,
unsigned long long capacity)
{
- int ret = -1;
- virStoragePoolPtr pool = NULL;
- virStorageVolPtr vol = NULL;
- char *volName = NULL, *basePath = NULL;
- char *volStr = NULL;
+ g_autoptr(virStoragePool) pool = NULL;
+ g_autoptr(virStorageVol) vol = NULL;
g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
+ char *volName = NULL;
+ g_autofree char *basePath = NULL;
const char *format = NULL;
const char *compat = NULL;
unsigned int flags = 0;
@@ -303,7 +302,7 @@ qemuMigrationDstPrecreateDisk(virConnectPtr *conn,
virReportError(VIR_ERR_INVALID_ARG,
_("malformed disk path: %1$s"),
disk->src->path);
- goto cleanup;
+ return -1;
}
*volName = '\0';
@@ -311,11 +310,11 @@ qemuMigrationDstPrecreateDisk(virConnectPtr *conn,
if (!*conn) {
if (!(*conn = virGetConnectStorage()))
- goto cleanup;
+ return -1;
}
if (!(pool = virStoragePoolLookupByTargetPath(*conn, basePath)))
- goto cleanup;
+ return -1;
format = virStorageFileFormatTypeToString(disk->src->format);
if (disk->src->format == VIR_STORAGE_FILE_QCOW2) {
flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA;
@@ -327,11 +326,11 @@ qemuMigrationDstPrecreateDisk(virConnectPtr *conn,
case VIR_STORAGE_TYPE_VOLUME:
if (!*conn) {
if (!(*conn = virGetConnectStorage()))
- goto cleanup;
+ return -1;
}
if (!(pool = virStoragePoolLookupByName(*conn,
disk->src->srcpool->pool)))
- goto cleanup;
+ return -1;
format = virStorageFileFormatTypeToString(disk->src->format);
volName = disk->src->srcpool->volume;
if (disk->src->format == VIR_STORAGE_FILE_QCOW2)
@@ -353,14 +352,13 @@ qemuMigrationDstPrecreateDisk(virConnectPtr *conn,
virReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot precreate storage for disk type
'%1$s'"),
virStorageTypeToString(disk->src->type));
- goto cleanup;
+ return -1;
}
if ((vol = virStorageVolLookupByName(pool, volName))) {
VIR_DEBUG("Skipping creation of already existing volume of name
'%s'",
volName);
- ret = 0;
- goto cleanup;
+ return 0;
}
virBufferAddLit(&buf, "<volume>\n");
@@ -377,22 +375,10 @@ qemuMigrationDstPrecreateDisk(virConnectPtr *conn,
virBufferAdjustIndent(&buf, -2);
virBufferAddLit(&buf, "</volume>\n");
- if (!(volStr = virBufferContentAndReset(&buf))) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("unable to create volume XML"));
- goto cleanup;
- }
-
- if (!(vol = virStorageVolCreateXML(pool, volStr, flags)))
- goto cleanup;
+ if (!(vol = virStorageVolCreateXML(pool, virBufferCurrentContent(&buf), flags)))
+ return -1;
- ret = 0;
- cleanup:
- VIR_FREE(basePath);
- VIR_FREE(volStr);
- virObjectUnref(vol);
- virObjectUnref(pool);
- return ret;
+ return 0;
}
static bool
--
2.43.0
7:49 a.m.
New subject: [PATCH 4/8] qemuMigrationDstPrepareStorage: Move block device specific logic
Now that we have a switch statement, the code adding the 'slice' for
block devices of non-equal sizes can be moved to appropriate location.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_migration.c | 41 +++++++++++++++++++--------------------
1 file changed, 20 insertions(+), 21 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 32569ecbb4..2ac10ab5ff 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -452,8 +452,27 @@ qemuMigrationDstPrepareStorage(virDomainObj *vm,
continue;
switch (virStorageSourceGetActualType(disk->src)) {
- case VIR_STORAGE_TYPE_FILE:
case VIR_STORAGE_TYPE_BLOCK:
+ /* In case the destination of the storage migration is a block
+ * device it might be possible that for various reasons the size
+ * will not be identical. Since qemu refuses to do a blockdev-mirror
+ * into an image which doesn't have the exact same size we need to
+ * install a slice on top of the top image */
+ if (disk->src->format == VIR_STORAGE_FILE_RAW &&
+ disk->src->sliceStorage == NULL) {
+ qemuDomainObjPrivate *priv = vm->privateData;
+ g_autoptr(virQEMUDriverConfig) cfg =
virQEMUDriverGetConfig(priv->driver);
+ qemuDomainDiskPrivate *diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
+
+ if (qemuDomainStorageUpdatePhysical(cfg, vm, disk->src) == 0
&&
+ disk->src->physical > nbd->disks[i].capacity) {
+ disk->src->sliceStorage = g_new0(virStorageSourceSlice, 1);
+ disk->src->sliceStorage->size = nbd->disks[i].capacity;
+ diskPriv->migrationslice = true;
+ }
+ }
+ G_GNUC_FALLTHROUGH;
+ case VIR_STORAGE_TYPE_FILE:
case VIR_STORAGE_TYPE_DIR:
diskSrcPath = virDomainDiskGetSource(disk);
break;
@@ -478,26 +497,6 @@ qemuMigrationDstPrepareStorage(virDomainObj *vm,
}
if (diskSrcPath) {
- /* In case the destination of the storage migration is a block
- * device it might be possible that for various reasons the size
- * will not be identical. Since qemu refuses to do a blockdev-mirror
- * into an image which doesn't have the exact same size we need to
- * install a slice on top of the top image */
- if (virStorageSourceIsBlockLocal(disk->src) &&
- disk->src->format == VIR_STORAGE_FILE_RAW &&
- disk->src->sliceStorage == NULL) {
- qemuDomainObjPrivate *priv = vm->privateData;
- g_autoptr(virQEMUDriverConfig) cfg =
virQEMUDriverGetConfig(priv->driver);
- qemuDomainDiskPrivate *diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
-
- if (qemuDomainStorageUpdatePhysical(cfg, vm, disk->src) == 0
&&
- disk->src->physical > nbd->disks[i].capacity) {
- disk->src->sliceStorage = g_new0(virStorageSourceSlice, 1);
- disk->src->sliceStorage->size = nbd->disks[i].capacity;
- diskPriv->migrationslice = true;
- }
- }
-
/* don't pre-create existing disks */
if (virFileExists(diskSrcPath)) {
VIR_DEBUG("Skipping pre-create of existing source for disk
'%s'", disk->dst);
--
2.43.0
7:49 a.m.
New subject: [PATCH 5/8] qemuMigrationDstPrepareStorage: Rework storage existence check
Check the existance of storage per-type rather than trying to come up
with a common "path".
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_migration.c | 38 ++++++++++++++++++++------------------
1 file changed, 20 insertions(+), 18 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 2ac10ab5ff..d248f87c2d 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -434,8 +434,7 @@ qemuMigrationDstPrepareStorage(virDomainObj *vm,
for (i = 0; i < nbd->ndisks; i++) {
virDomainDiskDef *disk;
- const char *diskSrcPath = NULL;
- g_autofree char *nvmePath = NULL;
+ bool exists = false;
VIR_DEBUG("Looking up disk target '%s' (capacity=%llu)",
nbd->disks[i].target, nbd->disks[i].capacity);
@@ -471,21 +470,27 @@ qemuMigrationDstPrepareStorage(virDomainObj *vm,
diskPriv->migrationslice = true;
}
}
- G_GNUC_FALLTHROUGH;
+
+ exists = virFileExists(disk->src->path);
+ break;
+
case VIR_STORAGE_TYPE_FILE:
case VIR_STORAGE_TYPE_DIR:
- diskSrcPath = virDomainDiskGetSource(disk);
+ exists = virFileExists(disk->src->path);
break;
- case VIR_STORAGE_TYPE_NVME:
- /* While NVMe disks are local, they are not accessible via src->path.
- * Therefore, we have to return false here. */
+ case VIR_STORAGE_TYPE_NVME: {
+ /* While NVMe disks are local, they are not accessible via src->path */
+ g_autofree char *nvmePath = NULL;
+
virPCIDeviceAddressGetSysfsFile(&disk->src->nvme->pciAddr,
&nvmePath);
- diskSrcPath = nvmePath;
+
+ exists = virFileExists(nvmePath);
break;
+ }
case VIR_STORAGE_TYPE_VHOST_VDPA:
- diskSrcPath = disk->src->vdpadev;
+ exists = virFileExists(disk->src->vdpadev);
break;
case VIR_STORAGE_TYPE_NETWORK:
@@ -496,20 +501,17 @@ qemuMigrationDstPrepareStorage(virDomainObj *vm,
break;
}
- 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;
- }
- }
+ VIR_DEBUG("target='%s' exists='%d'", disk->dst,
exists);
+
+ if (exists)
+ 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"),
- NULLSTR(diskSrcPath), nbd->disks[i].target);
+ _("pre-creation of storage target for incremental storage
migration of disk '%1$s' is not supported"),
+ nbd->disks[i].target);
return -1;
}
--
2.43.0
7:49 a.m.
New subject: [PATCH 6/8] qemuMigrationDstPrepareStorage: Reject migration into 'dir' and 'vhost-user' types
Migrating into a 'directory' won't ever work as we ask qemu to emulate a
fat filesystem, so restoring of the files won't be possible. Same for
'vhost-user' disks which don't support blockjobs as there's no block
backend used in qemu.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_migration.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index d248f87c2d..4c524fafe4 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -475,7 +475,6 @@ qemuMigrationDstPrepareStorage(virDomainObj *vm,
break;
case VIR_STORAGE_TYPE_FILE:
- case VIR_STORAGE_TYPE_DIR:
exists = virFileExists(disk->src->path);
break;
@@ -493,9 +492,15 @@ qemuMigrationDstPrepareStorage(virDomainObj *vm,
exists = virFileExists(disk->src->vdpadev);
break;
+ case VIR_STORAGE_TYPE_VHOST_USER:
+ case VIR_STORAGE_TYPE_DIR:
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
+ _("non-shared storage migration into '%1$s'
target is not supported"),
+
virStorageTypeToString(virStorageSourceGetActualType(disk->src)));
+ return -1;
+
case VIR_STORAGE_TYPE_NETWORK:
case VIR_STORAGE_TYPE_VOLUME:
- case VIR_STORAGE_TYPE_VHOST_USER:
case VIR_STORAGE_TYPE_LAST:
case VIR_STORAGE_TYPE_NONE:
break;
--
2.43.0
7:49 a.m.
New subject: [PATCH 7/8] qemuMigrationDstPrepareStorage: Move assumption that 'network' disks always exist
Move the assumption from the code pre-creating the storage to
qemuMigrationDstPrepareStorage where it's checked for other cases.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_migration.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 4c524fafe4..438aa4503e 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -338,10 +338,6 @@ qemuMigrationDstPrecreateDisk(virConnectPtr *conn,
break;
case VIR_STORAGE_TYPE_NETWORK:
- VIR_DEBUG("Skipping creation of network disk '%s'",
- disk->dst);
- return 0;
-
case VIR_STORAGE_TYPE_BLOCK:
case VIR_STORAGE_TYPE_DIR:
case VIR_STORAGE_TYPE_NVME:
@@ -492,6 +488,12 @@ qemuMigrationDstPrepareStorage(virDomainObj *vm,
exists = virFileExists(disk->src->vdpadev);
break;
+ case VIR_STORAGE_TYPE_NETWORK:
+ /* For network disks we always assume they exist as the storage drivec
+ * can't create them */
+ exists = true;
+ break;
+
case VIR_STORAGE_TYPE_VHOST_USER:
case VIR_STORAGE_TYPE_DIR:
virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
@@ -499,7 +501,6 @@ qemuMigrationDstPrepareStorage(virDomainObj *vm,
virStorageTypeToString(virStorageSourceGetActualType(disk->src)));
return -1;
- case VIR_STORAGE_TYPE_NETWORK:
case VIR_STORAGE_TYPE_VOLUME:
case VIR_STORAGE_TYPE_LAST:
case VIR_STORAGE_TYPE_NONE:
--
2.43.0
7:49 a.m.
New subject: [PATCH 8/8] qemuMigrationDstPrepareStorage: Annotate that existance of 'volume' disks is checked elswhere
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_migration.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 438aa4503e..1faab5dd23 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -502,6 +502,9 @@ qemuMigrationDstPrepareStorage(virDomainObj *vm,
return -1;
case VIR_STORAGE_TYPE_VOLUME:
+ /* Existance of 'volume' type disks are handled when pre-creating
them */
+ break;
+
case VIR_STORAGE_TYPE_LAST:
case VIR_STORAGE_TYPE_NONE:
break;
--
2.43.0
3:53 a.m.
New subject: [PATCH 0/8] qemuMigrationDstPrepareStorage: Refactor and fix for
VDPA storage
On 2/19/24 14:49, Peter Krempa wrote:
Peter Krempa (8):
qemuMigrationDstPrepareStorage: Use 'switch' statement to include all
storage types
qemuMigrationDstPrepareStorage: Properly consider path for 'vdpa'
devices
qemuMigrationDstPrecreateDisk: Refactor cleanup
qemuMigrationDstPrepareStorage: Move block device specific logic
qemuMigrationDstPrepareStorage: Rework storage existence check
qemuMigrationDstPrepareStorage: Reject migration into 'dir' and
'vhost-user' types
qemuMigrationDstPrepareStorage: Move assumption that 'network' disks
always exist
qemuMigrationDstPrepareStorage: Annotate that existance of 'volume'
disks is checked elswhere
src/qemu/qemu_migration.c | 117 +++++++++++++++++++++-----------------
1 file changed, 66 insertions(+), 51 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
Michal
310
days inactive
311
days old
9 comments
2 participants
participants (2)
-
Michal Prívozník -
Peter Krempa