From 1f61fc1ecabb654f1859245d92fa15728a54bb78 Mon Sep 17 00:00:00 2001
From: Dmitry Tantsur <dtantsur@protonmail.com>
Date: Fri, 30 Oct 2020 16:02:27 +0100
Subject: [PATCH] Fix virtual media with q35 machines
This machine type does not have IDE controllers, don't try using them.
Change-Id: I0be094e25e5d72230c2d4db1b50eafc782530f1b
---
releasenotes/notes/q35-aea1a63ff2c7c72b.yaml | 4 ++
.../resources/systems/libvirtdriver.py | 15 +++++-
.../tests/unit/emulator/domain-q35.xml | 28 ++++++++++
.../resources/systems/test_libvirt.py | 53 +++++++++++++++++++
4 files changed, 99 insertions(+), 1 deletion(-)
create mode 100644 releasenotes/notes/q35-aea1a63ff2c7c72b.yaml
create mode 100644 sushy_tools/tests/unit/emulator/domain-q35.xml
diff --git a/releasenotes/notes/q35-aea1a63ff2c7c72b.yaml b/releasenotes/notes/q35-aea1a63ff2c7c72b.yaml
new file mode 100644
index 00000000..d7f46148
--- /dev/null
+++ b/releasenotes/notes/q35-aea1a63ff2c7c72b.yaml
@@ -0,0 +1,4 @@
+---
+fixes:
+ - |
+ No longer tries to use IDE controllers on q35 machines.
diff --git a/sushy_tools/emulator/resources/systems/libvirtdriver.py b/sushy_tools/emulator/resources/systems/libvirtdriver.py
index e7c3a3e5..8151abf3 100644
--- a/sushy_tools/emulator/resources/systems/libvirtdriver.py
+++ b/sushy_tools/emulator/resources/systems/libvirtdriver.py
@@ -842,6 +842,18 @@ class LibvirtDriver(AbstractSystemsDriver):
return image_path
+ def _default_controller(self, domain_tree):
+ os_element = domain_tree.find('os')
+ if os_element is not None:
+ type_element = os_element.find('type')
+ if type_element is not None:
+ machine = type_element.attrib.get('machine')
+ if machine and 'q35' in machine:
+ # No IDE support for newer q35 machine types
+ return 'sata'
+
+ return 'ide'
+
def _add_boot_image(self, domain, domain_tree, device,
boot_image, write_protected):
@@ -853,6 +865,8 @@ class LibvirtDriver(AbstractSystemsDriver):
'"%(identity)s" configuration' % {'identity': identity})
raise error.FishyError(msg)
+ controller_type = self._default_controller(domain_tree)
+
with libvirt_open(self._uri) as conn:
image_path = self._upload_image(domain, conn, boot_image)
@@ -865,7 +879,6 @@ class LibvirtDriver(AbstractSystemsDriver):
'Unknown device %s at %s' % (device, identity))
disk_elements = device_element.findall('disk')
- controller_type = 'ide'
for disk_element in disk_elements:
target_element = disk_element.find('target')
if target_element is None:
diff --git a/sushy_tools/tests/unit/emulator/domain-q35.xml b/sushy_tools/tests/unit/emulator/domain-q35.xml
new file mode 100644
index 00000000..4e792154
--- /dev/null
+++ b/sushy_tools/tests/unit/emulator/domain-q35.xml
@@ -0,0 +1,28 @@
+<domain type='qemu'>
+ <name>QEmu-fedora-i686</name>
+ <uuid>c7a5fdbd-cdaf-9455-926a-d65c16db1809</uuid>
+ <memory>219200</memory>
+ <currentMemory>219200</currentMemory>
+ <vcpu>2</vcpu>
+ <os>
+ <type arch='x86_64' machine='q35'>hvm</type>
+ <boot dev='cdrom'/>
+ <loader type='rom'/>
+ </os>
+ <devices>
+ <emulator>/usr/bin/qemu-system-x86_64</emulator>
+ <disk type='file' device='cdrom'>
+ <source file='/home/user/boot.iso'/>
+ <target dev='hdc'/>
+ <readonly/>
+ </disk>
+ <disk type='file' device='disk'>
+ <source file='/home/user/fedora.img'/>
+ <target dev='hda'/>
+ </disk>
+ <interface type='network'>
+ <source network='default'/>
+ </interface>
+ <graphics type='vnc' port='-1'/>
+ </devices>
+</domain>
diff --git a/sushy_tools/tests/unit/emulator/resources/systems/test_libvirt.py b/sushy_tools/tests/unit/emulator/resources/systems/test_libvirt.py
index 8ba846c0..119126bb 100644
--- a/sushy_tools/tests/unit/emulator/resources/systems/test_libvirt.py
+++ b/sushy_tools/tests/unit/emulator/resources/systems/test_libvirt.py
@@ -540,6 +540,59 @@ class LibvirtDriverTestCase(base.BaseTestCase):
self.assertEqual(1, conn_mock.defineXML.call_count)
self.assertIn(expected_disk, conn_mock.defineXML.call_args[0][0])
+ @mock.patch('sushy_tools.emulator.resources.systems.libvirtdriver'
+ '.os.stat', autospec=True)
+ @mock.patch('sushy_tools.emulator.resources.systems.libvirtdriver'
+ '.open')
+ @mock.patch('libvirt.open', autospec=True)
+ @mock.patch('libvirt.openReadOnly', autospec=True)
+ def test_set_boot_image_q35(self, libvirt_mock, libvirt_rw_mock,
+ open_mock, stat_mock):
+ with open('sushy_tools/tests/unit/emulator/domain-q35.xml', 'r') as f:
+ data = f.read()
+
+ conn_mock = libvirt_rw_mock.return_value
+ domain_mock = conn_mock.lookupByUUID.return_value
+ domain_mock.XMLDesc.return_value = data
+
+ pool_mock = conn_mock.storagePoolLookupByName.return_value
+
+ with open('sushy_tools/tests/unit/emulator/pool.xml', 'r') as f:
+ data = f.read()
+
+ pool_mock.XMLDesc.return_value = data
+
+ with mock.patch.object(
+ self.test_driver, 'get_power_state', return_value='Off'):
+ with mock.patch.object(
+ self.test_driver, 'get_boot_device', return_value=None):
+
+ self.test_driver.set_boot_image(
+ self.uuid, 'Cd', '/tmp/image.iso')
+
+ conn_mock = libvirt_rw_mock.return_value
+ pool_mock.listAllVolumes.assert_called_once_with()
+ stat_mock.assert_called_once_with('/tmp/image.iso')
+ pool_mock.createXML.assert_called_once_with(mock.ANY)
+
+ volume_mock = pool_mock.createXML.return_value
+ volume_mock.upload.assert_called_once_with(mock.ANY, 0, mock.ANY)
+
+ expected_disk = ('<disk device="cdrom" type="file">'
+ '<target bus="sata" dev="sdc" />'
+ '<address bus="0" controller="0" '
+ 'target="0" type="drive" unit="0" />')
+
+ # NOTE(rpittau): starting from Python 3.8 the tostring() function
+ # preserves the attribute order specified by the user.
+ if sys.version_info[1] >= 8:
+ expected_disk = ('<disk type="file" device="cdrom">'
+ '<target dev="sdc" bus="sata" />'
+ '<address type="drive" controller="0"'
+ ' bus="0" target="0" unit="0" />')
+ self.assertEqual(1, conn_mock.defineXML.call_count)
+ self.assertIn(expected_disk, conn_mock.defineXML.call_args[0][0])
+
@mock.patch('sushy_tools.emulator.resources.systems.libvirtdriver'
'.os.stat', autospec=True)
@mock.patch('sushy_tools.emulator.resources.systems.libvirtdriver'