devicedisk: Break apart set_create_storage

And break about the vol_install vs local clone storage creators. And
just generally delete a lot of code. The commit aint pretty but it works
and deletes a ton of hacks
This commit is contained in:
Cole Robinson 2014-12-05 21:09:26 -05:00
parent 7e2ea81a2c
commit cf0206a58c
6 changed files with 120 additions and 255 deletions

View File

@ -18,7 +18,7 @@
<devices>
<emulator>/usr/bin/qemu-kvm</emulator>
<disk type='file' device='disk'>
<source file='/tmp/virtinst-test1.img'/>
<source file='/dev/default-pool/default-vol'/>
<target dev='hda' bus='ide'/>
</disk>
<disk type='file' device='floppy'>

View File

@ -29,7 +29,7 @@
<readonly/>
</disk>
<disk type='file' device='disk'>
<source file='/tmp/virtinst-test2.img'/>
<source file='/dev/default-pool/default-vol'/>
<target dev='sdb' bus='scsi'/>
</disk>
<interface type='network'>

View File

@ -41,16 +41,6 @@ DISKPOOL = "/dev/disk-pool"
local_files = [FILE1, FILE2]
clonexml_dir = os.path.join(os.getcwd(), "tests/clone-xml")
clone_files = []
for tmpf in os.listdir(clonexml_dir):
black_list = ["managed-storage", "cross-pool", "force", "skip",
"fullpool"]
if tmpf.endswith("-out.xml"):
tmpf = tmpf[0:(len(tmpf) - len("-out.xml"))]
if tmpf not in clone_files and tmpf not in black_list:
clone_files.append(tmpf)
conn = utils.open_testdriver()
@ -115,32 +105,6 @@ class TestClone(unittest.TestCase):
outxml = utils.read_file(outfile)
utils.test_create(conn, outxml)
# Skip this test, since libvirt can add new XML elements to the defined
# XML (<video>) that make roundtrip a pain
def notestCloneGuestLookup(self):
"""Test using a vm name lookup for cloning"""
for base in clone_files:
infile = os.path.join(clonexml_dir, base + "-in.xml")
vm = None
try:
vm = conn.defineXML(utils.read_file(infile))
cloneobj = Cloner(conn)
cloneobj.original_guest = ORIG_NAME
cloneobj = self._default_clone_values(cloneobj)
self._clone_compare(cloneobj, base)
finally:
if vm:
vm.undefine()
def testCloneFromFile(self):
"""Test using files for input and output"""
for base in clone_files:
self._clone_helper(base)
def testRemoteNoStorage(self):
"""Test remote clone where VM has no storage that needs cloning"""
useconn = utils.open_test_remote()

View File

@ -136,12 +136,12 @@ class Cloner(object):
disk.path = path
disk.device = device
if path and not self.preserve_dest_disks:
# We fake storage creation params for now, but we will
# update it later. Just use any clone_path to make sure
# validation doesn't trip up
clone_path = "/foo/bar"
disk.set_create_storage(fake=True, clone_path=clone_path)
if (not self.preserve_dest_disks and
disk.wants_storage_creation()):
vol_install = VirtualDisk.build_vol_install(
self.conn, os.path.basename(disk.path),
disk.get_parent_pool(), .000001, False)
disk.set_vol_install(vol_install)
disk.validate()
disklist.append(disk)
except Exception, e:
@ -323,11 +323,6 @@ class Cloner(object):
_("Clone onto existing storage volume is not "
"currently supported: '%s'") % clone_disk.path)
# Sync 'size' between the two
size = orig_disk.get_size()
vol_install = None
clone_path = None
# Setup proper cloning inputs for the new virtual disks
if (orig_disk.get_vol_object() and
clone_disk.get_vol_install()):
@ -347,11 +342,11 @@ class Cloner(object):
# Deliberately don't sync input_vol params here
clone_vol_install.input_vol = orig_disk.get_vol_object()
vol_install = clone_vol_install
else:
clone_path = orig_disk.path
clone_disk.set_create_storage(
size=size, vol_install=vol_install, clone_path=clone_path)
clone_disk.set_vol_install(vol_install)
elif orig_disk.path:
clone_disk.set_local_disk_to_clone(orig_disk, self.clone_sparse)
clone_disk.validate()
@ -533,8 +528,7 @@ class Cloner(object):
newd.driver_type = disk.driver_type
newd.target = disk.target
if validate:
newd.set_create_storage(fake=True)
if newd.creating_storage() and disk.path is not None:
if newd.wants_storage_creation():
raise ValueError("Disk path '%s' does not exist." %
newd.path)
except Exception, e:

