* Eduardo Habkost (ehabkost@xxxxxxxxxx) wrote: > On Mon, Nov 09, 2015 at 08:33:55AM +0800, haozhong.zhang@xxxxxxxxx wrote: > > On 11/06/15 13:12, Eduardo Habkost wrote: > > > On Fri, Nov 06, 2015 at 10:32:24AM +0800, haozhong.zhang@xxxxxxxxx wrote: > > > > On 11/05/15 14:05, Eduardo Habkost wrote: > > > > > On Thu, Nov 05, 2015 at 09:30:51AM +0800, Haozhong Zhang wrote: > > > > > > On 11/04/15 19:42, Eduardo Habkost wrote: > > > [...] > > > > > > > > + env->tsc_khz_saved = r; > > > > > > > > + } > > > > > > > > > > > > > > Why do you need a separate tsc_khz_saved field, and don't simply use > > > > > > > tsc_khz? It would have the additional feature of letting QMP clients > > > > > > > query the current TSC rate by asking for the tsc-freq property on CPU > > > > > > > objects. > > > > > > > > > > > > > > > > > > > It's to avoid overriding env->tsc_khz on the destination in the > > > > > > migration. I can change this line to > > > > > > env->tsc_khz = env->tsc_khz_saved = r; > > > > > > > > > > You are already avoiding overriding env->tsc_khz, because you use > > > > > KVM_GET_TSC_KHZ only if tsc_khz is not set yet. I still don't see why > > > > > you need a tsc_khz_saved field that requires duplicating the SET_TSC_KHZ > > > > > code, if you could just do this: > > > > > > > > > > if (!env->tsc_khz) { > > > > > env->tsc_khz = kvm_vcpu_ioctl(cs, KVM_GET_TSC_KHZ); > > > > > } > > > > > > > > > > > > > Consider an example that we migrate a VM from machine A to machine B > > > > and then to machine C, and QEMU on machine B is launched with the cpu > > > > option 'tsc-freq' (i.e. env->tsc_khz on B is non-zero at the > > > > beginning): > > > > 1) In the migration from B to C, the user-specified TSC frequency by > > > > 'tsc-freq' on B is expected to be migrated to C. That is, the > > > > value of env->tsc_khz on B is migrated. > > > > 2) If TSC frequency is migrated through env->tsc_khz, then > > > > env->tsc_khz on B will be overrode in the migration from A to B > > > > before kvm_arch_setup_tsc_khz(). If the guest TSC frequency is > > > > different than the user-specified TSC frequency on B, the > > > > expectation in 1) will not be satisfied anymore. > > > > > > Setting tsc-freq on B when tsc-freq was not used on A is invalid usage. > > > This is not different from changing the CPU model and adding or removing > > > CPU flags when migrating, which is also incorrect. The command-line > > > parameters defining the VM must be the same when you migrate. > > > > > > > Good to know it's an invalid usage. Then the question is what QEMU is > > expected to do for this invalid usage? > > > > 1) Abort the migration? But I find that the current QEMU does not > > abort the migration between different CPU models (e.g. Nehalem and > > Haswell). > > > > 2) Or do not abort the migration and ignore tsc-freq option? If so, > > tsc_khz_saved will be not needed. > > My first choice is to abort migration. If we decide to abort today and > find it to cause problems, we can easily fix it. If we decide to > continue without aborting, it is difficult to change that behavior > without breaking existing setups. Yes, if it's a bad config please abort the migration and put a clear message in the log so we can tell easily. Dave > > -- > Eduardo -- Dr. David Alan Gilbert / dgilbert@xxxxxxxxxx / Manchester, UK -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html