[virt-tools-list] [virt-manager v3]Add virtio-scsi disk bus option

[Cole Robinson]

On 12/03/2012 05:25 AM, Chen Hanxiao wrote:
> From: ChenHanxiao <chenhanxiao at cn.fujitsu.com>
> 
> This patch will add virtio-scsi bus option on "Add New Virtual
> Hardware" GUI page. It will support users to add a virtual disk
> using SCSI bus with a controller model virtio-scsi.
> If there is no SCSI controller existed, a new SCSI controller by
> model 'virtio-scsi' will be added automatically.
> 
> Signed-off-by: ChenHanxiao <chenhanxiao at cn.fujitsu.com>
> 
> diff --git a/src/virtManager/addhardware.py b/src/virtManager/addhardware.py
> index 4d894eb..f0b8319 100644
> --- a/src/virtManager/addhardware.py
> +++ b/src/virtManager/addhardware.py
> @@ -34,6 +34,7 @@ import virtManager.uihelpers as uihelpers
>  from virtManager.asyncjob import vmmAsyncJob
>  from virtManager.storagebrowse import vmmStorageBrowser
>  from virtManager.baseclass import vmmGObjectUI
> +from virtinst.VirtualController import VirtualControllerSCSI    
>  
>  PAGE_ERROR = 0
>  PAGE_DISK = 1
> @@ -539,6 +540,8 @@ class vmmAddHardware(vmmGObjectUI):
>          if self.vm.get_hv_type() == "kvm":
>              add_dev("sata", virtinst.VirtualDisk.DEVICE_DISK, "SATA disk")
>              add_dev("virtio", virtinst.VirtualDisk.DEVICE_DISK, "Virtio disk")
> +            add_dev("virtio-scsi", virtinst.VirtualDisk.DEVICE_DISK, 
> +                "Virtio SCSI disk")
>          if self.conn.is_xen():
>              add_dev("xen", virtinst.VirtualDisk.DEVICE_DISK, "Virtual disk")
>  
> @@ -1151,10 +1154,16 @@ class vmmAddHardware(vmmGObjectUI):
>  
>          self._dev.get_xml_config()
>          logging.debug("Adding device:\n" + self._dev.get_xml_config())
> +        if self.has_controller():  
> +            if self._controller is not None:
> +                logging.debug("Adding controller:\n" 
> +                            + self._controller.get_xml_config())
>  
>          # Hotplug device
>          attach_err = False
>          try:
> +            if self.has_controller() and self._controller is not None:
> +                self.vm.attach_device(self._controller)    
>              self.vm.attach_device(self._dev)
>          except Exception, e:
>              logging.debug("Device could not be hotplugged: %s", str(e))
> @@ -1176,6 +1185,13 @@ class vmmAddHardware(vmmGObjectUI):
>                  return (False, None)
>  
>          # Alter persistent config
> +        if self.has_controller() and self._controller is not None:
> +            try:
> +                self.vm.add_device(self._controller)    
> +            except Excpetion, e:
> +                self.err.show_err(_("Error adding device: %s" % str(e)))
> +                return (True, None)
> +

Similar to the hotplug case, move the has_controller() bit into the same
exception block as the add_device() call below.

>          try:
>              self.vm.add_device(self._dev)
>          except Exception, e:
> @@ -1184,6 +1200,14 @@ class vmmAddHardware(vmmGObjectUI):
>  
>          return (False, None)
>  
> +    #check whether device has attribute '_controller'
> +    def has_controller(self):  
> +        try:
> +            self._controller
> +        except AttributeError:
> +            return False
> +        else:
> +            return True
>  

This is overkill, just replace instances of has_controller() with
getattr(self, "_controller", None)

Though I have another recommendation below

>      ###########################
>      # Page validation methods #
> @@ -1222,6 +1246,10 @@ class vmmAddHardware(vmmGObjectUI):
>          bus, device = self.get_config_disk_target()
>          cache = self.get_config_disk_cache()
>          fmt = self.get_config_disk_format()
> +        model = None
> +        if bus == "virtio-scsi":
> +           bus = "scsi"
> +           model = "virtio-scsi"
>  
>          # Make sure default pool is running
>          if self.is_default_storage():
> @@ -1271,6 +1299,7 @@ class vmmAddHardware(vmmGObjectUI):
>                                          device=device,
>                                          bus=bus,
>                                          driverCache=cache,
> +                                        model=model,
>                                          format=fmt)
>  
>              if not fmt:
> @@ -1316,6 +1345,15 @@ class vmmAddHardware(vmmGObjectUI):
>          uihelpers.check_path_search_for_qemu(self.topwin,
>                                               self.conn, disk.path)
>  
> +        if disk.model == "virtio-scsi": 
> +            controllers = self.vm.get_controller_devices()
> +            controller = VirtualControllerSCSI(conn = self.conn.vmm)
> +            controller.set_model(disk.model)
> +            self._controller = controller
> +            for d in controllers:
> +                if disk.model == d.model:
> +                   self._controller = None
> +
>          self._dev = disk
>          return True
>  
> 

If we drop the virtinst patch, you can still make all this work. Just stick
the 'model' value in disk.vmm__model and the controller in
disk.vmm__controller. It's kinda hacky, but it's also one of the nice things
about python is that you can just monkey patch everything.

This will actually solve a real bug: if someone adds a virtio-scsi disk,
self._controller is set. Then if the user later tries to add an audio device,
self._controller is still set and we will attempt to add it again.

Thanks,
Cole