[virt-tools-list] [virt-manager PATCH 1/2] refactor detection of guest type capabilities
Cole Robinson
crobinso at redhat.com
Wed Jul 8 16:28:32 UTC 2015
On 07/08/2015 05:33 AM, Pavel Hrdina wrote:
> Each guest type can have its own capabilities and we should always ask
> only for those capabilities.
>
> The old approach was to get capabilities from libvirt and then for
> example cycle trough all guests and return True, if any guest type
> supports kvm or pae, etc.
>
> Now we check those capabilities only for the correct guest type
> according to defaults and input from user.
>
$ python setup.py pylint
************* Module virtManager.create
E:881, 0: invalid syntax (syntax-error)
************* Module virtManager.engine
F: 42, 0: Unable to import 'create' (invalid syntax (<string>, line 881))
(import-error)
running pep8
virtManager/create.py:882: [E113] unexpected indentation
I also get this failure:
======================================================================
FAIL: testVMX2Libvirt (tests.virtconvtest.TestVirtConv)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/crobinso/src/virt-manager/tests/virtconvtest.py", line 91, in
testVMX2Libvirt
self._compare_files("vmx")
File "/home/crobinso/src/virt-manager/tests/virtconvtest.py", line 86, in
_compare_files
self._compare_single_file(in_path, in_type)
File "/home/crobinso/src/virt-manager/tests/virtconvtest.py", line 75, in
_compare_single_file
self._convert_helper(in_path, out_path, in_type, disk_format)
File "/home/crobinso/src/virt-manager/tests/virtconvtest.py", line 58, in
_convert_helper
utils.diff_compare(out_expect, outfile)
File "/home/crobinso/src/virt-manager/tests/utils.py", line 206, in diff_compare
raise AssertionError("Conversion outputs did not match.\n%s" % diff)
AssertionError: Conversion outputs did not match.
---
/home/crobinso/src/virt-manager/tests/virtconv-files/libvirt_output/vmx2libvirt_test-vmx-zip.libvirt
+++ Generated Output
@@ -12,7 +12,6 @@
<features>
<acpi/>
<apic/>
- <pae/>
<vmport state="off"/>
</features>
<cpu mode="custom" match="exact">
Few other comments below
> diff --git a/tests/xmlconfig.py b/tests/xmlconfig.py
> index 048675d..bf41c07 100644
> --- a/tests/xmlconfig.py
> +++ b/tests/xmlconfig.py
> @@ -52,6 +52,8 @@ def _make_guest(installer=None, conn=None):
> g.os.arch = "i686"
> g.os.os_type = "hvm"
>
> + g.capsinfo = conn.caps.guest_lookup(g.os.os_type, g.os.arch)
> +
Instead I'd switch this routine to use lookup_virtinst_guest
> g.add_default_input_device()
> g.add_default_console_device()
> g.add_device(virtinst.VirtualAudio(g.conn))
> diff --git a/virtManager/create.py b/virtManager/create.py
> index 8401a7a..45de3c5 100644
> --- a/virtManager/create.py
> +++ b/virtManager/create.py
> @@ -504,7 +504,7 @@ class vmmCreate(vmmGObjectUI):
> self.widget("startup-error-box").hide()
> self.widget("create-forward").set_sensitive(True)
>
> - if not self.conn.caps.has_install_options():
> + if not self.capsinfo.guest.has_install_options():
> error = _("No hypervisor options were found for this "
> "connection.")
>
> @@ -539,7 +539,7 @@ class vmmCreate(vmmGObjectUI):
> self.startup_warning(error)
>
> elif self.conn.is_qemu():
> - if not self.conn.caps.is_kvm_available():
> + if not self.capsinfo.guest.is_kvm_available():
> error = _("KVM is not available. This may mean the KVM "
> "package is not installed, or the KVM kernel modules "
> "are not loaded. Your virtual machines may perform poorly.")
> @@ -875,11 +875,10 @@ class vmmCreate(vmmGObjectUI):
> break
>
> capsinfo = self.conn.caps.guest_lookup(os_type=gtype, arch=arch)
> - newg, newdom = capsinfo.get_caps_objects()
>
> if self.capsinfo:
> - oldg, olddom = self.capsinfo.get_caps_objects()
> - if oldg == newg and olddom and newdom:
> + if (self.capsinfo.guest == capsinfo.guest and
> + self.capsinfo.domain == capsinfo.domain)
> return
>
> self.capsinfo = capsinfo
> diff --git a/virtinst/capabilities.py b/virtinst/capabilities.py
> index bed8596..a26b4a2 100644
> --- a/virtinst/capabilities.py
> +++ b/virtinst/capabilities.py
> @@ -268,6 +268,36 @@ class _CapsGuest(XMLBuilder):
> # Fallback, just return last item in list
> return domains[-1]
>
> + def has_install_options(self):
> + """
> + Return True if there are any install options available
> + """
> + if len(self.domains) > 0:
> + return True
> +
> + return False
> +
> + def is_kvm_available(self):
> + """
> + Return True if kvm guests can be installed
> + """
> + if self.os_type != "hvm":
> + return False
> +
> + for d in self.domains:
> + if d.hypervisor_type == "kvm":
> + return True
> +
> + return False
> +
> + def supports_pae(self):
> + """
> + Return True if capabilities report support for PAE
> + """
> + if self.features.pae:
> + return True
> + return False
> +
>
> ############################
> # Main capabilities object #
> @@ -280,23 +310,17 @@ class _CapsInfo(object):
> """
> def __init__(self, conn, guest, domain, requested_machine):
> self.conn = conn
> - self._guest = guest
> - self._domain = domain
> + self.guest = guest
> + self.domain = domain
> self._requested_machine = requested_machine
>
> - self.hypervisor_type = self._domain.hypervisor_type
> - self.os_type = self._guest.os_type
> - self.arch = self._guest.arch
> - self.loader = self._guest.loader
> + self.hypervisor_type = self.domain.hypervisor_type
> + self.os_type = self.guest.os_type
> + self.arch = self.guest.arch
> + self.loader = self.guest.loader
>
> - self.emulator = self._domain.emulator
> - self.machines = self._domain.machines[:]
> -
> - def get_caps_objects(self):
> - """
> - Return the raw backing caps objects
> - """
> - return self._guest, self._domain
> + self.emulator = self.domain.emulator
> + self.machines = self.domain.machines[:]
>
> def get_recommended_machine(self):
> """
> @@ -360,39 +384,6 @@ class Capabilities(XMLBuilder):
> # Public API #
> ##############
>
> - def has_install_options(self):
> - """
> - Return True if there are any install options available
> - """
> - for g in self.guests:
> - if len(g.domains) > 0:
> - return True
> -
> - return False
> -
> - def is_kvm_available(self):
> - """
> - Return True if kvm guests can be installed
> - """
> - for g in self.guests:
> - if g.os_type != "hvm":
> - continue
> -
> - for d in g.domains:
> - if d.hypervisor_type == "kvm":
> - return True
> -
> - return False
> -
> - def supports_pae(self):
> - """
> - Return True if capabilities report support for PAE
> - """
> - for g in self.guests:
> - if g.features.pae:
> - return True
> - return False
> -
> def get_cpu_values(self, arch):
> if not arch:
> return []
> @@ -498,6 +489,8 @@ class Capabilities(XMLBuilder):
>
> gobj.os.machine = capsinfo.get_recommended_machine()
>
> + gobj.capsinfo = capsinfo
> +
> return gobj
>
> def lookup_virtinst_guest(self, *args, **kwargs):
> diff --git a/virtinst/guest.py b/virtinst/guest.py
> index bf4b70b..7ff7bd5 100644
> --- a/virtinst/guest.py
> +++ b/virtinst/guest.py
> @@ -844,7 +844,7 @@ class Guest(XMLBuilder):
> if self.features.apic == "default":
> self.features.apic = self._os_object.supports_apic(default)
> if self.features.pae == "default":
> - self.features.pae = self.conn.caps.supports_pae()
> + self.features.pae = self.capsinfo.guest.supports_pae()
>
> if (self.features.vmport == "default" and
> self.has_spice() and
>
Add self.capsinfo = None to Guest.__init__ with a comment that it's set via
Capabilities.build_virtinst_guest
Thanks,
Cole
More information about the virt-tools-list
mailing list