[virt-tools-list] [virt-manager PATCH v2]RFC: addhardware: Add controller options to addhw wizard

Cole Robinson crobinso at redhat.com
Wed Apr 9 15:20:31 UTC 2014


On 04/09/2014 05:46 AM, Chen Hanxiao wrote:
> Add controller options to addhw wizard.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1076607
> 
> Signed-off-by: Chen Hanxiao <chenhanxiao at cn.fujitsu.com>
> ---
> v2: move ui 'controller' after 'disk'
>     hide model label and combo when controller doesn't have model option.
>     use same controller models as details shows
>     disable adding pci controller
> 
>  ui/addhardware.ui          | 137 ++++++++++++++++++++++++++++++++++++---------
>  virtManager/addhardware.py | 119 ++++++++++++++++++++++++++++++++++-----
>  2 files changed, 214 insertions(+), 42 deletions(-)
> 

> diff --git a/virtManager/addhardware.py b/virtManager/addhardware.py
> index bd999cc..fb112da 100644
> --- a/virtManager/addhardware.py
> +++ b/virtManager/addhardware.py
> @@ -44,20 +44,21 @@ from virtManager.addstorage import vmmAddStorage
>  
>  PAGE_ERROR = 0
>  PAGE_DISK = 1
> -PAGE_NETWORK = 2
> -PAGE_INPUT = 3
> -PAGE_GRAPHICS = 4
> -PAGE_SOUND = 5
> -PAGE_HOSTDEV = 6
> -PAGE_CHAR = 7
> -PAGE_VIDEO = 8
> -PAGE_WATCHDOG = 9
> -PAGE_FILESYSTEM = 10
> -PAGE_SMARTCARD = 11
> -PAGE_USBREDIR = 12
> -PAGE_TPM = 13
> -PAGE_RNG = 14
> -PAGE_PANIC = 15
> +PAGE_CONTROLLER = 2
> +PAGE_NETWORK = 3
> +PAGE_INPUT = 4
> +PAGE_GRAPHICS = 5
> +PAGE_SOUND = 6
> +PAGE_HOSTDEV = 7
> +PAGE_CHAR = 8
> +PAGE_VIDEO = 9
> +PAGE_WATCHDOG = 10
> +PAGE_FILESYSTEM = 11
> +PAGE_SMARTCARD = 12
> +PAGE_USBREDIR = 13
> +PAGE_TPM = 14
> +PAGE_RNG = 15
> +PAGE_PANIC = 16
>  

Can you change this to use the range() trick like we use in details.py for the
EDIT_* bits?

>  
>  class vmmAddHardware(vmmGObjectUI):
> @@ -111,6 +112,8 @@ class vmmAddHardware(vmmGObjectUI):
>              "on_rng_type_changed": self.change_rng,
>              "on_rng_backend_mode_changed": self.change_rng,
>              "on_rng_backend_type_changed": self.change_rng,
> +
> +            "on_controller_type_changed": self.populate_controller_model,
>          })
>          self.bind_escape_key_close()
>  
> @@ -312,6 +315,16 @@ class vmmAddHardware(vmmGObjectUI):
>          combo = self.widget("panic-type")
>          self.build_panic_address_type(combo)
>  
> +        # Controller widgets
> +        combo = self.widget("controller-type")
> +        target_model = Gtk.ListStore(str, str)
> +        combo.set_model(target_model)
> +        uiutil.set_combo_text_column(combo, 1)
> +        combo = self.widget("controller-model")
> +        target_model = Gtk.ListStore(str, str)
> +        combo.set_model(target_model)
> +        uiutil.set_combo_text_column(combo, 1)
> +
>          # Available HW options
>          is_local = not self.conn.is_remote()
>          is_storage_capable = self.conn.is_storage_capable()
> @@ -331,6 +344,7 @@ class vmmAddHardware(vmmGObjectUI):
>  
>          add_hw_option("Storage", "drive-harddisk", PAGE_DISK, have_storage,
>                        have_storage and storage_tooltip or None)
> +        add_hw_option("Controller", "device_pci", PAGE_CONTROLLER, True, None)
>          add_hw_option("Network", "network-idle", PAGE_NETWORK, True, None)
>          add_hw_option("Input", "input-mouse", PAGE_INPUT, self.vm.is_hvm(),
>                        _("Not supported for this guest type."))
> @@ -454,6 +468,9 @@ class vmmAddHardware(vmmGObjectUI):
>          # Panic device params
>          self.widget("panic-iobase").set_text("0x505")
>  
> +        # Controller device params
> +        self.populate_controller_type()
> +
>          self.set_hw_selection(0)
>  
>  
> @@ -812,6 +829,44 @@ class vmmAddHardware(vmmGObjectUI):
>          if not create:
>              format_list.get_child().set_text("")
>  
> +    def populate_controller_type(self):
> +        widget = self.widget("controller-type")
> +        model = widget.get_model()
> +        model.clear()
> +
> +        for t in VirtualController.TYPES:
> +            if t == VirtualController.TYPE_PCI:
> +                continue
> +            model.append([t, VirtualController.pretty_type(t)])
> +
> +        if len(model) > 0:
> +            widget.set_active(0)
> +
> +    def populate_controller_model(self, src):
> +        ignore = src
> +
> +        controller_type = self.get_config_controller_type()
> +        modellist = self.widget("controller-model")
> +        modellabel = self.widget("controller-model-label")
> +        modellist.set_visible(True)
> +        modellabel.set_visible(True)
> +        model = modellist.get_model()
> +        model.clear()
> +
> +        if controller_type == "scsi":
> +            model.append([None, "Default"])
> +            model.append(["virtio-scsi", "VirtIO SCSI"])
> +        elif controller_type == "usb":
> +            model.append([None, "Default"])
> +            model.append(["ich9-ehci1", "USB 2"])
> +            model.append(["nec-xhci", "USB 3"])
> +        else:
> +            model.append([None, "None"])
> +            modellist.set_visible(False)
> +            modellabel.set_visible(False)
> +
> +        if len(model) > 0:
> +            modellist.set_active(0)
>  

