On Thu, Mar 29, 2018 at 12:47:42PM +0300, Roman Kagan wrote: > On Wed, Mar 28, 2018 at 03:53:31PM -0300, Eduardo Habkost wrote: > > On Wed, Mar 28, 2018 at 06:30:24PM +0300, Roman Kagan wrote: > > > In order to guarantee compatibility on migration, QEMU should have > > > complete control over the features it announces to the guest via CPUID. > > > > > > However, for a number of Hyper-V-related cpu properties, if the > > > corresponding feature is not supported by the underlying KVM, the > > > propery is silently ignored and the feature is not announced to the > > > guest. > > > > > > Refuse to start with an error instead. > > > > > > Signed-off-by: Roman Kagan <rka...@virtuozzo.com> > > > > Something I didn't consider before: > > > > Will this block migration before it even starts, or will crash > > the VM only after all migration data was sent to the destination? > > > > I didn't test it, but kvm_arch_init_vcpu() seems to be too late > > to block an invalid/unsupport configuration. > > I just did a simple test, force-failing one of the checks and starting > QEMU with -incoming defer. It refused to start with the expected error > message. IOW in the migration case the destination will abort before > the source have started to send any data. > > > Maybe we can simply call hyperv_handle_properties() earlier, > > inside x86_cpu_realizefn()? > > Now it's > > ... > x86_cpu_realizefn > qemu_init_vcpu > qemu_kvm_start_vcpu > qemu_thread_create(qemu_kvm_cpu_thread_fn) > [in vcpu thread] > qemu_kvm_cpu_thread_fn
Oh, this is the part that I missed. I thought the vCPU thread wouldn't start until migration was finished. This means the patch is OK as-is. Sorry for the confusion. > kvm_init_vcpu > kvm_arch_init_vcpu > hyperv_handle_properties > [error return] > [error return] > [error return] > exit(1) > > > (I know it's very late for this kind of intrusive change in > > v2.12, but I still think it's a good idea to fix this as soon as > > possible.) > > I agree that the current hyperv flag handling begs for cleanup but I > think this can wait for post-2.12. > > > > --- > > > v1 -> v2: > > > - indicate what flag requested the feature that can't be enabled in the > > > error message > > > - fix a typo in the error message for VP_RUNTIME > > I just noticed that I missed hv-time being silently cleared, too (just > in a slightly different pattern), so I'll have to respin. > > Thanks, > Roman. -- Eduardo
