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

[Cole Robinson]

Hi Chen, thanks for the patch! Sorry for the review delay, comments inline.

On 11/12/2012 03:12 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.
> In virt-manager, we will provide virtio-scsi disk XML for libvirt:
> 
> <disk type='block' device='disk'>
>   <driver name='qemu'/>
>   <source dev='/dev/sdb'/>
>   <target dev='sda' bus='scsi' model='virtio-scsi'/>
> </disk>
> 
> We provided tag 'model' for libvirt to determine which SCSI
> controller model to use.
> 
> Signed-off-by: ChenHanxiao <chenhanxiao at cn.fujitsu.com>
> ---
>  src/virtManager/addhardware.py |    7 +++++++
>  src/virtManager/details.py     |    6 +++++-
>  2 files changed, 12 insertions(+), 1 deletions(-)
> 
> diff --git a/src/virtManager/addhardware.py b/src/virtManager/addhardware.py
> index 4d894eb..9a7602c 100644
> --- a/src/virtManager/addhardware.py
> +++ b/src/virtManager/addhardware.py
> @@ -539,6 +539,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")

That last string is a disk label, so please make it something like _('Virtio
SCSI disk')

>          if self.conn.is_xen():
>              add_dev("xen", virtinst.VirtualDisk.DEVICE_DISK, "Virtual disk")
>  
> @@ -1222,6 +1224,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 +1277,7 @@ class vmmAddHardware(vmmGObjectUI):
>                                          device=device,
>                                          bus=bus,
>                                          driverCache=cache,
> +                                        model=model,
>                                          format=fmt)
>  
>              if not fmt:
> diff --git a/src/virtManager/details.py b/src/virtManager/details.py
> index d20e748..0e86648 100644
> --- a/src/virtManager/details.py
> +++ b/src/virtManager/details.py
> @@ -2219,7 +2219,11 @@ class vmmDetails(vmmGObjectUI):
>              if bus == "spapr-vscsi":
>                  bus = "scsi"
>                  addr = "spapr-vio"
> -            add_define(self.vm.define_disk_bus, dev_id_info, bus, addr)
> +            if bus == "virtio-scsi":
> +                bus = "scsi"
> +                model = "virtio-scsi"
> +            add_define(self.vm.define_disk_bus, dev_id_info, bus, 
> +                addr, model)
>  
>          return self._change_config_helper(df, da, hf, ha)
>  
> 

Does this actually work? By my reading this breaks all disk adding because
model is only defined for bus == "virtio-scsi", and define_disk_bus in
domain.py wasn't changed to accept the model parameter.

An easy way to test all this stuff is to do

  virt-manager --connect test:///default

That uses the libvirt stub driver which will allow you to add all manner of
devices to a fake guest and nothing will affect the local machine.

I've added a few commits to help with that, so make sure you rebase your
changes on latest virt-manager git.

- Cole