Move Storage.clone_disk_files logic to XenPool

- Add XenVolume to identify volumes which can be cloned even if they are not in
the same pool

qubes/storage/__init__.py

@@ -31,7 +31,6 @@ from __future__ import absolute_import
 import os
 import os.path
 import shutil
-import subprocess
 
 import pkg_resources
 import qubes
@@ -146,7 +145,7 @@ class Storage(object):
 
         old_umask = os.umask(002)
 
-        self.vm.log.info('Creating directory: {0}'.format(self.vm.dir_path))
+        self.log.info('Creating directory: {0}'.format(self.vm.dir_path))
         os.makedirs(self.vm.dir_path)
         for name, volume in self.vm.volumes.items():
             source_volume = None
@@ -156,25 +155,17 @@ class Storage(object):
 
         os.umask(old_umask)
 
-    # TODO migrate this
-    def clone_disk_files(self, src_vm):
-        # :pylint: disable=missing-docstring
+    def clone(self, src_vm):
         self.vm.log.info('Creating directory: {0}'.format(self.vm.dir_path))
-        os.mkdir(self.vm.dir_path)
-
-        if hasattr(src_vm, 'private_img'):
-            self.vm.log.info('Copying the private image: {} -> {}'.format(
-                src_vm.private_img, self.vm.private_img))
-            self._copy_file(src_vm.private_img, self.vm.private_img)
-
-        if src_vm.updateable and hasattr(src_vm, 'root_img'):
-            self.vm.log.info(
-                'Copying the root image: {} -> {}'.format(
-                    src_vm.volume['root'].path_origin,
-                    self.vm.volume['root'].path_origin)
-            )
-            self._copy_file(src_vm.volume['root'].path_origin,
-                            self.vm.volume['root'].path_origin)
+        if not os.path.exists(self.vm.dir_path):
+            self.log.info('Creating directory: {0}'.format(self.vm.dir_path))
+            os.makedirs(self.vm.dir_path)
+        for name, target in self.vm.volumes.items():
+            pool = self.get_pool(target)
+            source = src_vm.volumes[name]
+            volume = pool.clone(source, target)
+            assert volume, "%s.clone() returned '%s'" % (pool.__class__, volume)
+            self.vm.volumes[name] = volume
 
     # TODO migrate this
     @staticmethod
@@ -265,33 +256,16 @@ class Pool(object):
         raise NotImplementedError("Pool %s has config() not implemented" %
                                   self.name)
 
-    @staticmethod
-    def _copy_file(source, destination):
-        '''Effective file copy, preserving sparse files etc.
-        '''
-        # TODO: Windows support
-        # We prefer to use Linux's cp, because it nicely handles sparse files
-        assert os.path.exists(source), \
-            "Missing the source %s to copy from" % source
-        assert not os.path.exists(destination), \
-            "Destination %s already exists" % destination
-        try:
-            subprocess.check_call(['cp', '--reflink=auto', source, destination
-                                   ])
-        except subprocess.CalledProcessError:
-            raise IOError('Error while copying {!r} to {!r}'.format(
-                source, destination))
+    def clone(self, source, target):
+        ''' Clone volume '''
+        raise NotImplementedError("Pool %s has clone() not implemented" %
+                                  self.name)
 
     def remove(self, volume):
         ''' Remove volume'''
         raise NotImplementedError("Pool %s has remove() not implemented" %
                                   self.name)
 
