On 03/29/2012 03:44 AM, Li Zhang wrote:
> From: Qing Lin <qinglbj@xxxxxxxxxxxxxxxxxx>
>
> When installing OS from cdrom, we cannot change its address type
> to spapr-vio (Power support bus type) through GUI. So, it needs
> to change default cdrom disk type into vSCSI on pSeries machine.
> All the vSCSI disks share one sPAPR-vSCSI controller. So,
> when the first vSCSI disk is added, sPAPR-vSCSI controller
> should be added, if sPAPR-vSCSI controller is already existed,
> it's not necessary to add it.
>
> Signed-off-by: Qing Lin <qinglbj@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Li Zhang <zhlcindy@xxxxxxxxxxxxxxxxxx>
> ---
> tests/xmlconfig-xml/boot-many-devices.xml | 12 ++++++++--
> tests/xmlconfig.py | 4 +++
> virtinst/Guest.py | 28 ++++++++++++++++----------
> 3 files changed, 30 insertions(+), 16 deletions(-)
>
> diff --git a/tests/xmlconfig-xml/boot-many-devices.xml b/tests/xmlconfig-xml/boot-many-devices.xml
> index 043e291..58107be 100644
> --- a/tests/xmlconfig-xml/boot-many-devices.xml
> +++ b/tests/xmlconfig-xml/boot-many-devices.xml
> @@ -39,9 +39,12 @@
> <source file='/default-pool/testvol1.img'/>
> <target dev='sdb' bus='scsi'/>
> </disk>
> - <controller type='scsi' index='0'>
> - <address type='spapr-vio'/>
> - </controller>
> + <disk type='file' device='cdrom'>
> + <driver name='qemu' type='qcow2'/>
> + <source file='/default-pool/testvol2.img'/>
> + <target dev='sdc' bus='scsi'/>
> + <readonly/>
> + </disk>
> <controller type='ide' index='3'/>
> <controller type='virtio-serial' index='0' ports='32' vectors='17'/>
> <interface type='network'>
> @@ -93,6 +96,9 @@
> </source>
> </hostdev>
> <watchdog model='ib700' action='none'/>
> + <controller type='scsi' index='0'>
> + <address type='spapr-vio'/>
> + </controller>
> </devices>
> <seclabel type='static' model='selinux'>
> <label>foolabel</label>
> diff --git a/tests/xmlconfig.py b/tests/xmlconfig.py
> index 1dd447d..f237fb2 100644
> --- a/tests/xmlconfig.py
> +++ b/tests/xmlconfig.py
> @@ -716,6 +716,10 @@ class TestXMLConfig(unittest.TestCase):
> bus="scsi", driverName="qemu")
> d3.address.type = "spapr-vio"
> g.disks.append(d3)
> + d4 = VirtualDisk(conn=g.conn, path="/default-pool/testvol2.img", device="cdrom",
> + bus="scsi", driverName="qemu")
> + d4.address.type = "spapr-vio"
> + g.disks.append(d4)
>
> # Controller devices
> c1 = VirtualController.get_class_for_type(VirtualController.CONTROLLER_TYPE_IDE)(g.conn)
> diff --git a/virtinst/Guest.py b/virtinst/Guest.py
> index cd529aa..3d69b96 100644
> --- a/virtinst/Guest.py
> +++ b/virtinst/Guest.py
> @@ -870,21 +870,23 @@ class Guest(XMLBuilderDomain.XMLBuilderDomain):
> finally:
> if origpath:
> dev.path = origpath
> - def get_vscsi_ctrl_xml():
> - vscsi_class = virtinst.VirtualController.get_class_for_type(
> - virtinst.VirtualController.CONTROLLER_TYPE_SCSI)
> - ctrl = vscsi_class(self.conn)
> - ctrl.set_address("spapr-vio")
> - return ctrl.get_xml_config()
> -
> xml = self._get_emulator_xml()
> # Build XML
> + vscsi_need_add = False
> + vscsi_is_exist = False
> for dev in devs:
> xml = _util.xml_append(xml, get_dev_xml(dev))
> - if (dev.address.type == "spapr-vio" and
> - dev.virtual_device_type == virtinst.VirtualDevice.VIRTUAL_DEV_DISK):
> - xml = _util.xml_append(xml, get_vscsi_ctrl_xml())
> -
> + if (dev.virtual_device_type == virtinst.VirtualDevice.VIRTUAL_DEV_DISK and
> + dev.address.type == "spapr-vio"):
> + vscsi_need_add = True
> + if (dev.get_virtual_device_type() == VirtualDevice.VIRTUAL_DEV_CONTROLLER and
> + dev.address.type == "spapr-vio"):
> + vscsi_is_exist = True
> + if (vscsi_need_add and not vscsi_is_exist):
> + ctrl = virtinst.VirtualController.get_class_for_type(virtinst.VirtualController.CONTROLLER_TYPE_SCSI)(self.conn)
> + ctrl.set_address("spapr-vio")
> + self.add_device(ctrl)
> + xml = _util.xml_append(xml, ctrl.get_xml_config())
> return xml
>
Hmm, this is getting a little out of hand. We wouldn't need any of this if
libvirt was adding an implicit controller for us. Which I think is the correct
way forward here, since libvirt does it for just about everything else.
In fact I should have just rejected the original patch that changed this function.
Ideally what we would do here is just specify a cdrom device like
<disk type='file' device='cdrom'>
<source file='foobar'/>
</disk>
and libvirt could generate a device target, and fill in the default bus value.
Then users/apps wouldn't need to know the supported buses for each
hv+arch+machine combination. But that's for another day. Until then, the rest
of the patch is fine.
- Cole
> def _get_emulator_xml(self):
> @@ -1503,6 +1505,10 @@ class Guest(XMLBuilderDomain.XMLBuilderDomain):
> else:
> if self.installer.is_hvm():
> disk.bus = "ide"
> + if (self.installer.type == "kvm" and
> + self.installer.machine == "pseries"):
> + disk.bus = "scsi"
> + disk.set_address("spapr-vio")
> elif self.installer.is_xenpv():
> disk.bus = "xen"
> used_targets.append(disk.generate_target(used_targets))
