On 03/29/2012 03:44 AM, Li Zhang wrote:
> From: Qing Lin <qinglbj@xxxxxxxxxxxxxxxxxx>
>
> 1.Add "vSCSI disk" and "vSCSI cdrom" option into device type on
> "add storage page".
> 2.Add sPAPR-vSCSI controller when it's needed.
> When vSCSI controller doesn't exist,there are two situations to
> add it:one is when adding a vSCSI disk, another is when
> an existing disk is changed into sPAPR-vSCSI bus type.
> 3.Fix Disk Lable text on details page.
>
> Signed-off-by: Qing Lin <qinglbj@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Li Zhang <zhlcindy@xxxxxxxxxxxxxxxxxx>
> ---
> src/virtManager/addhardware.py | 30 ++++++++++++++++++++++++++++++
> src/virtManager/details.py | 33 +++++++++++++++++++++++++++------
> 2 files changed, 57 insertions(+), 6 deletions(-)
>
> diff --git a/src/virtManager/addhardware.py b/src/virtManager/addhardware.py
> index 50852ed..f7b3b3f 100644
> --- a/src/virtManager/addhardware.py
> +++ b/src/virtManager/addhardware.py
> @@ -535,6 +535,9 @@ class vmmAddHardware(vmmGObjectUI):
> add_dev("ide", virtinst.VirtualDisk.DEVICE_CDROM, "IDE cdrom")
> add_dev("fdc", virtinst.VirtualDisk.DEVICE_FLOPPY, "Floppy disk")
>
> + if (self.vm.get_machtype() == "pseries"):
> + add_dev("spapr-vscsi", virtinst.VirtualDisk.DEVICE_DISK, "vSCSI disk")
> + add_dev("spapr-vscsi", virtinst.VirtualDisk.DEVICE_CDROM, "vSCSI cdrom")
Does a pseries machine really support IDE and all these other options? If not,
might as well limit the device list to the actually supported values while you
are here.
Also, I think you should add an extra value to the disk type combo, storing
the disk address. So for these spapr devices, you'd have an add_dev call like
add_dev("scsi", virtinst.VirtualDisk.DEVICE_DISK, "vSCSI disk",
address="spapr-vscsi")
All other devices wouldn't set address, and itll just default to None. Then
you can avoid all the conditionalizing later on, and just do
if address:
dev.set_address(address)
We should have done that in details.py as well
> if self.vm.rhel6_defaults():
> add_dev("scsi", virtinst.VirtualDisk.DEVICE_DISK, "SCSI disk")
> add_dev("usb", virtinst.VirtualDisk.DEVICE_DISK, "USB disk")
> @@ -1176,6 +1179,25 @@ class vmmAddHardware(vmmGObjectUI):
> if not res:
> return (False, None)
>
> + if self.vm._guest:
Why this check? The fact that you are accessing a private variable (as denoted
by the leading underscore) is an indication something is wrong here.
> + if (self._dev.get_virtual_device_type() == VirtualDevice.VIRTUAL_DEV_DISK and
> + self._dev.address.type == "spapr-vio"):
> + vscsi_need_add = True
> + for dev in self.vm._guest._devices:
> + if (dev.get_virtual_device_type() == VirtualDevice.VIRTUAL_DEV_CONTROLLER and
> + dev.address.type == "spapr-vio"):
> + vscsi_need_add = False
> + break
> + if vscsi_need_add:
> + ctrl = virtinst.VirtualController.get_class_for_type(
> + virtinst.VirtualController.CONTROLLER_TYPE_SCSI)(self.vm._guest.conn)
> + ctrl.set_address("spapr-vio")
> + try:
> + self.vm.add_device(ctrl)
> + except Exception, e:
> + self.err.show_err(_("Error adding device: %s" % str(e)))
> + return (True, None)
> +
libvirt needs to handle this for us, as I mentioned in the virtinst patch
review. Until then, NACK to this bit.
> # Alter persistent config
> try:
> self.vm.add_device(self._dev)
> @@ -1264,6 +1286,11 @@ class vmmAddHardware(vmmGObjectUI):
> if do_use:
> diskpath = ideal
>
> + is_vscsi = False
> + if bus == "spapr-vscsi":
> + bus = "scsi"
> + is_vscsi = True
> +
> disk = virtinst.VirtualDisk(conn=self.conn.vmm,
> path=diskpath,
> size=disksize,
> @@ -1274,6 +1301,9 @@ class vmmAddHardware(vmmGObjectUI):
> driverCache=cache,
> format=fmt)
>
> + if is_vscsi:
> + disk.set_address("spapr-vio")
> +
> if not fmt:
> fmt = self.config.get_storage_format()
> if (self.is_default_storage() and
> diff --git a/src/virtManager/details.py b/src/virtManager/details.py
> index 5395087..368b444 100644
> --- a/src/virtManager/details.py
> +++ b/src/virtManager/details.py
> @@ -2217,6 +2217,22 @@ class vmmDetails(vmmGObjectUI):
> if bus == "spapr-vscsi":
> bus = "scsi"
> addr = "spapr-vio"
> + vscsi_need_add = True
> + if self.vm._guest:
> + for dev in self.vm._guest._devices:
> + if (dev.get_virtual_device_type() == virtinst.VirtualDevice.VIRTUAL_DEV_CONTROLLER and
> + dev.address.type == "spapr-vio"):
> + vscsi_need_add = False
> + break
> + if vscsi_need_add:
> + ctrl = virtinst.VirtualController.get_class_for_type(
> + virtinst.VirtualController.CONTROLLER_TYPE_SCSI)(self.vm._guest.conn)
> + ctrl.set_address("spapr-vio")
> + try:
> + self.vm.add_device(ctrl)
> + except Exception, e:
> + self.err.show_err(_("Error adding device: %s" % str(e)))
> + return (True, None)
> add_define(self.vm.define_disk_bus, dev_id_info, bus, addr)
>
> return self._change_config_helper(df, da, hf, ha)
> @@ -2806,7 +2822,6 @@ 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
> @@ -2830,9 +2845,11 @@ class vmmDetails(vmmGObjectUI):
> is_cdrom = (devtype == virtinst.VirtualDisk.DEVICE_CDROM)
> is_floppy = (devtype == virtinst.VirtualDisk.DEVICE_FLOPPY)
>
> - if addr == "spapr-vio":
> - bus = "spapr-vscsi"
>
> + if (self.vm.get_hv_type() == "kvm" and
> + self.vm.get_machtype() == "pseries"):
> + if bus == "scsi":
> + bus = "spapr-vscsi"
> pretty_name = prettyify_disk(devtype, bus, idx)
>
> self.widget("disk-source-path").set_text(path or "-")
> @@ -3289,6 +3306,9 @@ class vmmDetails(vmmGObjectUI):
> buses.append(["ide", "IDE"])
> if self.vm.rhel6_defaults():
> buses.append(["scsi", "SCSI"])
> + if (self.vm.get_hv_type() == "kvm" and
> + self.vm.get_machtype() == "pseries"):
> + buses.append(["spapr-vscsi", "sPAPR-vSCSI"])
> else:
> if self.vm.is_hvm():
> buses.append(["ide", "IDE"])
> @@ -3386,9 +3406,10 @@ class vmmDetails(vmmGObjectUI):
> elif devtype == "floppy":
> icon = "media-floppy"
>
> - if disk.address.type == "spapr-vio":
> - bus = "spapr-vscsi"
> -
> + if (self.vm.get_hv_type() == "kvm" and
> + self.vm.get_machtype() == "pseries"):
> + if bus == "scsi":
> + bus = "spapr-vscsi"
> label = prettyify_disk(devtype, bus, idx)
>
> update_hwlist(HW_LIST_TYPE_DISK, disk, label, icon)
This should all be changed to explicity pass the address type to prettify_disk
as well. Then it doesn't even matter if we are a pseries machine, if we see
bus == scsi and addresstype == spapr-vscsi, we know its a vscsi disk.
Thanks,
Cole