View File

@ -97,34 +97,6 @@ def _is_dir_searchable(uid, username, path):
return bool(re.search("user:%s:..x" % username, out))
def _make_storage_backend(conn, nomanaged, path, vol_object):
parent_pool = None
if (not vol_object and path and not nomanaged):
(vol_object, parent_pool) = diskbackend.manage_path(conn, path)
backend = diskbackend.StorageBackend(conn, path, vol_object, parent_pool)
return backend
def _make_storage_creator(conn, backend, vol_install, clone_path,
*creator_args):
parent_pool = backend.get_parent_pool()
if backend.exists(auto_check=False) and backend.path is not None:
if not clone_path:
return
if backend.path and not (vol_install or parent_pool or clone_path):
raise RuntimeError(_("Don't know how to create storage for "
"path '%s'. Use libvirt APIs to manage the parent directory "
"as a pool first.") % backend.path)
if not (backend.path or vol_install or parent_pool or clone_path):
return
return diskbackend.StorageCreator(conn, backend.path,
parent_pool, vol_install, clone_path, *creator_args)
class VirtualDisk(VirtualDevice):
virtual_device_type = VirtualDevice.VIRTUAL_DEV_DISK
@ -423,8 +395,44 @@ class VirtualDisk(VirtualDevice):
return (True, 0)
@staticmethod
def build_vol_install(*args, **kwargs):
return diskbackend.build_vol_install(*args, **kwargs)
def build_vol_install(conn, volname, poolobj, size, sparse,
fmt=None, backing_store=None):
"""
Helper for building a StorageVolume instance to pass to VirtualDisk
for eventual storage creation.
:param volname: name of the volume to be created
:param size: size in bytes
"""
from .storage import StorageVolume
if size is None:
raise ValueError(_("Size must be specified for non "
"existent volume '%s'" % volname))
logging.debug("Creating volume '%s' on pool '%s'",
volname, poolobj.name())
cap = (size * 1024 * 1024 * 1024)
if sparse:
alloc = 0
else:
alloc = cap
volinst = StorageVolume(conn)
volinst.pool = poolobj
volinst.name = volname
volinst.capacity = cap
volinst.allocation = alloc
volinst.backing_store = backing_store
if fmt:
if not volinst.supports_property("format"):
raise ValueError(_("Format attribute not supported for this "
"volume type"))
volinst.format = fmt
return volinst
@staticmethod
def num_to_target(num):
@ -485,8 +493,6 @@ class VirtualDisk(VirtualDevice):
self.__storage_backend = None
self._storage_creator = None
self.nomanaged = False
#############################
# Public property-esque API #
@ -509,8 +515,8 @@ class VirtualDisk(VirtualDevice):
self._set_xmlpath(self.path)
def set_vol_install(self, vol_install):
self._storage_creator = diskbackend.StorageCreator(self.conn,
None, None, vol_install, None, None, None, None, None)
self._storage_creator = diskbackend.ManagedStorageCreator(
self.conn, vol_install)
self._set_xmlpath(self.path)
def get_vol_object(self):
@ -655,68 +661,12 @@ class VirtualDisk(VirtualDevice):
self.__storage_backend = val
_storage_backend = property(_get_storage_backend, _set_storage_backend)
def set_create_storage(self, size=None, sparse=True,
fmt=None, vol_install=None,
clone_path=None, backing_store=None,
fake=False):
def set_local_disk_to_clone(self, disk, sparse):
"""
Function that sets storage creation parameters. If this isn't
called, we assume that no storage creation is taking place and
will error accordingly.
@size is in gigs
@fake: If true, make like we are creating storage but fail
if we ever asked to do so.
Set a path to manually clone (as in, not through libvirt)
"""
def _validate_path(p):
if p is None:
return
try:
d = VirtualDisk(self.conn)
d.path = p
# If this disk isn't managed, make sure we only perform
# non-managed lookup.
if (self._storage_creator or
(self.path and self._storage_backend.exists())):
d.nomanaged = not self.__managed_storage()
d.set_create_storage(fake=True)
d.validate()
except Exception, e:
raise ValueError(_("Error validating path %s: %s") % (p, e))
path = self.path
# Validate clone_path
if clone_path is not None:
clone_path = os.path.abspath(clone_path)
if backing_store is not None:
backing_store = os.path.abspath(backing_store)
if not fake:
_validate_path(clone_path)
_validate_path(backing_store)
if fake and size is None:
size = .000001
backend = _make_storage_backend(self.conn,
self.nomanaged, path, None)
creator_args = (backing_store, size, sparse, fmt)
creator = _make_storage_creator(self.conn, backend,
vol_install, clone_path,
*creator_args)
self._storage_creator = creator
if self._storage_creator:
self._storage_creator.fake = bool(fake)
self._set_xmlpath(self.path)
else:
if (vol_install or clone_path):
raise RuntimeError("Need storage creation but it "
"didn't happen.")
if fmt and self.driver_name == self.DRIVER_QEMU:
self.driver_type = fmt
self._storage_creator = diskbackend.CloneStorageCreator(self.conn,
self.path, disk.path, disk.get_size(), sparse)
def is_cdrom(self):
return self.device == self.DEVICE_CDROM
@ -729,8 +679,15 @@ class VirtualDisk(VirtualDevice):
return self.is_floppy() or self.is_cdrom()
def _change_backend(self, path, vol_object):
backend = _make_storage_backend(self.conn, self.nomanaged,
path, vol_object)
# User explicitly changed 'path', so try to lookup its storage
# object since we may need it
parent_pool = None
if path and not vol_object:
(vol_object, parent_pool) = diskbackend.manage_path(self.conn,
path)
backend = diskbackend.StorageBackend(self.conn, path,
vol_object, parent_pool)
self._storage_backend = backend
def sync_path_props(self):

