Merge branch 'core3-storage3'

doc/index.rst

@@ -16,6 +16,7 @@ manpages and API documentation. For primary user documentation, see
    qubes
    qubes-vm/index
    qubes-events
+   qubes-storage
    qubes-exc
    qubes-ext
    qubes-log

doc/qubes-storage.rst

@@ -0,0 +1,145 @@
+:py:mod:`qubes.storage` -- Qubes data storage
+=============================================
+
+Qubes provide extensible API for domains data storage. Each domain have
+multiple storage volumes, for different purposes. Each volume is provided by
+some storage pool. Qubes support different storage pool drivers, and it's
+possible to register additional 3rd-party drivers.
+
+Domain's storage volumes:
+
+ - `root` - this is where operating system is installed. The volume is
+   available read-write to :py:class:`~qubes.vm.templatevm.TemplateVM` and
+   :py:class:`~qubes.vm.standalonevm.StandaloneVM`, and read-only to others
+   (:py:class:`~qubes.vm.appvm.AppVM` and :py:class:`~qubes.vm.dispvm.DispVM`).
+ - `private` - this is where domain's data live. The volume is available
+   read-write to all domain classes (including :py:class:`~qubes.vm.dispvm.DispVM`,
+   but data written there is discarded on domain shutdown).
+ - `volatile` - this is used for any data that do not to persist. This include
+   swap, copy-on-write layer for `root` volume etc.
+ - `kernel` - domain boot files - operating system kernel, initial ramdisk,
+   kernel modules etc. This volume is provided read-only and should be provided by
+   a storage pool respecting :py:attr:`qubes.vm.qubesvm.QubesVM.kernel` property.
+
+Storage pool concept
+--------------------
+
+Storage pool is responsible for managing its volumes. Qubes have defined
+storage pool driver API, allowing to put domains storage in various places. By
+default two drivers are provided: :py:class:`qubes.storage.file.FilePool`
+(named `file`) and :py:class:`qubes.storage.lvm.ThinPool` (named `lvm_thin`).
+But the API allow to implement variety of other drivers (like additionally
+encrypted storage, external disk, drivers using special features of some
+filesystems like btrfs, etc).
+
+Most of storage API focus on storage volumes. Each volume have at least those
+properties:
+ - :py:attr:`~qubes.storage.Volume.rw` - should the volume be available
+   read-only or read-write to the domain
+ - :py:attr:`~qubes.storage.Volume.snap_on_start` - should the domain start
+   with its own state of the volume, or rather a snapshot of its template volume
+   (pointed by a :py:attr:`~qubes.storage.Volume.source` property). This can be
+   set to `True` only if a domain do have `template` property (AppVM and DispVM).
+   If the domain's template is running already, the snapshot should be made out of
+   the template's before its startup.
+ - :py:attr:`~qubes.storage.Volume.save_on_stop` - should the volume state be
+   saved or discarded on domain
+   stop. In either case, while the domain is running, volume's current state
+   should not be committed immediately. This is to allow creating snapshots of the
+   volume's state from before domain start (see
+   :py:attr:`~qubes.storage.Volume.snap_on_start`).
+ - :py:attr:`~qubes.storage.Volume.revisions_to_keep` - number of volume
+   revisions to keep. If greater than zero, at each domain stop (and if
+   :py:attr:`~qubes.storage.Volume.save_on_stop` is `True`) new revision is saved
+   and old ones exceeding :py:attr:`~qubes.storage.Volume.revisions_to_keep` limit
+   are removed.
+ - :py:attr:`~qubes.storage.Volume.source` - source volume for
+   :py:attr:`~qubes.storage.Volume.snap_on_start` volumes
+ - :py:attr:`~qubes.storage.Volume.vid` - pool specific volume identifier, must
+   be unique inside given pool
+ - :py:attr:`~qubes.storage.Volume.pool` - storage pool object owning this volume
+ - :py:attr:`~qubes.storage.Volume.name` - name of the volume inside owning
+   domain (like `root`, or `private`)
+ - :py:attr:`~qubes.storage.Volume.size` - size of the volume, in bytes
+
+Storage pool driver may define additional properties.
+
+Storage pool driver API
+-----------------------
+
+Storage pool driver need to implement two classes:
+ - pool class - inheriting from :py:class:`qubes.storage.Pool`
+ - volume class - inheriting from :py:class:`qubes.storage.Volume`
+
+Pool class should be registered with `qubes.storage` entry_point, under the
+name of storage pool driver. Volume class instances should be returned by
+:py:meth:`qubes.storage.Pool.init_volume` method of pool class instance.
+
+Methods required to be implemented by the pool class:
+ - :py:meth:`~qubes.storage.Pool.init_volume` - return instance of appropriate
+   volume class; this method should not alter any persistent disk state, it is
+   used to instantiate both existing volumes and create new ones
+ - :py:meth:`~qubes.storage.Pool.setup` - setup new storage pool
+ - :py:meth:`~qubes.storage.Pool.destroy` - destroy storage pool
+
+Methods and properties required to be implemented by the volume class:
+ - :py:meth:`~qubes.storage.Volume.create` - create volume on disk
+ - :py:meth:`~qubes.storage.Volume.remove` - remove volume from disk
+ - :py:meth:`~qubes.storage.Volume.start` - prepare the volume for domain start;
+   this include making a snapshot if
+   :py:attr:`~qubes.storage.Volume.snap_on_start` is `True`
+ - :py:meth:`~qubes.storage.Volume.stop` - cleanup after domain shutdown; this
+   include committing changes to the volume if
+   :py:attr:`~qubes.storage.Volume.save_on_stop` is `True`
+ - :py:meth:`~qubes.storage.Volume.export` - return a path to be read to extract
+   volume data; for complex formats, this can be a pipe (connected to some
+   data-extracting process)
+ - :py:meth:`~qubes.storage.Volume.import_data` - return a path the data should
+   be written to, to import volume data; for complex formats, this can be pipe
+   (connected to some data-importing process)
+ - :py:meth:`~qubes.storage.Volume.import_data_end` - finish data import
+   operation (cleanup temporary files etc); this methods is called always after
+   :py:meth:`~qubes.storage.Volume.import_data` regardless if operation was
+   successful or not
+ - :py:meth:`~qubes.storage.Volume.import_volume` - import data from another volume
+ - :py:meth:`~qubes.storage.Volume.resize` - resize volume
+ - :py:meth:`~qubes.storage.Volume.revert` - revert volume state to a given revision
+ - :py:attr:`~qubes.storage.Volume.revisions` - collection of volume revisions (to use
+   with :py:meth:`qubes.storage.Volume.revert`)
+ - :py:meth:`~qubes.storage.Volume.is_dirty` - is volume properly committed
+   after domain shutdown? Applies only to volumes with
+   :py:attr:`~qubes.storage.Volume.save_on_stop` set to `True`
+ - :py:meth:`~qubes.storage.Volume.is_outdated` - have the source volume started
+   since domain startup? applies only to volumes with
+   :py:attr:`~qubes.storage.Volume.snap_on_start` set to `True`
+ - :py:attr:`~qubes.storage.Volume.config` - volume configuration, this should
+   be enough to later reinstantiate the same volume object
+ - :py:meth:`~qubes.storage.Volume.block_device` - return
+   :py:class:`qubes.storage.BlockDevice` instance required to configure volume in
+   libvirt
+
+Some storage pool drivers can provide limited functionality only - for example
+support only `volatile` volumes (those with
+:py:attr:`~qubes.storage.Volume.snap_on_start` is `False`,
+:py:attr:`~qubes.storage.Volume.save_on_stop` is `False`, and
+:py:attr:`~qubes.storage.Volume.rw` is `True`). In that case, it should raise
+:py:exc:`NotImplementedError` in :py:meth:`qubes.storage.Pool.init_volume` when
+trying to instantiate unsupported volume.
+
+Note that pool driver should be prepared to recover from power loss before
+stopping a domain - so, if volume have
+:py:attr:`~qubes.storage.Volume.save_on_stop` is `True`, and
+:py:meth:`qubes.storage.Volume.stop` wasn't called, next
+:py:meth:`~qubes.storage.Volume.start` should pick up previous (not committed)
+state.
+
+See specific methods documentation for details.
+
+Module contents
+---------------
+
+.. automodule:: qubes.storage
+   :members:
+   :show-inheritance:
+
+.. vim: ts=3 sw=3 et

qubes/api/admin.py

