From bba9b38e8eae9816d1aeda58b5accd07261efa80 Mon Sep 17 00:00:00 2001
From: Bahtiar `kalkin-` Gadimov <bahtiar@gadimov.de>
Date: Thu, 23 Jun 2016 18:41:11 +0200
Subject: [PATCH 01/21] Avoid libvirt access in qubes.vm.qubesvm.QubesVM

---
 qubes/vm/qubesvm.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/qubes/vm/qubesvm.py b/qubes/vm/qubesvm.py
index afe1a21e..02f452e1 100644
--- a/qubes/vm/qubesvm.py
+++ b/qubes/vm/qubesvm.py
@@ -1099,7 +1099,11 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
         :param qubes.vm.qubesvm.QubesVM src: source VM
         '''
 
-        if not self.is_halted():
+        # If the current vm name is not a part of `self.app.domains.keys()`,
+        # then the current vm is in creation process. Calling
+        # `self.is_halted()` at this point, would instantiate libvirt, we want
+        # avoid that.
+        if self.name in self.app.domains.keys() and not self.is_halted():
             raise qubes.exc.QubesVMNotHaltedError(
                 self, 'Cannot clone a running domain {!r}'.format(self.name))
 

From 4cc7b8d2a8726de1aa103b5c7ffe3cfd0498194b Mon Sep 17 00:00:00 2001
From: Bahtiar `kalkin-` Gadimov <bahtiar@gadimov.de>
Date: Tue, 12 Jul 2016 18:05:10 +0200
Subject: [PATCH 02/21] Fix qubes.tests.storage

---
 qubes/tests/storage.py | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/qubes/tests/storage.py b/qubes/tests/storage.py
index 2b027e50..9afddb9f 100644
--- a/qubes/tests/storage.py
+++ b/qubes/tests/storage.py
@@ -20,19 +20,16 @@ import qubes.log
 from qubes.exc import QubesException
 from qubes.storage import pool_drivers
 from qubes.storage.file import FilePool
-from qubes.tests import QubesTestCase, SystemTestsMixin
+from qubes.tests import QubesTestCase
 
 # :pylint: disable=invalid-name
 
 
-class TestApp(qubes.tests.TestEmitter):
-    pass
-
-
 class TestVM(object):
     def __init__(self, test, template=None):
         self.app = test.app
         self.name = test.make_vm_name('appvm')
+        self.dir_path = '/var/lib/qubes/appvms/' + self.name
         self.log = qubes.log.get_vm_logger(self.name)
 
         if template:
@@ -50,6 +47,10 @@ class TestVM(object):
 class TestTemplateVM(TestVM):
     dir_path_prefix = qubes.config.system_path['qubes_templates_dir']
 
+    def __init__(self, test, template=None):
+        super(TestTemplateVM, self).__init__(test, template)
+        self.dir_path = '/var/lib/qubes/vm-templates/' + self.name
+
     def is_template(self):
         return True
 
@@ -59,7 +60,7 @@ class TestDisposableVM(TestVM):
         return True
 
 class TestApp(qubes.Qubes):
-    def __init__(self, *args, **kwargs):
+    def __init__(self, *args, **kwargs):  # pylint: disable=unused-argument
         super(TestApp, self).__init__('/tmp/qubes-test.xml',
             load=False, offline_mode=True, **kwargs)
         self.load_initial_values()

From 3952cef5565b8bc095f19016328ee3fd4a681757 Mon Sep 17 00:00:00 2001
From: Bahtiar `kalkin-` Gadimov <bahtiar@gadimov.de>
Date: Tue, 12 Jul 2016 17:56:49 +0200
Subject: [PATCH 03/21] QubesVM serialize bool values from XML

---
 qubes/vm/qubesvm.py | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/qubes/vm/qubesvm.py b/qubes/vm/qubesvm.py
index 02f452e1..bb15da0e 100644
--- a/qubes/vm/qubesvm.py
+++ b/qubes/vm/qubesvm.py
@@ -421,10 +421,15 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
                     name = node.get('name')
                     assert name
                     for key, value in node.items():
-                        self.volume_config[name][key] = value
+                        # pylint: disable=no-member
+                        if value == 'True':
+                            self.volume_config[name][key] = True
+                        else:
+                            self.volume_config[name][key] = value
 
             for name, conf in volume_config.items():
                 for key, value in conf.items():
+                    # pylint: disable=no-member
                     self.volume_config[name][key] = value
 
         elif volume_config:

From 7841e3f6c057963fae3581f969d41daab6f865cc Mon Sep 17 00:00:00 2001
From: Bahtiar `kalkin-` Gadimov <bahtiar@gadimov.de>
Date: Tue, 12 Jul 2016 18:56:06 +0200
Subject: [PATCH 04/21] qubes.storage rework api

---
 qubes/storage/__init__.py | 250 ++++++++++++++++++++++++++------------
 1 file changed, 172 insertions(+), 78 deletions(-)

diff --git a/qubes/storage/__init__.py b/qubes/storage/__init__.py
index a8aa7ae2..b573dfc9 100644
--- a/qubes/storage/__init__.py
+++ b/qubes/storage/__init__.py
@@ -29,7 +29,7 @@ from __future__ import absolute_import
 
 import os
 import os.path
-import string
+import string  # pylint: disable=deprecated-module
 
 import lxml.etree
 import pkg_resources
@@ -49,58 +49,118 @@ class StoragePoolException(qubes.exc.QubesException):
 class Volume(object):
     ''' Encapsulates all data about a volume for serialization to qubes.xml and
         libvirt config.
+
+
+        Keep in mind!
+        volatile        = not snap_on_start and not save_on_stop
+        snapshot        =     snap_on_start and not save_on_stop
+        origin          = not snap_on_start and     save_on_stop
+        origin_snapshot =     snap_on_start and     save_on_stop
     '''
 
     devtype = 'disk'
     domain = None
     path = None
-    rw = True
     script = None
     usage = 0
 
-    def __init__(self, name, pool, volume_type, vid=None, size=0,
-                 removable=False, internal=False, **kwargs):
+    def __init__(self, name, pool, vid, internal=False, removable=False,
+            revisions_to_keep=0, rw=False, save_on_stop=False, size=0,
+            snap_on_start=False, source=None, **kwargs):
+        ''' Initialize a volume.
+
+            :param str name: The domain name
+            :param str pool: The pool name
+            :param str vid:  Volume identifier needs to be unique in pool
+            :param bool internal: If `True` volume is hidden when qvm-block ls
+                is used
+            :param bool removable: If `True` volume can be detached from vm at
+                run time
+            :param int revisions_to_keep: Amount of revisions to keep around
+            :param bool rw: If true volume will be mounted read-write
+            :param bool snap_on_start: Create a snapshot from source on start
+            :param bool save_on_stop: Write changes to disk in vm.stop()
+            :param str source: Vid of other volume in same pool
+            :param str/int size: Size of the volume
+
+        '''
+
         super(Volume, self).__init__(**kwargs)
+
         self.name = str(name)
         self.pool = str(pool)
-        self.vid = vid
-        self.size = size
-        self.volume_type = volume_type
-        self.removable = removable
         self.internal = internal
+        self.removable = removable
+        self.revisions_to_keep = revisions_to_keep
+        self.rw = rw
+        self.save_on_stop = save_on_stop
+        self.size = int(size)
+        self.snap_on_start = snap_on_start
+        self.source = source
+        self.vid = vid
 
-    def __xml__(self):
-        return lxml.etree.Element('volume', **self.config)
+    def __eq__(self, other):
+        return other.pool == self.pool and other.vid == self.vid
 
-    @property
-    def config(self):
-        ''' return config data for serialization to qubes.xml '''
-        return {'name': self.name,
-                'pool': self.pool,
-                'volume_type': self.volume_type}
+    def __hash__(self):
+        return hash('%s:%s' % (self.pool, self.vid))
+
+    def __neq__(self, other):
+        return not self.__eq__(other)
 
     def __repr__(self):
         return '{!r}'.format(self.pool + ':' + self.vid)
 
+    def __str__(self):
+        return str(self.vid)
+
+    def __xml__(self):
+        config = _sanitize_config(self.config)
+        return lxml.etree.Element('volume', **config)
+
     def block_device(self):
         ''' Return :py:class:`qubes.devices.BlockDevice` for serialization in
             the libvirt XML template as <disk>.
         '''
         return qubes.devices.BlockDevice(self.path, self.name, self.script,
-            self.rw, self.domain, self.devtype)
+                                         self.rw, self.domain, self.devtype)
 
-    def __eq__(self, other):
-        return other.pool == self.pool and other.vid == self.vid \
-            and other.volume_type == self.volume_type
+    @property
+    def revisions(self):
+        ''' Returns a `dict` containing revision identifiers and paths '''
+        msg = "{!s} has revisions not implemented".format(self.__class__)
+        raise NotImplementedError(msg)
 
-    def __neq__(self, other):
-        return not self.__eq__(other)
+    @property
+    def config(self):
+        ''' return config data for serialization to qubes.xml '''
+        result = {'name': self.name, 'pool': self.pool, 'vid': self.vid, }
 
-    def __hash__(self):
-        return hash('%s:%s %s' % (self.pool, self.vid, self.volume_type))
+        if self.internal:
+            result['internal'] = self.internal
 
-    def __str__(self):
-        return "{!s}:{!s}".format(self.pool, self.vid)
+        if self.removable:
+            result['removable'] = self.removable
+
+        if self.revisions_to_keep:
+            result['revisions_to_keep'] = self.revisions_to_keep
+
+        if self.rw:
+            result['rw'] = self.rw
+
+        if self.save_on_stop:
+            result['save_on_stop'] = self.save_on_stop
+
+        if self.size:
+            result['size'] = self.size
+
+        if self.snap_on_start:
+            result['snap_on_start'] = self.snap_on_start
+
+        if self.source:
+            result['source'] = self.source
+
+        return result
 
 
 class Storage(object):
@@ -119,6 +179,7 @@ class Storage(object):
         #: Additional drive (currently used only by HVM)
         self.drive = None
         self.pools = {}
+
         if hasattr(vm, 'volume_config'):
             for name, conf in self.vm.volume_config.items():
                 assert 'pool' in conf, "Pool missing in volume_config" % str(
@@ -187,7 +248,7 @@ class Storage(object):
         If :py:attr:`self.vm.kernel` is :py:obj:`None`, the this points inside
         :py:attr:`self.vm.dir_path`
         '''
-        assert 'kernel' in self.vm.volumes, "VM has no kernel pool"
+        assert 'kernel' in self.vm.volumes, "VM has no kernel volume"
         return self.vm.volumes['kernel'].kernels_dir
 
     def get_disk_utilization(self):
@@ -248,11 +309,15 @@ class Storage(object):
     def rename(self, old_name, new_name):
         ''' Notify the pools that the domain was renamed '''
         volumes = self.vm.volumes
+        vm = self.vm
+        old_dir_path = os.path.join(os.path.dirname(vm.dir_path), old_name)
+        new_dir_path = os.path.join(os.path.dirname(vm.dir_path), new_name)
+        os.rename(old_dir_path, new_dir_path)
         for name, volume in volumes.items():
             pool = self.get_pool(volume)
             volumes[name] = pool.rename(volume, old_name, new_name)
 
-    def verify_files(self):
+    def verify(self):
         '''Verify that the storage is sane.
 
         On success, returns normally. On failure, raises exception.
@@ -264,6 +329,7 @@ class Storage(object):
         for volume in self.vm.volumes.values():
             self.get_pool(volume).verify(volume)
         self.vm.fire_event('domain-verify-files')
+        return True
 
     def remove(self):
         ''' Remove all the volumes.
@@ -280,7 +346,8 @@ class Storage(object):
     def start(self):
         ''' Execute the start method on each pool '''
         for volume in self.vm.volumes.values():
-            self.get_pool(volume).start(volume)
+            pool = self.get_pool(volume)
+            volume = pool.start(volume)
 
     def stop(self):
         ''' Execute the start method on each pool '''
@@ -289,8 +356,12 @@ class Storage(object):
 
     def get_pool(self, volume):
         ''' Helper function '''
-        assert isinstance(volume, Volume), "You need to pass a Volume"
-        return self.pools[volume.name]
+        assert isinstance(volume, (Volume, basestring)), \
+            "You need to pass a Volume or pool name as str"
+        if isinstance(volume, Volume):
+            return self.pools[volume.name]
+        else:
+            return self.vm.app.pools[volume]
 
     def commit_template_changes(self):
         ''' Makes changes to an 'origin' volume persistent '''
@@ -320,106 +391,129 @@ class Pool(object):
 
         3rd Parties providing own storage implementations will need to extend
         this class.
-    '''
+    '''  # pylint: disable=unused-argument
     private_img_size = qubes.config.defaults['private_img_size']
     root_img_size = qubes.config.defaults['root_img_size']
 
+    def __init__(self, name, revisions_to_keep=1, **kwargs):
+        super(Pool, self).__init__(**kwargs)
+        self.name = name
+        self.revisions_to_keep = revisions_to_keep
+        kwargs['name'] = self.name
+
     def __eq__(self, other):
         return self.name == other.name
 
-
     def __neq__(self, other):
         return not self.__eq__(other)
 
-    def __init__(self, name, **kwargs):
-        super(Pool, self).__init__(**kwargs)
-        self.name = name
-        kwargs['name'] = self.name
-
     def __str__(self):
         return self.name
 
     def __xml__(self):
-        return lxml.etree.Element('pool', **self.config)
+        config = _sanitize_config(self.config)
+        return lxml.etree.Element('pool', **config)
 
-    def create(self, volume, source_volume=None):
+    def create(self, volume):
         ''' Create the given volume on disk or copy from provided
             `source_volume`.
         '''
-        raise NotImplementedError("Pool %s has create() not implemented" %
-                                  self.name)
+        raise self._not_implemented("create")
 
-    def commit_template_changes(self, volume):
-        ''' Update origin device '''
-        raise NotImplementedError(
-            "Pool %s has commit_template_changes() not implemented" %
-            self.name)
+    def commit(self, volume):  # pylint: disable=no-self-use
+        ''' Write the snapshot to disk '''
+        msg = "Got volume_type {!s} when expected 'snap'"
+        msg = msg.format(volume.volume_type)
+        assert volume.volume_type == 'snap', msg
 
     @property
     def config(self):
         ''' Returns the pool config to be written to qubes.xml '''
-        raise NotImplementedError("Pool %s has config() not implemented" %
-                                  self.name)
+        raise self._not_implemented("config")
 
     def clone(self, source, target):
         ''' Clone volume '''
-        raise NotImplementedError("Pool %s has clone() not implemented" %
-                                  self.name)
+        raise self._not_implemented("clone")
 
     def destroy(self):
         ''' Called when removing the pool. Use this for implementation specific
             clean up.
         '''
-        raise NotImplementedError("Pool %s has destroy() not implemented" %
-                                  self.name)
+        raise self._not_implemented("destroy")
+
+    def export(self, volume):
+        ''' Returns an object that can be `open()`. '''
+        raise self._not_implemented("export")
+
+    def import_volume(self, dst_pool, dst_volume, src_pool, src_volume):
+        ''' Imports data to a volume in this pool '''
+        raise self._not_implemented("import_volume")
+
+    def init_volume(self, vm, volume_config):
+        ''' Initialize a :py:class:`qubes.storage.Volume` from `volume_config`.
+        '''
+        raise self._not_implemented("init_volume")
+
+    def is_dirty(self, volume):
+        ''' Return `True` if volume was not properly shutdown and commited '''
+        raise self._not_implemented("is_dirty")
 
     def is_outdated(self, volume):
-        raise NotImplementedError("Pool %s has is_outdated() not implemented" %
-                                  self.name)
+        ''' Returns `True` if the currently used `volume.source` of a snapshot
+            volume is outdated.
+        '''
+        raise self._not_implemented("is_outdated")
+
+    def recover(self, volume):
+        ''' Try to recover a :py:class:`Volume` or :py:class:`SnapVolume` '''
+        raise self._not_implemented("recover")
 
     def remove(self, volume):
         ''' Remove volume'''
