Extract the hostdev check from qemuMigrationIsAllowed into a separate function since that is the only part that needs to be done in the v2 migration protocol prepare phase on the destination. All other checks were added when the v3 protocol existed so that they don't need to be extracted. This change will allow to dro the @def argument for qemuMigrationIsAllowed and further simplify the function. --- src/qemu/qemu_migration.c | 44 +++++++++++++++++++++++++++++++------------- 1 file changed, 31 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index fa81a16..8f28bd5 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2156,6 +2156,34 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver, } +/** + * qemuMigrationIsAllowedHostdev: + * @def: domain definition + * + * Checks that @def does not contain any host devices unsupported accross + * migrations. Returns true if the vm is allowed to migrate. + */ +static bool +qemuMigrationIsAllowedHostdev(const virDomainDef *def) +{ + size_t i; + + /* Migration with USB host devices is allowed, all other devices are + * forbidden. */ + for (i = 0; i < def->nhostdevs; i++) { + virDomainHostdevDefPtr hostdev = def->hostdevs[i]; + if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || + hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("domain has assigned non-USB host devices")); + return false; + } + } + + return true; +} + + /* Validate whether the domain is safe to migrate. If vm is NULL, * then this is being run in the v2 Prepare stage on the destination * (where we only have the target xml); if vm is provided, then this @@ -2213,17 +2241,8 @@ qemuMigrationIsAllowed(virQEMUDriverPtr driver, virDomainObjPtr vm, def = vm->def; } - /* Migration with USB host devices is allowed, all other devices are - * forbidden. */ - for (i = 0; i < def->nhostdevs; i++) { - virDomainHostdevDefPtr hostdev = def->hostdevs[i]; - if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || - hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("domain has assigned non-USB host devices")); - return false; - } - } + if (!qemuMigrationIsAllowedHostdev(def)) + return false; if (def->cpu && def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH) { for (i = 0; i < def->cpu->nfeatures; i++) { @@ -3195,7 +3214,6 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, unsigned int cookieFlags; virCapsPtr caps = NULL; char *migrateFrom = NULL; - bool abort_on_error = !!(flags & VIR_MIGRATE_ABORT_ON_ERROR); bool taint_hook = false; virNWFilterReadLockFilterUpdates(); @@ -3225,7 +3243,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup; - if (!qemuMigrationIsAllowed(driver, NULL, *def, true, abort_on_error)) + if (!qemuMigrationIsAllowedHostdev(*def)) goto cleanup; /* Let migration hook filter domain XML */ -- 2.4.5

