[virt-tools-list] [virt-manager PATCH 2/4] addhardware: addcontroller UI

[Chen Hanxiao]

On 04/16/2014 08:28 PM, Giuseppe Scrivano wrote:
> Chen Hanxiao <chenhanxiao at cn.fujitsu.com> writes:
>
>> This patch will enable adding controller
>> without model.
>>
>> Signed-off-by: Chen Hanxiao <chenhanxiao at cn.fujitsu.com>
>> ---
>>   ui/addhardware.ui          | 137 ++++++++++++++++++++++++++++++++++++---------
>>   virtManager/addhardware.py |  76 ++++++++++++++++++++++++-
>>   2 files changed, 184 insertions(+), 29 deletions(-)
>>
>> diff --git a/virtManager/addhardware.py b/virtManager/addhardware.py
>> index 39ec63f..b22a339 100644
>> --- a/virtManager/addhardware.py
>> +++ b/virtManager/addhardware.py
>> @@ -1725,6 +1774,31 @@ class vmmAddHardware(vmmGObjectUI):
>>           except Exception, e:
>>               return self.err.val_err(_("Panic device parameter error"), e)
>>   
>> +    def validate_page_controller(self):
>> +        conn = self.conn.get_backend()
>> +        controller_type = self.get_config_controller_type()
>> +        model = self.get_config_controller_model()
>> +        self._dev = VirtualController(conn)
>> +        # FIXME
>> +        model = "none"
>> +
>> +        if model == "ich9-ehci1":
>> +            # FIXME: for USB 2
>> +            pass
>> +        else:
> I see you've fixed it in 4/4 but, can't we just drop the unimplemented
> part and leave only what now is under the "else" branch?  We get rid of
> two FIXME, even if temporary ones :-)
>
Forgive my laziness :)
v2 will only keep "else branch".

>> +            controllers = self.vm.get_controller_devices()
>> +            controller_num = [x for x in controllers if
>> +                    (x.type == controller_type)]
>> +            if len(controller_num) > 0:
>> +                index_new = max([x.index for x in controller_num]) + 1
>> +                setattr(self._dev, "index", index_new)
> "self._dev.index = index_new" should have the same effect.
>
>
>> +            setattr(self._dev, "type", controller_type)
>> +            if model != "none":
>> +                if model == "default":
>> +                    model = None
>> +                setattr(self._dev, "model", model)
> same here.
Nice idea, v2 will improve.

Thanks,

- Chen
>
> Thanks,
> Giuseppe
>
> _______________________________________________
> virt-tools-list mailing list
> virt-tools-list at redhat.com
> https://www.redhat.com/mailman/listinfo/virt-tools-list
>