-        raise NotImplementedError("Pool %s has remove() not implemented" %
-                                  self.name)
+        raise self._not_implemented("remove")
 
     def rename(self, volume, old_name, new_name):
         ''' Called when the domain changes its name '''
-        raise NotImplementedError("Pool %s has rename() not implemented" %
-                                  self.name)
+        raise self._not_implemented("rename")
 
-    def start(self, volume):
-        ''' Do what ever is needed on start '''
-        raise NotImplementedError("Pool %s has start() not implemented" %
-                                  self.name)
+    def reset(self, volume):
+        ''' Drop and recreate volume without copying it's content from source.
+        '''
+        raise self._not_implemented("reset")
+
+    def revert(self, volume, revision=None):
+        ''' Revert volume to previous revision  '''
+        raise self._not_implemented("revert")
 
     def setup(self):
         ''' Called when adding a pool to the system. Use this for implementation
             specific set up.
         '''
-        raise NotImplementedError("Pool %s has setup() not implemented" %
-                                  self.name)
+        raise self._not_implemented("setup")
 
-    def stop(self, volume):
+    def start(self, volume):  # pylint: disable=no-self-use
+        ''' Do what ever is needed on start '''
+        raise self._not_implemented("start")
+
+    def stop(self, volume):  # pylint: disable=no-self-use
         ''' Do what ever is needed on stop'''
-        raise NotImplementedError("Pool %s has stop() not implemented" %
-                                  self.name)
-
-    def init_volume(self, vm, volume_config):
-        ''' Initialize a :py:class:`qubes.storage.Volume` from `volume_config`.
-        '''
-        raise NotImplementedError("Pool %s has init_volume() not implemented" %
-                                  self.name)
 
     def verify(self, volume):
         ''' Verifies the volume. '''
-        raise NotImplementedError("Pool %s has verify() not implemented" %
-                                  self.name)
+        raise self._not_implemented("verify")
 
     @property
     def volumes(self):
         ''' Return a list of volumes managed by this pool '''
-        raise NotImplementedError("Pool %s has volumes() not implemented" %
-                                  self.name)
+        raise self._not_implemented("volumes")
+
+    def _not_implemented(self, method_name):
+        ''' Helper for emitting helpful `NotImplementedError` exceptions '''
+        msg = "Pool driver {!s} has {!s}() not implemented"
+        msg = msg.format(str(self.__class__.__name__), method_name)
+        return NotImplementedError(msg)
+
+
 
 
 def pool_drivers():

From 1bccb146d8b8096cb72fc82f0ed4afcc6917f767 Mon Sep 17 00:00:00 2001
From: Bahtiar `kalkin-` Gadimov <bahtiar@gadimov.de>
Date: Tue, 12 Jul 2016 18:57:04 +0200
Subject: [PATCH 05/21] Add qubes.storage.isodate() helper function

---
 qubes/storage/__init__.py | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/qubes/storage/__init__.py b/qubes/storage/__init__.py
index b573dfc9..7bb6ea6f 100644
--- a/qubes/storage/__init__.py
+++ b/qubes/storage/__init__.py
@@ -30,6 +30,8 @@ from __future__ import absolute_import
 import os
 import os.path
 import string  # pylint: disable=deprecated-module
+import time
+from datetime import datetime
 
 import lxml.etree
 import pkg_resources
@@ -520,3 +522,8 @@ def pool_drivers():
     """ Return a list of EntryPoints names """
     return [ep.name
             for ep in pkg_resources.iter_entry_points(STORAGE_ENTRY_POINT)]
+
+
+def isodate(seconds=time.time()):
+    ''' Helper method which returns an iso date '''
+    return datetime.utcfromtimestamp(seconds).isoformat("T")

From 7e1563c88d0eae3cdf0e72ac880d2cbd98affef4 Mon Sep 17 00:00:00 2001
From: Bahtiar `kalkin-` Gadimov <bahtiar@gadimov.de>
Date: Tue, 12 Jul 2016 18:57:43 +0200
Subject: [PATCH 06/21] Add handling for old volume config

---
 qubes/storage/__init__.py | 44 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/qubes/storage/__init__.py b/qubes/storage/__init__.py
index 7bb6ea6f..0cc0a2de 100644
--- a/qubes/storage/__init__.py
+++ b/qubes/storage/__init__.py
@@ -186,10 +186,54 @@ class Storage(object):
             for name, conf in self.vm.volume_config.items():
                 assert 'pool' in conf, "Pool missing in volume_config" % str(
                     conf)
+                if 'volume_type' in conf:
+                    conf = self._migrate_config(conf)
+
                 pool = self.vm.app.get_pool(conf['pool'])
                 self.vm.volumes[name] = pool.init_volume(self.vm, conf)
                 self.pools[name] = pool
 
+    def _migrate_config(self, conf):
+        ''' Migrates from the old config style to new
+        '''  # FIXME: Remove this compatibility hack
+        assert 'volume_type' in conf
+        _type = conf['volume_type']
+        old_volume_types = [
+            'read-write', 'read-only', 'origin', 'snapshot', 'volatile'
+        ]
+        msg = "Volume {!s} has unknown type {!s}".format(conf['name'], _type)
+        assert conf['volume_type'] in old_volume_types, msg
+        if _type == 'origin':
+            conf['rw'] = True
+            conf['source'] = None
+            conf['save_on_stop'] = True
+            conf['revisions_to_keep'] = 1
+        elif _type == 'snapshot':
+            conf['rw'] = False
+            if conf['pool'] == 'default':
+                template_vid = os.path.join('vm-templates',
+                    self.vm.template.name, conf['name'])
+            elif conf['pool'] == 'qubes_dom0':
+                template_vid = os.path.join(
+                    'qubes_dom0', self.vm.template.name + '-' + conf['name'])
+            conf['source'] = template_vid
+            conf['snap_on_start'] = True
+        elif _type == 'read-write':
+            conf['rw'] = True
+            conf['save_on_stop'] = True
+            conf['revisions_to_keep'] = 0
+        elif _type == 'read-only':
+            conf['rw'] = False
+            conf['snap_on_start'] = True
+            conf['save_on_stop'] = False
+            conf['revisions_to_keep'] = 0
+        elif _type == 'volatile':
+            conf['snap_on_start'] = False
+            conf['save_on_stop'] = False
+            conf['revisions_to_keep'] = 0
+        del conf['volume_type']
+        return conf
+
     def attach(self, volume, rw=False):
         ''' Attach a volume to the domain '''
         assert self.vm.is_running()

From 1cbabc79ffbb26b1cb13997004d6661584803356 Mon Sep 17 00:00:00 2001
From: Bahtiar `kalkin-` Gadimov <bahtiar@gadimov.de>
Date: Tue, 12 Jul 2016 17:58:30 +0200
Subject: [PATCH 07/21] qubes.vm.QubesVM use new storage api

---
 qubes/vm/qubesvm.py | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/qubes/vm/qubesvm.py b/qubes/vm/qubesvm.py
index bb15da0e..e052e186 100644
--- a/qubes/vm/qubesvm.py
+++ b/qubes/vm/qubesvm.py
@@ -547,6 +547,8 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
         # pylint: disable=unused-argument
         self.init_log()
 
+        self.storage.rename(old_name, new_name)
+
         if self._libvirt_domain is not None:
             self.libvirt_domain.undefine()
             self._libvirt_domain = None
@@ -554,8 +556,6 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
             self._qdb_connection.close()
             self._qdb_connection = None
 
-        self.storage.rename(old_name, new_name)
-
         self._update_libvirt_domain()
 
         if self.autostart:
@@ -680,7 +680,7 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
         self.fire_event_pre('domain-pre-start', preparing_dvm=preparing_dvm,
             start_guid=start_guid, mem_required=mem_required)
 
-        self.storage.verify_files()
+        self.storage.verify()
 
         if self.netvm is not None:
             # pylint: disable = no-member
@@ -1066,21 +1066,19 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
             user="root", passio_popen=True, gui=False, wait=True)
         p.communicate(input=self.default_user)
 
-    def create_on_disk(self, source_template=None):
+    def create_on_disk(self, pool=None, pools=None):
         '''Create files needed for VM.
-
-        :param qubes.vm.templatevm.TemplateVM source_template: Template to use
-            (if :py:obj:`None`, use domain's own template
         '''
 
-        if source_template is None and hasattr(self, 'template'):
-            # pylint: disable=no-member
-            source_template = self.template
-
         self.log.info('Creating directory: {0}'.format(self.dir_path))
         os.makedirs(self.dir_path, mode=0o775)
 
-        self.storage.create(source_template)
+        if pool or pools:
+            self.volume_config = _patch_volume_config(self.volume_config, pool,
+                                                      pools)
+            self.storage = qubes.storage.Storage(self)
+
+        self.storage.create()
 
         self.log.info('Creating icon symlink: {} -> {}'.format(
             self.icon_path, self.label.icon_path))
@@ -1090,13 +1088,13 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
             shutil.copy(self.label.icon_path, self.icon_path)
 
         # fire hooks
-        self.fire_event('domain-create-on-disk', source_template)
+        self.fire_event('domain-create-on-disk')
 
     def remove_from_disk(self):
         '''Remove domain remnants from disk.'''
         self.fire_event('domain-remove-from-disk')
-        self.storage.remove()
         shutil.rmtree(self.dir_path)
+        self.storage.remove()
 
     def clone_disk_files(self, src):
         '''Clone files from other vm.
@@ -1117,6 +1115,8 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
             self.volume_config = src.volume_config
         self.storage = qubes.storage.Storage(self)
         self.storage.clone(src)
+        self.storage.verify()
+        assert self.volumes != {}
 
         if src.icon_path is not None \
                 and os.path.exists(src.dir_path) \

From ca9797bb6be02c79c97f72fa86792b5d19eaf020 Mon Sep 17 00:00:00 2001
From: Bahtiar `kalkin-` Gadimov <bahtiar@gadimov.de>
Date: Tue, 12 Jul 2016 17:59:10 +0200
Subject: [PATCH 08/21] qubes.tests.int.basic use new storage API

---
 qubes/tests/int/basic.py | 70 +++++++++++++++++++++-------------------
 1 file changed, 36 insertions(+), 34 deletions(-)

diff --git a/qubes/tests/int/basic.py b/qubes/tests/int/basic.py
index 4cfe154f..66c08139 100644
--- a/qubes/tests/int/basic.py
+++ b/qubes/tests/int/basic.py
@@ -1,6 +1,6 @@
 #!/usr/bin/python
 # vim: fileencoding=utf-8
-
+# pylint: disable=invalid-name
 #
 # The Qubes OS Project, https://www.qubes-os.org/
 #
@@ -35,7 +35,7 @@ import qubes.vm.appvm
 import qubes.vm.qubesvm
 import qubes.vm.templatevm
 
-import libvirt
+import libvirt  # pylint: disable=import-error
 
 
 class TC_00_Basic(qubes.tests.SystemTestsMixin, qubes.tests.QubesTestCase):
@@ -46,17 +46,12 @@ class TC_00_Basic(qubes.tests.SystemTestsMixin, qubes.tests.QubesTestCase):
     def test_000_qubes_create(self):
         self.assertIsInstance(self.app, qubes.Qubes)
 
-    def test_001_qvm_create_default_template(self):
-        self.app.add_new_vm
-
-
     def test_100_qvm_create(self):
         vmname = self.make_vm_name('appvm')
 
         vm = self.app.add_new_vm(qubes.vm.appvm.AppVM,
             name=vmname, template=self.app.default_template,
-            label='red'
-        )
+            label='red')
 
         self.assertIsNotNone(vm)
         self.assertEqual(vm.name, vmname)
@@ -64,17 +59,18 @@ class TC_00_Basic(qubes.tests.SystemTestsMixin, qubes.tests.QubesTestCase):
         vm.create_on_disk()
 
         with self.assertNotRaises(qubes.exc.QubesException):
-            vm.verify_files()
+            vm.storage.verify()
 
 
 class TC_01_Properties(qubes.tests.SystemTestsMixin, qubes.tests.QubesTestCase):
+    # pylint: disable=attribute-defined-outside-init
     def setUp(self):
         super(TC_01_Properties, self).setUp()
         self.init_default_template()
         self.vmname = self.make_vm_name('appvm')
-        self.vm = self.app.add_new_vm(qubes.vm.appvm.AppVM,
-            name=self.vmname,
-            label='red')
+        self.vm = self.app.add_new_vm(qubes.vm.appvm.AppVM, name=self.vmname,
+                                      template=self.app.default_template,
+                                      label='red')
         self.vm.create_on_disk()
 
     def save_and_reload_db(self):
@@ -82,7 +78,7 @@ class TC_01_Properties(qubes.tests.SystemTestsMixin, qubes.tests.QubesTestCase):
         if hasattr(self, 'vm'):
             self.vm = self.app.domains[self.vm.qid]
         if hasattr(self, 'netvm'):
-            self.netvm = self.app[self.netvm.qid]
+            self.netvm = self.app.domains[self.netvm.qid]
 
     def test_000_rename(self):
         newname = self.make_vm_name('newname')
@@ -103,13 +99,11 @@ class TC_01_Properties(qubes.tests.SystemTestsMixin, qubes.tests.QubesTestCase):
             os.path.join(
                 qubes.config.system_path['qubes_base_dir'],
                 qubes.config.system_path['qubes_appvms_dir'], newname))
-        self.assertEqual(self.vm.conf_file,
-            os.path.join(self.vm.dir_path, 'libvirt.xml'))
         self.assertTrue(os.path.exists(
             os.path.join(self.vm.dir_path, "apps", newname + "-vm.directory")))
         # FIXME: set whitelisted-appmenus.list first
-        self.assertTrue(os.path.exists(
-            os.path.join(self.vm.dir_path, "apps", newname + "-firefox.desktop")))
+        self.assertTrue(os.path.exists(os.path.join(
+            self.vm.dir_path, "apps", newname + "-firefox.desktop")))
         self.assertTrue(os.path.exists(
             os.path.join(os.getenv("HOME"), ".local/share/desktop-directories",
                 newname + "-vm.directory")))
@@ -135,7 +129,7 @@ class TC_01_Properties(qubes.tests.SystemTestsMixin, qubes.tests.QubesTestCase):
 
     def test_001_rename_libvirt_undefined(self):
         self.vm.libvirt_domain.undefine()
-        self.vm._libvirt_domain = None
+        self.vm._libvirt_domain = None  # pylint: disable=protected-access
 
         newname = self.make_vm_name('newname')
         with self.assertNotRaises(
@@ -146,15 +140,20 @@ class TC_01_Properties(qubes.tests.SystemTestsMixin, qubes.tests.QubesTestCase):
         testvm1 = self.app.add_new_vm(
             qubes.vm.appvm.AppVM,
             name=self.make_vm_name("vm"),
+            template=self.app.default_template,
             label='red')
         testvm1.create_on_disk()
         testvm2 = self.app.add_new_vm(testvm1.__class__,
                                      name=self.make_vm_name("clone"),
                                      template=testvm1.template,
-                                     label='red',
-                                     )
+                                     label='red')
         testvm2.clone_properties(testvm1)
         testvm2.clone_disk_files(testvm1)
+        self.assertTrue(testvm1.storage.verify())
+        self.assertIn('source', testvm1.volumes['root'].config)
+        self.assertNotEquals(testvm2, None)
+        self.assertNotEquals(testvm2.volumes, {})
+        self.assertIn('source', testvm2.volumes['root'].config)
 
         # qubes.xml reload
         self.save_and_reload_db()
@@ -200,8 +199,7 @@ class TC_01_Properties(qubes.tests.SystemTestsMixin, qubes.tests.QubesTestCase):
         testvm3 = self.app.add_new_vm(testvm1.__class__,
                                      name=self.make_vm_name("clone2"),
                                      template=testvm1.template,
-                                     label='red',
-                                     )
+                                     label='red',)
         testvm3.clone_properties(testvm1)
         testvm3.clone_disk_files(testvm1)
 
@@ -235,14 +233,15 @@ class TC_01_Properties(qubes.tests.SystemTestsMixin, qubes.tests.QubesTestCase):
         # TODO decide what exception should be here
         with self.assertRaises((qubes.exc.QubesException, ValueError)):
             self.vm2 = self.app.add_new_vm(qubes.vm.appvm.AppVM,
-                name=self.vmname, template=self.app.default_template)
+                name=self.vmname, template=self.app.default_template,
+                label='red')
             self.vm2.create_on_disk()
 
     def test_021_name_conflict_template(self):
         # TODO decide what exception should be here
         with self.assertRaises((qubes.exc.QubesException, ValueError)):
             self.vm2 = self.app.add_new_vm(qubes.vm.templatevm.TemplateVM,
-                name=self.vmname)
+                name=self.vmname, label='red')
             self.vm2.create_on_disk()
 
     def test_030_rename_conflict_app(self):
@@ -257,6 +256,7 @@ class TC_01_Properties(qubes.tests.SystemTestsMixin, qubes.tests.QubesTestCase):
                 self.vm2.name = self.vmname
 
 class TC_02_QvmPrefs(qubes.tests.SystemTestsMixin, qubes.tests.QubesTestCase):
+    # pylint: disable=attribute-defined-outside-init
 
     def setUp(self):
         super(TC_02_QvmPrefs, self).setUp()
@@ -349,7 +349,7 @@ class TC_02_QvmPrefs(qubes.tests.SystemTestsMixin, qubes.tests.QubesTestCase):
             ('invalid', '', False),
             ('[invalid]', '', False),
             # TODO:
-            #('["12:12.0"]', '', False)
+            # ('["12:12.0"]', '', False)
         ])
 
     @unittest.skip('test not converted to core3 API')
@@ -368,6 +368,7 @@ class TC_02_QvmPrefs(qubes.tests.SystemTestsMixin, qubes.tests.QubesTestCase):
 
 class TC_03_QvmRevertTemplateChanges(qubes.tests.SystemTestsMixin,
                                      qubes.tests.QubesTestCase):
+    # pylint: disable=attribute-defined-outside-init
 
     def setUp(self):
         super(TC_03_QvmRevertTemplateChanges, self).setUp()
@@ -395,7 +396,7 @@ class TC_03_QvmRevertTemplateChanges(qubes.tests.SystemTestsMixin,
 
     def get_rootimg_checksum(self):
         p = subprocess.Popen(
-            ['sha1sum', self.test_template.volumes['root'].vid],
+            ['sha1sum', self.test_template.volumes['root'].path_cow],
             stdout=subprocess.PIPE)
         return p.communicate()[0]
 
@@ -407,9 +408,11 @@ class TC_03_QvmRevertTemplateChanges(qubes.tests.SystemTestsMixin,
         if checksum_before == checksum_changed:
             self.log.warning("template not modified, test result will be "
                              "unreliable")
+        self.assertNotEqual(self.test_template.volumes['root'].revisions, {})
         with self.assertNotRaises(subprocess.CalledProcessError):
-            subprocess.check_call(['sudo', 'qvm-revert-template-changes',
-                                   '--force', self.test_template.name])
+            pool_vid = repr(self.test_template.volumes['root']).strip("'")
+            revert_cmd = ['qvm-block', 'revert', pool_vid]
+            subprocess.check_call(revert_cmd)
 
         checksum_after = self.get_rootimg_checksum()
         self.assertEquals(checksum_before, checksum_after)
@@ -442,8 +445,7 @@ class TC_04_DispVM(qubes.tests.SystemTestsMixin,
         self.disp_tpl = self.app.add_new_vm(disp_tpl.__class__,
             name=disp_tpl.name,
             template=disp_tpl.template,
-            label='red'
-        )
+            label='red')
         self.app.save()
 
     @staticmethod
@@ -458,6 +460,7 @@ class TC_04_DispVM(qubes.tests.SystemTestsMixin,
         """
 
         testvm1 = self.app.add_new_vm(qubes.vm.appvm.AppVM,
+                                     template=self.app.default_template,
                                      name=self.make_vm_name('vm1'),
                                      label='red')
         testvm1.create_on_disk()
@@ -512,6 +515,7 @@ class TC_04_DispVM(qubes.tests.SystemTestsMixin,
         Check firewall propagation VM->DispVM, when VM have no firewall rules
         """
         testvm1 = self.app.add_new_vm(qubes.vm.appvm.AppVM,
+                                     template=self.app.default_template,
                                      name=self.make_vm_name('vm1'),
                                      label='red')
         testvm1.create_on_disk()
@@ -544,8 +548,8 @@ class TC_04_DispVM(qubes.tests.SystemTestsMixin,
             dispvm_name = p.stdout.readline().strip()
             self.reload_db()
             dispvm = self.app.domains[dispvm_name]
-            self.assertIsNotNone(dispvm, "DispVM {} not found in qubes.xml".format(
-                dispvm_name))
+            self.assertIsNotNone(
+                dispvm, "DispVM {} not found in qubes.xml".format(dispvm_name))
             # check if firewall was propagated to the DispVM from the right VM
             self.assertEquals(testvm1.get_firewall_conf(),
                               dispvm.get_firewall_conf())
@@ -625,6 +629,4 @@ class TC_04_DispVM(qubes.tests.SystemTestsMixin,
         self.assertIsNone(dispvm, "DispVM {} still exists in qubes.xml".format(
             dispvm_name))
 
-
-
 # vim: ts=4 sw=4 et

From 1f735669bc181fcc094eb2557e9c3770da5f3098 Mon Sep 17 00:00:00 2001
From: Bahtiar `kalkin-` Gadimov <bahtiar@gadimov.de>
Date: Tue, 12 Jul 2016 18:03:14 +0200
Subject: [PATCH 09/21] Migrate qubes.vm modules to new API

---
 qubes/app.py             |  5 ++--
 qubes/vm/appvm.py        | 57 +++++++++++++++++++++++++++++++++-------
 qubes/vm/dispvm.py       | 16 ++++++++---
 qubes/vm/standalonevm.py | 17 +++++++++---
 qubes/vm/templatevm.py   | 26 +++++++++---------
 5 files changed, 89 insertions(+), 32 deletions(-)

diff --git a/qubes/app.py b/qubes/app.py
index 805a64f4..2df571e2 100644
--- a/qubes/app.py
+++ b/qubes/app.py
@@ -415,7 +415,6 @@ class VMCollection(object):
 
         return value
 
-
     def __getitem__(self, key):
         if isinstance(key, int):
             return self._dict[key]
@@ -858,7 +857,6 @@ class Qubes(qubes.PropertyHolder):
                 'no such VM class: {!r}'.format(clsname))
         # don't catch TypeError
 
-
     def add_new_vm(self, cls, qid=None, **kwargs):
         '''Add new Virtual Machine to colletion
 
