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
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.
