| Message ID | 20181119182330.17147-1-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] target/i386: kvm: add VMX migration blocker | expand |
* Paolo Bonzini (pbonzini@redhat.com) wrote: > Nested VMX does not support live migration yet. Add a blocker > until that is worked out. > > Nested SVM only does not support it, but unfortunately it is > enabled by default for -cpu host so we cannot really disable it. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> So I'm OK with this, but it does need a release note warning whenever it goes in, because it'll surprise those who've already enabled nesting but don't use it on all their VMs. Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > target/i386/kvm.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/target/i386/kvm.c b/target/i386/kvm.c > index f524e7d929..27dcca5365 100644 > --- a/target/i386/kvm.c > +++ b/target/i386/kvm.c > @@ -854,6 +854,7 @@ static int hyperv_init_vcpu(X86CPU *cpu) > } > > static Error *invtsc_mig_blocker; > +static Error *vmx_mig_blocker; > > #define KVM_MAX_CPUID_ENTRIES 100 > > @@ -1246,6 +1247,17 @@ int kvm_arch_init_vcpu(CPUState *cs) > !!(c->ecx & CPUID_EXT_SMX); > } > > + if ((env->features[FEAT_1_ECX] & CPUID_EXT_VMX) && !vmx_mig_blocker) { > + error_setg(&vmx_mig_blocker, > + "Nested VMX virtualization does not support live migration yet"); > + r = migrate_add_blocker(vmx_mig_blocker, &local_err); > + if (local_err) { > + error_report_err(local_err); > + error_free(vmx_mig_blocker); > + return r; > + } > + } > + > if (env->mcg_cap & MCG_LMCE_P) { > has_msr_mcg_ext_ctl = has_msr_feature_control = true; > } > -- > 2.19.1 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On 20/11/18 12:44, Dr. David Alan Gilbert wrote: > * Paolo Bonzini (pbonzini@redhat.com) wrote: >> Nested VMX does not support live migration yet. Add a blocker >> until that is worked out. >> >> Nested SVM only does not support it, but unfortunately it is >> enabled by default for -cpu host so we cannot really disable it. >> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > So I'm OK with this, but it does need a release note warning whenever it > goes in, because it'll surprise those who've already enabled nesting > but don't use it on all their VMs. Yes, of course. Paolo
On 11/20/18 6:44 AM, Dr. David Alan Gilbert wrote: > * Paolo Bonzini (pbonzini@redhat.com) wrote: >> Nested VMX does not support live migration yet. Add a blocker >> until that is worked out. >> >> Nested SVM only does not support it, but unfortunately it is >> enabled by default for -cpu host so we cannot really disable it. >> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > So I'm OK with this, but it does need a release note warning whenever it > goes in, because it'll surprise those who've already enabled nesting > but don't use it on all their VMs. > We are hitting this in Fedora 30. Now that nested VMX is enabled by default at the kernel level, and virt-manager/boxes will use the equivalent of -cpu host by default, libvirt managedsave (migrate to file) and virt-manager snapshots (savevm) are rejected for default created VMs on intel. That's quite unfortunate. Any ideas on how to resolve this? FYI it's the root of this bug though there's libvirt issues mixed in: https://bugzilla.redhat.com/show_bug.cgi?id=1697997 Thanks, Cole
On 10/04/19 20:26, Cole Robinson wrote: > On 11/20/18 6:44 AM, Dr. David Alan Gilbert wrote: >> * Paolo Bonzini (pbonzini@redhat.com) wrote: >>> Nested VMX does not support live migration yet. Add a blocker >>> until that is worked out. >>> >>> Nested SVM only does not support it, but unfortunately it is >>> enabled by default for -cpu host so we cannot really disable it. >>> >>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >> >> So I'm OK with this, but it does need a release note warning whenever it >> goes in, because it'll surprise those who've already enabled nesting >> but don't use it on all their VMs. >> > > We are hitting this in Fedora 30. Now that nested VMX is enabled by > default at the kernel level, and virt-manager/boxes will use the > equivalent of -cpu host by default, libvirt managedsave (migrate to > file) and virt-manager snapshots (savevm) are rejected for default > created VMs on intel. That's quite unfortunate. > > Any ideas on how to resolve this? I think the simplest solution is just to finish implementation of nested VMX live migration and backport it to Fedora 30. Paolo
On 4/12/19 3:47 AM, Paolo Bonzini wrote: > On 10/04/19 20:26, Cole Robinson wrote: >> On 11/20/18 6:44 AM, Dr. David Alan Gilbert wrote: >>> * Paolo Bonzini (pbonzini@redhat.com) wrote: >>>> Nested VMX does not support live migration yet. Add a blocker >>>> until that is worked out. >>>> >>>> Nested SVM only does not support it, but unfortunately it is >>>> enabled by default for -cpu host so we cannot really disable it. >>>> >>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >>> >>> So I'm OK with this, but it does need a release note warning whenever it >>> goes in, because it'll surprise those who've already enabled nesting >>> but don't use it on all their VMs. >>> >> >> We are hitting this in Fedora 30. Now that nested VMX is enabled by >> default at the kernel level, and virt-manager/boxes will use the >> equivalent of -cpu host by default, libvirt managedsave (migrate to >> file) and virt-manager snapshots (savevm) are rejected for default >> created VMs on intel. That's quite unfortunate. >> >> Any ideas on how to resolve this? > > I think the simplest solution is just to finish implementation of nested > VMX live migration and backport it to Fedora 30. > That would simplify things :) Any guess on the timeframe? This is kernel work I presume? If changes aren't landing in the near term I think we should disable nested VMX by default in Fedora, maybe just with modules.d/kvm.conf override. (Or revert this patch downstream, but I presume that's not a good idea). The alternative of just letting it sit is going to generate a lot of complaints I suspect. And the only solutions will be 1) disable nested VMx for your whole machine and reboot, or 2) run this virt-xml command to disable VMX in your domain config... and probably forget that it's there and then a year later when this is all sorted out file a bug asking why nested virt isn't working for this one VM ;) I guess #2 might not be avoidable anyways for the amount of people that have already opted into nested VMX Thanks, Cole
----- Cole Robinson <crobinso@redhat.com> ha scritto: > On 4/12/19 3:47 AM, Paolo Bonzini wrote: > > On 10/04/19 20:26, Cole Robinson wrote: > >> On 11/20/18 6:44 AM, Dr. David Alan Gilbert wrote: > >>> * Paolo Bonzini (pbonzini@redhat.com) wrote: > >>>> Nested VMX does not support live migration yet. Add a blocker > >>>> until that is worked out. > >>>> > >>>> Nested SVM only does not support it, but unfortunately it is > >>>> enabled by default for -cpu host so we cannot really disable it. > >>>> > >>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > >>> > >>> So I'm OK with this, but it does need a release note warning whenever it > >>> goes in, because it'll surprise those who've already enabled nesting > >>> but don't use it on all their VMs. > >>> > >> > >> We are hitting this in Fedora 30. Now that nested VMX is enabled by > >> default at the kernel level, and virt-manager/boxes will use the > >> equivalent of -cpu host by default, libvirt managedsave (migrate to > >> file) and virt-manager snapshots (savevm) are rejected for default > >> created VMs on intel. That's quite unfortunate. > >> > >> Any ideas on how to resolve this? > > > > I think the simplest solution is just to finish implementation of nested > > VMX live migration and backport it to Fedora 30. > > > > That would simplify things :) Any guess on the timeframe? This is kernel > work I presume? No, the kernel part is already in. As a contingency plan, you could just revert this QEMU patch. Paolo > > If changes aren't landing in the near term I think we should disable > nested VMX by default in Fedora, maybe just with modules.d/kvm.conf > override. (Or revert this patch downstream, but I presume that's not a > good idea). > > The alternative of just letting it sit is going to generate a lot of > complaints I suspect. And the only solutions will be 1) disable nested > VMx for your whole machine and reboot, or 2) run this virt-xml command > to disable VMX in your domain config... and probably forget that it's > there and then a year later when this is all sorted out file a bug > asking why nested virt isn't working for this one VM ;) > > I guess #2 might not be avoidable anyways for the amount of people that > have already opted into nested VMX > > Thanks, > Cole
* Paolo Bonzini (pbonzini@redhat.com) wrote: > > ----- Cole Robinson <crobinso@redhat.com> ha scritto: > > On 4/12/19 3:47 AM, Paolo Bonzini wrote: > > > On 10/04/19 20:26, Cole Robinson wrote: > > >> On 11/20/18 6:44 AM, Dr. David Alan Gilbert wrote: > > >>> * Paolo Bonzini (pbonzini@redhat.com) wrote: > > >>>> Nested VMX does not support live migration yet. Add a blocker > > >>>> until that is worked out. > > >>>> > > >>>> Nested SVM only does not support it, but unfortunately it is > > >>>> enabled by default for -cpu host so we cannot really disable it. > > >>>> > > >>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > >>> > > >>> So I'm OK with this, but it does need a release note warning whenever it > > >>> goes in, because it'll surprise those who've already enabled nesting > > >>> but don't use it on all their VMs. > > >>> > > >> > > >> We are hitting this in Fedora 30. Now that nested VMX is enabled by > > >> default at the kernel level, and virt-manager/boxes will use the > > >> equivalent of -cpu host by default, libvirt managedsave (migrate to > > >> file) and virt-manager snapshots (savevm) are rejected for default > > >> created VMs on intel. That's quite unfortunate. > > >> > > >> Any ideas on how to resolve this? > > > > > > I think the simplest solution is just to finish implementation of nested > > > VMX live migration and backport it to Fedora 30. > > > > > > > That would simplify things :) Any guess on the timeframe? This is kernel > > work I presume? > > No, the kernel part is already in. As a contingency plan, you could just revert this QEMU patch. With a new-enough kernel, how hard is it to detect that this actual migration is safe? Dave > Paolo > > > > > If changes aren't landing in the near term I think we should disable > > nested VMX by default in Fedora, maybe just with modules.d/kvm.conf > > override. (Or revert this patch downstream, but I presume that's not a > > good idea). > > > > The alternative of just letting it sit is going to generate a lot of > > complaints I suspect. And the only solutions will be 1) disable nested > > VMx for your whole machine and reboot, or 2) run this virt-xml command > > to disable VMX in your domain config... and probably forget that it's > > there and then a year later when this is all sorted out file a bug > > asking why nested virt isn't working for this one VM ;) > > > > I guess #2 might not be avoidable anyways for the amount of people that > > have already opted into nested VMX > > > > Thanks, > > Cole > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On 15/04/19 13:22, Dr. David Alan Gilbert wrote: > * Paolo Bonzini (pbonzini@redhat.com) wrote: >> >> ----- Cole Robinson <crobinso@redhat.com> ha scritto: >>> On 4/12/19 3:47 AM, Paolo Bonzini wrote: >>>> On 10/04/19 20:26, Cole Robinson wrote: >>>>> On 11/20/18 6:44 AM, Dr. David Alan Gilbert wrote: >>>>>> * Paolo Bonzini (pbonzini@redhat.com) wrote: >>>>>>> Nested VMX does not support live migration yet. Add a blocker >>>>>>> until that is worked out. >>>>>>> >>>>>>> Nested SVM only does not support it, but unfortunately it is >>>>>>> enabled by default for -cpu host so we cannot really disable it. >>>>>>> >>>>>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >>>>>> >>>>>> So I'm OK with this, but it does need a release note warning whenever it >>>>>> goes in, because it'll surprise those who've already enabled nesting >>>>>> but don't use it on all their VMs. >>>>>> >>>>> >>>>> We are hitting this in Fedora 30. Now that nested VMX is enabled by >>>>> default at the kernel level, and virt-manager/boxes will use the >>>>> equivalent of -cpu host by default, libvirt managedsave (migrate to >>>>> file) and virt-manager snapshots (savevm) are rejected for default >>>>> created VMs on intel. That's quite unfortunate. >>>>> >>>>> Any ideas on how to resolve this? >>>> >>>> I think the simplest solution is just to finish implementation of nested >>>> VMX live migration and backport it to Fedora 30. >>>> >>> >>> That would simplify things :) Any guess on the timeframe? This is kernel >>> work I presume? >> >> No, the kernel part is already in. As a contingency plan, you could just revert this QEMU patch. > > With a new-enough kernel, how hard is it to detect that this actual > migration is safe? Not hard, but it would fail whenever the KVM kernel module is loaded, so pretty much for all Linux guests. Paolo
diff --git a/target/i386/kvm.c b/target/i386/kvm.c index f524e7d929..27dcca5365 100644 --- a/target/i386/kvm.c +++ b/target/i386/kvm.c @@ -854,6 +854,7 @@ static int hyperv_init_vcpu(X86CPU *cpu) } static Error *invtsc_mig_blocker; +static Error *vmx_mig_blocker; #define KVM_MAX_CPUID_ENTRIES 100 @@ -1246,6 +1247,17 @@ int kvm_arch_init_vcpu(CPUState *cs) !!(c->ecx & CPUID_EXT_SMX); } + if ((env->features[FEAT_1_ECX] & CPUID_EXT_VMX) && !vmx_mig_blocker) { + error_setg(&vmx_mig_blocker, + "Nested VMX virtualization does not support live migration yet"); + r = migrate_add_blocker(vmx_mig_blocker, &local_err); + if (local_err) { + error_report_err(local_err); + error_free(vmx_mig_blocker); + return r; + } + } + if (env->mcg_cap & MCG_LMCE_P) { has_msr_mcg_ext_ctl = has_msr_feature_control = true; }
Nested VMX does not support live migration yet. Add a blocker until that is worked out. Nested SVM only does not support it, but unfortunately it is enabled by default for -cpu host so we cannot really disable it. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- target/i386/kvm.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)