@@ -871,10 +869,11 @@ class Qubes(qubes.PropertyHolder):
         # override it with default template)
         if 'template' not in kwargs and hasattr(cls, 'template'):
             kwargs['template'] = self.default_template
+        elif 'template' in kwargs and isinstance(kwargs['template'], str):
+            kwargs['template'] = self.domains[kwargs['template']]
 
         return self.domains.add(cls(self, None, qid=qid, **kwargs))
 
-
     def get_label(self, label):
         '''Get label as identified by index or name
 
diff --git a/qubes/vm/appvm.py b/qubes/vm/appvm.py
index f567213a..de6d30f1 100644
--- a/qubes/vm/appvm.py
+++ b/qubes/vm/appvm.py
@@ -21,12 +21,12 @@
 # with this program; if not, write to the Free Software Foundation, Inc.,
 # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 #
-
 ''' This module contains the AppVM implementation '''
 
+import copy
+
 import qubes.events
 import qubes.vm.qubesvm
-
 from qubes.config import defaults
 
 
@@ -39,36 +39,75 @@ class AppVM(qubes.vm.qubesvm.QubesVM):
                                 ls_width=31,
                                 doc='Template, on which this AppVM is based.')
 
-    def __init__(self, *args, **kwargs):
+    def __init__(self, app, xml, template=None, **kwargs):
         self.volume_config = {
             'root': {
                 'name': 'root',
                 'pool': 'default',
-                'volume_type': 'snapshot',
+                'snap_on_start': True,
+                'save_on_stop': False,
+                'rw': False,
                 'internal': True
             },
             'private': {
                 'name': 'private',
                 'pool': 'default',
-                'volume_type': 'origin',
+                'snap_on_start': False,
+                'save_on_stop': True,
+                'rw': True,
+                'source': None,
                 'size': defaults['private_img_size'],
                 'internal': True
             },
             'volatile': {
                 'name': 'volatile',
                 'pool': 'default',
-                'volume_type': 'volatile',
                 'size': defaults['root_img_size'],
-                'internal': True
+                'internal': True,
+                'rw': True,
             },
             'kernel': {
                 'name': 'kernel',
                 'pool': 'linux-kernel',
-                'volume_type': 'read-only',
+                'snap_on_start': True,
+                'rw': False,
                 'internal': True
             }
         }
-        super(AppVM, self).__init__(*args, **kwargs)
+
+        if template is not None:
+            # template is only passed if the AppVM is created, in other cases we
+            # don't need to patch the volume_config because the config is
+            # coming from XML, already as we need it
+
+            for name, conf in self.volume_config.items():
+                tpl_volume = template.volumes[name]
+
+                conf['size'] = tpl_volume.size
+                conf['pool'] = tpl_volume.pool
+
+                has_source = ('source' in conf and conf['source'] is not None)
+                is_snapshot = 'snap_on_start' in conf and conf['snap_on_start']
+                if is_snapshot and not has_source:
+                    if tpl_volume.source is not None:
+                        conf['source'] = tpl_volume.source
+                    else:
+                        conf['source'] = tpl_volume.vid
+
+            for name, config in template.volume_config.items():
+                # in case the template vm has more volumes add them to own
+                # config
+                if name not in self.volume_config:
+                    self.volume_config[name] = copy.deepcopy(config)
+                    if 'vid' in self.volume_config[name]:
+                        del self.volume_config[name]['vid']
+
+        super(AppVM, self).__init__(app, xml, **kwargs)
+        if not hasattr(template, 'template') and template is not None:
+            self.template = template
+        if 'source' not in self.volume_config['root']:
+            msg = 'missing source for root volume'
+            raise qubes.exc.QubesException(msg)
 
     @qubes.events.handler('domain-load')
     def on_domain_loaded(self, event):
diff --git a/qubes/vm/dispvm.py b/qubes/vm/dispvm.py
index 6d28a705..44691695 100644
--- a/qubes/vm/dispvm.py
+++ b/qubes/vm/dispvm.py
@@ -46,24 +46,32 @@ class DispVM(qubes.vm.qubesvm.QubesVM):
             'root': {
                 'name': 'root',
                 'pool': 'default',
-                'volume_type': 'snapshot',
+                'snap_on_start': True,
+                'save_on_stop': False,
+                'rw': False,
+                'internal': True
             },
             'private': {
                 'name': 'private',
                 'pool': 'default',
-                'volume_type': 'snapshot',
+                'snap_on_start': True,
+                'save_on_stop': False,
+                'internal': True,
+                'rw': True,
             },
             'volatile': {
                 'name': 'volatile',
                 'pool': 'default',
-                'volume_type': 'volatile',
+                'internal': True,
                 'size': qubes.config.defaults['root_img_size'] +
                         qubes.config.defaults['private_img_size'],
             },
             'kernel': {
                 'name': 'kernel',
                 'pool': 'linux-kernel',
-                'volume_type': 'read-only',
+                'snap_on_start': True,
+                'rw': False,
+                'internal': True
             }
         }
 
diff --git a/qubes/vm/standalonevm.py b/qubes/vm/standalonevm.py
index 4ab3f78a..7e12c807 100644
--- a/qubes/vm/standalonevm.py
+++ b/qubes/vm/standalonevm.py
@@ -34,25 +34,34 @@ class StandaloneVM(qubes.vm.qubesvm.QubesVM):
             'root': {
                 'name': 'root',
                 'pool': 'default',
-                'volume_type': 'origin',
+                'snap_on_start': False,
+                'save_on_stop': True,
+                'rw': True,
+                'source': None,
+                'internal': True,
                 'size': qubes.config.defaults['root_img_size'],
             },
             'private': {
                 'name': 'private',
                 'pool': 'default',
-                'volume_type': 'origin',
+                'snap_on_start': False,
+                'save_on_stop': True,
+                'rw': True,
+                'source': None,
+                'internal': True,
                 'size': qubes.config.defaults['private_img_size'],
             },
             'volatile': {
                 'name': 'volatile',
                 'pool': 'default',
-                'volume_type': 'volatile',
+                'internal': True,
                 'size': qubes.config.defaults['root_img_size'],
             },
             'kernel': {
                 'name': 'kernel',
                 'pool': 'linux-kernel',
-                'volume_type': 'read-only',
+                'rw': False,
+                'internal': True
             }
         }
         super(StandaloneVM, self).__init__(*args, **kwargs)
diff --git a/qubes/vm/templatevm.py b/qubes/vm/templatevm.py
index e681f5a0..9ca789f2 100644
--- a/qubes/vm/templatevm.py
+++ b/qubes/vm/templatevm.py
@@ -60,39 +60,41 @@ class TemplateVM(QubesVM):
             'root': {
                 'name': 'root',
                 'pool': 'default',
-                'volume_type': 'origin',
+                'snap_on_start': False,
+                'save_on_stop': True,
+                'rw': True,
+                'source': None,
                 'size': defaults['root_img_size'],
                 'internal': True
             },
             'private': {
                 'name': 'private',
                 'pool': 'default',
-                'volume_type': 'read-write',
+                'snap_on_start': False,
+                'save_on_stop': True,
+                'rw': True,
+                'source': None,
                 'size': defaults['private_img_size'],
+                'revisions_to_keep': 0,
                 'internal': True
             },
             'volatile': {
                 'name': 'volatile',
                 'pool': 'default',
                 'size': defaults['root_img_size'],
-                'volume_type': 'volatile',
-                'internal': True
+                'internal': True,
+                'rw': True,
             },
             'kernel': {
                 'name': 'kernel',
                 'pool': 'linux-kernel',
-                'volume_type': 'read-only',
-                'internal': True
+                'snap_on_start': True,
+                'internal': True,
+                'rw': False
             }
         }
         super(TemplateVM, self).__init__(*args, **kwargs)
 
-    def clone_disk_files(self, src):
-        super(TemplateVM, self).clone_disk_files(src)
-
-        # Create root-cow.img
-        self.commit_changes()
-
     def commit_changes(self):
         '''Commit changes to template'''
         self.log.debug('commit_changes()')

From d1c606b952a39aa5419204fe4b51801572d1fdb5 Mon Sep 17 00:00:00 2001
From: Bahtiar `kalkin-` Gadimov <bahtiar@gadimov.de>
Date: Tue, 12 Jul 2016 17:53:31 +0200
Subject: [PATCH 10/21] qubes.storage.file use new storage API

---
 qubes/storage/file.py       | 572 ++++++++++++++++--------------------
 qubes/tests/storage_file.py | 213 +++++++-------
 2 files changed, 355 insertions(+), 430 deletions(-)

diff --git a/qubes/storage/file.py b/qubes/storage/file.py
index d66be0fb..3f899f90 100644
--- a/qubes/storage/file.py
+++ b/qubes/storage/file.py
@@ -22,10 +22,8 @@
 # with this program; if not, write to the Free Software Foundation, Inc.,
 # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 #
-
 ''' This module contains pool implementations backed by file images'''
 
-
 from __future__ import absolute_import
 
 import os
@@ -33,88 +31,102 @@ import os.path
 import re
 import subprocess
 
-from qubes.storage import Pool, StoragePoolException, Volume
+import qubes.storage
 
 BLKSIZE = 512
 
 
-class FilePool(Pool):
-    ''' File based 'original' disk implementation '''
+class FilePool(qubes.storage.Pool):
+    ''' File based 'original' disk implementation
+    '''  # pylint: disable=protected-access
     driver = 'file'
 
-    def __init__(self, name=None, dir_path=None):
-        super(FilePool, self).__init__(name=name)
+    def __init__(self, revisions_to_keep=1, dir_path=None, **kwargs):
+        super(FilePool, self).__init__(revisions_to_keep=revisions_to_keep,
+                                       **kwargs)
         assert dir_path, "No pool dir_path specified"
         self.dir_path = os.path.normpath(dir_path)
         self._volumes = []
 
-    def clone(self, source, target):
-        ''' Clones the volume if the `source.pool` if the source is a
-            :py:class:`FileVolume`.
-        '''
-        if issubclass(FileVolume, 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
-        if _type == 'origin':
-            create_sparse_file(volume.path_origin, size)
-            create_sparse_file(volume.path_cow, size)
-        elif _type in ['read-write'] and source_volume:
-            copy_file(source_volume.path, volume.path)
-        elif _type in ['read-write', 'volatile']:
-            create_sparse_file(volume.path, size)
-
-        return volume
-
     @property
     def config(self):
         return {
             'name': self.name,
             'dir_path': self.dir_path,
             'driver': FilePool.driver,
+            'revisions_to_keep': self.revisions_to_keep
         }
 
-    def is_outdated(self, volume):
-        # FIX: Implement or remove this at all?
-        raise NotImplementedError
+    def clone(self, source, target):
+        new_dir = os.path.dirname(target.path)
+        if target._is_origin or target._is_volume:
+            if not os.path.exists:
+                os.makedirs(new_dir)
+            copy_file(source.path, target.path)
+        return target
+
+    def create(self, volume):
+        assert isinstance(volume.size, (int, long)) and volume.size > 0, \
+            'Volatile volume size must be > 0'
+        if volume._is_origin:
+            create_sparse_file(volume.path, volume.size)
+            create_sparse_file(volume.path_cow, volume.size)
+        elif not volume._is_snapshot:
+            if volume.source is not None:
+                source_path = os.path.join(self.dir_path,
+                                           volume.source + '.img')
+                copy_file(source_path, volume.path)
+            elif volume._is_volatile:
+                pass
+            else:
+                create_sparse_file(volume.path, volume.size)
+
+    def init_volume(self, vm, volume_config):
+        volume_config['dir_path'] = self.dir_path
+        if os.path.join(self.dir_path, self._vid_prefix(vm)) == vm.dir_path:
+            volume_config['backward_comp'] = True
+
+        if 'vid' not in volume_config:
+            volume_config['vid'] = os.path.join(
+                self._vid_prefix(vm), volume_config['name'])
+
+        try:
+            if volume_config['reset_on_start']:
+                volume_config['revisions_to_keep'] = 0
+        except KeyError:
+            pass
+        finally:
+            if 'revisions_to_keep' not in volume_config:
+                volume_config['revisions_to_keep'] = self.revisions_to_keep
+
+        volume = FileVolume(**volume_config)
+        self._volumes += [volume]
+        return volume
+
+    def is_dirty(self, volume):
+        return False  # TODO: How to implement this?
 
     def resize(self, volume, size):
         ''' Expands volume, throws
