[virt-tools-list] [PATCH] virt-manager:Add "vSCSI disk" and "vSCSI cdrom" option into Device type in "add hardware" page
Li Zhang
zhlcindy at linux.vnet.ibm.com
Fri Mar 30 12:18:58 UTC 2012
On 03/29/2012 10:21 PM, Cole Robinson wrote:
> On 03/29/2012 03:44 AM, Li Zhang wrote:
>> From: Qing Lin<qinglbj at linux.vnet.ibm.com>
>>
>> 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 at linux.vnet.ibm.com>
>> Signed-off-by: Li Zhang<zhlcindy at linux.vnet.ibm.com>
>> ---
>> 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.
No, a pseries machine doesn't support IDE bus.
we will limit the device list. Thanks :)
>
> 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
OK. Good suggestion. I will consider to modify it.
>
> if address:
> dev.set_address(address)
>
> We should have done that in details.py as well
It also needs to set the address here.
Because this is another device which is added by "add hardware".
>
>> 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.
Oh, you are right. I didn't realize it.
I will review whether this check is necessary.
We will modify it. :)
>
>> + 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.
OK. We can have some discussion about it.
>
>> # 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.
As I mentioned in the mail for virinst,
There is a little tricky here, we pass the disk's address type as
spapr-vio from virt-manager GUI, in fact the address type of the
disk is "drive". So here the address type we get, it is not
spapr-vio.
So we use pseries machine to specify that it is vscsi.
>
> Thanks,
> Cole
>
More information about the virt-tools-list
mailing list