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

Chen Hanxiao <chen_han_xiao@xxxxxxx> · Wed, 09 Apr 2014 23:55:47 +0800

On 04/09/2014 11:20 PM, Cole Robinson wrote:
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@xxxxxxxxxxxxxx>
---
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?

Good idea!

  
  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?

I've noticed that devicecontroller.py do some maps, but take 
refresh_controller_page
as reference, sorry for my fault.


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

Thanks for your suggestion, I'll try to do something tomorrow.

- Chen

_______________________________________________
virt-tools-list mailing list
virt-tools-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/virt-tools-list



_______________________________________________
virt-tools-list mailing list
virt-tools-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/virt-tools-list