-    def clone(self, source, target):
-        ''' Clone volume '''
-        raise NotImplementedError("Pool %s has clone() not implemented" %
-                                  self.name)
-
     def start(self, volume):
         ''' Do what ever is needed on start '''
         raise NotImplementedError("Pool %s has start() not implemented" %

qubes/storage/xen.py

@@ -50,6 +50,20 @@ class XenPool(Pool):
         vm_templates_path = os.path.join(self.dir_path, 'vm-templates')
         create_dir_if_not_exists(vm_templates_path)
 
+    def clone(self, source, target):
+        ''' Clones the volume if the `source.pool` if the source is a
+            :py:class:`XenVolume`.
+        '''
+        if issubclass(XenVolume, source.__class__):
+            raise StoragePoolException('Volumes %s and %s use different pools'
+                                       % (source.__class__, target.__class__))
+
+        if source.volume_type not in ['origin', 'read-write']:
+            return target
+
+        copy_file(source.vid, target.vid)
+        return target
+
     def create(self, volume, source_volume=None):
         _type = volume.volume_type
         size = volume.size
@@ -200,7 +214,9 @@ class XenPool(Pool):
 
 
 class XenVolume(Volume):
-    ''' Parent class for the xen volumes implementation '''
+    ''' Parent class for the xen volumes implementation which expects a
+        `target_dir` param on initialization.
+    '''
 
     def __init__(self, target_dir, **kwargs):
         self.target_dir = target_dir
@@ -212,15 +228,11 @@ class SizeMixIn(XenVolume):
     ''' A mix in which expects a `size` param to be > 0 on initialization and
         provides a usage property wrapper.
     '''
-    def __init__(self, name=None, pool=None, vid=None, target_dir=None, size=0,
-                 **kwargs):
-        assert size > 0, 'Size for volume ' + name + ' is <=0'
-        super(SizeMixIn, self).__init__(name=name,
-                                        pool=pool,
-                                        vid=vid,
-                                        size=size,
-                                        **kwargs)
-        self.target_dir = target_dir
+
+    def __init__(self, size=0, **kwargs):
+        super(SizeMixIn, self).__init__(size=int(size), **kwargs)
+        assert size, 'Empty size provided'
+        assert size > 0, 'Size for volume ' + kwargs['name'] + ' is <=0'
 
     @property
     def usage(self):
@@ -237,19 +249,13 @@ class ReadWriteFile(SizeMixIn):
         self.vid = self.path
 
 
-class ReadOnlyFile(Volume):
+class ReadOnlyFile(XenVolume):
     # :pylint: disable=missing-docstring
     usage = 0
 
-    def __init__(self, name=None, pool=None, vid=None, target_dir=None,
-                 size=0, **kwargs):
+    def __init__(self, size=0, **kwargs):
         # :pylint: disable=unused-argument
-        assert os.path.exists(vid), "read-only volume missing vid"
-        super(ReadOnlyFile, self).__init__(name=name,
-                                           pool=pool,
-                                           vid=vid,
-                                           size=size,
-                                           **kwargs)
+        super(ReadOnlyFile, self).__init__(size=int(size), **kwargs)
         self.path = self.vid
 
 
@@ -277,22 +283,17 @@ class OriginFile(SizeMixIn):
         return result
 
 
-class SnapshotFile(Volume):
+class SnapshotFile(XenVolume):
     # :pylint: disable=missing-docstring
     script = 'block-snapshot'
     rw = False
     usage = 0
 
-    def __init__(self, name=None, pool=None, vid=None, target_dir=None,
-                 size=None, **kwargs):
+    def __init__(self, name=None, size=None, **kwargs):
         assert size
-        super(SnapshotFile, self).__init__(name=name,
-                                           pool=pool,
-                                           vid=vid,
-                                           size=size,
-                                           **kwargs)
-        self.path_origin = os.path.join(target_dir, name + '.img')
-        self.path_cow = os.path.join(target_dir, name + '-cow.img')
+        super(SnapshotFile, self).__init__(name=name, size=int(size), **kwargs)
+        self.path_origin = os.path.join(self.target_dir, name + '.img')
+        self.path_cow = os.path.join(self.target_dir, name + '-cow.img')
         self.path = '%s:%s' % (self.path_origin, self.path_cow)
         self.vid = self.path_origin
 

qubes/vm/qubesvm.py

@@ -1114,18 +1114,20 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
         self.storage.remove()
         shutil.rmtree(self.vm.dir_path)
 
-
     def clone_disk_files(self, src):
         '''Clone files from other vm.
 
         :param qubes.vm.qubesvm.QubesVM src: source VM
         '''
 
-        if src.is_running(): # XXX what about paused?
+        if src.is_running():  # XXX what about paused?
             raise qubes.exc.QubesVMNotHaltedError(
                 self, 'Cannot clone a running domain {!r}'.format(self.name))
 
-        self.storage.clone_disk_files(src)
+        if hasattr(src, 'volume_config'):
+            self.volume_config = src.volume_config
+        self.storage = qubes.storage.Storage(self)
+        self.storage.clone(src)
 
         if src.icon_path is not None \
                 and os.path.exists(src.dir_path) \