@@ -275,7 +275,7 @@ class QubesAdminAPI(qubes.api.AbstractQubesAPI):
         volume = self.dest.volumes[self.arg]
         # properties defined in API
         volume_properties = [
-            'pool', 'vid', 'size', 'usage', 'rw', 'internal', 'source',
+            'pool', 'vid', 'size', 'usage', 'rw', 'source',
             'save_on_stop', 'snap_on_start']
         return ''.join('{}={}\n'.format(key, getattr(volume, key)) for key in
             volume_properties)

qubes/app.py

@@ -28,6 +28,7 @@ import grp
 import logging
 import os
 import random
+import subprocess
 import sys
 import tempfile
 import time
@@ -548,6 +549,46 @@ class VMCollection(object):
             'https://xkcd.com/221/',
             'http://dilbert.com/strip/2001-10-25')[random.randint(0, 1)])
 
+def _default_pool(app):
+    ''' Default storage pool.
+
+    1. If there is one named 'default', use it.
+    2. Check if root fs is on LVM thin - use that
+    3. Look for file-based pool pointing /var/lib/qubes
+    4. Fail
+    '''
+    if 'default' in app.pools:
+        return app.pools['default']
+    else:
+        rootfs = os.stat('/')
+        root_major = (rootfs.st_dev & 0xff00) >> 8
+        root_minor = rootfs.st_dev & 0xff
+        for pool in app.pools.values():
+            if pool.config.get('driver', None) != 'lvm_thin':
+                continue
+            thin_pool = pool.config['thin_pool']
+            thin_volumes = subprocess.check_output(
+                ['lvs', '--select', 'pool_lv=' + thin_pool,
+                '-o', 'lv_kernel_major,lv_kernel_minor', '--noheadings'])
+            if any((str(root_major), str(root_minor)) == thin_vol.split()
+                    for thin_vol in thin_volumes.splitlines()):
+                return pool
+        # not a thin volume? look for file pools
+        for pool in app.pools.values():
+            if pool.config.get('driver', None) != 'file':
+                continue
+            if pool.config['dir_path'] == '/var/lib/qubes':
+                return pool
+        raise AttributeError('Cannot determine default storage pool')
+
+def _setter_pool(app, prop, value):
+    if isinstance(value, qubes.storage.Pool):
+        return value
+    try:
+        return app.pools[value]
+    except KeyError:
+        raise qubes.exc.QubesPropertyValueError(app, prop, value,
+            'No such storage pool')
 
 class Qubes(qubes.PropertyHolder):
     '''Main Qubes application
@@ -629,6 +670,27 @@ class Qubes(qubes.PropertyHolder):
     default_dispvm = qubes.VMProperty('default_dispvm', load_stage=3,
         doc='Default DispVM base for service calls')
 
+    default_pool = qubes.property('default_pool', load_stage=3,
+        default=_default_pool,
+        doc='Default storage pool')
+
+    default_pool_private = qubes.property('default_pool_private', load_stage=3,
+        default=lambda app: app.default_pool,
+        doc='Default storage pool for private volumes')
+
+    default_pool_root = qubes.property('default_pool_root', load_stage=3,
+        default=lambda app: app.default_pool,
+        doc='Default storage pool for root volumes')
+
+    default_pool_volatile = qubes.property('default_pool_volatile',
+        load_stage=3,
+        default=lambda app: app.default_pool,
+        doc='Default storage pool for volatile volumes')
+
+    default_pool_kernel = qubes.property('default_pool_kernel', load_stage=3,
+        default=lambda app: app.default_pool,
+        doc='Default storage pool for kernel volumes')
+
     # TODO #1637 #892
     check_updates_vm = qubes.property('check_updates_vm',
         type=bool, setter=qubes.property.bool,
@@ -662,7 +724,7 @@ class Qubes(qubes.PropertyHolder):
         else:
             self._store = os.environ.get('QUBES_XML_PATH',
                 os.path.join(
-                    qubes.config.system_path['qubes_base_dir'],
+                    qubes.config.qubes_base_dir,
                     qubes.config.system_path['qubes_store_filename']))
 
         super(Qubes, self).__init__(xml=None, **kwargs)
@@ -907,6 +969,8 @@ class Qubes(qubes.PropertyHolder):
         for name, config in qubes.config.defaults['pool_configs'].items():
             self.pools[name] = self._get_pool(**config)
 
+        self.default_pool_kernel = 'linux-kernel'
+
         self.domains.add(
             qubes.vm.adminvm.AdminVM(self, None, label='black'))
 

qubes/config.py

@@ -36,8 +36,6 @@ system_path = {
     'qrexec_client_path': '/usr/lib/qubes/qrexec-client',
     'qubesdb_daemon_path': '/usr/sbin/qubesdb-daemon',
 
-    'qubes_base_dir': qubes_base_dir,
-
     # Relative to qubes_base_dir
     'qubes_appvms_dir': 'appvms',
     'qubes_templates_dir': 'vm-templates',

qubes/storage/__init__.py

@@ -78,25 +78,26 @@ class Volume(object):
     domain = None
     path = None
     script = None
+    #: disk space used by this volume, can be smaller than :py:attr:`size`
+    #: for sparse volumes
     usage = 0
 
-    def __init__(self, name, pool, vid, internal=False, removable=False,
+    def __init__(self, name, pool, vid,
             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 name: The name of the volume inside owning domain
             :param Pool pool: The pool object
             :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 Volume source: other volume in same pool, or None
+            :param bool snap_on_start: Create a snapshot from source on
+                start, instead of using volume own data
+            :param bool save_on_stop: Write changes to the volume in
+                vm.stop(), otherwise - discard
+            :param Volume source: other volume in same pool to make snapshot
+                from, required if *snap_on_start*=`True`
             :param str/int size: Size of the volume
 
         '''
@@ -106,16 +107,34 @@ class Volume(object):
         assert source is None or (isinstance(source, Volume)
                                   and source.pool == pool)
 
+        if snap_on_start and source is None:
+            msg = "snap_on_start specified on {!r} but no volume source set"
+            msg = msg.format(name)
+            raise StoragePoolException(msg)
+        elif not snap_on_start and source is not None:
+            msg = "source specified on {!r} but no snap_on_start set"
+            msg = msg.format(name)
+            raise StoragePoolException(msg)
+
+        #: Name of the volume in a domain it's attached to (like `root` or
+        #: `private`).
         self.name = str(name)
+        #: :py:class:`Pool` instance owning this volume
         self.pool = pool
-        self.internal = internal
-        self.removable = removable
+        #: How many revisions of the volume to keep. Each revision is created
+        #  at :py:meth:`stop`, if :py:attr:`save_on_stop` is True
         self.revisions_to_keep = int(revisions_to_keep)
+        #: Should this volume be writable by domain.
         self.rw = rw
+        #: Should volume state be saved or discarded at :py:meth:`stop`
         self.save_on_stop = save_on_stop
         self._size = int(size)
+        #: Should the volume state be initialized with a snapshot of
+        #: same-named volume of domain's template.
         self.snap_on_start = snap_on_start
+        #: source volume for :py:attr:`snap_on_start` volumes
         self.source = source
+        #: Volume unique (inside given pool) identifier
         self.vid = vid
 
     def __eq__(self, other):
@@ -142,6 +161,10 @@ class Volume(object):
     def create(self):
         ''' Create the given volume on disk.
 
+            This method is called only once in the volume lifetime. Before
+            calling this method, no data on disk should be touched (in
+            context of this volume).
+
             This can be implemented as a coroutine.
         '''
         raise self._not_implemented("create")
@@ -152,24 +175,38 @@ class Volume(object):
         This can be implemented as a coroutine.'''
         raise self._not_implemented("remove")
 
-    def commit(self):
-        ''' Write the snapshot to disk
+    def export(self):
+        ''' Returns a path to read the volume data from.
 
-        This can be implemented as a coroutine.'''
-        raise self._not_implemented("commit")
+            Reading from this path when domain owning this volume is
+            running (i.e. when :py:meth:`is_dirty` is True) should return the
+            data from before domain startup.
 
-    def export(self):
-        ''' Returns an object that can be `open()`. '''
+            Reading from the path returned by this method should return the
+            volume data. If extracting volume data require something more
+            than just reading from file (for example connecting to some other
+            domain, or decompressing the data), the returned path may be a pipe.
+        '''
         raise self._not_implemented("export")
 
     def import_data(self):
-        ''' Returns an object that can be `open()`. '''
+        ''' Returns a path to overwrite volume data.
+
+            This method is called after volume was already :py:meth:`create`-ed.
+
+            Writing to this path should overwrite volume data. If importing
+            volume data require something more than just writing to a file (
+            for example connecting to some other domain, or converting data
+            on the fly), the returned path may be a pipe.
+        '''
         raise self._not_implemented("import")
 
     def import_data_end(self, success):
-        ''' End data import operation. This may be used by pool
+        ''' End the data import operation. This may be used by pool
         implementation to commit changes, cleanup temporary files etc.
 
+        This method is called regardless the operation was successful or not.
+
         :param success: True if data import was successful, otherwise False
         '''
         # by default do nothing
