openstack
/
nova
Code Issues Proposed changes

libvirt: Don't drop CPU flags with policy='disable' from guest XML 

Commit eeeca4ceff (Handle disabled CPU features to fix live migration
failures, 2020-10-06) literally drops every CPU flag entry from guest
XML that had policy="disable".  I noticed this while testing this
patch[1] that allows disabling CPU flags, where disabled CPU flags are
altogether dropped from the guest XML record.  We should retain them.

The bug[2] the above-mentioned commit was putting a band-aid to -- which
is to ignore disabled CPU flags when computing a baseline CPU, to allow
live migration on some Intel processors -- would've been addressed by
half of the original patch.  That is, in the class LibvirtConfigCPU(),
we can drop the check from the parse_dom(), but keep it in format_dom().
Effectively, this is a partial revert of the said commit (eeeca4ceff).

Example:  In both the below cases, I'm testing with this patch that
allows disabling CPU flags[1].  Test: In `[libvirt]cpu_model_flags`,
enable "md-clear" and "pcid", but disable "mtrr" and "pdpe1gb".

Result without this patch
-------------------------

  <cpu mode='custom' match='exact' check='full'>
    <model fallback='forbid'>Nehalem-IBRS</model>
    <feature policy='require' name='md-clear'/>
    <feature policy='require' name='pcid'/>
  </cpu>

Only the enabled flags are shown for the guest.

Result with this patch
----------------------

  <cpu mode='custom' match='exact' check='full'>
    <model fallback='forbid'>Nehalem-IBRS</model>
    <feature policy='require' name='md-clear'/>
    <feature policy='require' name='pcid'/>
    <feature policy='disable' name='mtrr'/>
    <feature policy='disable' name='pdpe1gb'/>
  </cpu>

Here we see both enabled _and_ disabled flags in the guest XML, which is
much clearer.  So go with this approach.

[1] https://review.opendev.org/c/openstack/nova/+/774240/ (libvirt: Allow
    disabling CPU flags via `cpu_model_extra_flags`)
[2] https://launchpad.net/bugs/1898715 -- Live migration fails despite
    matching CPUs

blueprint: allow-disabling-cpu-flags

Change-Id: I47451e2b776eba0b038711f2027d565fc950c841
Signed-off-by: Kashyap Chamarthy <kchamart@redhat.com>
This commit is contained in:
Kashyap Chamarthy 2021-02-12 18:12:16 +01:00
parent 726e2fd03b
commit 2e8e04a8f6
2 changed files with 1 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
nova/tests/unit/virt/libvirt/test_config.py
View File

@ -457,27 +457,6 @@ class LibvirtConfigCPUTest(LibvirtConfigBaseTest):
</cpu>
""")
def test_config_disabled_features(self):
obj = config.LibvirtConfigCPU()
obj.model = "Penryn"
obj.vendor = "Intel"
obj.arch = obj_fields.Architecture.X86_64
disabled_feature = config.LibvirtConfigCPUFeature("mtrr")
disabled_feature.policy = "disable"
obj.add_feature(disabled_feature)
obj.add_feature(config.LibvirtConfigCPUFeature("apic"))
xml = obj.to_xml()
self.assertXmlEqual(xml, """
<cpu>
<arch>x86_64</arch>
<model>Penryn</model>
<vendor>Intel</vendor>
<feature name="apic"/>
</cpu>
""")
def test_only_uniq_cpu_featues(self):
obj = config.LibvirtConfigCPU()
obj.model = "Penryn"

3
nova/virt/libvirt/config.py
View File

@ -755,8 +755,7 @@ class LibvirtConfigCPU(LibvirtConfigObject):
# sorting the features to allow more predictable tests
for f in sorted(self.features, key=lambda x: x.name):
if f.policy != "disable":
cpu.append(f.format_dom())
cpu.append(f.format_dom())
return cpu