Now that qemuMigrationIsAllowed is always called with @vm, we can drop the @def argument and simplify the control flow. Additionally the comment is invalid so drop it. --- src/qemu/qemu_driver.c | 8 ++--- src/qemu/qemu_migration.c | 87 +++++++++++++++++++++-------------------------- src/qemu/qemu_migration.h | 3 +- 3 files changed, 43 insertions(+), 55 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index aff1915..d3017d2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3182,7 +3182,7 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, virDomainPtr dom, if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup; - if (!qemuMigrationIsAllowed(driver, vm, vm->def, false, false)) + if (!qemuMigrationIsAllowed(driver, vm, false, false)) goto cleanup; if (qemuDomainObjBeginAsyncJob(driver, vm, QEMU_ASYNC_JOB_SAVE) < 0) @@ -3614,7 +3614,7 @@ doCoreDump(virQEMUDriverPtr driver, goto cleanup; } - if (!qemuMigrationIsAllowed(driver, vm, vm->def, false, false)) + if (!qemuMigrationIsAllowed(driver, vm, false, false)) goto cleanup; ret = qemuMigrationToFile(driver, vm, fd, 0, path, @@ -13688,7 +13688,7 @@ qemuDomainSnapshotCreateActiveInternal(virConnectPtr conn, bool resume = false; int ret = -1; - if (!qemuMigrationIsAllowed(driver, vm, vm->def, false, false)) + if (!qemuMigrationIsAllowed(driver, vm, false, false)) goto cleanup; if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { @@ -14503,7 +14503,7 @@ qemuDomainSnapshotCreateActiveExternal(virConnectPtr conn, /* do the memory snapshot if necessary */ if (memory) { /* check if migration is possible */ - if (!qemuMigrationIsAllowed(driver, vm, vm->def, false, false)) + if (!qemuMigrationIsAllowed(driver, vm, false, false)) goto cleanup; /* allow the migration job to be cancelled or the domain to be paused */ diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 8f28bd5..3279ab2 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2184,69 +2184,58 @@ qemuMigrationIsAllowedHostdev(const virDomainDef *def) } -/* Validate whether the domain is safe to migrate. If vm is NULL, - * then this is being run in the v2 Prepare stage on the destination - * (where we only have the target xml); if vm is provided, then this - * is being run in either v2 Perform or v3 Begin (where we also have - * access to all of the domain's metadata, such as whether it is - * marked autodestroy or has snapshots). While it would be nice to - * assume that checking on source is sufficient to prevent ever - * talking to the destination in the first place, we are stuck with - * the fact that older servers did not do checks on the source. */ bool -qemuMigrationIsAllowed(virQEMUDriverPtr driver, virDomainObjPtr vm, - virDomainDefPtr def, bool remote, bool abort_on_error) +qemuMigrationIsAllowed(virQEMUDriverPtr driver, + virDomainObjPtr vm, + bool remote, + bool abort_on_error) { int nsnapshots; int pauseReason; size_t i; - if (vm) { - if (qemuProcessAutoDestroyActive(driver, vm)) { + if (qemuProcessAutoDestroyActive(driver, vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is marked for auto destroy")); + return false; + } + + /* perform these checks only when migrating to remote hosts */ + if (remote) { + nsnapshots = virDomainSnapshotObjListNum(vm->snapshots, NULL, 0); + if (nsnapshots < 0) + return false; + + if (nsnapshots > 0) { virReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain is marked for auto destroy")); + _("cannot migrate domain with %d snapshots"), + nsnapshots); return false; } - /* perform these checks only when migrating to remote hosts */ - if (remote) { - nsnapshots = virDomainSnapshotObjListNum(vm->snapshots, NULL, 0); - if (nsnapshots < 0) - return false; - - if (nsnapshots > 0) { - virReportError(VIR_ERR_OPERATION_INVALID, - _("cannot migrate domain with %d snapshots"), - nsnapshots); - return false; - } - - /* cancel migration if disk I/O error is emitted while migrating */ - if (abort_on_error && - virDomainObjGetState(vm, &pauseReason) == VIR_DOMAIN_PAUSED && - pauseReason == VIR_DOMAIN_PAUSED_IOERROR) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("cannot migrate domain with I/O error")); - return false; - } - - } - - if (qemuDomainHasBlockjob(vm, false)) { + /* cancel migration if disk I/O error is emitted while migrating */ + if (abort_on_error && + virDomainObjGetState(vm, &pauseReason) == VIR_DOMAIN_PAUSED && + pauseReason == VIR_DOMAIN_PAUSED_IOERROR) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("domain has an active block job")); + _("cannot migrate domain with I/O error")); return false; } - def = vm->def; } - if (!qemuMigrationIsAllowedHostdev(def)) + if (qemuDomainHasBlockjob(vm, false)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("domain has an active block job")); + return false; + } + + if (!qemuMigrationIsAllowedHostdev(vm->def)) return false; - if (def->cpu && def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH) { - for (i = 0; i < def->cpu->nfeatures; i++) { - virCPUFeatureDefPtr feature = &def->cpu->features[i]; + if (vm->def->cpu && vm->def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH) { + for (i = 0; i < vm->def->cpu->nfeatures; i++) { + virCPUFeatureDefPtr feature = &vm->def->cpu->features[i]; if (feature->policy != VIR_CPU_FEATURE_REQUIRE) continue; @@ -2262,8 +2251,8 @@ qemuMigrationIsAllowed(virQEMUDriverPtr driver, virDomainObjPtr vm, } /* Verify that memory device config can be transferred reliably */ - for (i = 0; i < def->nmems; i++) { - virDomainMemoryDefPtr mem = def->mems[i]; + for (i = 0; i < vm->def->nmems; i++) { + virDomainMemoryDefPtr mem = vm->def->mems[i]; if (mem->model == VIR_DOMAIN_MEMORY_MODEL_DIMM && mem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM) { @@ -2980,7 +2969,7 @@ qemuMigrationBeginPhase(virQEMUDriverPtr driver, if (priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_OUT) qemuMigrationJobSetPhase(driver, vm, QEMU_MIGRATION_PHASE_BEGIN3); - if (!qemuMigrationIsAllowed(driver, vm, NULL, true, abort_on_error)) + if (!qemuMigrationIsAllowed(driver, vm, true, abort_on_error)) goto cleanup; if (!(flags & VIR_MIGRATE_UNSAFE) && @@ -5341,7 +5330,7 @@ qemuMigrationPerformJob(virQEMUDriverPtr driver, goto endjob; } - if (!qemuMigrationIsAllowed(driver, vm, NULL, true, abort_on_error)) + if (!qemuMigrationIsAllowed(driver, vm, true, abort_on_error)) goto endjob; if (!(flags & VIR_MIGRATE_UNSAFE) && diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index fa14274..54676df 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -174,8 +174,7 @@ int qemuMigrationConfirm(virConnectPtr conn, int cancelled); bool qemuMigrationIsAllowed(virQEMUDriverPtr driver, virDomainObjPtr vm, - virDomainDefPtr def, bool remote, - bool abort_on_error); + bool remote, bool abort_on_error); int qemuMigrationToFile(virQEMUDriverPtr driver, virDomainObjPtr vm, int fd, off_t offset, const char *path, -- 2.4.5

qemuMigrationIsAllowed would disallow offline migration if the VM contained host devices or memory modules. Since during offline migration we don't transfer any state we can safely migrate VMs with such configuration. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1265049 --- src/qemu/qemu_migration.c | 87 +++++++++++++++++++++++++---------------------- 1 file changed, 46 insertions(+), 41 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 851c9d0..658998c 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2208,12 +2208,6 @@ qemuMigrationIsAllowed(virQEMUDriverPtr driver, int pauseReason; size_t i; - if (qemuProcessAutoDestroyActive(driver, vm)) { - virReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain is marked for auto destroy")); - return false; - } - /* perform these checks only when migrating to remote hosts */ if (remote) { nsnapshots = virDomainSnapshotObjListNum(vm->snapshots, NULL, 0); @@ -2229,6 +2223,7 @@ qemuMigrationIsAllowed(virQEMUDriverPtr driver, /* cancel migration if disk I/O error is emitted while migrating */ if (flags & VIR_MIGRATE_ABORT_ON_ERROR && + !(flags & VIR_MIGRATE_OFFLINE) && virDomainObjGetState(vm, &pauseReason) == VIR_DOMAIN_PAUSED && pauseReason == VIR_DOMAIN_PAUSED_IOERROR) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", @@ -2238,46 +2233,56 @@ qemuMigrationIsAllowed(virQEMUDriverPtr driver, } - if (qemuDomainHasBlockjob(vm, false)) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("domain has an active block job")); - return false; - } - - if (!qemuMigrationIsAllowedHostdev(vm->def)) - return false; - - if (vm->def->cpu && vm->def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH) { - for (i = 0; i < vm->def->cpu->nfeatures; i++) { - virCPUFeatureDefPtr feature = &vm->def->cpu->features[i]; - - if (feature->policy != VIR_CPU_FEATURE_REQUIRE) - continue; - - /* QEMU blocks migration and save with invariant TSC enabled */ - if (STREQ(feature->name, "invtsc")) { - virReportError(VIR_ERR_OPERATION_INVALID, - _("domain has CPU feature: %s"), - feature->name); + /* following checks don't make sense for offline migration */ + if (!(flags & VIR_MIGRATE_OFFLINE)) { + if (qemuProcessAutoDestroyActive(driver, vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is marked for auto destroy")); + return false; + } + + + if (qemuDomainHasBlockjob(vm, false)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("domain has an active block job")); + return false; + } + + if (!qemuMigrationIsAllowedHostdev(vm->def)) + return false; + + if (vm->def->cpu && vm->def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH) { + for (i = 0; i < vm->def->cpu->nfeatures; i++) { + virCPUFeatureDefPtr feature = &vm->def->cpu->features[i]; + + if (feature->policy != VIR_CPU_FEATURE_REQUIRE) + continue; + + /* QEMU blocks migration and save with invariant TSC enabled */ + if (STREQ(feature->name, "invtsc")) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("domain has CPU feature: %s"), + feature->name); + return false; + } + } + } + + /* Verify that memory device config can be transferred reliably */ + for (i = 0; i < vm->def->nmems; i++) { + virDomainMemoryDefPtr mem = vm->def->mems[i]; + + if (mem->model == VIR_DOMAIN_MEMORY_MODEL_DIMM && + mem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("domain's dimm info lacks slot ID " + "or base address")); + return false; } } } - /* Verify that memory device config can be transferred reliably */ - for (i = 0; i < vm->def->nmems; i++) { - virDomainMemoryDefPtr mem = vm->def->mems[i]; - - if (mem->model == VIR_DOMAIN_MEMORY_MODEL_DIMM && - mem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("domain's dimm info lacks slot ID " - "or base address")); - - return false; - } - } - return true; } -- 2.4.5