[PATCH 0/8] migration fixes for non-shared storage migration
Peter Krempa (8): qemuMigrationDstPrecreateStorage: Improve error messages qemuMigrationDstPrecreateStorage: Refactor cleanup qemuMigrationDstPrecreateStorage: Fix and clarify logic qemu: migration: Improve handling of VIR_MIGRATE_PARAM_DEST_XML with VIR_MIGRATE_PERSIST_DEST qemuDomainStorageOpenStat: Remove unused 'driver' argument and untangle callers qemu: migration: Rename qemuMigrationDstPrecreateStorage to qemuMigrationDstPrepareStorage qemu: Move and export qemuDomainStorageUpdatePhysical and dependencies qemu: migration: Automatically fix non-shared-storage migration to bigger block devices src/qemu/qemu_domain.c | 114 +++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 16 ++++ src/qemu/qemu_driver.c | 149 ++++---------------------------------- src/qemu/qemu_migration.c | 146 +++++++++++++++++++++++++++---------- 4 files changed, 249 insertions(+), 176 deletions(-) -- 2.43.0
Change the error messages so that they can be used to identify the problematic disk or image. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index f9c34b72e8..83d986f6e6 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -457,7 +457,7 @@ qemuMigrationDstPrecreateStorage(virDomainObj *vm, if (!(disk = virDomainDiskByTarget(vm->def, nbd->disks[i].target))) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("unable to find disk by target: %1$s"), + _("unable to find disk target '%1$s' for non-shared-storage migration"), nbd->disks[i].target); goto cleanup; } @@ -476,8 +476,9 @@ qemuMigrationDstPrecreateStorage(virDomainObj *vm, } if (incremental) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("pre-creation of storage targets for incremental storage migration is not supported")); + 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); goto cleanup; } -- 2.43.0
Use automatic pointer freeing for 'conn' and remove the 'cleanup' label. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 83d986f6e6..15032512e2 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -440,9 +440,8 @@ qemuMigrationDstPrecreateStorage(virDomainObj *vm, const char **migrate_disks, bool incremental) { - int ret = -1; + g_autoptr(virConnect) conn = NULL; size_t i = 0; - virConnectPtr conn = NULL; if (!nbd || !nbd->ndisks) return 0; @@ -459,7 +458,7 @@ qemuMigrationDstPrecreateStorage(virDomainObj *vm, virReportError(VIR_ERR_INTERNAL_ERROR, _("unable to find disk target '%1$s' for non-shared-storage migration"), nbd->disks[i].target); - goto cleanup; + return -1; } if (disk->src->type == VIR_STORAGE_TYPE_NVME) { @@ -479,20 +478,16 @@ qemuMigrationDstPrecreateStorage(virDomainObj *vm, 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); - goto cleanup; + return -1; } VIR_DEBUG("Proceeding with disk source %s", NULLSTR(diskSrcPath)); - if (qemuMigrationDstPrecreateDisk(&conn, - disk, nbd->disks[i].capacity) < 0) - goto cleanup; + if (qemuMigrationDstPrecreateDisk(&conn, disk, nbd->disks[i].capacity) < 0) + return -1; } - ret = 0; - cleanup: - virObjectUnref(conn); - return ret; + return 0; } -- 2.43.0
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> --- 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 15032512e2..a6cfede49f 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.43.0
When a user provides a migration XML via the VIR_MIGRATE_PARAM_DEST_XML it's expected that they want to change ABI-compatible aspects of the XML such as the disk paths or similar. If the user requests persisting of the VM but does not provide an explicit persistent XML libvirt would take the persistent XML from the source of the migration as the persistent config. This usually involves the old paths to images. Doing this would result into failure to start the VM. It makes more sense to take the XML used for migration and use that as the base for persisting the config. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index a6cfede49f..0ae88eec03 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -4706,6 +4706,7 @@ qemuMigrationSrcCancel(virDomainObj *vm, static int qemuMigrationSrcRun(virQEMUDriver *driver, virDomainObj *vm, + const char *xmlin, const char *persist_xml, const char *cookiein, int cookieinlen, @@ -4779,6 +4780,15 @@ qemuMigrationSrcRun(virQEMUDriver *driver, persist_xml, NULL, NULL))) goto error; + } else if (xmlin) { + /* if input XML is provided, use that one as template for the + * persistent XML. Otherwise user's changes will be thrown away. + */ + if (!(persistDef = qemuMigrationAnyPrepareDef(driver, + priv->qemuCaps, + xmlin, + NULL, NULL))) + goto error; } else { virDomainDef *def = vm->newDef ? vm->newDef : vm->def; if (!(persistDef = qemuDomainDefCopy(driver, priv->qemuCaps, def, @@ -5117,6 +5127,7 @@ qemuMigrationSrcResume(virDomainObj *vm, static int qemuMigrationSrcPerformNative(virQEMUDriver *driver, virDomainObj *vm, + const char *xmlin, const char *persist_xml, const char *uri, const char *cookiein, @@ -5202,7 +5213,7 @@ qemuMigrationSrcPerformNative(virQEMUDriver *driver, ret = qemuMigrationSrcResume(vm, migParams, cookiein, cookieinlen, cookieout, cookieoutlen, &spec, flags); } else { - ret = qemuMigrationSrcRun(driver, vm, persist_xml, cookiein, cookieinlen, + ret = qemuMigrationSrcRun(driver, vm, xmlin, persist_xml, cookiein, cookieinlen, cookieout, cookieoutlen, flags, resource, &spec, dconn, graphicsuri, nmigrate_disks, migrate_disks, @@ -5220,6 +5231,7 @@ static int qemuMigrationSrcPerformTunnel(virQEMUDriver *driver, virDomainObj *vm, virStreamPtr st, + const char *xmlin, const char *persist_xml, const char *cookiein, int cookieinlen, @@ -5266,7 +5278,7 @@ qemuMigrationSrcPerformTunnel(virQEMUDriver *driver, goto cleanup; } - ret = qemuMigrationSrcRun(driver, vm, persist_xml, cookiein, cookieinlen, + ret = qemuMigrationSrcRun(driver, vm, xmlin, persist_xml, cookiein, cookieinlen, cookieout, cookieoutlen, flags, resource, &spec, dconn, graphicsuri, nmigrate_disks, migrate_disks, migParams, NULL); @@ -5305,7 +5317,7 @@ qemuMigrationSrcPerformResume(virQEMUDriver *driver, virCloseCallbacksDomainRemove(vm, NULL, qemuMigrationAnyConnectionClosed); qemuDomainCleanupRemove(vm, qemuProcessCleanupMigrationJob); - ret = qemuMigrationSrcPerformNative(driver, vm, NULL, uri, + ret = qemuMigrationSrcPerformNative(driver, vm, NULL, NULL, uri, cookiein, cookieinlen, cookieout, cookieoutlen, flags, 0, NULL, NULL, 0, NULL, migParams, NULL); @@ -5407,12 +5419,12 @@ qemuMigrationSrcPerformPeer2Peer2(virQEMUDriver *driver, VIR_DEBUG("Perform %p", sconn); ignore_value(qemuMigrationJobStartPhase(vm, QEMU_MIGRATION_PHASE_PERFORM2)); if (flags & VIR_MIGRATE_TUNNELLED) - ret = qemuMigrationSrcPerformTunnel(driver, vm, st, NULL, + ret = qemuMigrationSrcPerformTunnel(driver, vm, st, NULL, NULL, NULL, 0, NULL, NULL, flags, resource, dconn, NULL, 0, NULL, migParams); else - ret = qemuMigrationSrcPerformNative(driver, vm, NULL, uri_out, + ret = qemuMigrationSrcPerformNative(driver, vm, NULL, NULL, uri_out, cookie, cookielen, NULL, NULL, /* No out cookie with v2 migration */ flags, resource, dconn, NULL, 0, NULL, @@ -5668,14 +5680,14 @@ qemuMigrationSrcPerformPeer2Peer3(virQEMUDriver *driver, } else { ignore_value(qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_PERFORM3)); if (flags & VIR_MIGRATE_TUNNELLED) { - ret = qemuMigrationSrcPerformTunnel(driver, vm, st, persist_xml, + ret = qemuMigrationSrcPerformTunnel(driver, vm, st, xmlin, persist_xml, cookiein, cookieinlen, &cookieout, &cookieoutlen, flags, bandwidth, dconn, graphicsuri, nmigrate_disks, migrate_disks, migParams); } else { - ret = qemuMigrationSrcPerformNative(driver, vm, persist_xml, uri, + ret = qemuMigrationSrcPerformNative(driver, vm, xmlin, persist_xml, uri, cookiein, cookieinlen, &cookieout, &cookieoutlen, flags, bandwidth, dconn, graphicsuri, @@ -6075,7 +6087,7 @@ qemuMigrationSrcPerformJob(virQEMUDriver *driver, if (qemuMigrationJobStartPhase(vm, QEMU_MIGRATION_PHASE_PERFORM2) < 0) goto endjob; - ret = qemuMigrationSrcPerformNative(driver, vm, persist_xml, uri, cookiein, cookieinlen, + ret = qemuMigrationSrcPerformNative(driver, vm, xmlin, persist_xml, uri, cookiein, cookieinlen, cookieout, cookieoutlen, flags, resource, NULL, NULL, 0, NULL, migParams, nbdURI); @@ -6140,6 +6152,7 @@ static int qemuMigrationSrcPerformPhase(virQEMUDriver *driver, virConnectPtr conn, virDomainObj *vm, + const char *xmlin, const char *persist_xml, const char *uri, const char *graphicsuri, @@ -6177,7 +6190,7 @@ qemuMigrationSrcPerformPhase(virQEMUDriver *driver, virCloseCallbacksDomainRemove(vm, NULL, qemuMigrationAnyConnectionClosed); - if (qemuMigrationSrcPerformNative(driver, vm, persist_xml, uri, cookiein, cookieinlen, + if (qemuMigrationSrcPerformNative(driver, vm, xmlin, persist_xml, uri, cookiein, cookieinlen, cookieout, cookieoutlen, flags, resource, NULL, graphicsuri, nmigrate_disks, migrate_disks, migParams, nbdURI) < 0) @@ -6276,7 +6289,7 @@ qemuMigrationSrcPerform(virQEMUDriver *driver, } if (v3proto) { - return qemuMigrationSrcPerformPhase(driver, conn, vm, persist_xml, uri, + return qemuMigrationSrcPerformPhase(driver, conn, vm, xmlin, persist_xml, uri, graphicsuri, nmigrate_disks, migrate_disks, migParams, -- 2.43.0
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 41 ++++++++++++++++------------------------- 1 file changed, 16 insertions(+), 25 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 64afae6450..f4c2fbfb45 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10217,7 +10217,6 @@ qemuDomainMemoryPeek(virDomainPtr dom, /** - * @driver: qemu driver data * @cfg: driver configuration data * @vm: domain object * @src: storage source data @@ -10236,8 +10235,7 @@ qemuDomainMemoryPeek(virDomainPtr dom, * reported) or -1 otherwise (errors are reported). */ static int -qemuDomainStorageOpenStat(virQEMUDriver *driver G_GNUC_UNUSED, - virQEMUDriverConfig *cfg, +qemuDomainStorageOpenStat(virQEMUDriverConfig *cfg, virDomainObj *vm, virStorageSource *src, int *ret_fd, @@ -10296,7 +10294,6 @@ qemuDomainStorageCloseStat(virStorageSource *src, /** * qemuDomainStorageUpdatePhysical: - * @driver: qemu driver * @cfg: qemu driver configuration object * @vm: domain object * @src: storage source to update @@ -10308,8 +10305,7 @@ qemuDomainStorageCloseStat(virStorageSource *src, * reported but are reset (thus only logged)). */ static int -qemuDomainStorageUpdatePhysical(virQEMUDriver *driver, - virQEMUDriverConfig *cfg, +qemuDomainStorageUpdatePhysical(virQEMUDriverConfig *cfg, virDomainObj *vm, virStorageSource *src) { @@ -10320,7 +10316,7 @@ qemuDomainStorageUpdatePhysical(virQEMUDriver *driver, if (virStorageSourceIsEmpty(src)) return 0; - if ((ret = qemuDomainStorageOpenStat(driver, cfg, vm, src, &fd, &sb, true)) <= 0) { + if ((ret = qemuDomainStorageOpenStat(cfg, vm, src, &fd, &sb, true)) <= 0) { if (ret < 0) virResetLastError(); return -1; @@ -10335,7 +10331,6 @@ qemuDomainStorageUpdatePhysical(virQEMUDriver *driver, /** - * @driver: qemu driver data * @cfg: driver configuration data * @vm: domain object * @src: storage source data @@ -10366,8 +10361,7 @@ qemuDomainStorageUpdatePhysical(virQEMUDriver *driver, * are reported). */ static int -qemuStorageLimitsRefresh(virQEMUDriver *driver, - virQEMUDriverConfig *cfg, +qemuStorageLimitsRefresh(virQEMUDriverConfig *cfg, virDomainObj *vm, virStorageSource *src, bool skipInaccessible) @@ -10379,7 +10373,7 @@ qemuStorageLimitsRefresh(virQEMUDriver *driver, g_autofree char *buf = NULL; ssize_t len; - if ((rc = qemuDomainStorageOpenStat(driver, cfg, vm, src, &fd, &sb, + if ((rc = qemuDomainStorageOpenStat(cfg, vm, src, &fd, &sb, skipInaccessible)) <= 0) return rc; @@ -10470,7 +10464,7 @@ qemuDomainGetBlockInfo(virDomainPtr dom, /* for inactive domains we have to peek into the files */ if (!virDomainObjIsActive(vm)) { - if ((qemuStorageLimitsRefresh(driver, cfg, vm, disk->src, false)) < 0) + if ((qemuStorageLimitsRefresh(cfg, vm, disk->src, false)) < 0) goto endjob; info->capacity = disk->src->capacity; @@ -10510,7 +10504,7 @@ qemuDomainGetBlockInfo(virDomainPtr dom, if (info->allocation == 0) info->allocation = entry->physical; - if (qemuDomainStorageUpdatePhysical(driver, cfg, vm, disk->src) == 0) { + if (qemuDomainStorageUpdatePhysical(cfg, vm, disk->src) == 0) { info->physical = disk->src->physical; } else { info->physical = entry->physical; @@ -17220,8 +17214,7 @@ qemuDomainGetStatsInterface(virQEMUDriver *driver G_GNUC_UNUSED, /* refresh information by opening images on the disk */ static int -qemuDomainGetStatsOneBlockFallback(virQEMUDriver *driver, - virQEMUDriverConfig *cfg, +qemuDomainGetStatsOneBlockFallback(virQEMUDriverConfig *cfg, virDomainObj *dom, virTypedParamList *params, virStorageSource *src, @@ -17233,7 +17226,7 @@ qemuDomainGetStatsOneBlockFallback(virQEMUDriver *driver, if (virStorageSourceIsFD(src)) return 0; - if (qemuStorageLimitsRefresh(driver, cfg, dom, src, true) <= 0) { + if (qemuStorageLimitsRefresh(cfg, dom, src, true) <= 0) { virResetLastError(); return 0; } @@ -17252,8 +17245,7 @@ qemuDomainGetStatsOneBlockFallback(virQEMUDriver *driver, static int -qemuDomainGetStatsOneBlock(virQEMUDriver *driver, - virQEMUDriverConfig *cfg, +qemuDomainGetStatsOneBlock(virQEMUDriverConfig *cfg, virDomainObj *dom, virTypedParamList *params, const char *entryname, @@ -17266,7 +17258,7 @@ qemuDomainGetStatsOneBlock(virQEMUDriver *driver, /* the VM is offline so we have to go and load the stast from the disk by * ourselves */ if (!virDomainObjIsActive(dom)) { - return qemuDomainGetStatsOneBlockFallback(driver, cfg, dom, params, + return qemuDomainGetStatsOneBlockFallback(cfg, dom, params, src, block_idx); } @@ -17284,7 +17276,7 @@ qemuDomainGetStatsOneBlock(virQEMUDriver *driver, if (entry->physical) { virTypedParamListAddULLong(params, entry->physical, "block.%zu.physical", block_idx); } else { - if (qemuDomainStorageUpdatePhysical(driver, cfg, dom, src) == 0) { + if (qemuDomainStorageUpdatePhysical(cfg, dom, src) == 0) { virTypedParamListAddULLong(params, src->physical, "block.%zu.physical", block_idx); } } @@ -17362,7 +17354,6 @@ qemuDomainGetStatsBlockExportDisk(virDomainDiskDef *disk, virTypedParamList *params, size_t *recordnr, bool visitBacking, - virQEMUDriver *driver, virQEMUDriverConfig *cfg, virDomainObj *dom) @@ -17435,7 +17426,7 @@ qemuDomainGetStatsBlockExportDisk(virDomainDiskDef *disk, return -1; } - if (qemuDomainGetStatsOneBlock(driver, cfg, dom, params, + if (qemuDomainGetStatsOneBlock(cfg, dom, params, backendalias, n, *recordnr, stats) < 0) return -1; @@ -17462,7 +17453,7 @@ qemuDomainGetStatsBlockExportDisk(virDomainDiskDef *disk, if (qemuDomainGetStatsBlockExportHeader(disk, disk->mirror, *recordnr, params) < 0) return -1; - if (qemuDomainGetStatsOneBlock(driver, cfg, dom, params, + if (qemuDomainGetStatsOneBlock(cfg, dom, params, qemuBlockStorageSourceGetEffectiveNodename(disk->mirror), disk->mirror, *recordnr, @@ -17491,7 +17482,7 @@ qemuDomainGetStatsBlockExportDisk(virDomainDiskDef *disk, *recordnr, params) < 0) return -1; - if (qemuDomainGetStatsOneBlock(driver, cfg, dom, params, + if (qemuDomainGetStatsOneBlock(cfg, dom, params, qemuBlockStorageSourceGetEffectiveNodename(backupdisk->store), backupdisk->store, *recordnr, @@ -17548,7 +17539,7 @@ qemuDomainGetStatsBlock(virQEMUDriver *driver, for (i = 0; i < dom->def->ndisks; i++) { if (qemuDomainGetStatsBlockExportDisk(dom->def->disks[i], stats, blockparams, &visited, - visitBacking, driver, cfg, dom) < 0) + visitBacking, cfg, dom) < 0) return -1; } -- 2.43.0
The function will be used to setup storage for non-shared-storage migration, not just precreate images. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 0ae88eec03..2d91fc3e8b 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -434,11 +434,11 @@ qemuMigrationHasAnyStorageMigrationDisks(virDomainDef *def, static int -qemuMigrationDstPrecreateStorage(virDomainObj *vm, - qemuMigrationCookieNBD *nbd, - size_t nmigrate_disks, - const char **migrate_disks, - bool incremental) +qemuMigrationDstPrepareStorage(virDomainObj *vm, + qemuMigrationCookieNBD *nbd, + size_t nmigrate_disks, + const char **migrate_disks, + bool incremental) { g_autoptr(virConnect) conn = NULL; size_t i = 0; @@ -3097,9 +3097,9 @@ qemuMigrationDstPrepareActive(virQEMUDriver *driver, goto error; } - if (qemuMigrationDstPrecreateStorage(vm, mig->nbd, - nmigrate_disks, migrate_disks, - !!(flags & VIR_MIGRATE_NON_SHARED_INC)) < 0) + if (qemuMigrationDstPrepareStorage(vm, mig->nbd, + nmigrate_disks, migrate_disks, + !!(flags & VIR_MIGRATE_NON_SHARED_INC)) < 0) goto error; if (tunnel && -- 2.43.0
Move qemuDomainStorageUpdatePhysical, qemuDomainStorageOpenStat, qemuDomainStorageCloseStat to qemu_domain.c and export them. They'll be reused in the migration code. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 114 +++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 15 ++++++ src/qemu/qemu_driver.c | 114 ----------------------------------------- 3 files changed, 129 insertions(+), 114 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 953808fcfe..734d63f8a4 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -12675,3 +12675,117 @@ qemuDomainNumatuneMaybeFormatNodesetUnion(virDomainObj *vm, if (nodeset) *nodeset = g_steal_pointer(&unionMask); } + + +/** + * @cfg: driver configuration data + * @vm: domain object + * @src: storage source data + * @ret_fd: pointer to return open'd file descriptor + * @ret_sb: pointer to return stat buffer (local or remote) + * @skipInaccessible: Don't report error if files are not accessible + * + * For local storage, open the file using qemuDomainOpenFile and then use + * fstat() to grab the stat struct data for the caller. + * + * For remote storage, attempt to access the file and grab the stat + * struct data if the remote connection supports it. + * + * Returns 1 if @src was successfully opened (@ret_fd and @ret_sb is populated), + * 0 if @src can't be opened and @skipInaccessible is true (no errors are + * reported) or -1 otherwise (errors are reported). + */ +int +qemuDomainStorageOpenStat(virQEMUDriverConfig *cfg, + virDomainObj *vm, + virStorageSource *src, + int *ret_fd, + struct stat *ret_sb, + bool skipInaccessible) +{ + if (virStorageSourceIsLocalStorage(src)) { + if (skipInaccessible && !virFileExists(src->path)) + return 0; + + if ((*ret_fd = qemuDomainOpenFile(cfg, vm->def, src->path, O_RDONLY, + NULL)) < 0) + return -1; + + if (fstat(*ret_fd, ret_sb) < 0) { + virReportSystemError(errno, _("cannot stat file '%1$s'"), src->path); + VIR_FORCE_CLOSE(*ret_fd); + return -1; + } + } else { + if (skipInaccessible && virStorageSourceSupportsBackingChainTraversal(src) <= 0) + return 0; + + if (virStorageSourceInitAs(src, cfg->user, cfg->group) < 0) + return -1; + + if (virStorageSourceStat(src, ret_sb) < 0) { + virStorageSourceDeinit(src); + virReportSystemError(errno, _("failed to stat remote file '%1$s'"), + NULLSTR(src->path)); + return -1; + } + } + + return 1; +} + + +/** + * @src: storage source data + * @fd: file descriptor to close for local + * + * If local, then just close the file descriptor. + * else remote, then tear down the storage driver backend connection. + */ +void +qemuDomainStorageCloseStat(virStorageSource *src, + int *fd) +{ + if (virStorageSourceIsLocalStorage(src)) + VIR_FORCE_CLOSE(*fd); + else + virStorageSourceDeinit(src); +} + + +/** + * qemuDomainStorageUpdatePhysical: + * @cfg: qemu driver configuration object + * @vm: domain object + * @src: storage source to update + * + * Update the physical size of the disk by reading the actual size of the image + * on disk. + * + * Returns 0 on successful update and -1 otherwise (some uncommon errors may be + * reported but are reset (thus only logged)). + */ +int +qemuDomainStorageUpdatePhysical(virQEMUDriverConfig *cfg, + virDomainObj *vm, + virStorageSource *src) +{ + int ret; + int fd = -1; + struct stat sb; + + if (virStorageSourceIsEmpty(src)) + return 0; + + if ((ret = qemuDomainStorageOpenStat(cfg, vm, src, &fd, &sb, true)) <= 0) { + if (ret < 0) + virResetLastError(); + return -1; + } + + ret = virStorageSourceUpdatePhysicalSize(src, fd, &sb); + + qemuDomainStorageCloseStat(src, &fd); + + return ret; +} diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 1e56e50672..b8331a32d3 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -1129,3 +1129,18 @@ void qemuDomainNumatuneMaybeFormatNodesetUnion(virDomainObj *vm, virBitmap **nodeset, char **nodesetStr); + +int +qemuDomainStorageOpenStat(virQEMUDriverConfig *cfg, + virDomainObj *vm, + virStorageSource *src, + int *ret_fd, + struct stat *ret_sb, + bool skipInaccessible); +void +qemuDomainStorageCloseStat(virStorageSource *src, + int *fd); +int +qemuDomainStorageUpdatePhysical(virQEMUDriverConfig *cfg, + virDomainObj *vm, + virStorageSource *src); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f4c2fbfb45..9331369d4d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10216,120 +10216,6 @@ qemuDomainMemoryPeek(virDomainPtr dom, } -/** - * @cfg: driver configuration data - * @vm: domain object - * @src: storage source data - * @ret_fd: pointer to return open'd file descriptor - * @ret_sb: pointer to return stat buffer (local or remote) - * @skipInaccessible: Don't report error if files are not accessible - * - * For local storage, open the file using qemuDomainOpenFile and then use - * fstat() to grab the stat struct data for the caller. - * - * For remote storage, attempt to access the file and grab the stat - * struct data if the remote connection supports it. - * - * Returns 1 if @src was successfully opened (@ret_fd and @ret_sb is populated), - * 0 if @src can't be opened and @skipInaccessible is true (no errors are - * reported) or -1 otherwise (errors are reported). - */ -static int -qemuDomainStorageOpenStat(virQEMUDriverConfig *cfg, - virDomainObj *vm, - virStorageSource *src, - int *ret_fd, - struct stat *ret_sb, - bool skipInaccessible) -{ - if (virStorageSourceIsLocalStorage(src)) { - if (skipInaccessible && !virFileExists(src->path)) - return 0; - - if ((*ret_fd = qemuDomainOpenFile(cfg, vm->def, src->path, O_RDONLY, - NULL)) < 0) - return -1; - - if (fstat(*ret_fd, ret_sb) < 0) { - virReportSystemError(errno, _("cannot stat file '%1$s'"), src->path); - VIR_FORCE_CLOSE(*ret_fd); - return -1; - } - } else { - if (skipInaccessible && virStorageSourceSupportsBackingChainTraversal(src) <= 0) - return 0; - - if (virStorageSourceInitAs(src, cfg->user, cfg->group) < 0) - return -1; - - if (virStorageSourceStat(src, ret_sb) < 0) { - virStorageSourceDeinit(src); - virReportSystemError(errno, _("failed to stat remote file '%1$s'"), - NULLSTR(src->path)); - return -1; - } - } - - return 1; -} - - -/** - * @src: storage source data - * @fd: file descriptor to close for local - * - * If local, then just close the file descriptor. - * else remote, then tear down the storage driver backend connection. - */ -static void -qemuDomainStorageCloseStat(virStorageSource *src, - int *fd) -{ - if (virStorageSourceIsLocalStorage(src)) - VIR_FORCE_CLOSE(*fd); - else - virStorageSourceDeinit(src); -} - - -/** - * qemuDomainStorageUpdatePhysical: - * @cfg: qemu driver configuration object - * @vm: domain object - * @src: storage source to update - * - * Update the physical size of the disk by reading the actual size of the image - * on disk. - * - * Returns 0 on successful update and -1 otherwise (some uncommon errors may be - * reported but are reset (thus only logged)). - */ -static int -qemuDomainStorageUpdatePhysical(virQEMUDriverConfig *cfg, - virDomainObj *vm, - virStorageSource *src) -{ - int ret; - int fd = -1; - struct stat sb; - - if (virStorageSourceIsEmpty(src)) - return 0; - - if ((ret = qemuDomainStorageOpenStat(cfg, vm, src, &fd, &sb, true)) <= 0) { - if (ret < 0) - virResetLastError(); - return -1; - } - - ret = virStorageSourceUpdatePhysicalSize(src, fd, &sb); - - qemuDomainStorageCloseStat(src, &fd); - - return ret; -} - - /** * @cfg: driver configuration data * @vm: domain object -- 2.43.0
QEMU's blockdev-mirror job doesn't allow copy into a destination which isn't exactly the same size as source. This is a problem for non-shared-storage migration when migrating into a raw block device, as there it's very hard to ensure that the destination size will match the source size. Rather than failing the migration, we can add a storage slice in such case automatically and thus make the migration pass. To do this we need to probe the size of the block device on the destination and if it differs form the size detected on the source we'll install the 'slice'. An additional handling is required when persisting the VM as we want to propagate the slice even there to ensure that the device sizes won't change. Resolves: https://issues.redhat.com/browse/RHEL-4607 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.h | 1 + src/qemu/qemu_migration.c | 50 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index b8331a32d3..fa566dded6 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -275,6 +275,7 @@ struct _qemuDomainDiskPrivate { bool migrating; /* the disk is being migrated */ virStorageSource *migrSource; /* disk source object used for NBD migration */ + bool migrationslice; /* storage slice was added for migration purposes */ /* information about the device */ bool tray; /* device has tray */ diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 2d91fc3e8b..a0f131aa61 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -473,6 +473,25 @@ 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)) { @@ -6381,6 +6400,37 @@ qemuMigrationDstPersist(virQEMUDriver *driver, priv->qemuCaps))) goto error; + if (vm->def) { + size_t i; + + for (i = 0; i < vm->def->ndisks; i++) { + virDomainDiskDef *disk = vm->def->disks[i]; + qemuDomainDiskPrivate *diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); + virDomainDiskDef *persistDisk; + + /* transfer over any slices added for migration */ + if (!disk->src->sliceStorage) + continue; + + if (!diskPriv->migrationslice) + continue; + + diskPriv->migrationslice = false; + + if (!(persistDisk = virDomainDiskByTarget(vm->newDef, disk->dst))) + continue; + + if (persistDisk->src->sliceStorage) + continue; + + if (!virStorageSourceIsSameLocation(disk->src, persistDisk->src)) + continue; + + persistDisk->src->sliceStorage = g_new0(virStorageSourceSlice, 1); + persistDisk->src->sliceStorage->size = disk->src->sliceStorage->size; + } + } + if (!oldPersist) { eventDetail = VIR_DOMAIN_EVENT_DEFINED_ADDED; -- 2.43.0
On a Thursday in 2023, Peter Krempa wrote:
Peter Krempa (8): qemuMigrationDstPrecreateStorage: Improve error messages qemuMigrationDstPrecreateStorage: Refactor cleanup qemuMigrationDstPrecreateStorage: Fix and clarify logic qemu: migration: Improve handling of VIR_MIGRATE_PARAM_DEST_XML with VIR_MIGRATE_PERSIST_DEST qemuDomainStorageOpenStat: Remove unused 'driver' argument and untangle callers qemu: migration: Rename qemuMigrationDstPrecreateStorage to qemuMigrationDstPrepareStorage qemu: Move and export qemuDomainStorageUpdatePhysical and dependencies qemu: migration: Automatically fix non-shared-storage migration to bigger block devices
src/qemu/qemu_domain.c | 114 +++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 16 ++++ src/qemu/qemu_driver.c | 149 ++++---------------------------------- src/qemu/qemu_migration.c | 146 +++++++++++++++++++++++++++---------- 4 files changed, 249 insertions(+), 176 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Peter Krempa