@@ -179,54 +216,60 @@ class Volume(object):
         ''' Imports data from a different volume (possibly in a different
         pool.
 
-        The needs to be create()d first.
+        The volume needs to be create()d first.
 
         This can be implemented as a coroutine. '''
         # pylint: disable=unused-argument
         raise self._not_implemented("import_volume")
 
     def is_dirty(self):
-        ''' Return `True` if volume was not properly shutdown and commited '''
+        ''' Return `True` if volume was not properly shutdown and committed.
+
+            This include the situation when domain owning the volume is still
+            running.
+
+        '''
         raise self._not_implemented("is_dirty")
 
     def is_outdated(self):
-        ''' Returns `True` if the currently used `volume.source` of a snapshot
-            volume is outdated.
+        ''' Returns `True` if this snapshot of a source volume (for
+        `snap_on_start`=True) is outdated.
         '''
         raise self._not_implemented("is_outdated")
 
-    def recover(self):
-        ''' Try to recover a :py:class:`Volume` or :py:class:`SnapVolume` '''
-        raise self._not_implemented("recover")
-
-    def reset(self):
-        ''' Drop and recreate volume without copying it's content from source.
-        '''
-        raise self._not_implemented("reset")
-
     def resize(self, size):
         ''' Expands volume, throws
             :py:class:`qubes.storage.StoragePoolException` if
             given size is less than current_size
 
             This can be implemented as a coroutine.
+
+            :param int size: new size in bytes
         '''
         # pylint: disable=unused-argument
         raise self._not_implemented("resize")
 
     def revert(self, revision=None):
-        ''' Revert volume to previous revision  '''
+        ''' Revert volume to previous revision
+
+        :param revision: revision to revert volume to, see :py:attr:`revisions`
+        '''
         # pylint: disable=unused-argument
         raise self._not_implemented("revert")
 
     def start(self):
-        ''' Do what ever is needed on start
+        ''' Do what ever is needed on start.
+
+        This include making a snapshot of template's volume if
+        :py:attr:`snap_on_start` is set.
 
         This can be implemented as a coroutine.'''
         raise self._not_implemented("start")
 
     def stop(self):
-        ''' Do what ever is needed on stop
+        ''' Do what ever is needed on stop.
+
+        This include committing data if :py:attr:`save_on_stop` is set.
 
         This can be implemented as a coroutine.'''
 
@@ -245,12 +288,14 @@ class Volume(object):
 
     @property
     def revisions(self):
-        ''' Returns a `dict` containing revision identifiers and paths '''
+        ''' Returns a dict containing revision identifiers and time of their
+        creation '''
         msg = "{!s} has revisions not implemented".format(self.__class__)
         raise NotImplementedError(msg)
 
     @property
     def size(self):
+        ''' Volume size in bytes '''
         return self._size
 
     @size.setter
@@ -262,29 +307,19 @@ class Volume(object):
     @property
     def config(self):
         ''' return config data for serialization to qubes.xml '''
-        result = {'name': self.name, 'pool': str(self.pool), 'vid': self.vid, }
-
-        if self.internal:
-            result['internal'] = self.internal
-
-        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
+        result = {
+            'name': self.name,
+            'pool': str(self.pool),
+            'vid': self.vid,
+            'revisions_to_keep': self.revisions_to_keep,
+            'rw': self.rw,
+            'save_on_stop': self.save_on_stop,
+            'snap_on_start': self.snap_on_start,
+        }
 
         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'] = str(self.source)
 
@@ -335,7 +370,13 @@ class Storage(object):
 
         if 'name' not in volume_config:
             volume_config['name'] = name
-        pool = self.vm.app.get_pool(volume_config['pool'])
+        if 'pool' not in volume_config:
+            pool = getattr(self.vm.app, 'default_pool_' + name)
+        else:
+            pool = self.vm.app.get_pool(volume_config['pool'])
+        if 'internal' in volume_config:
+            # migrate old config
+            del volume_config['internal']
         volume = pool.init_volume(self.vm, volume_config)
         self.vm.volumes[name] = volume
         return volume
@@ -486,13 +527,6 @@ class Storage(object):
 
         return result
 
-    def rename(self, old_name, new_name):
-        ''' Notify the pools that the domain was renamed '''
-        volumes = self.vm.volumes
-        for name, volume in volumes.items():
-            pool = volume.pool
-            volumes[name] = pool.rename(volume, old_name, new_name)
-
     @asyncio.coroutine
     def verify(self):
         '''Verify that the storage is sane.
@@ -559,19 +593,6 @@ class Storage(object):
         if futures:
             yield from asyncio.wait(futures)
 
-    @asyncio.coroutine
-    def commit(self):
-        ''' Makes changes to an 'origin' volume persistent '''
-        futures = []
-        for volume in self.vm.volumes.values():
-            if volume.save_on_stop:
-                ret = volume.commit()
-                if asyncio.iscoroutine(ret):
-                    futures.append(ret)
-
-        if futures:
-            yield asyncio.wait(futures)
-
     def unused_frontend(self):
         ''' Find an unused device name '''
         unused_frontends = self.AVAILABLE_FRONTENDS.difference(
@@ -722,10 +743,6 @@ class Pool(object):
         '''
         raise self._not_implemented("init_volume")
 
-    def rename(self, volume, old_name, new_name):
-        ''' Called when the domain changes its name '''
-        raise self._not_implemented("rename")
-
     def setup(self):
         ''' Called when adding a pool to the system. Use this for implementation
             specific set up.

qubes/storage/file.py

@@ -37,6 +37,20 @@ BLKSIZE = 512
 
 class FilePool(qubes.storage.Pool):
     ''' File based 'original' disk implementation
+
+    Volumes are stored in sparse files. Additionally device-mapper is used for
+    applying copy-on-write layer.
+
+    Quick reference on device-mapper layers:
+
+    snap_on_start save_on_stop layout
+    yes           yes          not supported
+    no            yes          snapshot-origin(volume.img, volume-cow.img)
+    yes           no           snapshot(
+                                   snapshot(source.img, source-cow.img),
+                                   volume-cow.img)
+    no            no           volume.img directly
+
     '''  # pylint: disable=protected-access
     driver = 'file'
 
@@ -57,16 +71,18 @@ class FilePool(qubes.storage.Pool):
         }
 
     def init_volume(self, vm, volume_config):
+        if volume_config.get('snap_on_start', False) and \
+                volume_config.get('save_on_stop', False):
+            raise NotImplementedError(
+                'snap_on_start + save_on_stop not supported by file driver')
         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']:
+            if not volume_config.get('save_on_stop', False):
                 volume_config['revisions_to_keep'] = 0
         except KeyError:
             pass
@@ -74,31 +90,15 @@ class FilePool(qubes.storage.Pool):
             if 'revisions_to_keep' not in volume_config:
                 volume_config['revisions_to_keep'] = self.revisions_to_keep
 
+        if int(volume_config['revisions_to_keep']) > 1:
+            raise NotImplementedError(
+                'FilePool supports maximum 1 volume revision to keep')
+
         volume_config['pool'] = self
         volume = FileVolume(**volume_config)
         self._volumes += [volume]
         return volume
 
-    def rename(self, volume, old_name, new_name):
-        assert issubclass(volume.__class__, FileVolume)
-        subdir, _, volume_path = volume.vid.split('/', 2)
-
-        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, 0o755)
-            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.vid = os.path.join(subdir, new_name, volume_path)
-
-        return volume
-
     def destroy(self):
         pass
 
@@ -150,59 +150,37 @@ class FileVolume(qubes.storage.Volume):
     ''' Parent class for the xen volumes implementation which expects a
         `target_dir` param on initialization.  '''
 
-    def __init__(self, dir_path, backward_comp=False, **kwargs):
+    def __init__(self, dir_path, **kwargs):
         self.dir_path = dir_path
-        self.backward_comp = backward_comp
         assert self.dir_path, "dir_path not specified"
         super(FileVolume, self).__init__(**kwargs)
 
-        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:
+        if self.snap_on_start:
             img_name = self.source.vid + '-cow.img'
             self.path_source_cow = os.path.join(self.dir_path, img_name)
-        elif self._is_volume or self._is_volatile:
-            pass
-        elif self._is_origin:
-            pass
-        else:
-            assert False, 'This should not happen'
 
     def create(self):
         assert isinstance(self.size, int) and self.size > 0, \
