On Fri, May 16, 2014 at 11:05:28AM +0200, Andreas Färber wrote:
> Am 15.05.2014 22:22, schrieb Andreas Färber:
> > Am 30.04.2014 18:48, schrieb Eduardo Habkost:
> >> From: Marcelo Tosatti <mtosa...@redhat.com>
> >>
> >> Invariant TSC documentation mentions that "invariant TSC will run at a
> >> constant rate in all ACPI P-, C-. and T-states".
> >>
> >> This is not the case if migration to a host with different TSC frequency
> >> is allowed, or if savevm is performed. So block migration/savevm.
> >>
> >> Also do not expose invariant tsc flag by default.
> >>
> >> Cc: Juan Quintela <quint...@redhat.com>
> >> Signed-off-by: Marcelo Tosatti <mtosa...@redhat.com>
> >> Reviewed-by: Eduardo Habkost <ehabk...@redhat.com>
> >> Signed-off-by: Eduardo Habkost <ehabk...@redhat.com>
> >> ---
> >> target-i386/cpu-qom.h | 2 +-
> >> target-i386/kvm.c | 13 +++++++++++++
> >> target-i386/machine.c | 2 +-
> >> 3 files changed, 15 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
> >> index 016f90d..473d803 100644
> >> --- a/target-i386/cpu-qom.h
> >> +++ b/target-i386/cpu-qom.h
> >> @@ -121,7 +121,7 @@ static inline X86CPU *x86_env_get_cpu(CPUX86State *env)
> >> #define ENV_OFFSET offsetof(X86CPU, env)
> >>
> >> #ifndef CONFIG_USER_ONLY
> >> -extern const struct VMStateDescription vmstate_x86_cpu;
> >> +extern struct VMStateDescription vmstate_x86_cpu;
> >> #endif
> >>
> >> /**
> >> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> >> index 4389959..99cc7e3 100644
> >> --- a/target-i386/kvm.c
> >> +++ b/target-i386/kvm.c
> >> @@ -33,6 +33,8 @@
> >> #include "exec/ioport.h"
> >> #include <asm/hyperv.h>
> >> #include "hw/pci/pci.h"
> >> +#include "migration/migration.h"
> >> +#include "qapi/qmp/qerror.h"
> >>
> >> //#define DEBUG_KVM
> >>
> >> @@ -447,6 +449,8 @@ static bool hyperv_enabled(X86CPU *cpu)
> >> cpu->hyperv_relaxed_timing);
> >> }
> >>
> >> +Error *invtsc_mig_blocker;
> >
> > This should be static, even if no zero-initialization is needed below.
> >
> >> +
> >> #define KVM_MAX_CPUID_ENTRIES 100
> >>
> >> int kvm_arch_init_vcpu(CPUState *cs)
> >> @@ -702,6 +706,15 @@ int kvm_arch_init_vcpu(CPUState *cs)
> >> !!(c->ecx & CPUID_EXT_SMX);
> >> }
> >>
> >> + c = cpuid_find_entry(&cpuid_data.cpuid, 0x80000007, 0);
> >> + if (c && (c->edx & 1<<8) && invtsc_mig_blocker == NULL) {
> >> + /* for migration */
> >> + error_set(&invtsc_mig_blocker, QERR_MIGRATION_NOT_SUPPORTED,
> >> "cpu");
> >
> > This does not compile for me. error_setg()? With what text?
>
> http://git.qemu-project.org/?p=qemu.git;a=blobdiff;f=include/qapi/qmp/qerror.h;h=01d1d0661c607ace1c5d3831e5c79eeab851f6b7;hp=a72bbc98503fe261bb8d2c407220252b1e6a85a4;hb=f231b88db14f13ee9a41599896f57f3594c1ca8b;hpb=d73f0beadb57f885e678bffc362864f4401262e0
>
> -#define QERR_MIGRATION_NOT_SUPPORTED \
> - ERROR_CLASS_GENERIC_ERROR, "State blocked by non-migratable device
> '%s'"
>
> Suggesting something nicer than "device 'cpu'":
>
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 1fe8512..9b09a1a 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -710,7 +710,8 @@ int kvm_arch_init_vcpu(CPUState *cs)
> c = cpuid_find_entry(&cpuid_data.cpuid, 0x80000007, 0);
> if (c && (c->edx & 1<<8) && invtsc_mig_blocker == NULL) {
> /* for migration */
> - error_set(&invtsc_mig_blocker, QERR_MIGRATION_NOT_SUPPORTED,
> "cpu");
> + error_setg(&invtsc_mig_blocker,
> + "State blocked by non-migratable CPU device");
> migrate_add_blocker(invtsc_mig_blocker);
> /* for savevm */
> vmstate_x86_cpu.unmigratable = 1;
v3 sent by Marcelo had a different error message, but my queue had v2. I
suggest this:
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index f9ffa4b..7099a51 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -710,8 +710,8 @@ int kvm_arch_init_vcpu(CPUState *cs)
c = cpuid_find_entry(&cpuid_data.cpuid, 0x80000007, 0);
if (c && (c->edx & 1<<8) && invtsc_mig_blocker == NULL) {
/* for migration */
- error_setg(&invtsc_mig_blocker,
- "State blocked by non-migratable CPU device");
+ error_set(&invtsc_mig_blocker,
+ QERR_DEVICE_FEATURE_BLOCKS_MIGRATION, "invtsc", "cpu");
migrate_add_blocker(invtsc_mig_blocker);
/* for savevm */
vmstate_x86_cpu.unmigratable = 1;
Requiring the user to look at QEMU source code to find out which feature
is blocking migration would just cause confusion.
BTW, applying this patch without applying "Set migratable=yes by
default" will break existing setups where users expect migration to
work. See the discussion at:
http://marc.info/?l=qemu-devel&m=139838802220184&w=2
--
Eduardo
