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

[Giuseppe Scrivano]

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


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

Thanks,
Giuseppe