[virt-tools-list] [PATCH v2] CPU model capabilities vs domcapabilities

[Cole Robinson]

On 03/26/2018 05:08 PM, Charles Arnold wrote:
> The issue is when the host has been updated with microcode for Spectre
> but qemu has _not_ been updated. In this scenario (as an example),
> 'virsh capabilities' shows the host cpu model as IvyBridge-IBRS, which is
> correct. However, 'virsh domcapabilities' shows IvyBridge as the host-model
> and does not show any of the '-IBRS' flavors available under the custom model,
> which is also correct since the qemu does not have Spectre patches.
> 
> Be default, virt-manager uses 'custom' for the cpu mode XML. For example,
> 
>   <cpu mode="custom" match="exact">
>     <model>IvyBridge-IBRS</model>
>   </cpu>
> 
> Starting an installation in the above scenario will fail because qemu
> doesn't understand '-IBRS' until it has been updated to do so.
> 
> The patch below validates that the capabilities cpu is present in domcapabilities,
> and if they differ, log a message and set the CPU model to None.
> 
> Signed-off-by: Charles Arnold <carnold at suse.com>
> 
> diff --git a/virtinst/domain/cpu.py b/virtinst/domain/cpu.py
> index 3c1089c0..f3055dc7 100644
> --- a/virtinst/domain/cpu.py
> +++ b/virtinst/domain/cpu.py
> @@ -6,6 +6,8 @@
>  # See the COPYING file in the top-level directory.
>  
>  from ..xmlbuilder import XMLBuilder, XMLProperty, XMLChildProperty
> +from .domcapabilities import DomainCapabilities
> +import logging
>  

Hmm I'm guessing this is a patch to older code that was rebased to git
master, because this import fails immediately. After fixing that there's
also quite a few test failures

>  
>  class _CPUCellSibling(XMLBuilder):
> @@ -79,7 +81,7 @@ class DomainCpu(XMLBuilder):
>      SPECIAL_MODES = [SPECIAL_MODE_HOST_MODEL_ONLY, SPECIAL_MODE_HV_DEFAULT,
>                       SPECIAL_MODE_HOST_COPY, SPECIAL_MODE_HOST_MODEL,
>                       SPECIAL_MODE_HOST_PASSTHROUGH, SPECIAL_MODE_CLEAR]
> -    def set_special_mode(self, val):
> +    def set_special_mode(self, val, guest=None):
>          if (val == self.SPECIAL_MODE_HOST_MODEL or
>              val == self.SPECIAL_MODE_HOST_PASSTHROUGH):
>              self.model = None
> @@ -97,6 +99,15 @@ class DomainCpu(XMLBuilder):
>              if self.conn.caps.host.cpu.model:
>                  self.clear()
>                  self.model = self.conn.caps.host.cpu.model
> +                if guest:
> +                    domcaps = DomainCapabilities.build_from_guest(guest)
> +                    domcaps_model = domcaps.cpu.get_mode("host-model").get_models()
> +                    if (isinstance(domcaps_model, list) and len(domcaps_model) and
> +                        domcaps_model[0] != self.conn.caps.host.cpu.model):
> +                        logging.debug("Host capabilities CPU '%s' does not match "
> +                              "domain capabilities CPU '%s'. Leaving CPU model unset.",
> +                              self.conn.caps.host.cpu.model, domcaps_model[0])
> +                        self.model = None

Since we are really only interested in fixing this case via the
virt-manager or virt-install default, I'd take all this logic and move
it to after the cpu.set_special_mode call in guest.py


>          else:
>              raise RuntimeError("programming error: unknown "
>                  "special cpu mode '%s'" % val)
> diff --git a/virtinst/domcapabilities.py b/virtinst/domcapabilities.py
> index f98c807b..8387838c 100644
> --- a/virtinst/domcapabilities.py
> +++ b/virtinst/domcapabilities.py
> @@ -29,9 +29,27 @@ class _Enum(_HasValues):
>      name = XMLProperty("./@name")
>  
>  
> +class _Model(XMLBuilder):
> +    _XML_ROOT_NAME = "model"
> +    model = XMLProperty(".")
> +
> +
> +class _HasModels(XMLBuilder):
> +    models = XMLChildProperty(_Model)
> +
> +    def get_models(self):
> +        return [m.model for m in self.models]
> +
> +
> +class _CPUMode(_HasModels):
> +    _XML_ROOT_NAME = "mode"

s/_XML_ROOT_NAME/XML_NAME/g

Also if this domcapabilities stuff is its own prep patch it will be
easier to make sure it has test coverage.

Thanks,
Cole