[virt-tools-list] [RFC PATCH] Add machine type option in virt-manager

[Cole Robinson]

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 at linux.vnet.ibm.com>
> ---
>  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
>