-            'Volatile volume size must be > 0'
-        if self._is_origin:
+            'Volume size must be > 0'
+        if not self.snap_on_start:
             create_sparse_file(self.path, self.size)
+        # path_cow not needed only in volatile volume
+        if self.save_on_stop or self.snap_on_start:
             create_sparse_file(self.path_cow, self.size)
-        elif not self._is_snapshot:
-            if self.source is not None:
-                source_path = os.path.join(self.dir_path,
-                    self.source.vid + '.img')
-                copy_file(source_path, self.path)
-            elif self._is_volatile:
-                pass
-            else:
-                create_sparse_file(self.path, self.size)
 
     def remove(self):
-        if not self.internal:
-            return  # do not remove random attached file volumes
-        elif self._is_snapshot:
-            return  # no need to remove, because it's just a snapshot
-        else:
+        if not self.snap_on_start:
             _remove_if_exists(self.path)
-            if self._is_origin:
-                _remove_if_exists(self.path_cow)
+        if self.snap_on_start or self.save_on_stop:
+            _remove_if_exists(self.path_cow)
 
     def is_dirty(self):
-        return False  # TODO: How to implement this?
+        if not self.save_on_stop:
+            return False
+        if os.path.exists(self.path_cow):
+            stat = os.stat(self.path_cow)
+            return stat.st_blocks > 0
+        return False
 
     def resize(self, size):
         ''' Expands volume, throws
@@ -223,7 +201,7 @@ class FileVolume(qubes.storage.Volume):
         with open(self.path, 'a+b') as fd:
             fd.truncate(size)
 
-        p = subprocess.Popen(['sudo', 'losetup', '--associated', self.path],
+        p = subprocess.Popen(['losetup', '--associated', self.path],
                              stdout=subprocess.PIPE)
         result = p.communicate()
 
@@ -232,28 +210,27 @@ class FileVolume(qubes.storage.Volume):
             loop_dev = m.group(1)
 
             # resize loop device
-            subprocess.check_call(['sudo', 'losetup', '--set-capacity',
+            subprocess.check_call(['losetup', '--set-capacity',
                                    loop_dev])
         self.size = size
 
     def commit(self):
         msg = 'Tried to commit a non commitable volume {!r}'.format(self)
-        assert (self._is_origin or self._is_volume) and self.rw, msg
-
-        if self._is_volume:
-            return self
+        assert self.save_on_stop and self.rw, msg
 
         if os.path.exists(self.path_cow):
-            old_path = self.path_cow + '.old'
-            os.rename(self.path_cow, old_path)
+            if self.revisions_to_keep:
+                old_path = self.path_cow + '.old'
+                os.rename(self.path_cow, old_path)
+            else:
+                os.unlink(self.path_cow)
 
-        old_umask = os.umask(0o002)
-        with open(self.path_cow, 'w') as f_cow:
-            f_cow.truncate(self.size)
-        os.umask(old_umask)
+        create_sparse_file(self.path_cow, self.size)
         return self
 
     def export(self):
+        # FIXME: this should rather return snapshot(self.path, self.path_cow)
+        #  if domain is running
         return self.path
 
     def import_volume(self, src_volume):
@@ -271,53 +248,30 @@ class FileVolume(qubes.storage.Volume):
 
     def reset(self):
         ''' Remove and recreate a volatile volume '''
-        assert self._is_volatile, "Not a volatile volume"
+        assert not self.snap_on_start and not self.save_on_stop, \
+            "Not a volatile volume"
         assert isinstance(self.size, int) and self.size > 0, \
             'Volatile volume size must be > 0'
 
         _remove_if_exists(self.path)
-
-        with open(self.path, "w") as f_volatile:
-            f_volatile.truncate(self.size)
+        create_sparse_file(self.path, self.size)
         return self
 
-    def revert(self, revision=None):
-        if revision is not None:
-            try:
-                return self.revisions[revision]
-            except KeyError:
-                msg = "Volume {!r} does not have revision {!s}"
-                msg = msg.format(self, revision)
-                raise qubes.storage.StoragePoolException(msg)
-        else:
-            try:
-                old_path = self.revisions.values().pop()
-                os.rename(old_path, self.path_cow)
-            except IndexError:
-                msg = "Volume {!r} does not have old revisions".format(self)
-                raise qubes.storage.StoragePoolException(msg)
-
     def start(self):
-        if self._is_volatile:
+        if not self.save_on_stop and not self.snap_on_start:
             self.reset()
         else:
-            _check_path(self.path)
-            if self.snap_on_start:
-                if not self.save_on_stop:
-                    # make sure previous snapshot is removed - even if VM
-                    # shutdown routing wasn't called (power interrupt or so)
-                    _remove_if_exists(self.path_cow)
-                try:
-                    _check_path(self.path_cow)
-                except qubes.storage.StoragePoolException:
-                    create_sparse_file(self.path_cow, self.size)
-                    _check_path(self.path_cow)
-                if hasattr(self, 'path_source_cow'):
-                    try:
-                        _check_path(self.path_source_cow)
-                    except qubes.storage.StoragePoolException:
-                        create_sparse_file(self.path_source_cow, self.size)
-                        _check_path(self.path_source_cow)
+            if not self.save_on_stop:
+                # make sure previous snapshot is removed - even if VM
+                # shutdown routine wasn't called (power interrupt or so)
+                _remove_if_exists(self.path_cow)
+            if not os.path.exists(self.path_cow):
+                create_sparse_file(self.path_cow, self.size)
+            if not self.snap_on_start:
+                _check_path(self.path)
+            if hasattr(self, 'path_source_cow'):
+                if not os.path.exists(self.path_source_cow):
+                    create_sparse_file(self.path_source_cow, self.size)
         return self
 
     def stop(self):
@@ -331,7 +285,7 @@ class FileVolume(qubes.storage.Volume):
 
     @property
     def path(self):
-        if self._is_snapshot:
+        if self.snap_on_start:
             return os.path.join(self.dir_path, self.source.vid + '.img')
         return os.path.join(self.dir_path, self.vid + '.img')
 
@@ -342,18 +296,19 @@ class FileVolume(qubes.storage.Volume):
 
     def verify(self):
         ''' Verifies the volume. '''
-        if not os.path.exists(self.path) and not self._is_volatile:
+        if not os.path.exists(self.path) and \
+                (self.snap_on_start or self.save_on_stop):
             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:
+        if not self.snap_on_start and not self.save_on_stop:
             return None
-        elif self._is_origin:
+        elif not self.snap_on_start and self.save_on_stop:
             return 'block-origin'
-        elif self._is_origin_snapshot or self._is_snapshot:
+        elif self.snap_on_start:
             return 'block-snapshot'
 
     def block_device(self):
@@ -361,9 +316,9 @@ class FileVolume(qubes.storage.Volume):
             the libvirt XML template as <disk>.
         '''
         path = self.path
-        if self._is_snapshot:
+        if self.snap_on_start:
             path += ":" + self.path_source_cow
-        if self._is_origin or self._is_snapshot:
+        if self.snap_on_start or self.save_on_stop:
             path += ":" + self.path_cow
         return qubes.storage.BlockDevice(path, self.name, self.script, self.rw,
                                          self.domain, self.devtype)
@@ -380,39 +335,13 @@ class FileVolume(qubes.storage.Volume):
 
         seconds = os.path.getctime(old_revision)
         iso_date = qubes.storage.isodate(seconds).split('.', 1)[0]
-        return {iso_date: old_revision}
+        return {'old': iso_date}
 
     @property
     def usage(self):
         ''' Returns the actualy used space '''
         return get_disk_usage(self.vid)
 
