Commit fba6bc4 introduced the non-migratable invtsc feature, breaking save/migration with host-model and host-passthrough. On hosts with this feature present it was automatically included in the CPU definition, regardless of QEMU support. Commit de0aeaf stopped including it by default for host-model, but failed to fix host-passthrough. This commit ignores checking of CPU features with host-passthrough, since we don't pass them to QEMU (only -cpu host is passed), allowing domains using host-passthrough that were saved with the broken version of libvirtd to be restored. https://bugzilla.redhat.com/show_bug.cgi?id=1147584 --- src/qemu/qemu_migration.c | 22 ++++++++++++---------- src/qemu/qemu_process.c | 5 +++++ 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 6b38592..284cd5a 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1714,18 +1714,20 @@ qemuMigrationIsAllowed(virQEMUDriverPtr driver, virDomainObjPtr vm, return false; } - for (i = 0; def->cpu && i < def->cpu->nfeatures; i++) { - virCPUFeatureDefPtr feature = &def->cpu->features[i]; + 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 (feature->policy != VIR_CPU_FEATURE_REQUIRE) - continue; + 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; + /* 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; + } } } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 1b8931e..ca78aac 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3787,6 +3787,11 @@ qemuProcessVerifyGuestCPU(virQEMUDriverPtr driver, bool ret = false; size_t i; + /* no features are passed to QEMU with -cpu host + * so it makes no sense to verify them */ + if (def->cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH) + return true; + switch (arch) { case VIR_ARCH_I686: case VIR_ARCH_X86_64: -- 1.8.5.5

On 09/30/2014 01:09 AM, John Ferlan wrote: The features aren't really allowed. We just fill the cpu def with host-passthrough with what we think QEMU will do to inform the user, but the features aren't passed to QEMU. So this would be the side effect. This patch fixes the already saved files where we added invtsc even though QEMU didn't and I think the second one should be enough to fix it for new saves. Thanks for catching that. Jan

On 09/29/2014 10:27 AM, Ján Tomko wrote: Could this be expanded a bit - so one doesn't have to chase into the bz in order to understand what is/was being fixed? So if I understand correctly the above loop removes not migrate-able features - from the passed in migrated cpu and the loop below is updating features for anything left. So given my question from patch 1/2 - it seems if features weren't allowed for host-passthrough, well then neither of these would be necessary. In any case - even if we added something that removed features, the incoming, saved/restore, etc. guest with host-passthrough could still have it so this would be necessary - which I guess means it's an ACK (typing and thinking). John