[virt-tools-list] [virt-manager PATCH 2/2] capabilities: detect ACPI and APIC capabilites properly

[Cole Robinson]

On 07/08/2015 05:33 AM, Pavel Hrdina wrote:
> Instead of hard-coding that ACPI and APIC are enabled by default, detect
> their presence from libvirt capabilities and use it.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1215692
> 
> Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> ---

Comments inline

> diff --git a/virtinst/capabilities.py b/virtinst/capabilities.py
> index a26b4a2..553a430 100644
> --- a/virtinst/capabilities.py
> +++ b/virtinst/capabilities.py
> @@ -206,6 +206,8 @@ class _CapsGuestFeatures(XMLBuilder):
>      _XML_ROOT_NAME = "features"
>  
>      pae = XMLProperty("./pae", is_bool=True)
> +    acpi = XMLProperty("./acpi/@default", is_onoff=True)
> +    apic = XMLProperty("./apic/@default", is_onoff=True)
>  
>  
>  class _CapsGuest(XMLBuilder):
> @@ -298,6 +300,16 @@ class _CapsGuest(XMLBuilder):
>              return True
>          return False
>  
> +    def supports_acpi(self):
> +        if self.features.acpi:
> +            return True
> +        return False
> +
> +    def supports_apic(self):
> +        if self.features.apic:
> +            return True
> +        return False
> +
>  

You can just do: return bool(self.features.apic)

>  ############################
>  # Main capabilities object #
> diff --git a/virtinst/guest.py b/virtinst/guest.py
> index 7ff7bd5..3f91ae9 100644
> --- a/virtinst/guest.py
> +++ b/virtinst/guest.py
> @@ -840,9 +840,12 @@ class Guest(XMLBuilder):
>              default = False
>  
>          if self.features.acpi == "default":
> -            self.features.acpi = self._os_object.supports_acpi(default)
> +            if default:
> +                self.features.acpi = self.capsinfo.guest.supports_acpi()
> +            else:
> +                self.features.acpi = False
>          if self.features.apic == "default":
> -            self.features.apic = self._os_object.supports_apic(default)
> +            self.features.apic = self.capsinfo.guest.supports_apic()
>          if self.features.pae == "default":
>              self.features.pae = self.capsinfo.guest.supports_pae()
>  

You still need to abide the os_object bits. I'd say leave these 'if'
conditionals alone, and set the 'default' variable above to also depend on
capsinfo.guest.supports_*. But you'll probably need to have separate
acpi_default and apic_default vars

Thanks,
Cole

> diff --git a/virtinst/osdict.py b/virtinst/osdict.py
> index 76c2260..bd82ae1 100644
> --- a/virtinst/osdict.py
> +++ b/virtinst/osdict.py
> @@ -447,14 +447,6 @@ class _OsVariant(object):
>      def supports_virtiommio(self):
>          return self._is_related_to(["fedora19"])
>  
> -    def supports_acpi(self, default):
> -        if self._family in ['msdos']:
> -            return False
> -        return default
> -
> -    def supports_apic(self, default):
> -        return self.supports_acpi(default)
> -
>      def default_netmodel(self):
>          """
>          Default non-virtio net-model, since we check for that separately
>