devices: disk: Always set a stub storage backend

This reworks the existing code to never have storage_backend = None,
instead carrying around a stub class, and resolving the actual
storage info when necessary. This makes the logic easier to follow.

Signed-off-by: Cole Robinson <crobinso@redhat.com>
This commit is contained in:
Cole Robinson 2020-01-29 06:18:15 -05:00
parent d46b89ec09
commit 04c0d48ef7
3 changed files with 74 additions and 42 deletions

View File

@ -1570,10 +1570,10 @@ class XMLParseTest(unittest.TestCase):
disk.validate()
disk.is_size_conflict()
disk.build_storage(None)
self.assertEqual(getattr(disk, "_storage_backend"), None)
self.assertTrue(getattr(disk, "_storage_backend").is_stub())
disk.set_backend_for_existing_path()
self.assertEqual(bool(getattr(disk, "_storage_backend")), True)
self.assertFalse(getattr(disk, "_storage_backend").is_stub())
with self.assertRaises(ValueError):
disk.validate()

View File

@ -291,7 +291,7 @@ class DeviceDisk(Device):
_XML_PROP_ORDER = [
"_type", "_device", "snapshot_policy",
"_xmltype", "_device", "snapshot_policy",
"driver_name", "driver_type",
"driver_cache", "driver_discard", "driver_detect_zeroes",
"driver_io", "error_policy",
@ -307,25 +307,23 @@ class DeviceDisk(Device):
Device.__init__(self, *args, **kwargs)
self._source_volume_err = None
self._storage_backend = None
self.storage_was_created = False
self._storage_backend = diskbackend.StorageBackendStub(
self.conn, self._get_xmlpath(), self._xmltype, self.driver_type)
#############################
# Public property-esque API #
#############################
def _get_path(self):
if not self._storage_backend:
xmlpath = self._get_xmlpath()
if xmlpath:
return xmlpath
self._set_default_storage_backend()
if (self._storage_backend.is_stub() and not
self._storage_backend.get_path()):
self._resolve_storage_backend()
return self._storage_backend.get_path()
def _set_path(self, newpath):
if (self._storage_backend and
self._storage_backend.will_create_storage()):
if self._storage_backend.will_create_storage():
xmlutil.raise_programming_error(None,
"Can't change disk path if storage creation info "
"has been set.")
@ -343,8 +341,8 @@ class DeviceDisk(Device):
# a _storage_backend to be initialized from the XML path. That
# will cause validate() to actually validate the path exists.
# We need this so addhw XML editing will still validate the disk path
if not self._storage_backend:
self._set_default_storage_backend()
if self._storage_backend.is_stub():
self._resolve_storage_backend()
def set_vol_object(self, vol_object, parent_pool):
log.debug("disk.set_vol_object: volxml=\n%s",
@ -362,18 +360,11 @@ class DeviceDisk(Device):
self._set_xmlpath(self.path)
def get_vol_object(self):
if not self._storage_backend:
return None
return self._storage_backend.get_vol_object()
def get_vol_install(self):
if not self._storage_backend:
return None
return self._storage_backend.get_vol_install()
def get_parent_pool(self):
if self.get_vol_install():
return self.get_vol_install().pool
return self._storage_backend.get_parent_pool()
def get_size(self):
return self._storage_backend.get_size()
@ -526,7 +517,7 @@ class DeviceDisk(Device):
def _get_default_type(self):
if self.source_pool or self.source_volume:
return DeviceDisk.TYPE_VOLUME
if self._storage_backend:
if not self._storage_backend.is_stub():
return self._storage_backend.get_dev_type()
if self.source_protocol:
return DeviceDisk.TYPE_NETWORK
@ -590,13 +581,13 @@ class DeviceDisk(Device):
# type, device, driver_name, driver_type handling
# These are all weirdly intertwined so require some special handling
def _get_type(self):
if self._type:
return self._type
if self._xmltype:
return self._xmltype
return self._get_default_type()
def _set_type(self, val):
self._type = val
self._xmltype = val
type = property(_get_type, _set_type)
_type = XMLProperty("./@type")
_xmltype = XMLProperty("./@type")
def _get_device(self):
if self._device:
@ -660,7 +651,7 @@ class DeviceDisk(Device):
# Validation assistance methods #
#################################
def _set_default_storage_backend(self):
def _resolve_storage_backend(self):
path = None
vol_object = None
parent_pool = None
@ -730,8 +721,7 @@ class DeviceDisk(Device):
If true, this disk needs storage creation parameters or things
will error.
"""
return (self._storage_backend and
not self._storage_backend.exists())
return not self._storage_backend.exists()
def validate(self):
if self.path is None:
@ -744,9 +734,6 @@ class DeviceDisk(Device):
return
if not self._storage_backend:
return
if (not self._storage_backend.will_create_storage() and
not self._storage_backend.exists()):
raise ValueError(
@ -762,8 +749,7 @@ class DeviceDisk(Device):
If storage doesn't exist (a non-existent file 'path', or 'vol_install'
was specified), we create it.
"""
if (not self._storage_backend or
not self._storage_backend.will_create_storage()):
if not self._storage_backend.will_create_storage():
return
meter = progress.ensure_meter(meter)
@ -785,8 +771,6 @@ class DeviceDisk(Device):
Non fatal conflicts (sparse disk exceeds available space) will
return (False, "description of collision")
"""
if not self._storage_backend:
return (False, None)
return self._storage_backend.is_size_conflict()
def is_conflict_disk(self):
@ -924,8 +908,8 @@ class DeviceDisk(Device):
def set_defaults(self, guest):
if not self._device:
self._device = self._get_device()
if not self._type:
self._type = self._get_default_type()
if not self._xmltype:
self._xmltype = self._get_default_type()
if not self.driver_name:
self.driver_name = self._get_default_driver_name()
if not self.driver_type:

View File

@ -15,6 +15,7 @@ import libvirt
from .logger import log
from .storage import StoragePool, StorageVolume
from . import xmlutil
def _lookup_vol_by_path(conn, path):
@ -389,15 +390,20 @@ class _StorageBase(object):
raise NotImplementedError()
def get_path(self):
raise NotImplementedError()
def is_stub(self):
return False
# Storage creation routines
def is_size_conflict(self):
raise NotImplementedError()
def create(self, progresscb):
raise NotImplementedError()
def will_create_storage(self):
raise NotImplementedError()
def create(self, progresscb):
ignore = progresscb # pragma: no cover
xmlutil.raise_programming_error(None,
"%s can't create storage" % self.__class__.__name__)
class _StorageCreator(_StorageBase):
"""
@ -627,6 +633,48 @@ class ManagedStorageCreator(_StorageCreator):
return self._vol_install.is_size_conflict()
class StorageBackendStub(_StorageBase):
"""
Class representing a storage path for a parsed XML disk, that we
don't want to do slow resolving of unless requested
"""
def __init__(self, conn, path, dev_type, driver_type):
_StorageBase.__init__(self, conn)
self._path = path
self._dev_type = dev_type
self._driver_type = driver_type
def get_path(self):
return self._path
def get_vol_object(self):
return None
def get_vol_xml(self):
return None
def get_parent_pool(self):
return None
def get_size(self):
return 0
def exists(self):
return True
def get_dev_type(self):
return self._dev_type
def get_driver_type(self):
return self._driver_type
def validate(self, disk):
ignore = disk
return
def get_vol_install(self):
return None
def is_size_conflict(self):
return (False, None)
def is_stub(self):
return True
def will_create_storage(self):
return False
class StorageBackend(_StorageBase):
"""
Class that carries all the info about any existing storage that
@ -643,8 +691,8 @@ class StorageBackend(_StorageBase):
self._path = None
if self._vol_object and not self._parent_pool:
raise RuntimeError(
"programming error: parent_pool must be specified")
xmlutil.raise_programming_error(None,
"parent_pool must be specified")
# Cached bits
self._vol_xml = None