[virt-tools-list] [PATCH v2] virt-manager:Add spapr-vscsi disk type for pseries guest.

[Cole Robinson]

On 02/22/2012 05:13 AM, Li Zhang wrote:
> This patch adds spapr-vscsi disk type on disk GUI page.
> It provides an options to user to select the scsi disk
> which is based on spapr-vio address type for pseries guest.
> 
> Signed-off-by: Li Zhang <zhlcindy at linux.vnet.ibm.com>
> ---
>  src/virtManager/details.py |   18 ++++++++++++++++--
>  src/virtManager/domain.py  |    7 ++++++-
>  2 files changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/src/virtManager/details.py b/src/virtManager/details.py
> index 55ea957..b24c2db 100644
> --- a/src/virtManager/details.py
> +++ b/src/virtManager/details.py
> @@ -2791,6 +2791,7 @@ class vmmDetails(vmmGObjectUI):
>          ro = disk.read_only
>          share = disk.shareable
>          bus = disk.bus
> +        addr = disk.address.type
>          idx = disk.disk_bus_index
>          cache = disk.driver_cache
>          io = disk.driver_io
> @@ -2814,7 +2815,10 @@ class vmmDetails(vmmGObjectUI):
>          is_cdrom = (devtype == virtinst.VirtualDisk.DEVICE_CDROM)
>          is_floppy = (devtype == virtinst.VirtualDisk.DEVICE_FLOPPY)
>  
> -        pretty_name = prettyify_disk(devtype, bus, idx)
> +        if addr == "spapr-vio":
> +            pretty_name = "vSCSI %s" % idx
> +        else:
> +            pretty_name = prettyify_disk(devtype, bus, idx)
>  
>          self.widget("disk-source-path").set_text(path or "-")
>          self.widget("disk-target-type").set_text(pretty_name)
> @@ -2832,6 +2836,10 @@ class vmmDetails(vmmGObjectUI):
>          no_default = not self.is_customize_dialog
>  
>          self.populate_disk_bus_combo(devtype, no_default)
> +
> +        if addr == "spapr-vio":
> +            bus = "spapr-vscsi"
> +
>          self.set_combo_label("disk-bus", bus)
>          self.widget("disk-serial").set_text(serial or "")
>  
> @@ -3279,6 +3287,9 @@ class vmmDetails(vmmGObjectUI):
>              if self.vm.get_hv_type() in ["kvm", "test"]:
>                  buses.append(["sata", "SATA"])
>                  buses.append(["virtio", "Virtio"])
> +            if (self.vm.get_hv_type() == "kvm" and
> +                    self.vm.get_machtype() == "pseries"):
> +                buses.append(["spapr-vscsi", "sPAPR-vSCSI"])
>              if self.vm.conn.is_xen() or self.vm.get_hv_type() == "test":
>                  buses.append(["xen", "Xen"])
>  
> @@ -3364,7 +3375,10 @@ class vmmDetails(vmmGObjectUI):
>              elif devtype == "floppy":
>                  icon = "media-floppy"
>  
> -            label = prettyify_disk(devtype, bus, idx)
> +            if disk.address.type == "spapr-vio":
> +                label = "vSCSI %s" % idx
> +            else:
> +                label = prettyify_disk(devtype, bus, idx)
> 

Can you extend prettyify_disk to handle this case, rather than open coding it?

>              update_hwlist(HW_LIST_TYPE_DISK, disk, label, icon)
>  
> diff --git a/src/virtManager/domain.py b/src/virtManager/domain.py
> index 7f382e0..911a210 100644
> --- a/src/virtManager/domain.py
> +++ b/src/virtManager/domain.py
> @@ -581,13 +581,18 @@ class vmmDomain(vmmLibvirtObject):
>          def change(editdev):
>              oldprefix = editdev.get_target_prefix()[0]
>              oldbus = editdev.bus
> -            editdev.bus = newval
>  

Due to some UI weirdness with default values in the 'customize before install'
wizard, we want to keep this part here.

I'd say move the below logic out of domain.py and into
details.py:config_disk_apply. Change domain.py:define_disk_bus to take an
optional addrstr parameter. Then this code will end up like

oldbus = editdev.bus

Hmm, I'd rather keep this logic out of the domain class. Can you move this
special casing to details.py, in config_disk_apply? You'll probably need to
adjust define_disk_bus to take an extra address type parameter, which in all
other cases will be None.

Thanks,
Cole

>              if oldbus == newval:
>                  return
>  
>              editdev.address.clear()
>  
> +            if newval == "spapr-vscsi":
> +                editdev.bus = "scsi"
> +                editdev.set_address("spapr-vio")
> +            else:
> +                editdev.bus = newval
> +
>              if oldprefix == editdev.get_target_prefix()[0]:
>                  return
>