We should share this with details.py. Make this into a staticmethod like
populate_network_model_combo. The actual hiding of the 'model' row will
probably need to be moved to the callers.

>      ########################
>      # get_config_* methods #
> @@ -879,6 +934,13 @@ class vmmAddHardware(vmmGObjectUI):
>          self.build_combo_with_values(combo, types,
>                  virtinst.VirtualPanicDevice.ADDRESS_TYPE_ISA)
>  
> +    def build_controller_type(self, combo):
> +        types = []
> +        for t in VirtualController.TYPES:
> +            types.append([t, VirtualController.pretty_type(t)])
> +        self.build_combo_with_values(combo, types)
> +
> +
>      def get_config_hardware_type(self):
>          row = self.get_hw_selection()
>          if not row:
> @@ -1007,6 +1069,13 @@ class vmmAddHardware(vmmGObjectUI):
>      def get_config_rng_backend_mode(self):
>          return uiutil.get_list_selection(self.widget("rng-backend-mode"), 0)
>  
> +    # CONTROLLER getters
> +    def get_config_controller_type(self):
> +        return uiutil.get_list_selection(self.widget("controller-type"), 0)
> +
> +    def get_config_controller_model(self):
> +        return uiutil.get_list_selection(self.widget("controller-model"), 0)
> +
>      ################
>      # UI listeners #
>      ################
> @@ -1112,6 +1181,8 @@ class vmmAddHardware(vmmGObjectUI):
>              return _("Error")
>          if page == PAGE_DISK:
>              return _("Storage")
> +        if page == PAGE_CONTROLLER:
> +            return _("Controller")
>          if page == PAGE_NETWORK:
>              return _("Network")
>          if page == PAGE_INPUT:
> @@ -1369,6 +1440,8 @@ class vmmAddHardware(vmmGObjectUI):
>              return True
>          elif page_num == PAGE_DISK:
>              return self.validate_page_storage()
> +        elif page_num == PAGE_CONTROLLER:
> +            return self.validate_page_controller()
>          elif page_num == PAGE_NETWORK:
>              return self.validate_page_network()
>          elif page_num == PAGE_INPUT:
> @@ -1724,6 +1797,24 @@ 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()
> +        self._dev = VirtualController(conn)
> +
> +        controller_type = self.get_config_controller_type()
> +        model = self.get_config_controller_model()
> +
> +        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)
> +
> +        setattr(self._dev, "type", controller_type)
> +        if model != "none":
> +            setattr(self._dev, "model", model)
> +

This actually won't do the correct thing for the USB 2 controller at least,
since USB 2 maps to multiple devices. This patch is already getting quite long
though. Can you split things up a bit?

patch 1) The range() fix I suggested
patch 2) Just the controller 'type' UI and code. Remove the 'model' handling
bits (well, you can keep the UI, just unconditionally hide it)
patch 3) Add the populate_controller_model function and use it in details.py
patch 4) Add the 'model' UI to addhardware, and handle the multi device USB issue.

patch 1 + 2 can be applied first while we figure out the details for patch 3 + 4.

- Cole




More information about the virt-tools-list mailing list