[virt-tools-list] [virt-manager PATCH 6/8] domcapabilities: introduce get_cpu_security_features

[Cole Robinson]

On 3/15/19 12:41 PM, Pavel Hrdina wrote:
> Get all CPU security features that we should enable for guests.
> 
> In order to do that we need to get CPU definition from domain
> capabilities and modify the XML so it is in required format for
> libvirt CPU baseline APIs.  We will prefer the baselineHypervisorCPU
> API because that considers what QEMU actually supports and we will
> fallback to baselineCPU API if the better one is not supported by
> libvirt.
> 
> This way we can figure out which of the security features are actually
> available on that specific host for that specific QEMU binary.
> 
> Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> ---
>   virtinst/domcapabilities.py | 43 +++++++++++++++++++++++++++++++++++++
>   virtinst/support.py         |  4 ++++
>   2 files changed, 47 insertions(+)
> 
> diff --git a/virtinst/domcapabilities.py b/virtinst/domcapabilities.py
> index deef9e6a..7576472d 100644
> --- a/virtinst/domcapabilities.py
> +++ b/virtinst/domcapabilities.py
> @@ -8,7 +8,11 @@
>   
>   import logging
>   import re
> +import xml.etree.ElementTree as ET
>   
> +import libvirt
> +
> +from .domain import DomainCpu
>   from .xmlbuilder import XMLBuilder, XMLChildProperty, XMLProperty
>   
>   
> @@ -243,6 +247,45 @@ class DomainCapabilities(XMLBuilder):
>   
>           return models
>   
> +    def _convert_mode_to_cpu(self, xml):
> +        root = ET.fromstring(xml)
> +        root.tag = "cpu"
> +        root.attrib = None
> +        arch = ET.SubElement(root, "arch")
> +        arch.text = self.arch
> +        return ET.tostring(root, encoding="unicode")
> +
> +    def get_cpu_security_features(self):
> +        sec_features = [
> +                'pcid',
> +                'spec-ctrl',
> +                'ssbd',
> +                'pdpe1gb',
> +                'ibpb',
> +                'virt-ssbd',
> +                'amd-ssbd',
> +                'amd-no-ssb']
> +
> +        features = []
> +
> +        for m in self.cpu.modes:
> +            if m.name == "host-model":

If you check for != "host-model" you can 'continue' and save a bunch of 
indentation

Adding new API calls for the default virt-install invocation makes me a 
bit nervous, so let's be on the safe side, I suggest moving everything 
up to the 'cpu' building into its own function, then wrap the call in 
try/except to make errors non-fatal. Add a logging.warn call when it 
fails so if virt-install hits this the user is notified and it's more 
likely we will get a bug report.

> +                cpuXML = self._convert_mode_to_cpu(m.get_xml())

Please log the generated XML here, something like 'CPU XML for security 
flag baseline: %s

> +                if self.conn.check_support(self.conn.SUPPORT_CONN_HYPERVISOR_BASELINE):
> +                    expandedXML = self.conn.baselineHypervisorCPU(
> +                            self.path, self.arch, self.machine, self.domain, [cpuXML],
> +                            libvirt.VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES)
> +                else:
> +                    expandedXML = self.conn.baselineCPU([cpuXML],
> +                            libvirt.VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES)

And log the expandedXML here too

> +                cpu = DomainCpu(self.conn, expandedXML) > +
> +                for feature in cpu.features:
> +                    if feature.name in sec_features:
> +                        features.append(feature.name)
> +
> +        return features
> +
>   
>       XML_NAME = "domainCapabilities"
>       os = XMLChildProperty(_OS, is_single=True)
> diff --git a/virtinst/support.py b/virtinst/support.py
> index 7ace3ef6..077be426 100644
> --- a/virtinst/support.py
> +++ b/virtinst/support.py
> @@ -278,6 +278,10 @@ SUPPORT_CONN_DISK_DRIVER_NAME_QEMU = _make(
>       hv_version={"qemu": 0, "xen": "4.2.0"},
>       hv_libvirt_version={"qemu": 0, "xen": "1.1.0"})
>   
> +SUPPORT_CONN_HYPERVISOR_BASELINE = _make(
> +    version="4.4.0",
> +    hv_libvirt_version={"test": 0})
> +
>

I think this should be hv_libvirt_version={"qemu", 0}, meaning: assume 
support if libvirt version >= 4.4.0, driver=qemu, qemu driver version > 
0. The test driver doesn't appear to implement this API

Thanks,
Cole