-            :py:class:`qubst.storage.StoragePoolException` if given size is
-            less than current_size
+            :py:class:`qubst.storage.qubes.storage.StoragePoolException` if
+            given size is less than current_size
         '''  # pylint: disable=no-self-use
-        _type = volume.volume_type
-        if _type not in ['origin', 'read-write', 'volatile']:
-            raise StoragePoolException('Can not resize a %s volume %s' %
-                                       (_type, volume.vid))
+        if not volume.rw:
+            msg = 'Can not resize reađonly volume {!s}'.format(volume)
+            raise qubes.storage.StoragePoolException(msg)
 
         if size <= volume.size:
-            raise StoragePoolException(
+            raise qubes.storage.StoragePoolException(
                 'For your own safety, shrinking of %s is'
                 ' disabled. If you really know what you'
                 ' are doing, use `truncate` on %s manually.' %
                 (volume.name, volume.vid))
 
-        if _type == 'origin':
-            path = volume.path_origin
-        elif _type in ['read-write', 'volatile']:
-            path = volume.path
-
-        with open(path, 'a+b') as fd:
+        with open(volume.path, 'a+b') as fd:
             fd.truncate(size)
 
-        p = subprocess.Popen(
-            ['sudo', 'losetup', '--associated', path],
-            stdout=subprocess.PIPE)
+        p = subprocess.Popen(['sudo', 'losetup', '--associated', volume.path],
+                             stdout=subprocess.PIPE)
         result = p.communicate()
 
         m = re.match(r'^(/dev/loop\d+):\s', result[0])
@@ -122,34 +134,57 @@ class FilePool(Pool):
             loop_dev = m.group(1)
 
             # resize loop device
-            subprocess.check_call(['sudo', 'losetup', '--set-capacity', loop_dev
-                                   ])
+            subprocess.check_call(['sudo', 'losetup', '--set-capacity',
+                                   loop_dev])
 
     def remove(self, volume):
-        if volume.volume_type in ['read-write', 'volatile']:
+        if not volume.internal:
+            return  # do not remove random attached file volumes
+        elif volume._is_snapshot:
+            return  # no need to remove, because it's just a snapshot
+        else:
             _remove_if_exists(volume.path)
-        elif volume.volume_type == 'origin':
-            _remove_if_exists(volume.path)
-            _remove_if_exists(volume.path_cow)
+            if volume._is_origin:
+                _remove_if_exists(volume.path_cow)
 
     def rename(self, volume, old_name, new_name):
         assert issubclass(volume.__class__, FileVolume)
-        old_dir = os.path.dirname(volume.path)
-        new_dir = os.path.join(os.path.dirname(old_dir), new_name)
+        subdir, _, volume_path = volume.vid.split('/', 2)
 
-        if not os.path.exists(new_dir):
-            os.makedirs(new_dir)
+        if volume._is_origin:
+            # TODO: Renaming the old revisions
+            new_path = os.path.join(self.dir_path, subdir, new_name)
+            if not os.path.exists(new_path):
+                os.mkdir(new_path, 0755)
+            new_volume_path = os.path.join(new_path, self.name + '.img')
+            if not volume.backward_comp:
+                os.rename(volume.path, new_volume_path)
+            new_volume_path_cow = os.path.join(new_path, self.name + '-cow.img')
+            if os.path.exists(new_volume_path_cow) and not volume.backward_comp:
+                os.rename(volume.path_cow, new_volume_path_cow)
 
-        volume.rename_target_dir(old_name, new_name)
+        volume.vid = os.path.join(subdir, new_name, volume_path)
 
         return volume
 
-    def commit_template_changes(self, volume):
-        if volume.volume_type != 'origin':
+    def import_volume(self, dst_pool, dst_volume, src_pool, src_volume):
+        msg = "Can not import snapshot volume {!s} in to pool {!s} "
+        msg = msg.format(src_volume, self)
+        assert not src_volume.snap_on_start, msg
+        if dst_volume.save_on_stop:
+            copy_file(src_pool.export(src_volume), dst_volume.path)
+        return dst_volume
+
+    def commit(self, volume):
+        msg = 'Tried to commit a non commitable volume {!r}'.format(volume)
+        assert (volume._is_origin or volume._is_volume) and volume.rw, msg
+
+        if volume._is_volume:
             return volume
 
         if os.path.exists(volume.path_cow):
-            os.rename(volume.path_cow, volume.path_cow + '.old')
+            old_path = volume.path_cow + '.old'
+            os.rename(volume.path_cow, old_path)
 
         old_umask = os.umask(002)
         with open(volume.path_cow, 'w') as f_cow:
@@ -160,6 +195,37 @@ class FilePool(Pool):
     def destroy(self):
         pass
 
+    def export(self, volume):
+        return volume.path
+
+    def reset(self, volume):
+        ''' Remove and recreate a volatile volume '''
+        assert volume._is_volatile, "Not a volatile volume"
+        assert isinstance(volume.size, (int, long)) and volume.size > 0, \
+            'Volatile volume size must be > 0'
+
+        _remove_if_exists(volume.path)
+
+        with open(volume.path, "w") as f_volatile:
+            f_volatile.truncate(volume.size)
+        return volume
+
+    def revert(self, volume, revision=None):
+        if revision is not None:
+            try:
+                return volume.revisions[revision]
+            except KeyError:
+                msg = "Volume {!r} does not have revision {!s}"
+                msg = msg.format(volume, revision)
+                raise qubes.storage.StoragePoolException(msg)
+        else:
+            try:
+                old_path = volume.revisions.values().pop()
+                os.rename(old_path, volume.path_cow)
+            except IndexError:
+                msg = "Volume {!r} does not have old revisions".format(volume)
+                raise qubes.storage.StoragePoolException(msg)
+
     def setup(self):
         create_dir_if_not_exists(self.dir_path)
         appvms_path = os.path.join(self.dir_path, 'appvms')
@@ -168,18 +234,39 @@ class FilePool(Pool):
         create_dir_if_not_exists(vm_templates_path)
 
     def start(self, volume):
-        if volume.volume_type == 'volatile':
-            _reset_volume(volume)
-        if volume.volume_type in ['origin', 'snapshot']:
-            _check_path(volume.path_origin)
-            _check_path(volume.path_cow)
-        else:
+        if volume._is_snapshot or volume._is_origin:
             _check_path(volume.path)
-
+            try:
+                _check_path(volume.path_cow)
+            except qubes.storage.StoragePoolException:
+                create_sparse_file(volume.path_cow, volume.size)
+                _check_path(volume.path_cow)
+        elif volume._is_volatile:
+            self.reset(volume)
         return volume
 
     def stop(self, volume):
-        pass
+        if volume.save_on_stop:
+            self.commit(volume)
+        elif volume._is_volatile:
+            _remove_if_exists(volume.path)
+        return volume
+
+    @staticmethod
+    def _vid_prefix(vm):
+        ''' Helper to create a prefix for the vid for volume
+        '''  # FIX Remove this if we drop the file backend
+        import qubes.vm.templatevm  # pylint: disable=redefined-outer-name
+        import qubes.vm.dispvm  # pylint: disable=redefined-outer-name
+        if isinstance(vm, qubes.vm.templatevm.TemplateVM):
+            subdir = 'vm-templates'
+        elif isinstance(vm, qubes.vm.dispvm.DispVM):
+            subdir = 'appvms'
+            return os.path.join(subdir, vm.template.name + '-dvm')
+        else:
+            subdir = 'appvms'
+
+        return os.path.join(subdir, vm.name)
 
     def target_dir(self, vm):
         """ Returns the path to vmdir depending on the type of the VM.
@@ -198,61 +285,8 @@ class FilePool(Pool):
                 string (str) absolute path to the directory where the vm files
                              are stored
         """
-        # FIX Remove this if we drop the file backend
-        import qubes.vm.templatevm  # nopep8
-        import qubes.vm.dispvm  # nopep8
-        if isinstance(vm, qubes.vm.templatevm.TemplateVM):
-            subdir = 'vm-templates'
-        elif isinstance(vm, qubes.vm.dispvm.DispVM):
-            subdir = 'appvms'
-            return os.path.join(self.dir_path, subdir,
-                                vm.template.name + '-dvm')
-        else:
-            subdir = 'appvms'
 
-        return os.path.join(self.dir_path, subdir, vm.name)
-
-    def init_volume(self, vm, volume_config):
-        assert 'volume_type' in volume_config, "Volume type missing " \
-            + str(volume_config)
-        volume_type = volume_config['volume_type']
-        known_types = {
-            'read-write': ReadWriteFile,
-            'read-only': ReadOnlyFile,
-            'origin': OriginFile,
-            'snapshot': SnapshotFile,
-            'volatile': VolatileFile,
-        }
-        if volume_type not in known_types:
-            raise StoragePoolException("Unknown volume type " + volume_type)
-
-        if volume_type in ['snapshot', 'read-only']:
-            name = volume_config['name']
-
-            origin_vm = vm.template
-            while origin_vm.volume_config[name]['volume_type'] == volume_type:
-                origin_vm = origin_vm.template
-
-            expected_origin_type = {
-                'snapshot': 'origin',
-                'read-only': 'read-write',  # FIXME: really?
-            }[volume_type]
-            assert origin_vm.volume_config[name]['volume_type'] == \
-                expected_origin_type
-
-            origin_pool = vm.app.get_pool(origin_vm.volume_config[name]['pool'])
-
-            assert isinstance(origin_pool,
-                              FilePool), 'Origin volume not a file volume'
-
-            volume_config['target_dir'] = origin_pool.target_dir(origin_vm)
-            volume_config['size'] = origin_vm.volume_config[name]['size']
-        else:
-            volume_config['target_dir'] = self.target_dir(vm)
-
-        volume = known_types[volume_type](**volume_config)
-        self._volumes += [volume]
-        return volume
+        return os.path.join(self.dir_path, self._vid_prefix(vm))
 
     def verify(self, volume):
         return volume.verify()
@@ -262,33 +296,77 @@ class FilePool(Pool):
         return self._volumes
 
 
-class FileVolume(Volume):
+class FileVolume(qubes.storage.Volume):
     ''' Parent class for the xen volumes implementation which expects a
-        `target_dir` param on initialization.
-    '''
+        `target_dir` param on initialization.  '''
 
-    def __init__(self, target_dir, **kwargs):
-        self.target_dir = target_dir
-        assert self.target_dir, "target_dir not specified"
+    def __init__(self, dir_path, backward_comp=False, **kwargs):
+        self.dir_path = dir_path
+        self.backward_comp = backward_comp
+        assert self.dir_path, "dir_path not specified"
         super(FileVolume, self).__init__(**kwargs)
 