-    @property
-    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 _is_origin(self):
-        ''' Internal helper. Useful for differentiating volume handling '''
-        # pylint: disable=line-too-long
-        return self.save_on_stop and self.revisions_to_keep > 0  # NOQA
-
-    @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
-
-    @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 '''

qubes/storage/kernels.py

@@ -138,6 +138,14 @@ class LinuxKernel(Pool):
     def init_volume(self, vm, volume_config):
         assert not volume_config['rw']
 
+        # migrate old config
+        if volume_config.get('snap_on_start', False) and not \
+                volume_config.get('source', None):
+            volume_config['snap_on_start'] = False
+
+        if volume_config.get('save_on_stop', False):
+            raise NotImplementedError(
+                'LinuxKernel pool does not support save_on_stop=True')
         volume_config['pool'] = self
         volume = LinuxModules(self.dir_path, lambda: vm.kernel, **volume_config)
 
@@ -157,9 +165,6 @@ class LinuxKernel(Pool):
     def import_volume(self, dst_pool, dst_volume, src_pool, src_volume):
         pass
 
-    def rename(self, volume, old_name, new_name):
-        return volume
-
     def setup(self):
         pass
 
@@ -170,7 +175,6 @@ class LinuxKernel(Pool):
                              kernel_version,
                              pool=self,
                              name=kernel_version,
-                             internal=True,
                              rw=False
                              )
                 for kernel_version in os.listdir(self.dir_path)]

qubes/storage/lvm.py

@@ -21,9 +21,14 @@
 ''' Driver for storing vm images in a LVM thin pool '''
 
 import logging
+import operator
 import os
 import subprocess
 
+import time
+
+import asyncio
+
 import qubes
 import qubes.storage
 import qubes.utils
@@ -88,23 +93,6 @@ class ThinPool(qubes.storage.Pool):
         volume_config['pool'] = self
         return ThinVolume(**volume_config)
 
-    def rename(self, volume, old_name, new_name):
-        ''' Called when the domain changes its name '''
-        new_vid = "{!s}/vm-{!s}-{!s}".format(self.volume_group, new_name,
-                                          volume.name)
-        if volume.save_on_stop:
-            cmd = ['clone', volume.vid, new_vid]
-            qubes_lvm(cmd, self.log)
-            cmd = ['remove', volume.vid]
-            qubes_lvm(cmd, self.log)
-
-        volume.vid = new_vid
-
-        if volume.snap_on_start:
-            volume._vid_snap = volume.vid + '-snap'
-        reset_cache()
-        return volume
-
     def setup(self):
         pass  # TODO Should we create a non existing pool?
 
@@ -119,6 +107,9 @@ class ThinPool(qubes.storage.Pool):
             if vid.endswith('-snap'):
                 # implementation detail volume
                 continue
+            if vid.endswith('-back'):
+                # old revisions
+                continue
             config = {
                 'pool': self,
                 'vid': vid,
@@ -132,7 +123,7 @@ class ThinPool(qubes.storage.Pool):
 
 def init_cache(log=logging.getLogger('qube.storage.lvm')):
     cmd = ['lvs', '--noheadings', '-o',
-           'vg_name,pool_lv,name,lv_size,data_percent,lv_attr',
+           'vg_name,pool_lv,name,lv_size,data_percent,lv_attr,origin',
            '--units', 'b', '--separator', ',']
     if os.getuid() != 0:
         cmd.insert(0, 'sudo')
@@ -149,14 +140,15 @@ def init_cache(log=logging.getLogger('qube.storage.lvm')):
 
     for line in out.splitlines():
         line = line.decode().strip()
-        pool_name, pool_lv, name, size, usage_percent, attr = line.split(',', 5)
+        pool_name, pool_lv, name, size, usage_percent, attr, \
+            origin = line.split(',', 6)
         if '' in [pool_name, pool_lv, name, size, usage_percent]:
             continue
         name = pool_name + "/" + name
         size = int(size[:-1])
         usage = int(size / 100 * float(usage_percent))
         result[name] = {'size': size, 'usage': usage, 'pool_lv': pool_lv,
-            'attr': attr}
+            'attr': attr, 'origin': origin}
 
     return result
 
@@ -173,16 +165,7 @@ class ThinVolume(qubes.storage.Volume):
         super(ThinVolume, self).__init__(size=size, **kwargs)
         self.log = logging.getLogger('qube.storage.lvm.%s' % str(self.pool))
 
-        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.snap_on_start:
+        if self.snap_on_start or self.save_on_stop:
             self._vid_snap = self.vid + '-snap'
 
         self._size = size
@@ -193,28 +176,18 @@ class ThinVolume(qubes.storage.Volume):
 
     @property
     def revisions(self):
-        path = self.path + '-back'
-        if os.path.exists(path):
-            seconds = os.path.getctime(path)
+        name_prefix = self.vid + '-'
+        revisions = {}
+        for revision_vid in size_cache:
+            if not revision_vid.startswith(name_prefix):
+                continue
+            if not revision_vid.endswith('-back'):
+                continue
+            revision_vid = revision_vid[len(name_prefix):]
+            seconds = int(revision_vid[:-len('-back')])
             iso_date = qubes.storage.isodate(seconds).split('.', 1)[0]
-            return {iso_date: path}
-        return {}
-
-    @property
-    def _is_origin(self):
-        return not self.snap_on_start and self.save_on_stop
-
-    @property
-    def _is_origin_snapshot(self):
-        return self.snap_on_start and self.save_on_stop
-
-    @property
-    def _is_snapshot(self):
-        return self.snap_on_start and not self.save_on_stop
-
-    @property
-    def _is_volatile(self):
-        return not self.snap_on_start and not self.save_on_stop
+            revisions[revision_vid] = iso_date
+        return revisions
 
     @property
     def size(self):
@@ -230,8 +203,8 @@ class ThinVolume(qubes.storage.Volume):
 
     def _reset(self):
         ''' Resets a volatile volume '''
-        assert self._is_volatile, \
-            'Expected a volatile volume, but got {!r}'.format(self)
+        assert not self.snap_on_start and not self.save_on_stop, \
+            "Not a volatile volume"
         self.log.debug('Resetting volatile ' + self.vid)
         try:
             cmd = ['remove', self.vid]
@@ -243,6 +216,27 @@ class ThinVolume(qubes.storage.Volume):
                str(self.size)]
         qubes_lvm(cmd, self.log)
 
+    def _remove_revisions(self, revisions=None):
+        '''Remove old volume revisions.
+
+        If no revisions list is given, it removes old revisions according to
+        :py:attr:`revisions_to_keep`
+
+        :param revisions: list of revisions to remove
+        '''
+        if revisions is None:
+            revisions = sorted(self.revisions.items(),
+                key=operator.itemgetter(1))
+            revisions = revisions[:-self.revisions_to_keep]
+            revisions = [rev_id for rev_id, _ in revisions]
+
+        for rev_id in revisions:
+            try:
+                cmd = ['remove', self.vid + rev_id]
+                qubes_lvm(cmd, self.log)
+            except qubes.storage.StoragePoolException:
+                pass
+
     def _commit(self):
         msg = "Trying to commit {!s}, but it has save_on_stop == False"
         msg = msg.format(self)
@@ -253,13 +247,11 @@ class ThinVolume(qubes.storage.Volume):
         assert self.rw, msg
         assert hasattr(self, '_vid_snap')
 
-        try:
-            cmd = ['remove', self.vid + "-back"]
+        if self.revisions_to_keep > 0:
+            cmd = ['clone', self.vid,
+                '{}-{}-back'.format(self.vid, int(time.time()))]
             qubes_lvm(cmd, self.log)
-        except qubes.storage.StoragePoolException:
-            pass
-        cmd = ['clone', self.vid, self.vid + "-back"]
-        qubes_lvm(cmd, self.log)
+            self._remove_revisions()
 
         cmd = ['remove', self.vid]
         qubes_lvm(cmd, self.log)
@@ -290,6 +282,7 @@ class ThinVolume(qubes.storage.Volume):
             cmd = ['remove', self._vid_snap]
             qubes_lvm(cmd, self.log)
 
+        self._remove_revisions(self.revisions.keys())
         if not os.path.exists(self.path):
             return
         cmd = ['remove', self.vid]
@@ -301,6 +294,7 @@ class ThinVolume(qubes.storage.Volume):
         devpath = '/dev/' + self.vid
         return devpath
 
+    @asyncio.coroutine
     def import_volume(self, src_volume):
         if not src_volume.save_on_stop:
             return self
@@ -316,9 +310,14 @@ class ThinVolume(qubes.storage.Volume):
             qubes_lvm(cmd, self.log)
         else:
             src_path = src_volume.export()
-            cmd = ['sudo', 'dd', 'if=' + src_path, 'of=/dev/' + self.vid,
+            cmd = ['dd', 'if=' + src_path, 'of=/dev/' + self.vid,
                 'conv=sparse']
-            subprocess.check_call(cmd)
+            p = yield from asyncio.create_subprocess_exec(*cmd)
+            yield from p.wait()
+            if p.returncode != 0:
+                raise qubes.storage.StoragePoolException(
+                    'Failed to import volume {!r}, dd exit code: {}'.format(
+                        src_volume, p.returncode))
             reset_cache()
 
         return self
@@ -330,18 +329,30 @@ class ThinVolume(qubes.storage.Volume):
 
     def is_dirty(self):
         if self.save_on_stop:
-            return os.path.exists(self.path + '-snap')
+            return os.path.exists('/dev/' + self._vid_snap)
         return False
 
+    def is_outdated(self):
+        if not self.snap_on_start:
+            return False
+        if self._vid_snap not in size_cache:
+            return False
+        return (size_cache[self._vid_snap]['origin'] !=
+               self.source.vid.split('/')[1])
+
+
     def revert(self, revision=None):
-        old_path = self.path + '-back'
+        if revision is None:
+            revision = \
+                max(self.revisions.items(), key=operator.itemgetter(1))[0]
+        old_path = self.path + '-' + revision
         if not os.path.exists(old_path):
             msg = "Volume {!s} has no {!s}".format(self, old_path)
             raise qubes.storage.StoragePoolException(msg)
 
         cmd = ['remove', self.vid]
         qubes_lvm(cmd, self.log)
-        cmd = ['clone', self.vid + '-back', self.vid]
+        cmd = ['clone', self.vid + '-' + revision, self.vid]
         qubes_lvm(cmd, self.log)
         reset_cache()
         return self
@@ -381,22 +392,22 @@ class ThinVolume(qubes.storage.Volume):
 
 
     def start(self):
-        if self.snap_on_start:
+        if self.snap_on_start or self.save_on_stop:
             if not self.save_on_stop or not self.is_dirty():
                 self._snapshot()
-        elif not self.save_on_stop:
+        else:
             self._reset()
 
         reset_cache()
         return self
 
     def stop(self):
-        if self.save_on_stop and self.snap_on_start:
+        if self.save_on_stop:
             self._commit()
-        if self.snap_on_start:
+        if self.snap_on_start or self.save_on_stop:
             cmd = ['remove', self._vid_snap]
             qubes_lvm(cmd, self.log)
-        elif not self.save_on_stop:
+        else:
             cmd = ['remove', self.vid]
             qubes_lvm(cmd, self.log)
         reset_cache()
@@ -415,7 +426,7 @@ class ThinVolume(qubes.storage.Volume):
         ''' Return :py:class:`qubes.storage.BlockDevice` for serialization in
             the libvirt XML template as <disk>.
         '''
-        if self.snap_on_start:
+        if self.snap_on_start or self.save_on_stop:
             return qubes.storage.BlockDevice(
                 '/dev/' + self._vid_snap, self.name, self.script,
                 self.rw, self.domain, self.devtype)

qubes/tests/__init__.py

@@ -585,7 +585,7 @@ class SystemTestsMixin(object):
         # need some information from the real qubes.xml - at least installed
         # templates; should not be used for testing, only to initialize self.app
         self.host_app = qubes.Qubes(os.path.join(
-            qubes.config.system_path['qubes_base_dir'],
+            qubes.config.qubes_base_dir,
             qubes.config.system_path['qubes_store_filename']))
         if os.path.exists(CLASS_XMLPATH):
             shutil.copy(CLASS_XMLPATH, XMLPATH)
@@ -729,7 +729,7 @@ class SystemTestsMixin(object):
                 'qubes_appvms_dir',
                 'qubes_servicevms_dir',
                 'qubes_templates_dir'):
-            dirpath = os.path.join(qubes.config.system_path['qubes_base_dir'],
+            dirpath = os.path.join(qubes.config.qubes_base_dir,
                 qubes.config.system_path[dirspec], vmname)
             if os.path.exists(dirpath):
                 if os.path.isdir(dirpath):
@@ -793,7 +793,7 @@ class SystemTestsMixin(object):
                 'qubes_appvms_dir',
                 'qubes_servicevms_dir',
                 'qubes_templates_dir'):
-            dirpath = os.path.join(qubes.config.system_path['qubes_base_dir'],
+            dirpath = os.path.join(qubes.config.qubes_base_dir,
                 qubes.config.system_path[dirspec])
             for name in os.listdir(dirpath):
                 if name.startswith(prefix):

qubes/tests/api_admin.py

@@ -37,7 +37,7 @@ import qubes.storage
 
 # properties defined in API
 volume_properties = [
-    'pool', 'vid', 'size', 'usage', 'rw', 'internal', 'source',
+    'pool', 'vid', 'size', 'usage', 'rw', 'source',
     'save_on_stop', 'snap_on_start']
 
 

qubes/tests/integ/basic.py

@@ -77,68 +77,6 @@ class TC_01_Properties(qubes.tests.SystemTestsMixin, qubes.tests.QubesTestCase):
                                       label='red')
         self.loop.run_until_complete(self.vm.create_on_disk())
 
-    @unittest.expectedFailure
-    def test_000_rename(self):
-        newname = self.make_vm_name('newname')
-
-        self.assertEqual(self.vm.name, self.vmname)
-        self.vm.firewall.policy = 'drop'
-        self.vm.firewall.rules = [
-            qubes.firewall.Rule(None, action='accept', specialtarget='dns')
-        ]
-        self.vm.firewall.save()
-        self.vm.autostart = True
-        self.addCleanup(os.system,
-                        'sudo systemctl -q disable qubes-vm@{}.service || :'.
-                        format(self.vmname))
-        pre_rename_firewall = self.vm.firewall.rules
-
-        with self.assertNotRaises(
-                (OSError, libvirt.libvirtError, qubes.exc.QubesException)):
-            self.vm.name = newname
-        self.assertEqual(self.vm.name, newname)
-        self.assertEqual(self.vm.dir_path,
-            os.path.join(
-                qubes.config.system_path['qubes_base_dir'],
-                qubes.config.system_path['qubes_appvms_dir'], newname))
-        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(os.getenv("HOME"), ".local/share/desktop-directories",
-                newname + "-vm.directory")))
-        self.assertTrue(os.path.exists(
-            os.path.join(os.getenv("HOME"), ".local/share/applications",
-                newname + "-firefox.desktop")))
-        self.assertFalse(os.path.exists(
-            os.path.join(os.getenv("HOME"), ".local/share/desktop-directories",
-                self.vmname + "-vm.directory")))
-        self.assertFalse(os.path.exists(
-            os.path.join(os.getenv("HOME"), ".local/share/applications",
-                self.vmname + "-firefox.desktop")))
-        self.vm.firewall.load()
-        self.assertEqual(pre_rename_firewall, self.vm.firewall.rules)
-        with self.assertNotRaises((qubes.exc.QubesException, OSError)):
-            self.vm.firewall.save()
-        self.assertTrue(self.vm.autostart)
-        self.assertTrue(os.path.exists(
-            '/etc/systemd/system/multi-user.target.wants/'
-            'qubes-vm@{}.service'.format(newname)))
-        self.assertFalse(os.path.exists(
-            '/etc/systemd/system/multi-user.target.wants/'
-            'qubes-vm@{}.service'.format(self.vmname)))
-
-    def test_001_rename_libvirt_undefined(self):
-        self.vm.libvirt_domain.undefine()
-        self.vm._libvirt_domain = None  # pylint: disable=protected-access
-
-        newname = self.make_vm_name('newname')
-        with self.assertNotRaises(
-                (OSError, libvirt.libvirtError, qubes.exc.QubesException)):
-            self.vm.name = newname
-
     @unittest.expectedFailure
     def test_030_clone(self):
         testvm1 = self.app.add_new_vm(

qubes/tests/integ/storage.py

@@ -59,7 +59,6 @@ class StorageTestMixin(qubes.tests.SystemTestsMixin):
         volume_config = {
             'pool': self.pool.name,
             'size': size,
-            'internal': False,
             'save_on_stop': False,
             'rw': True,
         }
@@ -91,7 +90,6 @@ class StorageTestMixin(qubes.tests.SystemTestsMixin):
         volume_config = {
             'pool': self.pool.name,
             'size': size,
-            'internal': False,
             'save_on_stop': True,
             'rw': True,
         }
@@ -126,7 +124,6 @@ class StorageTestMixin(qubes.tests.SystemTestsMixin):
         volume_config = {
             'pool': self.pool.name,
             'size': size,
-            'internal': False,
             'save_on_stop': False,
             'rw': False,
         }
@@ -157,7 +154,6 @@ class StorageTestMixin(qubes.tests.SystemTestsMixin):
         volume_config = {
             'pool': self.pool.name,
             'size': size,
-            'internal': False,
             'save_on_stop': True,
             'rw': True,
         }
@@ -166,7 +162,6 @@ class StorageTestMixin(qubes.tests.SystemTestsMixin):
         volume_config = {
             'pool': self.pool.name,
             'size': size,
-            'internal': False,
             'snap_on_start': True,
             'source': testvol.vid,
             'rw': True,
@@ -225,7 +220,6 @@ class StorageTestMixin(qubes.tests.SystemTestsMixin):
         volume_config = {
             'pool': self.pool.name,
             'size': size,
-            'internal': False,
             'save_on_stop': True,
             'rw': True,
         }
@@ -234,7 +228,6 @@ class StorageTestMixin(qubes.tests.SystemTestsMixin):
         volume_config = {
             'pool': self.pool.name,
             'size': size,
-            'internal': False,
             'snap_on_start': True,
             'source': testvol.vid,
             'rw': True,

qubes/tests/storage_file.py

@@ -136,6 +136,15 @@ class TC_01_FileVolumes(qubes.tests.QubesTestCase):
         self.assertFalse(volume.snap_on_start)
         self.assertTrue(volume.save_on_stop)
         self.assertTrue(volume.rw)
+        block = volume.block_device()
+        self.assertEqual(block.path,
+            '{base}.img:{base}-cow.img'.format(
+                base=self.POOL_DIR + '/appvms/' + vm.name + '/root'))
+        self.assertEqual(block.script, 'block-origin')
+        self.assertEqual(block.rw, True)
+        self.assertEqual(block.name, 'root')
+        self.assertEqual(block.devtype, 'disk')
+        self.assertIsNone(block.domain)
 
     def test_001_snapshot_volume(self):
         template_vm = self.app.default_template
@@ -169,6 +178,19 @@ class TC_01_FileVolumes(qubes.tests.QubesTestCase):
         self.assertFalse(volume.save_on_stop)
         self.assertFalse(volume.rw)
         self.assertEqual(volume.usage, 0)
+        block = volume.block_device()
+        assert isinstance(block, qubes.storage.BlockDevice)
+        self.assertEqual(block.path,
+            '{base}/{src}.img:{base}/{src}-cow.img:'
+            '{base}/{dst}-cow.img'.format(
+                base=self.POOL_DIR,
+                src='vm-templates/' + template_vm.name + '/root',
+                dst='appvms/' + vm.name + '/root',
+        ))
+        self.assertEqual(block.name, 'root')
+        self.assertEqual(block.script, 'block-snapshot')
+        self.assertEqual(block.rw, False)
+        self.assertEqual(block.devtype, 'disk')
 
     def test_002_read_write_volume(self):
         config = {
@@ -186,6 +208,12 @@ class TC_01_FileVolumes(qubes.tests.QubesTestCase):
         self.assertFalse(volume.snap_on_start)
         self.assertTrue(volume.save_on_stop)
         self.assertTrue(volume.rw)
+        block = volume.block_device()
+        self.assertEqual(block.name, 'root')
+        self.assertEqual(block.path, '{base}.img:{base}-cow.img'.format(
+            base=self.POOL_DIR + '/appvms/' + vm.name + '/root'))
+        self.assertEqual(block.rw, True)
+        self.assertEqual(block.script, 'block-origin')
 
     def test_003_read_only_volume(self):
         template = self.app.default_template
@@ -207,6 +235,8 @@ class TC_01_FileVolumes(qubes.tests.QubesTestCase):
         self.assertFalse(volume.snap_on_start)
         self.assertFalse(volume.save_on_stop)
         self.assertFalse(volume.rw)
+        block = volume.block_device()
+        self.assertEqual(block.rw, False)
 
     def test_004_volatile_volume(self):
         config = {
@@ -248,7 +278,8 @@ class TC_01_FileVolumes(qubes.tests.QubesTestCase):
 
         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'
+        expected = vm.dir_path + '/private.img:' + \
+                   vm.dir_path + '/private-cow.img'
         self.assertVolumePath(vm, 'private', expected, rw=True)
         expected = vm.dir_path + '/volatile.img'
         self.assertVolumePath(vm, 'volatile', expected, rw=True)
@@ -275,8 +306,8 @@ class TC_03_FilePool(qubes.tests.QubesTestCase):
     def setUp(self):
         """ Add a test file based storage pool """
         super(TC_03_FilePool, self).setUp()
-        self._orig_qubes_base_dir = qubes.config.system_path['qubes_base_dir']
-        qubes.config.system_path['qubes_base_dir'] = '/tmp/qubes-test'
+        self._orig_qubes_base_dir = qubes.config.qubes_base_dir
+        qubes.config.qubes_base_dir = '/tmp/qubes-test'
         self.app = TestApp()
         self.app.create_dummy_template()
         self.app.add_pool(**self.POOL_CONFIG)
@@ -289,7 +320,7 @@ class TC_03_FilePool(qubes.tests.QubesTestCase):
         shutil.rmtree(self.POOL_DIR, ignore_errors=True)
         if os.path.exists('/tmp/qubes-test'):
             shutil.rmtree('/tmp/qubes-test')
-        qubes.config.system_path['qubes_base_dir'] = self._orig_qubes_base_dir
+        qubes.config.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 """

qubes/tests/vm/qubesvm.py

@@ -80,50 +80,6 @@ class TC_00_setters(qubes.tests.QubesTestCase):
             qubes.vm.qubesvm._setter_qid(self.vm,
                 self.prop, qubes.config.max_qid + 5)
 
-
-    def test_010_setter_name(self):
-        self.assertEqual(
-            qubes.vm.qubesvm._setter_name(self.vm, self.prop, 'test_name-1'),
-            'test_name-1')
-
-    def test_011_setter_name_not_a_string(self):
-        # pylint: disable=invalid-name
-        with self.assertRaises(TypeError):
-            qubes.vm.qubesvm._setter_name(self.vm, self.prop, False)
-
-    def test_012_setter_name_longer_than_31(self):
-        # pylint: disable=invalid-name
-        with self.assertRaises(ValueError):
-            qubes.vm.qubesvm._setter_name(self.vm, self.prop, 't' * 32)
-
-    def test_013_setter_name_illegal_character(self):
-        # pylint: disable=invalid-name
-        with self.assertRaises(ValueError):
-            qubes.vm.qubesvm._setter_name(self.vm, self.prop, 'test#')
-
-    def test_014_setter_name_first_not_letter(self):
-        # pylint: disable=invalid-name
-        with self.assertRaises(ValueError):
-            qubes.vm.qubesvm._setter_name(self.vm, self.prop, '1test')
-
-    def test_015_setter_name_running(self):
-        self.vm.running = True
-        with self.assertRaises(qubes.exc.QubesVMNotHaltedError):
-            qubes.vm.qubesvm._setter_name(self.vm, self.prop, 'testname')
-
-    def test_016_setter_name_installed_by_rpm(self):
-        # pylint: disable=invalid-name
-        self.vm.installed_by_rpm = True
-        with self.assertRaises(qubes.exc.QubesException):
-            qubes.vm.qubesvm._setter_name(self.vm, self.prop, 'testname')
-
-    def test_017_setter_name_duplicate(self):
-        # pylint: disable=invalid-name
-        self.vm.app.domains['duplicate'] = TestVM(name='duplicate')
-        with self.assertRaises(qubes.exc.QubesException):
-            qubes.vm.qubesvm._setter_name(self.vm, self.prop, 'duplicate')
-
-
     @unittest.skip('test not implemented')
     def test_020_setter_kernel(self):
         pass

qubes/vm/appvm.py

@@ -49,7 +49,6 @@ class AppVM(qubes.vm.qubesvm.QubesVM):
                 'save_on_stop': False,
                 'rw': False,
                 'source': None,
-                'internal': True
             },
             'private': {
                 'name': 'private',
@@ -58,20 +57,21 @@ class AppVM(qubes.vm.qubesvm.QubesVM):
                 'save_on_stop': True,
                 'rw': True,
                 'size': defaults['private_img_size'],
-                'internal': True
             },
             'volatile': {
                 'name': 'volatile',
                 'pool': 'default',
+                'snap_on_start': False,
+                'save_on_stop': False,
                 'size': defaults['root_img_size'],
-                'internal': True,
                 'rw': True,
             },
             'kernel': {
                 'name': 'kernel',
                 'pool': 'linux-kernel',
+                'snap_on_start': False,
+                'save_on_stop': False,
                 'rw': False,
-                'internal': True
             }
         }
 

qubes/vm/dispvm.py

@@ -47,20 +47,19 @@ class DispVM(qubes.vm.qubesvm.QubesVM):
                 'snap_on_start': True,
                 'save_on_stop': False,
                 'rw': False,
-                'internal': True
             },
             'private': {
                 'name': 'private',
                 'pool': 'default',
                 'snap_on_start': True,
                 'save_on_stop': False,
-                'internal': True,
                 'rw': True,
             },
             'volatile': {
                 'name': 'volatile',
                 'pool': 'default',
-                'internal': True,
+                'snap_on_start': False,
+                'save_on_stop': False,
                 'rw': True,
                 'size': qubes.config.defaults['root_img_size'] +
                         qubes.config.defaults['private_img_size'],
@@ -68,8 +67,9 @@ class DispVM(qubes.vm.qubesvm.QubesVM):
             'kernel': {
                 'name': 'kernel',
                 'pool': 'linux-kernel',
+                'snap_on_start': False,
+                'save_on_stop': False,
                 'rw': False,
-                'internal': True
             }
         }
         if 'name' not in kwargs and 'dispid' in kwargs:

qubes/vm/qubesvm.py

@@ -72,28 +72,6 @@ def _setter_qid(self, prop, value):
     return value
 
 
-def _setter_name(self, prop, value):
-    ''' Helper for setting the domain name '''
-    qubes.vm.validate_name(self, prop, value)
-
-    if self.is_running():
-        raise qubes.exc.QubesVMNotHaltedError(
-            self, 'Cannot change name of running VM')
-
-    try:
-        if self.installed_by_rpm:
-            raise qubes.exc.QubesException('Cannot rename VM installed by RPM '
-                '-- first clone VM and then use yum to remove package.')
-    except AttributeError:
-        pass
-
-    if value in self.app.domains:
-        raise qubes.exc.QubesPropertyValueError(self, prop, value,
-            'VM named {} alread exists'.format(value))
-
-    return value
-
-
 def _setter_kernel(self, prop, value):
     ''' Helper for setting the domain kernel and running sanity checks on it.
     '''  # pylint: disable=unused-argument
@@ -615,7 +593,7 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
     def dir_path(self):
         '''Root directory for files related to this domain'''
         return os.path.join(
-            qubes.config.system_path['qubes_base_dir'],
+            qubes.config.qubes_base_dir,
             self.dir_path_prefix,
             self.name)
 
@@ -748,33 +726,13 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
             else:
                 shutil.copy(newvalue.icon_path, self.icon_path)
 
-    @qubes.events.handler('property-pre-set:name')
-    def on_property_pre_set_name(self, event, name, newvalue, oldvalue=None):
-        # pylint: disable=unused-argument
-        try:
-            self.app.domains[newvalue]
-        except KeyError:
-            pass
-        else:
-            raise qubes.exc.QubesValueError(
-                'VM named {!r} already exists'.format(newvalue))
-
-        # TODO not self.is_stopped() would be more appropriate
-        if self.is_running():
-            raise qubes.exc.QubesVMNotHaltedError(
-                'Cannot change name of running domain {!r}'.format(oldvalue))
-
-        if self.autostart:
-            subprocess.check_call(['sudo', 'systemctl', '-q', 'disable',
-                                   'qubes-vm@{}.service'.format(oldvalue)])
-
     @qubes.events.handler('property-pre-set:kernel')
     def on_property_pre_set_kernel(self, event, name, newvalue, oldvalue=None):
         # pylint: disable=unused-argument
         if not newvalue:
             return
         dirname = os.path.join(
-            qubes.config.system_path['qubes_base_dir'],
+            qubes.config.qubes_base_dir,
             qubes.config.system_path['qubes_kernels_base_dir'],
             newvalue)
         if not os.path.exists(dirname):
@@ -788,29 +746,6 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
                     'Kernel {!r} not properly installed: '
                     'missing {!r} file'.format(newvalue, filename))
 
-    @qubes.events.handler('property-set:name')
-    def on_property_set_name(self, event, name, newvalue, oldvalue=None):
-        # pylint: disable=unused-argument
-        self.init_log()
-
-        old_dir_path = os.path.join(os.path.dirname(self.dir_path), oldvalue)
-        new_dir_path = os.path.join(os.path.dirname(self.dir_path), newvalue)
-        os.rename(old_dir_path, new_dir_path)
-
-        self.storage.rename(oldvalue, newvalue)
-
-        if self._libvirt_domain is not None:
-            self.libvirt_domain.undefine()
-            self._libvirt_domain = None
-        if self._qdb_connection is not None:
-            self._qdb_connection.close()
-            self._qdb_connection = None
-
-        self._update_libvirt_domain()
-
-        if self.autostart:
-            self.autostart = self.autostart
-
     @qubes.events.handler('property-pre-set:autostart')
     def on_property_pre_set_autostart(self, event, name, newvalue,
             oldvalue=None):
@@ -1728,45 +1663,6 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
 
         return None
 
-    def is_outdated(self):
-        '''Check whether domain needs restart to update root image from \
-            template.
-
-        :returns: :py:obj:`True` if is outdated, :py:obj:`False` otherwise.
-        :rtype: bool
-        '''
-        # pylint: disable=no-member
-
-        # Makes sense only on VM based on template
-        if self.template is None:
-            return False
-
-        if not self.is_running():
-            return False
-
-        if not hasattr(self.template, 'rootcow_img'):
-            return False
-
-        rootimg_inode = os.stat(self.template.root_img)
-        try:
-            rootcow_inode = os.stat(self.template.rootcow_img)
-        except OSError:
-            # The only case when rootcow_img doesn't exists is in the middle of
-            # commit_changes, so VM is outdated right now
-            return True
-
-        current_dmdev = "/dev/mapper/snapshot-{0:x}:{1}-{2:x}:{3}".format(
-                rootimg_inode[2], rootimg_inode[1],
-                rootcow_inode[2], rootcow_inode[1])
-
-        # FIXME
-        # 51712 (0xCA00) is xvda
-        #  backend node name not available through xenapi :(
-        used_dmdev = self.app.vmm.xs.read('',
-            '/local/domain/0/backend/vbd/{}/51712/node'.format(self.xid))
-
-        return used_dmdev != current_dmdev
-
     #
     # helper methods
     #
@@ -1900,29 +1796,26 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
 
 
 def _clean_volume_config(config):
-    common_attributes = ['name', 'pool', 'size', 'internal', 'removable',
-                            'revisions_to_keep', 'rw', 'snap_on_start',
-                            'save_on_stop', 'source']
+    common_attributes = ['name', 'pool', 'size',
+                         '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
+    is_snapshot = config['snap_on_start']
+    is_rw = config['rw']
 
     name = config['name']
 
-    if pool and is_exportable and config['pool'] == 'default':
+    if pool and not is_snapshot and is_rw:
         config['pool'] = str(pool)
-    elif pool and not is_exportable:
+    elif pool:
         pass
     elif pools and name in pools.keys():
-        if is_exportable:
+        if not is_snapshot:
             config['pool'] = str(pools[name])
         else:
             msg = "Can't clone a snapshot volume {!s} to pool {!s} " \

qubes/vm/standalonevm.py

@@ -35,7 +35,6 @@ class StandaloneVM(qubes.vm.qubesvm.QubesVM):
                 'save_on_stop': True,
                 'rw': True,
                 'source': None,
-                'internal': True,
                 'size': qubes.config.defaults['root_img_size'],
             },
             'private': {
@@ -45,21 +44,22 @@ class StandaloneVM(qubes.vm.qubesvm.QubesVM):
                 'save_on_stop': True,
                 'rw': True,
                 'source': None,
-                'internal': True,
                 'size': qubes.config.defaults['private_img_size'],
             },
             'volatile': {
                 'name': 'volatile',
                 'pool': 'default',
-                'internal': True,
+                'snap_on_start': False,
+                'save_on_stop': False,
                 'rw': True,
                 'size': qubes.config.defaults['root_img_size'],
             },
             'kernel': {
                 'name': 'kernel',
                 'pool': 'linux-kernel',
+                'snap_on_start': False,
+                'save_on_stop': False,
                 'rw': False,
-                'internal': True
             }
         }
         super(StandaloneVM, self).__init__(*args, **kwargs)

qubes/vm/templatevm.py

@@ -71,7 +71,6 @@ class TemplateVM(QubesVM):
                 'rw': True,
                 'source': None,
                 'size': defaults['root_img_size'],
-                'internal': True
             },
             'private': {
                 'name': 'private',
@@ -82,30 +81,21 @@ class TemplateVM(QubesVM):
                 'source': None,
                 'size': defaults['private_img_size'],
                 'revisions_to_keep': 0,
-                'internal': True
             },
             'volatile': {
                 'name': 'volatile',
                 'pool': 'default',
                 'size': defaults['root_img_size'],
-                'internal': True,
+                'snap_on_start': False,
+                'save_on_stop': False,
                 'rw': True,
             },
             'kernel': {
                 'name': 'kernel',
                 'pool': 'linux-kernel',
-                'internal': True,
+                'snap_on_start': False,
+                'save_on_stop': False,
                 'rw': False
             }
         }
         super(TemplateVM, self).__init__(*args, **kwargs)
-
-    def commit_changes(self):
-        '''Commit changes to template'''
-        self.log.debug('commit_changes()')
-
-        if not self.app.vmm.offline_mode:
-            assert not self.is_running(), \
-                'Attempt to commit changes on running Template VM!'
-
-        self.storage.commit()