View File

@ -151,45 +151,10 @@ def manage_path(conn, path):
return vol, pool
def build_vol_install(conn, volname, poolobj, size, sparse,
fmt=None, backing_store=None):
"""
Helper for building a StorageVolume instance to pass to VirtualDisk
for eventual storage creation.
:param volname: name of the volume to be created
:param size: size in bytes
"""
if size is None:
raise ValueError(_("Size must be specified for non "
"existent volume '%s'" % volname))
logging.debug("Creating volume '%s' on pool '%s'", volname, poolobj.name())
cap = (size * 1024 * 1024 * 1024)
if sparse:
alloc = 0
else:
alloc = cap
volinst = StorageVolume(conn)
volinst.pool = poolobj
volinst.name = volname
volinst.capacity = cap
volinst.allocation = alloc
volinst.backing_store = backing_store
if fmt:
if not volinst.supports_property("format"):
raise ValueError(_("Format attribute not supported for this "
"volume type"))
volinst.format = fmt
return volinst
class _StorageBase(object):
def __init__(self, conn):
self._conn = conn
def get_size(self):
raise NotImplementedError()
def get_dev_type(self):
@ -200,40 +165,14 @@ class _StorageBase(object):
raise NotImplementedError()
class StorageCreator(_StorageBase):
def __init__(self, conn, path, pool,
vol_install, clone_path, backing_store,
size, sparse, fmt):
_StorageBase.__init__(self)
class _StorageCreator(_StorageBase):
def __init__(self, conn):
_StorageBase.__init__(self, conn)
self._conn = conn
self._pool = pool
self._vol_install = vol_install
self._path = path
self._size = size
self._sparse = sparse
self._clone_path = clone_path
self.fake = False
if not self._vol_install and self._pool:
self._vol_install = build_vol_install(conn,
os.path.basename(path), pool,
size, sparse, fmt=fmt,
backing_store=backing_store)
if not self._vol_install:
if backing_store:
raise RuntimeError(_("Cannot set backing store for unmanaged "
"storage."))
if fmt and fmt != "raw":
raise RuntimeError(_("Format cannot be specified for "
"unmanaged storage."))
if self._vol_install:
self._path = None
self._size = None
# Cached bits
self._pool = None
self._vol_install = None
self._path = None
self._size = None
self._dev_type = None
@ -241,6 +180,9 @@ class StorageCreator(_StorageBase):
# Public API #
##############
def create(self, progresscb):
raise NotImplementedError()
def _get_path(self):
if self._vol_install and not self._path:
xmlobj = StoragePool(self._conn,
@ -293,9 +235,20 @@ class StorageCreator(_StorageBase):
"'%s'" % self.path))
def is_size_conflict(self):
if self._vol_install:
return self._vol_install.is_size_conflict()
raise NotImplementedError()
class CloneStorageCreator(_StorageCreator):
def __init__(self, conn, output_path, input_path, size, sparse):
_StorageCreator.__init__(self, conn)
self._path = output_path
self._output_path = output_path
self._input_path = input_path
self._size = size
self._sparse = sparse
def is_size_conflict(self):
ret = False
msg = None
vfs = os.statvfs(os.path.dirname(self._path))
@ -316,55 +269,36 @@ class StorageCreator(_StorageBase):
((need / (1024 * 1024)), (avail / (1024 * 1024))))
return (ret, msg)
#############################
# Storage creation routines #
#############################
def create(self, progresscb):
if self.fake:
raise RuntimeError("Storage creator is fake but creation "
"requested.")
# If a clone_path is specified, but not vol_install.input_vol,
# that means we are cloning unmanaged -> managed, so skip this
if (self._vol_install and
(not self._clone_path or self._vol_install.input_vol)):
return self._vol_install.install(meter=progresscb)
if not self._clone_path:
raise RuntimeError("Local storage creation requested, "
"this shouldn't happen.")
text = (_("Cloning %(srcfile)s") %
{'srcfile' : os.path.basename(self._clone_path)})
{'srcfile' : os.path.basename(self._input_path)})
size_bytes = long(self.get_size() * 1024L * 1024L * 1024L)
progresscb.start(filename=self._path, size=long(size_bytes),
progresscb.start(filename=self._output_path, size=long(size_bytes),
text=text)
# Plain file clone
self._clone_local(progresscb, size_bytes)
def _clone_local(self, meter, size_bytes):
if self._clone_path == "/dev/null":
if self._input_path == "/dev/null":
# Not really sure why this check is here,
# but keeping for compat
logging.debug("Source dev was /dev/null. Skipping")
return
if self._clone_path == self._path:
if self._input_path == self._output_path:
logging.debug("Source and destination are the same. Skipping.")
return
# if a destination file exists and sparse flg is True,
# this priority takes a existing file.
if (not os.path.exists(self._path) and self._sparse):
if (not os.path.exists(self._output_path) and self._sparse):
clone_block_size = 4096
sparse = True
fd = None
try:
fd = os.open(self._path, os.O_WRONLY | os.O_CREAT, 0640)
fd = os.open(self._output_path, os.O_WRONLY | os.O_CREAT, 0640)
os.ftruncate(fd, size_bytes)
finally:
if fd:
@ -374,15 +308,17 @@ class StorageCreator(_StorageBase):
sparse = False
logging.debug("Local Cloning %s to %s, sparse=%s, block_size=%s",
self._clone_path, self._path, sparse, clone_block_size)
self._input_path, self._output_path,
sparse, clone_block_size)
zeros = '\0' * 4096
src_fd, dst_fd = None, None
try:
try:
src_fd = os.open(self._clone_path, os.O_RDONLY)
dst_fd = os.open(self._path, os.O_WRONLY | os.O_CREAT, 0640)
src_fd = os.open(self._input_path, os.O_RDONLY)
dst_fd = os.open(self._output_path,
os.O_WRONLY | os.O_CREAT, 0640)
i = 0
while 1:
@ -404,7 +340,7 @@ class StorageCreator(_StorageBase):
meter.update(i)
except OSError, e:
raise RuntimeError(_("Error cloning diskimage %s to %s: %s") %
(self._clone_path, self._path, str(e)))
(self._input_path, self._output_path, str(e)))
finally:
if src_fd is not None:
os.close(src_fd)
@ -412,15 +348,27 @@ class StorageCreator(_StorageBase):
os.close(dst_fd)
class ManagedStorageCreator(_StorageCreator):
def __init__(self, conn, vol_install):
_StorageCreator.__init__(self, conn)
self._pool = vol_install.pool
self._vol_install = vol_install
def create(self, progresscb):
return self._vol_install.install(meter=progresscb)
def is_size_conflict(self):
return self._vol_install.is_size_conflict()
class StorageBackend(_StorageBase):
"""
Class that carries all the info about any existing storage that
the disk references
"""
def __init__(self, conn, path, vol_object, parent_pool):
_StorageBase.__init__(self)
_StorageBase.__init__(self, conn)
self._conn = conn
self._vol_object = vol_object
self._parent_pool = parent_pool
self._path = path
@ -484,6 +432,8 @@ class StorageBackend(_StorageBase):
self._exists = True
elif not self._conn.is_remote() and os.path.exists(self._path):
self._exists = True
elif self._parent_pool:
self._exists = False
elif (auto_check and
self._conn.is_remote() and
not _can_auto_manage(self._path)):