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

[Li Zhang]

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