Re: [RFC PATCH] Add machine type option in virt-manager

Li Zhang <zhlcindy@xxxxxxxxxxxxxxxxxx> · Thu, 05 Jan 2012 17:45:49 +0800

On 12/23/2011 03:18 AM, Cole Robinson wrote:
On 12/15/2011 01:31 AM, Li Zhang wrote:
This patch adds the machine type combo callback
functions. When machine type is selected, and
click apply button, the details of overview page
are refreshed, and machine type value is set.


Thanks for the patch. A few comments:

- This doesn't include the .glade file changes

- This seems to apply on top of your hugepage patches. Since those might not
go in for a bit, please have this apply against current upstream.

- There is trailing whitespace, which pep8 should catch. See HACKING, or for
short just run make check-pylint to catch these issues.

- From examining the code, it looks like this shows the machine type combo
unconditionally for every VM that has a machine value in the XML. However I
don't think we ever want to show the machine value for x86, let alone allow
changing it. Please hide all the machine related UI if the guest is x86_64 or
i686.

Thanks,
Cole


Thanks for your comments.
I cook 2 new patches, including .glade file changes.
And set the visible of machine type UI as False for x86 architecture.

I do the test and it won't be visible on x86 platform.
I send the patches, which is on top of the latest version of
master. Please help review.

Thanks.
--Li

Signed-off-by: Li Zhang<zhlcindy@xxxxxxxxxxxxxxxxxx>
---
  src/virtManager/details.py |   33 ++++++++++++++++++++++++++++++++-
  src/virtManager/domain.py  |    7 +++++++
  2 files changed, 39 insertions(+), 1 deletions(-)

diff --git a/src/virtManager/details.py b/src/virtManager/details.py
index 92a13d8..e2fbf42 100644
--- a/src/virtManager/details.py
+++ b/src/virtManager/details.py
@@ -38,10 +38,11 @@ from virtManager import util as util
  import virtinst

  # Parameters that can be editted in the details window
-EDIT_TOTAL = 36
+EDIT_TOTAL = 37
  (EDIT_NAME,
  EDIT_ACPI,
  EDIT_APIC,
+EDIT_MACHTYPE,
  EDIT_CLOCK,
  EDIT_SECURITY,
  EDIT_DESC,
@@ -380,6 +381,7 @@ class vmmDetails(vmmGObjectUI):
              "on_overview_name_changed": (self.enable_apply, EDIT_NAME),
              "on_overview_acpi_changed": self.config_acpi_changed,
              "on_overview_apic_changed": self.config_apic_changed,
+            "on_machine_type_changed": (self.enable_apply, EDIT_MACHTYPE),
              "on_overview_clock_changed": (self.enable_apply, EDIT_CLOCK),
              "on_security_label_changed": (self.enable_apply, EDIT_SECURITY),
              "on_security_type_changed": self.security_type_changed,
@@ -733,6 +735,27 @@ class vmmDetails(vmmGObjectUI):
          clock_model.set_sort_column_id(0, gtk.SORT_ASCENDING)
          for offset in ["localtime", "utc"]:
              clock_model.append([offset])
+
+        caps = self.vm.conn.get_capabilities()
+        machines = []
+
+        if len(caps.guests)>  0:
+            for guest in caps.guests:
+                if len(guest.domains)>  0:
+                    for domain in guest.domains:
+                        machines = list(set(machines + domain.machines))
+
+        machtype_combo = self.widget("machine-type-combo")
+        machtype_model = gtk.ListStore(str)
+        machtype_combo.set_model(machtype_model)
+        text = gtk.CellRendererText()
+        machtype_combo.pack_start(text, True)
+        machtype_combo.add_attribute(text, 'text', 0)
+        machtype_model.set_sort_column_id(0, gtk.SORT_ASCENDING)
+
+        if len(machines)>  0:
+           for machine in machines:
+               machtype_model.append([machine])

          # Security info tooltips
          util.tooltip_wrapper(self.widget("security-static-info"),
@@ -1967,6 +1990,10 @@ class vmmDetails(vmmGObjectUI):
              clock = self.get_combo_label_value("overview-clock")
              add_define(self.vm.define_clock, clock)

+        if self.editted(EDIT_MACHTYPE):
+            machtype = self.get_combo_label_value("machine-type")
+            add_define(self.vm.define_machtype, machtype)
+
          if self.editted(EDIT_SECURITY):
              semodel = None
              setype = "static"
@@ -2532,6 +2559,7 @@ class vmmDetails(vmmGObjectUI):
          acpi = self.vm.get_acpi()
          apic = self.vm.get_apic()
          clock = self.vm.get_clock()
+        machtype = self.vm.get_machtype()

          # Hack in a way to represent 'default' acpi/apic for customize dialog
          self.widget("overview-acpi").set_active(bool(acpi))
@@ -2545,6 +2573,9 @@ class vmmDetails(vmmGObjectUI):
              clock = _("Same as host")
          self.set_combo_label("overview-clock", clock)

+        if machtype is not None:
+            self.set_combo_label("machine-type", machtype)
+
          # Security details
          semodel, ignore, vmlabel = self.vm.get_seclabel()
          caps = self.vm.conn.get_capabilities()
diff --git a/src/virtManager/domain.py b/src/virtManager/domain.py
index e29a1c0..6c30162 100644
--- a/src/virtManager/domain.py
+++ b/src/virtManager/domain.py
@@ -512,6 +512,11 @@ class vmmDomain(vmmLibvirtObject):
              guest.features["apic"] = newvalue
          return self._redefine_guest(change)

+    def define_machtype(self, newvalue):
+        def change(guest):
+            guest.installer.machine = newvalue
+        return self._redefine_guest(change)
+
      def define_clock(self, newvalue):
          def change(guest):
              guest.clock.offset = newvalue
@@ -840,6 +845,8 @@ class vmmDomain(vmmLibvirtObject):
          return self._get_guest().features["acpi"]
      def get_apic(self):
          return self._get_guest().features["apic"]
+    def get_machtype(self):
+        return self._get_guest().installer.machine
      def get_clock(self):
          return self._get_guest().clock.offset