-    def _new_dir(self, new_name):
-        ''' Returns a new directory path based on the new_name. This is a helper
-            method for moving file images during vm renaming.
+        if self.snap_on_start and self.source is None:
+            msg = "snap_on_start specified on {!r} but no volume source set"
+            msg = msg.format(self.name)
+            raise qubes.storage.StoragePoolException(msg)
+        elif not self.snap_on_start and self.source is not None:
+            msg = "source specified on {!r} but no snap_on_start set"
+            msg = msg.format(self.name)
+            raise qubes.storage.StoragePoolException(msg)
+
+        if self._is_snapshot:
+            self.path = os.path.join(self.dir_path, self.source + '.img')
+            img_name = self.source + '-cow.img'
+            self.path_cow = os.path.join(self.dir_path, img_name)
+        elif self._is_volume or self._is_volatile:
+            self.path = os.path.join(self.dir_path, self.vid + '.img')
+        elif self._is_origin:
+            self.path = os.path.join(self.dir_path, self.vid + '.img')
+            img_name = self.vid + '-cow.img'
+            self.path_cow = os.path.join(self.dir_path, img_name)
+        else:
+            assert False, 'This should not happen'
+
+    def verify(self):
+        ''' Verifies the volume. '''
+        if not os.path.exists(self.path) and not self._is_volatile:
+            msg = 'Missing image file: {!s}.'.format(self.path)
+            raise qubes.storage.StoragePoolException(msg)
+        return True
+
+    @property
+    def script(self):
+        if self._is_volume or self._is_volatile:
+            return None
+        elif self._is_origin:
+            return 'block-origin'
+        elif self._is_origin_snapshot or self._is_snapshot:
+            return 'block-snapshot'
+
+    def block_device(self):
+        ''' Return :py:class:`qubes.devices.BlockDevice` for serialization in
+            the libvirt XML template as <disk>.
         '''
-        old_dir = os.path.dirname(self.path)
-        return os.path.join(os.path.dirname(old_dir), new_name)
+        path = self.path
+        if self._is_origin or self._is_snapshot:
+            path += ":" + self.path_cow
+        return qubes.devices.BlockDevice(path, self.name, self.script, self.rw,
+                                         self.domain, self.devtype)
 
+    @property
+    def revisions(self):
+        if not hasattr(self, 'path_cow'):
+            return {}
 
-class SizeMixIn(FileVolume):
-    ''' A mix in which expects a `size` param to be > 0 on initialization and
-        provides a usage property wrapper.
-    '''
+        old_revision = self.path_cow + '.old'  # pylint: disable=no-member
 
-    def __init__(self, size=0, **kwargs):
-        assert size, 'Empty size provided'
-        assert size > 0, 'Size for volume ' + kwargs['name'] + ' is <=0'
-        super(SizeMixIn, self).__init__(size=int(size), **kwargs)
+        if not os.path.exists(old_revision):
+            return {}
+        else:
+            seconds = os.path.getctime(old_revision)
+            iso_date = qubes.storage.isodate(seconds).split('.', 1)[0]
+            return {iso_date: old_revision}
 
     @property
     def usage(self):
@@ -296,169 +374,31 @@ class SizeMixIn(FileVolume):
         return get_disk_usage(self.vid)
 
     @property
-    def config(self):
-        ''' return config data for serialization to qubes.xml '''
-        return {'name': self.name,
-                'pool': self.pool,
-                'size': str(self.size),
-                'volume_type': self.volume_type}
-
-
-class ReadWriteFile(SizeMixIn):
-    ''' Represents a readable & writable file image based volume '''
-
-    def __init__(self, **kwargs):
-        super(ReadWriteFile, self).__init__(**kwargs)
-        self.path = os.path.join(self.target_dir, self.name + '.img')
-        self.vid = self.path
-
-    def rename_target_dir(self, new_name, new_dir):
-        ''' Called by :py:class:`FilePool` when a domain changes it's name '''
-        # pylint: disable=unused-argument
-        old_path = self.path
-        file_name = os.path.basename(self.path)
-        new_path = os.path.join(new_dir, file_name)
-
-        os.rename(old_path, new_path)
-        self.target_dir = new_dir
-        self.path = new_path
-        self.vid = self.path
-
-    def verify(self):
-        ''' Verifies the volume. '''
-        if not os.path.exists(self.path):
-            raise StoragePoolException('Missing image file: %s' % self.path)
-
-
-class ReadOnlyFile(FileVolume):
-    ''' Represents a readonly file image based volume '''
-    usage = 0
-
-    def __init__(self, size=0, **kwargs):
-        super(ReadOnlyFile, self).__init__(size=int(size), **kwargs)
-        self.path = self.vid
-
-    def rename_target_dir(self, old_name, new_name):
-        """ Called by :py:class:`FilePool` when a domain changes it's name.
-
-        Only copies the volume if it belongs to the domain being renamed.
-        Currently if a volume is in a directory named the same as the domain,
-        it's ”owned” by the domain.
-        """
-        new_dir = self._new_dir(new_name)
-        if os.path.basename(self.target_dir) == old_name:
-            file_name = os.path.basename(self.path)
-            new_path = os.path.join(new_dir, file_name)
-            old_path = self.path
-
-            os.rename(old_path, new_path)
-
-            self.target_dir = new_dir
-            self.path = new_path
-            self.vid = self.path
-
-    def verify(self):
-        ''' Verifies the volume. '''
-        if not os.path.exists(self.path):
-            raise StoragePoolException('Missing image file: %s' % self.path)
-
-
-class OriginFile(SizeMixIn):
-    ''' Represents a readable, writeable & snapshotable file image based volume.
-
-        This is used for TemplateVM's
-    '''
-
-    script = 'block-origin'
-
-    def __init__(self, **kwargs):
-        super(OriginFile, self).__init__(**kwargs)
-        self.path_origin = os.path.join(self.target_dir, self.name + '.img')
-        self.path_cow = os.path.join(self.target_dir, self.name + '-cow.img')
-        self.path = '%s:%s' % (self.path_origin, self.path_cow)
-        self.vid = self.path_origin
-
-    def commit(self):
-        ''' Commit Template changes '''
-        raise NotImplementedError
-
-    def rename_target_dir(self, old_name, new_name):
-        ''' Called by :py:class:`FilePool` when a domain changes it's name.
-        '''  # pylint: disable=unused-argument
-        new_dir = self._new_dir(new_name)
-        old_path_origin = self.path_origin
-        old_path_cow = self.path_cow
-        new_path_origin = os.path.join(new_dir, self.name + '.img')
-        new_path_cow = os.path.join(new_dir, self.name + '-cow.img')
-        os.rename(old_path_origin, new_path_origin)
-        os.rename(old_path_cow, new_path_cow)
-        self.target_dir = new_dir
-        self.path_origin = new_path_origin
-        self.path_cow = new_path_cow
-        self.path = '%s:%s' % (self.path_origin, self.path_cow)
-        self.vid = self.path_origin
+    def _is_volatile(self):
+        ''' Internal helper. Useful for differentiating volume handling '''
+        return not self.snap_on_start and not self.save_on_stop
 
     @property
-    def usage(self):
-        result = 0
-        if os.path.exists(self.path_origin):
-            result += get_disk_usage(self.path_origin)
-        if os.path.exists(self.path_cow):
-            result += get_disk_usage(self.path_cow)
-        return result
+    def _is_origin(self):
+        ''' Internal helper. Useful for differentiating volume handling '''
+        # pylint: disable=line-too-long
+        return not self.snap_on_start and self.save_on_stop and self.revisions_to_keep > 0  # NOQA
 
-    def verify(self):
-        ''' Verifies the volume. '''
-        if not os.path.exists(self.path_origin):
-            raise StoragePoolException('Missing image file: %s' %
-                                       self.path_origin)
+    @property
+    def _is_snapshot(self):
+        ''' Internal helper. Useful for differentiating volume handling '''
+        return self.snap_on_start and not self.save_on_stop
 
+    @property
+    def _is_origin_snapshot(self):
+        ''' Internal helper. Useful for differentiating volume handling '''
+        return self.snap_on_start and self.save_on_stop
 
-class SnapshotFile(FileVolume):
-    ''' Represents a readonly snapshot of an :py:class:`OriginFile` volume '''
-    script = 'block-snapshot'
-    rw = False
-    usage = 0
-
-    def __init__(self, name=None, size=None, **kwargs):
-        assert size
-        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
-
-    def verify(self):
-        ''' Verifies the volume. '''
-        if not os.path.exists(self.path_origin):
-            raise StoragePoolException('Missing image file: %s' %
-                                       self.path_origin)
-
-
-class VolatileFile(SizeMixIn):
-    ''' Represents a readable & writeable file based volume, which will be
-        discarded and recreated at each startup.
-    '''
-    def __init__(self, **kwargs):
-        super(VolatileFile, self).__init__(**kwargs)
-        self.path = os.path.join(self.target_dir, self.name + '.img')
-        self.vid = self.path
-
-    def rename_target_dir(self, old_name, new_name):
-        ''' Called by :py:class:`FilePool` when a domain changes it's name.
-    '''  # pylint: disable=unused-argument
-        new_dir = self._new_dir(new_name)
-        _remove_if_exists(self.path)
-        file_name = os.path.basename(self.path)
-        self.target_dir = new_dir
-        new_path = os.path.join(new_dir, file_name)
-        self.path = new_path
-        self.vid = self.path
-
-    def verify(self):
-        ''' Verifies the volume. '''
-        pass
-
+    @property
+    def _is_volume(self):
+        ''' Internal helper. Usefull for differentiating volume handling '''
+        # pylint: disable=line-too-long
+        return not self.snap_on_start and self.save_on_stop and self.revisions_to_keep == 0  # NOQA
 
 def create_sparse_file(path, size):
     ''' Create an empty sparse file '''
@@ -534,7 +474,9 @@ def copy_file(source, destination):
         os.makedirs(parent_dir)
 
     try:
-        subprocess.check_call(['cp', '--reflink=auto', source, destination])
+        cmd = ['sudo', 'cp', '--sparse=auto',
+               '--reflink=auto', source, destination]
+        subprocess.check_call(cmd)
     except subprocess.CalledProcessError:
         raise IOError('Error while copying {!r} to {!r}'.format(source,
                                                                 destination))
@@ -549,17 +491,5 @@ def _remove_if_exists(path):
 def _check_path(path):
     ''' Raise an StoragePoolException if ``path`` does not exist'''
     if not os.path.exists(path):
-        raise StoragePoolException('Missing image file: %s' % path)
-
-
-def _reset_volume(volume):
-    ''' Remove and recreate a volatile volume '''
-    assert volume.volume_type == 'volatile', "Not a volatile volume"
-
-    assert volume.size
-
-    _remove_if_exists(volume.path)
-
-    with open(volume.path, "w") as f_volatile:
-        f_volatile.truncate(volume.size)
-    return volume
+        msg = 'Missing image file: %s' % path
+        raise qubes.storage.StoragePoolException(msg)
diff --git a/qubes/tests/storage_file.py b/qubes/tests/storage_file.py
index c5e79280..7cd787f4 100644
--- a/qubes/tests/storage_file.py
+++ b/qubes/tests/storage_file.py
@@ -16,29 +16,27 @@
 # with this program; if not, write to the Free Software Foundation, Inc.,
 # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 
+''' Tests for the file storage backend '''
+
 import os
 import shutil
 
 import qubes.storage
 import qubes.tests.storage
-import unittest
 from qubes.config import defaults
-from qubes.storage import Storage
-from qubes.storage.file import (OriginFile, ReadOnlyFile, ReadWriteFile,
-                                SnapshotFile, VolatileFile)
-from qubes.tests import QubesTestCase, SystemTestsMixin
-from qubes.tests.storage import TestVM
 
 # :pylint: disable=invalid-name
 
+
 class TestApp(qubes.Qubes):
-    def __init__(self, *args, **kwargs):
-        super(TestApp, self).__init__('/tmp/qubes-test.xml',
-            load=False, offline_mode=True, **kwargs)
+    ''' A Mock App object '''
+    def __init__(self, *args, **kwargs):  # pylint: disable=unused-argument
+        super(TestApp, self).__init__('/tmp/qubes-test.xml', load=False,
+                                      offline_mode=True, **kwargs)
         self.load_initial_values()
         self.pools['linux-kernel'].dir_path = '/tmp/qubes-test-kernel'
-        dummy_kernel = os.path.join(
-            self.pools['linux-kernel'].dir_path, 'dummy')
+        dummy_kernel = os.path.join(self.pools['linux-kernel'].dir_path,
+                                    'dummy')
         os.makedirs(dummy_kernel)
         open(os.path.join(dummy_kernel, 'vmlinuz'), 'w').close()
         open(os.path.join(dummy_kernel, 'modules.img'), 'w').close()
@@ -46,16 +44,23 @@ class TestApp(qubes.Qubes):
         self.default_kernel = 'dummy'
 
     def cleanup(self):
+        ''' Remove temporary directories '''
         shutil.rmtree(self.pools['linux-kernel'].dir_path)
 
     def create_dummy_template(self):
-        self.add_new_vm(qubes.vm.templatevm.TemplateVM,
-            name='test-template', label='red',
-            memory=1024, maxmem=1024)
-        self.default_template = 'test-template'
+        ''' Initalizes a dummy TemplateVM as the `default_template` '''
+        template = self.add_new_vm(qubes.vm.templatevm.TemplateVM,
+                                   name='test-template', label='red',
+                                   memory=1024, maxmem=1024)
+        self.default_template = template
 
-class TC_00_FilePool(QubesTestCase):
-    """ This class tests some properties of the 'default' pool. """
+
+class TC_00_FilePool(qubes.tests.QubesTestCase):
+    """ This class tests some properties of the 'default' pool.
+
+        This test might become obsolete if we change the driver for the default
+        pool to something else as 'file'.
+    """
 
     def setUp(self):
         super(TC_00_FilePool, self).setUp()
@@ -76,21 +81,23 @@ class TC_00_FilePool(QubesTestCase):
         self.assertEquals(result, expected)
 
     def test001_default_storage_class(self):
-        """ Check when using default pool the Storage is ``Storage``. """
+        """ Check when using default pool the Storage is
+            ``qubes.storage.Storage``. """
         result = self._init_app_vm().storage
-        self.assertIsInstance(result, Storage)
+        self.assertIsInstance(result, qubes.storage.Storage)
 
     def _init_app_vm(self):
         """ Return initalised, but not created, AppVm. """
         vmname = self.make_vm_name('appvm')
         self.app.create_dummy_template()
