Ensure non-empty libvirt <loader/> tag
When dynamic emulator changes domain boot mode, make sure that no empty <loader> tag is ever added into the domain configuration because it fails domain to boot later on. Whenever emulator's boot mode computation ends up in an empty boot loader path, just remove the entire <loader> tag from domain configuration effectively implying default boot mode. Change-Id: Icbf1eaa742da4ae6e9dd3ce4e6eb3a4c5a6f724f
This commit is contained in:
Ilya Etingof
parent
2dbb808dba
commit
3a91a559e8
|
|
@ -333,54 +333,51 @@ class LibvirtDriver(AbstractDriver):
|
|||
|
||||
raise error.FishyError(msg)
|
||||
|
||||
loaders = []
|
||||
|
||||
for os_element in tree.findall('os'):
|
||||
type_element = os_element.find('type')
|
||||
if type_element is None:
|
||||
os_arch = None
|
||||
else:
|
||||
os_arch = type_element.get('arch')
|
||||
|
||||
try:
|
||||
loader_path = self.BOOT_DEVICE_MAP[boot_mode][os_arch]
|
||||
|
||||
except KeyError:
|
||||
logger.warning('Boot loader is not configured for '
|
||||
'boot mode %s and OS architecture %s. '
|
||||
'Assuming no specific boot loader.',
|
||||
boot_mode, os_arch)
|
||||
loader_path = ''
|
||||
|
||||
# NOTE(etingof): here we collect all tree nodes to be
|
||||
# updated, but do not update them yet. The reason is to
|
||||
# make sure that previously configured boot loaders are
|
||||
# all present in our configuration, so we won't lose it
|
||||
# when setting ours.
|
||||
|
||||
for loader_element in os_element.findall('loader'):
|
||||
if loader_element.text not in self.KNOWN_BOOT_LOADERS:
|
||||
msg = ('Unknown boot loader path "%(path)s" in domain '
|
||||
'"%(identity)s" configuration encountered while '
|
||||
'setting boot mode "%(mode)s", system architecture '
|
||||
'"%(arch)s". Consider adding this loader path to '
|
||||
'emulator config.' % {'identity': identity,
|
||||
'mode': boot_mode,
|
||||
'arch': os_arch,
|
||||
'path': loader_element.text})
|
||||
raise error.FishyError(msg)
|
||||
|
||||
loaders.append((loader_element, loader_path))
|
||||
|
||||
if not loaders:
|
||||
msg = ('Can\'t set boot mode because no "os" elements are present '
|
||||
'in domain "%(identity)s" or not a single "os" element has '
|
||||
'"loader" configured.' % {'identity': identity})
|
||||
os_elements = tree.findall('os')
|
||||
if len(os_elements) != 1:
|
||||
msg = ('Can\'t set boot mode because "os" element must be present '
|
||||
'exactly once in domain "%(identity)s" '
|
||||
'configuration' % {'identity': identity})
|
||||
raise error.FishyError(msg)
|
||||
|
||||
for loader_element, loader_path in loaders:
|
||||
loader_element.set('type', loader_type)
|
||||
loader_element.text = loader_path
|
||||
os_element = os_elements[0]
|
||||
|
||||
type_element = os_element.find('type')
|
||||
if type_element is None:
|
||||
os_arch = None
|
||||
|
||||
else:
|
||||
os_arch = type_element.get('arch')
|
||||
|
||||
try:
|
||||
loader_path = self.BOOT_LOADER_MAP[boot_mode][os_arch]
|
||||
|
||||
except KeyError:
|
||||
logger.warning('Boot loader binary is not configured for '
|
||||
'boot mode %s and OS architecture %s. '
|
||||
'Assuming default boot loader for the domain.',
|
||||
boot_mode, os_arch)
|
||||
loader_path = None
|
||||
|
||||
for loader_element in os_element.findall('loader'):
|
||||
if loader_element.text not in self.KNOWN_BOOT_LOADERS:
|
||||
msg = ('Unknown boot loader path "%(path)s" in domain '
|
||||
'"%(identity)s" configuration encountered while '
|
||||
'setting boot mode "%(mode)s", system architecture '
|
||||
'"%(arch)s". Consider adding this loader path to '
|
||||
'emulator config.' % {'identity': identity,
|
||||
'mode': boot_mode,
|
||||
'arch': os_arch,
|
||||
'path': loader_element.text})
|
||||
raise error.FishyError(msg)
|
||||
|
||||
if loader_path:
|
||||
loader_element.set('type', loader_type)
|
||||
loader_element.text = loader_path
|
||||
|
||||
else:
|
||||
# NOTE(etingof): path must be present or element must be absent
|
||||
os_element.remove(loader_element)
|
||||
|
||||
with libvirt_open(self._uri) as conn:
|
||||
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ import uuid
|
|||
import libvirt
|
||||
from oslotest import base
|
||||
from six.moves import mock
|
||||
import xml.etree.ElementTree as ET
|
||||
|
||||
from sushy_tools.emulator.drivers.libvirtdriver import LibvirtDriver
|
||||
from sushy_tools import error
|
||||
|
|
@ -216,7 +217,30 @@ class LibvirtDriverTestCase(base.BaseTestCase):
|
|||
|
||||
@mock.patch('libvirt.open', autospec=True)
|
||||
@mock.patch('libvirt.openReadOnly', autospec=True)
|
||||
def test_set_boot_mode_unknown_path(self, libvirt_mock, libvirt_rw_mock):
|
||||
def test_set_boot_mode_known_loader(self, libvirt_mock, libvirt_rw_mock):
|
||||
with open('sushy_tools/tests/unit/emulator/domain.xml', 'r') as f:
|
||||
data = f.read()
|
||||
|
||||
conn_mock = libvirt_mock.return_value
|
||||
domain_mock = conn_mock.lookupByUUID.return_value
|
||||
domain_mock.XMLDesc.return_value = data
|
||||
|
||||
self.test_driver.set_boot_mode(self.uuid, 'Uefi')
|
||||
|
||||
conn_mock = libvirt_rw_mock.return_value
|
||||
xml_document = conn_mock.defineXML.call_args[0][0]
|
||||
tree = ET.fromstring(xml_document)
|
||||
for os_element in tree.findall('os'):
|
||||
loader_element = os_element.find('loader')
|
||||
self.assertEqual(
|
||||
'pflash', loader_element.get('type'))
|
||||
self.assertEqual(
|
||||
'/usr/share/OVMF/OVMF_CODE.fd',
|
||||
loader_element.text)
|
||||
|
||||
@mock.patch('libvirt.open', autospec=True)
|
||||
@mock.patch('libvirt.openReadOnly', autospec=True)
|
||||
def test_set_boot_mode_unknown_loader(self, libvirt_mock, libvirt_rw_mock):
|
||||
with open('sushy_tools/tests/unit/emulator/domain.xml', 'r') as f:
|
||||
data = f.read()
|
||||
|
||||
|
|
@ -230,6 +254,52 @@ class LibvirtDriverTestCase(base.BaseTestCase):
|
|||
error.FishyError, self.test_driver.set_boot_mode,
|
||||
self.uuid, 'Uefi')
|
||||
|
||||
@mock.patch('libvirt.open', autospec=True)
|
||||
@mock.patch('libvirt.openReadOnly', autospec=True)
|
||||
def test_set_boot_mode_no_os(self, libvirt_mock, libvirt_rw_mock):
|
||||
with open('sushy_tools/tests/unit/emulator/domain.xml', 'r') as f:
|
||||
data = f.read()
|
||||
|
||||
tree = ET.fromstring(data)
|
||||
for os_element in tree.findall('os'):
|
||||
tree.remove(os_element)
|
||||
|
||||
data = ET.tostring(tree)
|
||||
|
||||
conn_mock = libvirt_mock.return_value
|
||||
domain_mock = conn_mock.lookupByUUID.return_value
|
||||
domain_mock.XMLDesc.return_value = data
|
||||
|
||||
self.assertRaises(
|
||||
error.FishyError, self.test_driver.set_boot_mode,
|
||||
self.uuid, 'Uefi')
|
||||
|
||||
@mock.patch('libvirt.open', autospec=True)
|
||||
@mock.patch('libvirt.openReadOnly', autospec=True)
|
||||
def test_set_boot_mode_no_type(self, libvirt_mock, libvirt_rw_mock):
|
||||
with open('sushy_tools/tests/unit/emulator/domain.xml', 'r') as f:
|
||||
data = f.read()
|
||||
|
||||
tree = ET.fromstring(data)
|
||||
for os_element in tree.findall('os'):
|
||||
type_element = os_element.find('type')
|
||||
os_element.remove(type_element)
|
||||
|
||||
data = ET.tostring(tree)
|
||||
|
||||
conn_mock = libvirt_mock.return_value
|
||||
domain_mock = conn_mock.lookupByUUID.return_value
|
||||
domain_mock.XMLDesc.return_value = data
|
||||
|
||||
self.test_driver.set_boot_mode(self.uuid, 'Uefi')
|
||||
|
||||
conn_mock = libvirt_rw_mock.return_value
|
||||
xml_document = conn_mock.defineXML.call_args[0][0]
|
||||
tree = ET.fromstring(xml_document)
|
||||
for os_element in tree.findall('os'):
|
||||
# NOTE(etingof): should enforce default loader
|
||||
self.assertIsNone(os_element.find('loader'))
|
||||
|
||||
@mock.patch('libvirt.openReadOnly', autospec=True)
|
||||
def test_get_total_memory(self, libvirt_mock):
|
||||
conn_mock = libvirt_mock.return_value
|
||||
|
|
|
|||
Loading…
Reference in New Issue