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
> 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
>
