Re: [Qemu-devel] [qom/qom-cpu PATCH] i386: invtsc migration blocker is redudant

[Eduardo Habkost]

On Tue, May 27, 2014 at 11:39:20AM -0300, Marcelo Tosatti wrote:
> 
> Migration blocker is redudant: blocking savevm is sufficient.
> 

Removing the redundancy looks welcome, but at the same time the
migrate_add_blocker() call ensured we had a clearer error message (I
mean: if we did mention invtsc in the error message, which we still
don't, but should).

I am not familiar with how we deal with savevm/migration errors, so I
don't know what's best here. Juan, do you have any suggestions?

Additional small comment below:

> Signed-off-by: Marcelo Tosatti <address@hidden>
> 
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index f9ffa4b..b29098a 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -450,7 +450,7 @@ static bool hyperv_enabled(X86CPU *cpu)
>              cpu->hyperv_relaxed_timing);
>  }
>  
> -static Error *invtsc_mig_blocker;
> +bool invtsc_mig_blocked;
>  
>  #define KVM_MAX_CPUID_ENTRIES  100
>  
> @@ -708,13 +708,9 @@ 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");
> -        migrate_add_blocker(invtsc_mig_blocker);
> -        /* for savevm */
> +    if (c && (c->edx & 1<<8) && invtsc_mig_blocked == false) {
>          vmstate_x86_cpu.unmigratable = 1;
> +        invtsc_mig_blocked = true;

Why the invtsc_mig_blocked variable? Setting unmigratable=1 twice won't
hurt anybody.

>      }
>  

-- 
Eduardo