-        return self.app.add_new_vm(qubes.vm.appvm.AppVM,
-                                   name=vmname,
+        return self.app.add_new_vm(qubes.vm.appvm.AppVM, name=vmname,
                                    template=self.app.default_template,
                                    label='red')
 
 
-class TC_01_FileVolumes(QubesTestCase):
+class TC_01_FileVolumes(qubes.tests.QubesTestCase):
+    ''' Test correct handling of different types of volumes '''
+
     POOL_DIR = '/tmp/test-pool'
     POOL_NAME = 'test-pool'
     POOL_CONF = {'driver': 'file', 'dir_path': POOL_DIR, 'name': POOL_NAME}
@@ -113,91 +120,99 @@ class TC_01_FileVolumes(QubesTestCase):
         config = {
             'name': 'root',
             'pool': self.POOL_NAME,
-            'volume_type': 'origin',
+            'save_on_stop': True,
+            'rw': True,
             'size': defaults['root_img_size'],
         }
-        vm = TestVM(self)
-        result = self.app.get_pool(self.POOL_NAME).init_volume(vm, config)
-        self.assertIsInstance(result, OriginFile)
-        self.assertEqual(result.name, 'root')
-        self.assertEqual(result.pool, self.POOL_NAME)
-        self.assertEqual(result.size, defaults['root_img_size'])
+        vm = qubes.tests.storage.TestVM(self)
+        volume = self.app.get_pool(self.POOL_NAME).init_volume(vm, config)
+        self.assertEqual(volume.name, 'root')
+        self.assertEqual(volume.pool, self.POOL_NAME)
+        self.assertEqual(volume.size, defaults['root_img_size'])
+        self.assertFalse(volume.snap_on_start)
+        self.assertTrue(volume.save_on_stop)
+        self.assertTrue(volume.rw)
 
     def test_001_snapshot_volume(self):
-        original_path = '/var/lib/qubes/vm-templates/fedora-23/root.img'
+        source = 'vm-templates/fedora-23/root'
         original_size = qubes.config.defaults['root_img_size']
         config = {
             'name': 'root',
             'pool': 'default',
-            'volume_type': 'snapshot',
-            'vid': original_path,
+            'snap_on_start': True,
+            'rw': False,
+            'source': source,
+            'size': original_size,
         }
-        vm = TestVM(self, template=self.app.default_template)
-        result = self.app.get_pool(self.POOL_NAME).init_volume(vm, config)
-        self.assertIsInstance(result, SnapshotFile)
-        self.assertEqual(result.name, 'root')
-        self.assertEqual(result.pool, 'default')
-        self.assertEqual(result.size, original_size)
+
+        template_vm = self.app.default_template
+        vm = qubes.tests.storage.TestVM(self, template=template_vm)
+        volume = self.app.get_pool(self.POOL_NAME).init_volume(vm, config)
+        self.assertEqual(volume.name, 'root')
+        self.assertEqual(volume.pool, 'default')
+        self.assertEqual(volume.size, original_size)
+        self.assertTrue(volume.snap_on_start)
+        self.assertTrue(volume.snap_on_start)
+        self.assertFalse(volume.save_on_stop)
+        self.assertFalse(volume.rw)
+        self.assertEqual(volume.usage, 0)
 
     def test_002_read_write_volume(self):
         config = {
             'name': 'root',
             'pool': self.POOL_NAME,
-            'volume_type': 'read-write',
+            'rw': True,
+            'save_on_stop': True,
             'size': defaults['root_img_size'],
         }
-        vm = TestVM(self)
-        result = self.app.get_pool(self.POOL_NAME).init_volume(vm, config)
-        self.assertIsInstance(result, ReadWriteFile)
-        self.assertEqual(result.name, 'root')
-        self.assertEqual(result.pool, self.POOL_NAME)
-        self.assertEqual(result.size, defaults['root_img_size'])
+        vm = qubes.tests.storage.TestVM(self)
+        volume = self.app.get_pool(self.POOL_NAME).init_volume(vm, config)
+        self.assertEqual(volume.name, 'root')
+        self.assertEqual(volume.pool, self.POOL_NAME)
+        self.assertEqual(volume.size, defaults['root_img_size'])
+        self.assertFalse(volume.snap_on_start)
+        self.assertTrue(volume.save_on_stop)
+        self.assertTrue(volume.rw)
 
-    @unittest.expectedFailure
-    def test_003_read_volume(self):
+    def test_003_read_only_volume(self):
         template = self.app.default_template
-        original_path = template.volumes['root'].vid
-        original_size = qubes.config.defaults['root_img_size']
-        config = {
-            'name': 'root',
-            'pool': 'default',
-            'volume_type': 'read-only',
-            'vid': original_path
-        }
-        vm = TestVM(self, template=template)
+        vid = template.volumes['root'].vid
+        config = {'name': 'root', 'pool': 'default', 'rw': False, 'vid': vid}
+        vm = qubes.tests.storage.TestVM(self, template=template)
 
-        result = self.app.get_pool(self.POOL_NAME).init_volume(vm, config)
-        self.assertIsInstance(result, ReadOnlyFile)
-        self.assertEqual(result.name, 'root')
-        self.assertEqual(result.pool, 'default')
-        self.assertEqual(result.size, original_size)
+        volume = self.app.get_pool(self.POOL_NAME).init_volume(vm, config)
+        self.assertEqual(volume.name, 'root')
+        self.assertEqual(volume.pool, 'default')
+
+        # original_size = qubes.config.defaults['root_img_size']
+        # FIXME: self.assertEqual(volume.size, original_size)
+        self.assertFalse(volume.snap_on_start)
+        self.assertFalse(volume.save_on_stop)
+        self.assertFalse(volume.rw)
 
     def test_004_volatile_volume(self):
         config = {
             'name': 'root',
             'pool': self.POOL_NAME,
-            'volume_type': 'volatile',
             'size': defaults['root_img_size'],
+            'rw': True,
         }
-        vm = TestVM(self)
-        result = self.app.get_pool(self.POOL_NAME).init_volume(vm, config)
-        self.assertIsInstance(result, VolatileFile)
-        self.assertEqual(result.name, 'root')
-        self.assertEqual(result.pool, self.POOL_NAME)
-        self.assertEqual(result.size, defaults['root_img_size'])
+        vm = qubes.tests.storage.TestVM(self)
+        volume = self.app.get_pool(self.POOL_NAME).init_volume(vm, config)
+        self.assertEqual(volume.name, 'root')
+        self.assertEqual(volume.pool, self.POOL_NAME)
+        self.assertEqual(volume.size, defaults['root_img_size'])
+        self.assertFalse(volume.snap_on_start)
+        self.assertFalse(volume.save_on_stop)
+        self.assertTrue(volume.rw)
 
     def test_005_appvm_volumes(self):
         ''' Check if AppVM volumes are propertly initialized '''
         vmname = self.make_vm_name('appvm')
-        vm = self.app.add_new_vm(qubes.vm.appvm.AppVM,
-                                 name=vmname,
+        vm = self.app.add_new_vm(qubes.vm.appvm.AppVM, name=vmname,
                                  template=self.app.default_template,
                                  label='red')
 
-        volumes = vm.volumes
-        self.assertIsInstance(volumes['root'], SnapshotFile)
-        self.assertIsInstance(volumes['private'], OriginFile)
-        self.assertIsInstance(volumes['volatile'], VolatileFile)
         expected = vm.template.dir_path + '/root.img:' + vm.template.dir_path \
             + '/root-cow.img'
         self.assertVolumePath(vm, 'root', expected, rw=False)
@@ -210,14 +225,9 @@ class TC_01_FileVolumes(QubesTestCase):
     def test_006_template_volumes(self):
         ''' Check if TemplateVM volumes are propertly initialized '''
         vmname = self.make_vm_name('appvm')
-        vm = self.app.add_new_vm(qubes.vm.templatevm.TemplateVM,
-                                 name=vmname,
+        vm = self.app.add_new_vm(qubes.vm.templatevm.TemplateVM, name=vmname,
                                  label='red')
 
-        volumes = vm.volumes
-        self.assertIsInstance(volumes['root'], OriginFile)
-        self.assertIsInstance(volumes['private'], ReadWriteFile)
-        self.assertIsInstance(volumes['volatile'], VolatileFile)
         expected = vm.dir_path + '/root.img:' + vm.dir_path + '/root-cow.img'
         self.assertVolumePath(vm, 'root', expected, rw=True)
         expected = vm.dir_path + '/private.img'
@@ -233,7 +243,7 @@ class TC_01_FileVolumes(QubesTestCase):
         self.assertEquals(b_dev.path, expected)
 
 
-class TC_03_FilePool(QubesTestCase):
+class TC_03_FilePool(qubes.tests.QubesTestCase):
     """ Test the paths for the default file based pool (``FilePool``).
     """
 
@@ -263,7 +273,6 @@ class TC_03_FilePool(QubesTestCase):
             shutil.rmtree('/tmp/qubes-test')
         qubes.config.system_path['qubes_base_dir'] = self._orig_qubes_base_dir
 
-
     def test_001_pool_exists(self):
         """ Check if the storage pool was added to the storage pool config """
         self.assertIn('test-pool', self.app.pools.keys())
@@ -290,8 +299,7 @@ class TC_03_FilePool(QubesTestCase):
         """ Check if all the needed image files are created for an AppVm"""
 
         vmname = self.make_vm_name('appvm')
-        vm = self.app.add_new_vm(qubes.vm.appvm.AppVM,
-                                 name=vmname,
+        vm = self.app.add_new_vm(qubes.vm.appvm.AppVM, name=vmname,
                                  template=self.app.default_template,
                                  volume_config={
                                      'private': {
@@ -300,25 +308,17 @@ class TC_03_FilePool(QubesTestCase):
                                      'volatile': {
                                          'pool': 'test-pool'
                                      }
-                                 },
-                                 label='red')
-        vm.storage.create()
+                                 }, label='red')
+        vm.create_on_disk()
 
         expected_vmdir = os.path.join(self.APPVMS_DIR, vm.name)
 
-        expected_private_origin_path = \
-            os.path.join(expected_vmdir, 'private.img')
-        expected_private_cow_path = \
-            os.path.join(expected_vmdir, 'private-cow.img')
-        expected_private_path = '%s:%s' % (expected_private_origin_path,
-                                        expected_private_cow_path)
+        expected_private_path = os.path.join(expected_vmdir, 'private.img')
         self.assertEquals(vm.volumes['private'].path, expected_private_path)
-        self.assertEqualsAndExists(vm.volumes['private'].path_origin,
-            expected_private_origin_path)
-        self.assertEqualsAndExists(vm.volumes['private'].path_cow,
-            expected_private_cow_path)
 
         expected_volatile_path = os.path.join(expected_vmdir, 'volatile.img')
+        vm.storage.get_pool(vm.volumes['volatile'])\
+            .reset(vm.volumes['volatile'])
         self.assertEqualsAndExists(vm.volumes['volatile'].path,
                                    expected_volatile_path)
 
@@ -327,8 +327,7 @@ class TC_03_FilePool(QubesTestCase):
             created propertly by the storage system
         """
         vmname = self.make_vm_name('tmvm')
-        vm = self.app.add_new_vm(qubes.vm.templatevm.TemplateVM,
-                                 name=vmname,
+        vm = self.app.add_new_vm(qubes.vm.templatevm.TemplateVM, name=vmname,
                                  volume_config={
                                      'root': {
                                          'pool': 'test-pool'
@@ -339,8 +338,7 @@ class TC_03_FilePool(QubesTestCase):
                                      'volatile': {
                                          'pool': 'test-pool'
                                      }
-                                 },
-                                 label='red')
+                                 }, label='red')
         vm.create_on_disk()
 
         expected_vmdir = os.path.join(self.TEMPLATES_DIR, vm.name)
@@ -349,18 +347,14 @@ class TC_03_FilePool(QubesTestCase):
         expected_root_cow_path = os.path.join(expected_vmdir, 'root-cow.img')
         expected_root_path = '%s:%s' % (expected_root_origin_path,
                                         expected_root_cow_path)
-        self.assertEquals(vm.volumes['root'].path, expected_root_path)
-        self.assertExist(vm.volumes['root'].path_origin)
+        self.assertEquals(vm.volumes['root'].block_device().path,
+                          expected_root_path)
+        self.assertExist(vm.volumes['root'].path)
 
         expected_private_path = os.path.join(expected_vmdir, 'private.img')
         self.assertEqualsAndExists(vm.volumes['private'].path,
                                    expected_private_path)
 
-        expected_volatile_path = os.path.join(expected_vmdir, 'volatile.img')
-        self.assertEqualsAndExists(vm.volumes['volatile'].path,
-                                   expected_volatile_path)
-
-        vm.storage.commit_template_changes()
         expected_rootcow_path = os.path.join(expected_vmdir, 'root-cow.img')
         self.assertEqualsAndExists(vm.volumes['root'].path_cow,
                                    expected_rootcow_path)
@@ -377,4 +371,5 @@ class TC_03_FilePool(QubesTestCase):
     def assertExist(self, path):
         """ Assert that the given path exists. """
         # :pylint: disable=invalid-name
-        self.assertTrue(os.path.exists(path), "Path %s does not exist" % path)
+        self.assertTrue(
+            os.path.exists(path), "Path {!s} does not exist".format(path))

From f60ccb235da782750b750134986deda5aa15907c Mon Sep 17 00:00:00 2001
From: Bahtiar `kalkin-` Gadimov <bahtiar@gadimov.de>
Date: Tue, 12 Jul 2016 17:54:52 +0200
Subject: [PATCH 11/21] qubes.storage.domain use new storage API

---
 qubes/storage/domain.py | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/qubes/storage/domain.py b/qubes/storage/domain.py
index e2ab388c..d9d6207d 100644
--- a/qubes/storage/domain.py
+++ b/qubes/storage/domain.py
@@ -21,7 +21,7 @@
 #
 ''' Manages block devices in a domain '''
 
-import string
+import string  # pylint: disable=deprecated-module
 
 from qubes.storage import Pool, Volume
 
@@ -99,15 +99,12 @@ class DomainPool(Pool):
 class DomainVolume(Volume):
     ''' A volume provided by a block device in an domain '''
 
-    def __init__(self, name, pool, desc, mode, size):
-        if mode == 'w':
-            volume_type = 'read-write'
-        else:
-            volume_type = 'read-only'
+    def __init__(self, name, pool, desc, mode, **kwargs):
+        rw = (mode == 'w')
 
-        super(DomainVolume, self).__init__(desc,
-                                           pool,
-                                           volume_type,
-                                           vid=name,
-                                           size=size,
-                                           removable=True)
+        super(DomainVolume, self).__init__(desc, pool, vid=name, removable=True,
+                                           rw=rw, **kwargs)
+
+    @property
+    def revisions(self):
+        return {}

From 95fed1eb713987a0bf33b69aaeaa8ec2034fad1a Mon Sep 17 00:00:00 2001
From: Bahtiar `kalkin-` Gadimov <bahtiar@gadimov.de>
Date: Tue, 12 Jul 2016 18:16:15 +0200
Subject: [PATCH 12/21] qubes.linux.kernel use new storage api

---
 qubes/storage/kernels.py | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/qubes/storage/kernels.py b/qubes/storage/kernels.py
index cb7557ec..5d5e703d 100644
--- a/qubes/storage/kernels.py
+++ b/qubes/storage/kernels.py
@@ -33,12 +33,16 @@ class LinuxModules(Volume):
 
     def __init__(self, target_dir, kernel_version, **kwargs):
         kwargs['vid'] = kernel_version
+        kwargs['source'] = self
         super(LinuxModules, self).__init__(**kwargs)
         self.kernels_dir = os.path.join(target_dir, kernel_version)
         self.path = os.path.join(self.kernels_dir, 'modules.img')
         self.vmlinuz = os.path.join(self.kernels_dir, 'vmlinuz')
         self.initramfs = os.path.join(self.kernels_dir, 'initramfs')
 
+    @property
+    def revisions(self):
+        return {}
 
 class LinuxKernel(Pool):
     ''' Provides linux kernels '''
@@ -50,23 +54,22 @@ class LinuxKernel(Pool):
         self.dir_path = dir_path
 
     def init_volume(self, vm, volume_config):
-        assert 'volume_type' in volume_config, "Volume type missing " \
-            + str(volume_config)
-        volume_type = volume_config['volume_type']
-        if volume_type != 'read-only':
-            raise StoragePoolException("Unknown volume type " + volume_type)
+        assert not volume_config['rw']
 
         volume = LinuxModules(self.dir_path, vm.kernel, **volume_config)
 
         return volume
 
+    def is_dirty(self, volume):
+        return False
+
     def clone(self, source, target):
         return target
 
-    def create(self, volume, source_volume=None):
+    def create(self, volume):
         return volume
 
-    def commit_template_changes(self, volume):
+    def commit(self, volume):
         return volume
 
     @property
@@ -80,6 +83,12 @@ class LinuxKernel(Pool):
     def destroy(self):
         pass
 
+    def export(self, volume):
+        return volume.path
+
+    def import_volume(self, dst_pool, dst_volume, src_pool, src_volume):
+        pass
+
     def is_outdated(self, volume):
         return False
 
@@ -115,7 +124,8 @@ class LinuxKernel(Pool):
                              pool=self.name,
                              name=kernel_version,
                              internal=True,
-                             volume_type='read-only')
+                             rw=False
+                             )
                 for kernel_version in os.listdir(self.dir_path)]
 
 

From e07c4cc8e8c54ab803dc8340dcbfa326de75ac54 Mon Sep 17 00:00:00 2001
From: Bahtiar `kalkin-` Gadimov <bahtiar@gadimov.de>
Date: Sun, 3 Jul 2016 18:21:48 +0200
Subject: [PATCH 13/21] qvm-block use new storage API

---
 qubes/tools/qvm_block.py | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/qubes/tools/qvm_block.py b/qubes/tools/qvm_block.py
index 4f1d708a..d0cf99d7 100644
--- a/qubes/tools/qvm_block.py
+++ b/qubes/tools/qvm_block.py
@@ -45,19 +45,20 @@ def prepare_table(vd_list, full=False):
     '''
     output = []
     if sys.stdout.isatty():
-        output += [('POOL_NAME:VOLUME_ID', 'VOLUME_TYPE', 'VMNAME')]
+        output += [('POOL:VOLUME', 'VMNAME', 'VOLUME_NAME')]  # NOQA
 
     for volume in vd_list:
         if volume.domains:
-            vmname = volume.domains.pop()
-            output += [(str(volume), volume.volume_type, vmname)]
-            for vmname in volume.domains:
+            vmname, volume_name = volume.domains.pop()
+            output += [(str(volume), vmname, volume_name)]
+            for tupple in volume.domains:
+                vmname, volume_name = tupple
                 if full or not sys.stdout.isatty():
-                    output += [(str(volume), volume.volume_type, vmname)]
+                    output += [(str(volume), vmname, volume_name)]
                 else:
-                    output += [('', '', vmname)]
+                    output += [('', vmname, volume_name)]
         else:
-            output += [(str(volume), volume.volume_type)]
+            output += [(str(volume), "")]
 
     return output
 
@@ -70,7 +71,6 @@ class VolumeData(object):
     def __init__(self, volume):
         self.name = volume.name
         self.pool = volume.pool
-        self.volume_type = volume.volume_type
         self.vid = volume.vid
         self.domains = []
 

From 9acd46bddbafc8ec648354ea63a77d0029f32878 Mon Sep 17 00:00:00 2001
From: Bahtiar `kalkin-` Gadimov <bahtiar@gadimov.de>
Date: Tue, 12 Jul 2016 18:22:54 +0200
Subject: [PATCH 14/21] qvm-block show if old revisions are available

---
 qubes/tools/qvm_block.py | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/qubes/tools/qvm_block.py b/qubes/tools/qvm_block.py
index d0cf99d7..a13c6efb 100644
--- a/qubes/tools/qvm_block.py
+++ b/qubes/tools/qvm_block.py
@@ -50,13 +50,14 @@ def prepare_table(vd_list, full=False):
     for volume in vd_list:
         if volume.domains:
             vmname, volume_name = volume.domains.pop()
-            output += [(str(volume), vmname, volume_name)]
+            output += [(str(volume), vmname, volume_name, volume.revisions)]
             for tupple in volume.domains:
                 vmname, volume_name = tupple
                 if full or not sys.stdout.isatty():
-                    output += [(str(volume), vmname, volume_name)]
+                    output += [(str(volume), vmname, volume_name,
+                            volume.revisions)]
                 else:
-                    output += [('', vmname, volume_name)]
+                    output += [('', vmname, volume_name, '', volume.revisions)]
         else:
             output += [(str(volume), "")]
 
@@ -72,6 +73,10 @@ class VolumeData(object):
         self.name = volume.name
         self.pool = volume.pool
         self.vid = volume.vid
+        if volume.revisions != {}:
+            self.revisions = 'Yes'
+        else:
+            self.revisions = 'No'
         self.domains = []
 
     def __str__(self):
@@ -110,7 +115,7 @@ def list_volumes(args):
             for volume in domain.attached_volumes:
                 try:
                     volume_data = vd_dict[volume.pool][volume.vid]
-                    volume_data.domains += [domain.name]
+                    volume_data.domains += [(domain.name, volume.name)]
                 except KeyError:
                     # Skipping volume
                     continue

From 53ff88cd154eac3dd56c872cc09704941bf4c388 Mon Sep 17 00:00:00 2001
From: Bahtiar `kalkin-` Gadimov <bahtiar@gadimov.de>
Date: Tue, 12 Jul 2016 18:23:28 +0200
Subject: [PATCH 15/21] qvm-block add revert command

---
 doc/manpages/qvm-block.rst |  9 +++++++++
 qubes/tools/qvm_block.py   | 18 ++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/doc/manpages/qvm-block.rst b/doc/manpages/qvm-block.rst
index 60f4d22c..be8fa65f 100644
--- a/doc/manpages/qvm-block.rst
+++ b/doc/manpages/qvm-block.rst
@@ -86,6 +86,15 @@ Detach the volume with *POOL_NAME:VOLUME_ID* from domain *VMNAME*
 
 aliases: d, dt
 
+revert
+^^^^^^
+
+| :command:`qvm-block revert` [-h] [--verbose] [--quiet] *POOL_NAME:VOLUME_ID*
+
+Revert a volume to previous revision.
+
+aliases: rv, r
+
 Authors
 -------
 
diff --git a/qubes/tools/qvm_block.py b/qubes/tools/qvm_block.py
index a13c6efb..855370c2 100644
--- a/qubes/tools/qvm_block.py
+++ b/qubes/tools/qvm_block.py
@@ -131,6 +131,15 @@ def list_volumes(args):
         result = [x for p in vd_dict.itervalues() for x in p.itervalues()]
     qubes.tools.print_table(prepare_table(result, full=args.full))
 
+def revert_volume(args):
+    volume = args.volume
+    app = args.app
+    try:
+        pool = app.pools[volume.pool]
+        pool.revert(volume)
+    except qubes.storage.StoragePoolException as e:
+        print(e.message, file=sys.stderr)
+        sys.exit(1)
 
 def attach_volumes(args):
     ''' Called by the parser to execute the :program:`qvm-block attach`
@@ -178,6 +187,14 @@ def init_list_parser(sub_parsers):
     list_parser._mutually_exclusive_groups.append(vm_name_group)
     list_parser.set_defaults(func=list_volumes)
 
+def init_revert_parser(sub_parsers):
+    revert_parser = sub_parsers.add_parser(
+        'revert', aliases=('rv', 'r'),
+        help='revert volume to previous revision')
+    revert_parser.add_argument(metavar='POOL_NAME:VOLUME_ID', dest='volume',
+                               action=qubes.tools.VolumeAction)
+    revert_parser.set_defaults(func=revert_volume)
+
 
 def get_parser():
     '''Create :py:class:`argparse.ArgumentParser` suitable for
@@ -190,6 +207,7 @@ def get_parser():
         description="For more information see qvm-block command -h",
         dest='command')
     init_list_parser(sub_parsers)
+    init_revert_parser(sub_parsers)
     attach_parser = sub_parsers.add_parser(
         'attach', help="Attach volume to domain", aliases=('at', 'a'))
     attach_parser.add_argument('--ro', help='attach device read-only',

From bb8b58b04cd463458d1341b1a35a5c9ac9acd0ef Mon Sep 17 00:00:00 2001
From: Bahtiar `kalkin-` Gadimov <bahtiar@gadimov.de>
Date: Tue, 12 Jul 2016 18:25:37 +0200
Subject: [PATCH 16/21] qubes.backup fix verify_files

---
 qubes/backup.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qubes/backup.py b/qubes/backup.py
index cfce2eac..03636a72 100644
--- a/qubes/backup.py
+++ b/qubes/backup.py
@@ -2178,7 +2178,7 @@ class BackupRestore(object):
                         vm.features['backup-path']),
                         new_vm.dir_path)
 
-                new_vm.verify_files()
+                new_vm.storage.verify()
             except Exception as err:
                 self.log.error("ERROR: {0}".format(err))
                 self.log.warning("*** Skipping VM: {0}".format(vm.name))

From 61feb0ced74b79b395b3746bb9d366b15dde5730 Mon Sep 17 00:00:00 2001
From: Bahtiar `kalkin-` Gadimov <bahtiar@gadimov.de>
Date: Wed, 13 Jul 2016 02:22:52 +0200
Subject: [PATCH 17/21] Migrate backup to new storage api

---
 qubes/backup.py           | 13 ++++++-------
 qubes/tests/__init__.py   |  8 ++++----
 qubes/tests/int/backup.py | 14 ++++++++------
 3 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/qubes/backup.py b/qubes/backup.py
index 03636a72..777602d0 100644
--- a/qubes/backup.py
+++ b/qubes/backup.py
@@ -343,7 +343,7 @@ class Backup(object):
             # TODO this is file pool specific. Change it to a more general
             # solution
             if vm.volumes['private'] is not None:
-                path_to_private_img = vm.volumes['private'].vid
+                path_to_private_img = vm.volumes['private'].path
                 vm_files.append(self.FileToBackup(path_to_private_img, subdir))
 
             vm_files.append(self.FileToBackup(vm.icon_path, subdir))
@@ -358,7 +358,7 @@ class Backup(object):
             if vm.updateable:
                 # TODO this is file pool specific. Change it to a more general
                 # solution
-                path_to_root_img = vm.volumes['root'].vid
+                path_to_root_img = vm.volumes['root'].path
                 vm_files.append(self.FileToBackup(path_to_root_img, subdir))
             files_to_backup[vm.qid] = self.VMToBackup(vm, vm_files, subdir)
 
@@ -1725,11 +1725,10 @@ class BackupRestore(object):
             # wait for other processes (if any)
             for proc in self.processes_to_kill_on_cancel:
                 proc.wait()
-
-            if vmproc.returncode != 0:
-                raise qubes.exc.QubesException(
-                    "Backup completed, but VM receiving it reported an error "
-                    "(exit code {})".format(vmproc.returncode))
+                if proc.returncode != 0:
+                    raise qubes.exc.QubesException(
+                        "Backup completed, but VM receiving it reported an error "
+                        "(exit code {})".format(proc.returncode))
 
             if filename and filename != "EOF":
                 raise qubes.exc.QubesException(
diff --git a/qubes/tests/__init__.py b/qubes/tests/__init__.py
index 13e03593..44c884c7 100644
--- a/qubes/tests/__init__.py
+++ b/qubes/tests/__init__.py
@@ -820,7 +820,7 @@ class BackupTestsMixin(SystemTestsMixin):
             name=vmname, template=template, provides_network=True, label='red')
         testnet.create_on_disk()
         vms.append(testnet)
-        self.fill_image(testnet.volumes['private'].vid, 20*1024*1024)
+        self.fill_image(testnet.volumes['private'].path, 20*1024*1024)
 
         vmname = self.make_vm_name('test1')
         if self.verbose:
@@ -831,7 +831,7 @@ class BackupTestsMixin(SystemTestsMixin):
         testvm1.netvm = testnet
         testvm1.create_on_disk()
         vms.append(testvm1)
-        self.fill_image(testvm1.volumes['private'].vid, 100*1024*1024)
+        self.fill_image(testvm1.volumes['private'].path, 100*1024*1024)
 
         vmname = self.make_vm_name('testhvm1')
         if self.verbose:
@@ -841,7 +841,7 @@ class BackupTestsMixin(SystemTestsMixin):
                                       hvm=True,
                                       label='red')
         testvm2.create_on_disk()
-        self.fill_image(testvm2.volumes['root'].vid, 1024 * 1024 * 1024, True)
+        self.fill_image(testvm2.volumes['root'].path, 1024 * 1024 * 1024, True)
         vms.append(testvm2)
 
         vmname = self.make_vm_name('template')
@@ -850,7 +850,7 @@ class BackupTestsMixin(SystemTestsMixin):
         testvm3 = self.app.add_new_vm(qubes.vm.templatevm.TemplateVM,
             name=vmname, label='red')
         testvm3.create_on_disk()
-        self.fill_image(testvm3.root_img, 100*1024*1024, True)
+        self.fill_image(testvm3.volumes['root'].path, 100 * 1024 * 1024, True)
         vms.append(testvm3)
 
         vmname = self.make_vm_name('custom')
diff --git a/qubes/tests/int/backup.py b/qubes/tests/int/backup.py
index 91852913..57a41a9e 100644
--- a/qubes/tests/int/backup.py
+++ b/qubes/tests/int/backup.py
@@ -26,7 +26,6 @@
 
 import os
 
-import unittest
 import sys
 import qubes
 import qubes.exc
@@ -77,10 +76,13 @@ class TC_00_Backup(qubes.tests.BackupTestsMixin, qubes.tests.QubesTestCase):
         hvmtemplate = self.app.add_new_vm(
             qubes.vm.templatevm.TemplateVM, name=vmname, hvm=True, label='red')
         hvmtemplate.create_on_disk()
-        self.fill_image(os.path.join(hvmtemplate.dir_path, '00file'),
-                        195*1024*1024-4096*3)
-        self.fill_image(hvmtemplate.private_img, 195*1024*1024-4096*3)
-        self.fill_image(hvmtemplate.root_img, 1024*1024*1024, sparse=True)
+        self.fill_image(
+            os.path.join(hvmtemplate.dir_path, '00file'),
+            195 * 1024 * 1024 - 4096 * 3)
+        self.fill_image(hvmtemplate.volumes['private'].path,
+                        195 * 1024 * 1024 - 4096 * 3)
+        self.fill_image(hvmtemplate.volumes['root'].path, 1024 * 1024 * 1024,
+                        sparse=True)
         vms.append(hvmtemplate)
         self.app.save()
 
@@ -93,7 +95,7 @@ class TC_00_Backup(qubes.tests.BackupTestsMixin, qubes.tests.QubesTestCase):
 
     def test_005_compressed_custom(self):
         vms = self.create_backup_vms()
-        self.make_backup(vms, compressed="bzip2")
+        self.make_backup(vms, compression_filter="bzip2")
         self.remove_vms(reversed(vms))
         self.restore_backup()
         for vm in vms:

From 496434d8653a7c00f11ffb37a697d34a55a6aa9c Mon Sep 17 00:00:00 2001
From: Bahtiar `kalkin-` Gadimov <bahtiar@gadimov.de>
Date: Tue, 12 Jul 2016 18:36:53 +0200
Subject: [PATCH 18/21] qvm-create uses new api

- `-p` is now used for `--pool` instead of `--property`
- Documented pool usage
---
 doc/manpages/qvm-create.rst | 11 ++++++++---
 qubes/tools/qvm_create.py   | 22 +++++++++++++++-------
 2 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/doc/manpages/qvm-create.rst b/doc/manpages/qvm-create.rst
index ed008e97..69988a8c 100644
--- a/doc/manpages/qvm-create.rst
+++ b/doc/manpages/qvm-create.rst
@@ -32,7 +32,7 @@ Options
    The new domain class name (default: **AppVM** for
    :py:class:`qubes.vm.appvm.AppVM`).
 
-.. option:: --prop=NAME=VALUE, --property=NAME=VALUE, -p NAME=VALUE
+.. option:: --prop=NAME=VALUE, --property=NAME=VALUE
 
    Set domain's property, like "internal", "memory" or "vcpus". Any property may
    be set this way, even "qid".
@@ -57,9 +57,14 @@ Options
    Use provided :file:`root.img` instead of default/empty one (file will be
    *moved*). This option is mutually exclusive with :option:`--root-copy-from`.
 
-.. option:: --pool=POOL_NAME:VOLUME_NAME, -P POOL_NAME:VOLUME_NAME
+.. option:: -P POOL
 
-    Specify the pool to use for a volume
+    Pool to use for the new domain. All volumes besides snapshots volumes are
+    imported in to the specified POOL. ~HIS IS WHAT YOU WANT TO USE NORMALLY.
+
+.. option:: --pool=POOL:VOLUME, -p POOL:VOLUME
+
+    Specify the pool to use for the specific volume
 
 Options for internal use
 ------------------------
diff --git a/qubes/tools/qvm_create.py b/qubes/tools/qvm_create.py
index a3b47e13..acf2cbfa 100644
--- a/qubes/tools/qvm_create.py
+++ b/qubes/tools/qvm_create.py
@@ -42,15 +42,21 @@ parser.add_argument('--class', '-C', dest='cls',
     default='AppVM',
     help='specify the class of the new domain (default: %(default)s)')
 
-parser.add_argument('--property', '--prop', '-p',
+parser.add_argument('--property', '--prop',
     action=qubes.tools.PropertyAction,
     help='set domain\'s property, like "internal", "memory" or "vcpus"')
 
-parser.add_argument('--pool', '-P',
+parser.add_argument('--pool', '-p',
                     action='append',
                     metavar='POOL_NAME:VOLUME_NAME',
                     help='specify the pool to use for a volume')
 
+parser.add_argument('-P',
+                    metavar='POOL_NAME',
+                    dest='one_pool',
+                    default='',
+                    help='change all volume pools to specified pool')
+
 parser.add_argument('--template', '-t',
     action=qubes.tools.SinglePropertyAction,
     help='specify the TemplateVM to use')
@@ -80,16 +86,18 @@ parser.add_argument('name', metavar='VMNAME',
 def main(args=None):
     args = parser.parse_args(args)
 
-    if args.pool:
-        args.properties['volume_config'] = {}
+    pools = {}
+    pool = None
+    if hasattr(args, 'pools') and args.pools:
         for pool_vol in args.pool:
             try:
                 pool_name, volume_name = pool_vol.split(':')
-                config = {'pool': pool_name, 'name': volume_name}
-                args.properties['volume_config'][volume_name] = config
+                pools[volume_name] = pool_name
             except ValueError:
                 parser.error(
                     'Pool argument must be of form: -P pool_name:volume_name')
+    if args.one_pool:
+        pool = args.one_pool
 
     if 'label' not in args.properties:
         parser.error('--label option is mandatory')
@@ -144,7 +152,7 @@ def main(args=None):
 
     if not args.no_root:
         try:
-            vm.create_on_disk()
+            vm.create_on_disk(pool, pools)
 
             # TODO this is file pool specific. Change it to a more general
             # solution

From 1467f1ede540f532ce9163261b68321d4accc41e Mon Sep 17 00:00:00 2001
From: Bahtiar `kalkin-` Gadimov <bahtiar@gadimov.de>
Date: Tue, 12 Jul 2016 18:58:11 +0200
Subject: [PATCH 19/21] Storage add clone support

---
 qubes/storage/__init__.py | 91 ++++++++++++++++++++++++++++++---------
 1 file changed, 70 insertions(+), 21 deletions(-)

diff --git a/qubes/storage/__init__.py b/qubes/storage/__init__.py
index 0cc0a2de..dcdc32ff 100644
--- a/qubes/storage/__init__.py
+++ b/qubes/storage/__init__.py
@@ -305,37 +305,55 @@ class Storage(object):
         return result
 
     def resize(self, volume, size):
-        ''' Resize volume '''
+        ''' Resizes volume a read-writable volume '''
         self.get_pool(volume).resize(volume, size)
 
-    def create(self, source_template=None):
+    def create(self):
         ''' Creates volumes on disk '''
-        if source_template is None and hasattr(self.vm, 'template'):
-            source_template = self.vm.template
-
         old_umask = os.umask(002)
 
-        for name, volume in self.vm.volumes.items():
-            source_volume = None
-            if source_template and hasattr(source_template, 'volumes'):
-                source_volume = source_template.volumes[name]
-            self.get_pool(volume).create(volume, source_volume=source_volume)
+        for volume in self.vm.volumes.values():
+            self.get_pool(volume).create(volume)
 
         os.umask(old_umask)
 
     def clone(self, src_vm):
         ''' Clone volumes from the specified vm '''
-        self.vm.log.info('Creating directory: {0}'.format(self.vm.dir_path))
-        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
+
+        src_path = src_vm.dir_path
+        msg = "Source path {!s} does not exist".format(src_path)
+        assert os.path.exists(src_path), msg
+
+        dst_path = self.vm.dir_path
+        msg = "Destination {!s} already exists".format(dst_path)
+        assert not os.path.exists(dst_path), msg
+        os.mkdir(dst_path)
+
+        self.vm.volumes = {}
+        with VmCreationManager(self.vm):
+            for name, config in self.vm.volume_config.items():
+                dst_pool = self.get_pool(config['pool'])
+                dst = dst_pool.init_volume(self.vm, config)
+                src_volume = src_vm.volumes[name]
+                src_pool = self.vm.app.get_pool(src_volume.pool)
+                if dst_pool == src_pool:
+                    msg = "Cloning volume {!s} from vm {!s}"
+                    self.vm.log.info(msg.format(src_volume.name, src_vm.name))
+                    volume = dst_pool.clone(src_volume, dst)
+                else:
+                    msg = "Importing volume {!s} from vm {!s}"
+                    self.vm.log.info(msg.format(src_volume.name, src_vm.name))
+                    volume = dst_pool.import_volume(dst_pool, dst, src_pool,
+                                                    src_volume)
+
+                assert volume, "%s.clone() returned '%s'" % (
+                    dst_pool.__class__.__name__, volume)
+
+                self.vm.volumes[name] = volume
+
+        msg = "Cloning directory: {!s} to {!s}"
+        msg = msg.format(src_path, dst_path)
+        self.log.info(msg)
 
     @property
     def outdated_volumes(self):
@@ -560,6 +578,17 @@ class Pool(object):
         return NotImplementedError(msg)
 
 
+def _sanitize_config(config):
+    ''' Helper function to convert types to appropriate strings
+    '''  # FIXME: find another solution for serializing basic types
+    result = {}
+    for key, value in config.items():
+        if isinstance(value, bool):
+            if value:
+                result[key] = 'True'
+        else:
+            result[key] = str(value)
+    return result
 
 
 def pool_drivers():
@@ -571,3 +600,23 @@ def pool_drivers():
 def isodate(seconds=time.time()):
     ''' Helper method which returns an iso date '''
     return datetime.utcfromtimestamp(seconds).isoformat("T")
+
+
+class VmCreationManager(object):
+    ''' A `ContextManager` which cleans up if volume creation fails.
+    '''  # pylint: disable=too-few-public-methods
+    def __init__(self, vm):
+        self.vm = vm
+
+    def __enter__(self):
+        pass
+
+    def __exit__(self, type, value, tb):  # pylint: disable=redefined-builtin
+        if type is not None and value is not None and tb is not None:
+            for volume in self.vm.volumes.values():
+                try:
+                    pool = self.vm.storage.get_pool(volume)
+                    pool.remove(volume)
+                except Exception:  # pylint: disable=broad-except
+                    pass
+            os.rmdir(self.vm.dir_path)

From bcf1cfcb1fcc30f119764ff2b38e70a13795d2f9 Mon Sep 17 00:00:00 2001
From: Bahtiar `kalkin-` Gadimov <bahtiar@gadimov.de>
Date: Tue, 12 Jul 2016 18:24:43 +0200
Subject: [PATCH 20/21] Add qvm-clone(1)

---
 doc/manpages/qvm-clone.rst | 23 ++++++++----
 qubes/tools/qvm_clone.py   | 73 ++++++++++++++++++++++++++++++++++++++
 qubes/vm/qubesvm.py        | 60 +++++++++++++++++++++++++++++--
 rpm_spec/core-dom0.spec    |  1 +
 4 files changed, 147 insertions(+), 10 deletions(-)
 create mode 100644 qubes/tools/qvm_clone.py

diff --git a/doc/manpages/qvm-clone.rst b/doc/manpages/qvm-clone.rst
index 98cfb5b8..cd26aac3 100644
--- a/doc/manpages/qvm-clone.rst
+++ b/doc/manpages/qvm-clone.rst
@@ -1,30 +1,39 @@
 .. program:: qvm-clone
 
-===========================================================================
 :program:`qvm-clone` -- Clones an existing VM by copying all its disk files
 ===========================================================================
 
 Synopsis
-========
-:command:`qvm-clone` [*options*] <*src-name*> <*new-name*>
+--------
+:command:`qvm-clone` [-h] [--verbose] [--quiet] [-p *POOL:VOLUME* | -P POOL] *VMNAME* *NEWVM*
 
 Options
-=======
+-------
 
 .. option:: --help, -h
 
     Show this help message and exit
 
+.. option:: -P POOL
+
+    Pool to use for the new domain. All volumes besides snapshots volumes are
+    imported in to the specified POOL. ~HIS IS WHAT YOU WANT TO USE NORMALLY.
+
+.. option:: --pool=POOL:VOLUME, -p POOL:VOLUME
+
+    Specify the pool to use for the specific volume
+
 .. option:: --quiet, -q
 
     Be quiet
 
-.. option:: --path=DIR_PATH, -p DIR_PATH
+.. option:: --verbose, -v
 
-    Specify path to the template directory
+    Increase verbosity
 
 Authors
-=======
+-------
 | Joanna Rutkowska <joanna at invisiblethingslab dot com>
 | Rafal Wojtczuk <rafal at invisiblethingslab dot com>
 | Marek Marczykowski <marmarek at invisiblethingslab dot com>
+| Bahtiar `kalkin-` Gadimov <bahtiar at gadimov dot de> 
diff --git a/qubes/tools/qvm_clone.py b/qubes/tools/qvm_clone.py
new file mode 100644
index 00000000..f3444d1f
--- /dev/null
+++ b/qubes/tools/qvm_clone.py
@@ -0,0 +1,73 @@
+#!/usr/bin/python2
+# -*- encoding: utf8 -*-
+#
+# The Qubes OS Project, http://www.qubes-os.org
+#
+# Copyright (C) 2016 Bahtiar `kalkin-` Gadimov <bahtiar@gadimov.de>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program; if not, write to the Free Software Foundation, Inc.,
+# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+#
+
+''' Clone a domain '''
+
+import sys
+
+from qubes.tools import QubesArgumentParser, SinglePropertyAction
+
+parser = QubesArgumentParser(description=__doc__, vmname_nargs=1)
+parser.add_argument('new_name',
+                    metavar='NEWVM',
+                    action=SinglePropertyAction,
+                    help='name of the domain to create')
+
+group = parser.add_mutually_exclusive_group()
+group.add_argument('-P',
+                    metavar='POOL',
+                    dest='one_pool',
+                    default='',
+                    help='pool to use for the new domain')
+
+group.add_argument('-p',
+                    '--pool',
+                    action='append',
+                    metavar='POOL:VOLUME',
+                    help='specify the pool to use for the specific volume')
+
+
+def main(args=None):
+    ''' Clones an existing VM by copying all its disk files '''
+    args = parser.parse_args(args)
+    app = args.app
+    src_vm = args.domains[0]
+    new_name = args.properties['new_name']
+    dst_vm = app.add_new_vm(src_vm.__class__, name=new_name)
+    dst_vm.clone_properties(src_vm)
+
+    if args.one_pool:
+        dst_vm.clone_disk_files(src_vm, pool=args.one_pool)
+    elif hasattr(args, 'pools') and args.pools:
+        dst_vm.clone_disk_files(src_vm, pools=args.pools)
+    else:
+        dst_vm.clone_disk_files(src_vm)
+
+#   try:
+    app.save()  # HACK remove_from_disk on exception hangs for some reason
+#   except Exception as e:  # pylint: disable=broad-except
+#       dst_vm.remove_from_disk()
+#       parser.print_error(e)
+#   return 0
+
+if __name__ == '__main__':
+    sys.exit(main())
diff --git a/qubes/vm/qubesvm.py b/qubes/vm/qubesvm.py
index e052e186..e65e78be 100644
--- a/qubes/vm/qubesvm.py
+++ b/qubes/vm/qubesvm.py
@@ -25,6 +25,7 @@
 
 from __future__ import absolute_import
 
+import copy
 import base64
 import datetime
 import itertools
@@ -1074,6 +1075,7 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
         os.makedirs(self.dir_path, mode=0o775)
 
         if pool or pools:
+            # pylint: disable=attribute-defined-outside-init
             self.volume_config = _patch_volume_config(self.volume_config, pool,
                                                       pools)
             self.storage = qubes.storage.Storage(self)
@@ -1096,7 +1098,7 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
         shutil.rmtree(self.dir_path)
         self.storage.remove()
 
-    def clone_disk_files(self, src):
+    def clone_disk_files(self, src, pool=None, pools=None, ):
         '''Clone files from other vm.
 
         :param qubes.vm.qubesvm.QubesVM src: source VM
@@ -1110,9 +1112,11 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
             raise qubes.exc.QubesVMNotHaltedError(
                 self, 'Cannot clone a running domain {!r}'.format(self.name))
 
-        if hasattr(src, 'volume_config'):
+        if pool or pools:
             # pylint: disable=attribute-defined-outside-init
-            self.volume_config = src.volume_config
+            self.volume_config = _patch_volume_config(self.volume_config, pool,
+                                                      pools)
+
         self.storage = qubes.storage.Storage(self)
         self.storage.clone(src)
         self.storage.verify()
@@ -1589,3 +1593,53 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
         domain.memory_maximum = self.get_mem_static_max() * 1024
 
         return qubes.qmemman.algo.prefmem(domain) / 1024
+
+
+def _clean_volume_config(config):
+    common_attributes = ['name', 'pool', 'size', 'internal', 'removable',
+                            'revisions_to_keep', 'rw', 'snap_on_start',
+                            'save_on_stop', 'source']
+    config_copy = copy.deepcopy(config)
+    return {k: v for k, v in config_copy.items() if k in common_attributes}
+
+
+def _patch_pool_config(config, pool=None, pools=None):
+    assert pool is not None or pools is not None
+    is_saveable = 'save_on_stop' in config and config['save_on_stop']
+    is_resetable = not ('snap_on_start' in config and  # volatile
+                        config['snap_on_start'] and not is_saveable)
+
+    is_exportable = is_saveable or is_resetable
+
+    name = config['name']
+
+    if pool and is_exportable:
+        config['pool'] = str(pool)
+    elif pool and not is_exportable:
+        pass
+    elif pools and name in pools.keys():
+        if is_exportable:
+            config['pool'] = str(pools[name])
+        else:
+            msg = "Can't clone a snapshot volume {!s} to pool {!s} " \
+                .format(name, pools[name])
+            raise qubes.exc.QubesException(msg)
+    return config
+
+def _patch_volume_config(volume_config, pool=None, pools=None):
+    assert not (pool and pools), \
+        'You can not pass pool & pools parameter at same time'
+    assert pool or pools
+
+    result = {}
+
+    for name, config in volume_config.items():
+        # copy only the subset of volume_config key/values
+        dst_config = _clean_volume_config(config)
+
+        if pool is not None or pools is not None:
+            dst_config = _patch_pool_config(dst_config, pool, pools)
+
+        result[name] = dst_config
+
+    return result
diff --git a/rpm_spec/core-dom0.spec b/rpm_spec/core-dom0.spec
index b6a5927f..afca4fc8 100644
--- a/rpm_spec/core-dom0.spec
+++ b/rpm_spec/core-dom0.spec
@@ -247,6 +247,7 @@ fi
 %{python_sitelib}/qubes/tools/qvm_block.py*
 %{python_sitelib}/qubes/tools/qvm_create.py*
 %{python_sitelib}/qubes/tools/qvm_features.py*
+%{python_sitelib}/qubes/tools/qvm_clone.py*
 %{python_sitelib}/qubes/tools/qvm_kill.py*
 %{python_sitelib}/qubes/tools/qvm_ls.py*
 %{python_sitelib}/qubes/tools/qvm_pause.py*

From d8a90a77c29e36812d0da60816c260c0d53b3f3a Mon Sep 17 00:00:00 2001
From: Bahtiar `kalkin-` Gadimov <bahtiar@gadimov.de>
Date: Wed, 13 Jul 2016 20:38:46 +0200
Subject: [PATCH 21/21] =?UTF-8?q?Make=20pylint=20really=20happy=20?=
 =?UTF-8?q?=E2=99=A5=E2=99=A5=E2=99=A5?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 qubes/__init__.py         | 14 ++------------
 qubes/app.py              | 19 ++++++++++---------
 qubes/ext/qubesmanager.py |  2 +-
 qubes/utils.py            |  6 ++----
 4 files changed, 15 insertions(+), 26 deletions(-)

diff --git a/qubes/__init__.py b/qubes/__init__.py
index fafbcb65..233f480f 100644
--- a/qubes/__init__.py
+++ b/qubes/__init__.py
@@ -32,22 +32,12 @@ Qubes OS
 
 from __future__ import absolute_import
 
+import __builtin__
 import collections
-import errno
-import grp
-import logging
 import os
 import os.path
-import sys
-import tempfile
-import time
 
-import __builtin__
-
-import jinja2
 import lxml.etree
-import pkg_resources
-
 import qubes.config
 import qubes.events
 import qubes.exc
@@ -439,7 +429,7 @@ class PropertyHolder(qubes.events.Emitter):
         propvalues = {}
 
         all_names = set(prop.__name__ for prop in self.property_list())
-        for key in list(kwargs.keys()):
+        for key in list(kwargs):
             if not key in all_names:
                 continue
             propvalues[key] = kwargs.pop(key)
diff --git a/qubes/app.py b/qubes/app.py
index 2df571e2..d9b077f5 100644
--- a/qubes/app.py
+++ b/qubes/app.py
@@ -36,13 +36,14 @@ import tempfile
 import time
 import uuid
 
-import jinja2
 import lxml.etree
+
+import jinja2
 import libvirt
 
 try:
-    import xen.lowlevel.xs
-    import xen.lowlevel.xc
+    import xen.lowlevel.xs  # pylint: disable=wrong-import-order
+    import xen.lowlevel.xc  # pylint: disable=wrong-import-order
 except ImportError:
     pass
 
@@ -58,12 +59,12 @@ else:
     raise RuntimeError("Qubes works only on POSIX or WinNT systems")
 
 
-import qubes
-import qubes.ext
-import qubes.utils
-import qubes.vm.adminvm
-import qubes.vm.qubesvm
-import qubes.vm.templatevm
+import qubes  # pylint: disable=wrong-import-position
+import qubes.ext  # pylint: disable=wrong-import-position
+import qubes.utils  # pylint: disable=wrong-import-position
+import qubes.vm.adminvm  # pylint: disable=wrong-import-position
+import qubes.vm.qubesvm  # pylint: disable=wrong-import-position
+import qubes.vm.templatevm  # pylint: disable=wrong-import-position
 
 
 class VirDomainWrapper(object):
diff --git a/qubes/ext/qubesmanager.py b/qubes/ext/qubesmanager.py
index 41b489cf..9b002d99 100644
--- a/qubes/ext/qubesmanager.py
+++ b/qubes/ext/qubesmanager.py
@@ -29,8 +29,8 @@
 .. warning:: API defined here is not declared stable.
 '''
 
-import qubes.ext
 import dbus
+import qubes.ext
 
 
 class QubesManager(qubes.ext.Extension):
diff --git a/qubes/utils.py b/qubes/utils.py
index 8f0e3121..6b012c17 100644
--- a/qubes/utils.py
+++ b/qubes/utils.py
@@ -29,11 +29,11 @@ import os
 import re
 import subprocess
 
+import pkg_resources
+
 import docutils
 import docutils.core
 import docutils.io
-import pkg_resources
-
 import qubes.exc
 
 
@@ -158,5 +158,3 @@ def get_entry_point_one(group, name):
                 ', '.join('{}.{}'.format(ep.module_name, '.'.join(ep.attrs))
                     for ep in epoints)))
     return epoints[0].load()
-
-