Merge branch 'storage-properties'

* storage-properties:
  storage: use None for size/usage properties if unknown
  tests: call search_pool_containing_dir with various dirs and pools
  storage: make DirectoryThinPool helper less verbose, add sudo
  api/admin: add 'included_in' to admin.pool.Info call
  storage: add Pool.included_in() method for checking nested pools
  storage: move and generalize RootThinPool helper class
  storage/kernels: refuse changes to 'rw' and 'revisions_to_keep'
  api/admin: implement admin.vm.volume.Set.rw method
  api/admin: include 'revisions_to_keep' and 'is_outdated' in volume info

Makefile

@@ -30,6 +30,7 @@ ADMIN_API_METHODS_SIMPLE = \
 	admin.pool.volume.Resize \
 	admin.pool.volume.Revert \
 	admin.pool.volume.Set.revisions_to_keep \
+	admin.pool.volume.Set.rw \
 	admin.pool.volume.Snapshot \
 	admin.property.Get \
 	admin.property.GetDefault \
@@ -100,6 +101,7 @@ ADMIN_API_METHODS_SIMPLE = \
 	admin.vm.volume.Resize \
 	admin.vm.volume.Revert \
 	admin.vm.volume.Set.revisions_to_keep \
+	admin.vm.volume.Set.rw \
 	admin.vm.Stats \
 	$(null)
 

qubes/api/admin.py

@@ -336,9 +336,16 @@ class QubesAdminAPI(qubes.api.AbstractQubesAPI):
         # properties defined in API
         volume_properties = [
             'pool', 'vid', 'size', 'usage', 'rw', 'source',
-            'save_on_stop', 'snap_on_start']
+            'save_on_stop', 'snap_on_start', 'revisions_to_keep', 'is_outdated']
-        return ''.join('{}={}\n'.format(key, getattr(volume, key)) for key in
+
-            volume_properties)
+        def _serialize(value):
+            if callable(value):
+                value = value()
+            if value is None:
+                value = ''
+            return str(value)
+        return ''.join('{}={}\n'.format(key, _serialize(getattr(volume, key)))
+            for key in volume_properties)
 
     @qubes.api.method('admin.vm.volume.ListSnapshots', no_payload=True,
         scope='local', read=True)
@@ -496,6 +503,26 @@ class QubesAdminAPI(qubes.api.AbstractQubesAPI):
         self.dest.volumes[self.arg].revisions_to_keep = newvalue
         self.app.save()
 
+    @qubes.api.method('admin.vm.volume.Set.rw',
+        scope='local', write=True)
+    @asyncio.coroutine
+    def vm_volume_set_rw(self, untrusted_payload):
+        assert self.arg in self.dest.volumes.keys()
+        try:
+            newvalue = qubes.property.bool(None, None,
+                untrusted_payload.decode('ascii'))
+        except (UnicodeDecodeError, ValueError):
+            raise qubes.api.ProtocolError('Invalid value')
+        del untrusted_payload
+
+        self.fire_event_for_permission(newvalue=newvalue)
+
+        if not self.dest.is_halted():
+            raise qubes.exc.QubesVMNotHaltedError(self.dest)
+
+        self.dest.volumes[self.arg].rw = newvalue
+        self.app.save()
+
     @qubes.api.method('admin.vm.tag.List', no_payload=True,
         scope='local', read=True)
     @asyncio.coroutine
@@ -579,19 +606,25 @@ class QubesAdminAPI(qubes.api.AbstractQubesAPI):
 
         self.fire_event_for_permission(pool=pool)
 
-        size_info = ''
+        other_info = ''
+        pool_size = pool.size
+        if pool_size is not None:
+            other_info += 'size={}\n'.format(pool_size)
+
+        pool_usage = pool.usage
+        if pool_usage is not None:
+            other_info += 'usage={}\n'.format(pool_usage)
+
         try:
-            size_info += 'size={}\n'.format(pool.size)
+            included_in = pool.included_in(self.app)
-        except NotImplementedError:
+            if included_in:
-            pass
+                other_info += 'included_in={}\n'.format(str(included_in))
-        try:
-            size_info += 'usage={}\n'.format(pool.usage)
         except NotImplementedError:
             pass
 
         return ''.join('{}={}\n'.format(prop, val)
             for prop, val in sorted(pool.config.items())) + \
-            size_info
+            other_info
 
     @qubes.api.method('admin.pool.Add',
         scope='global', write=True)

qubes/app.py

@@ -24,21 +24,19 @@ import collections
 import errno
 import functools
 import grp
+import itertools
 import logging
 import os
 import random
-import subprocess
 import sys
 import tempfile
 import time
 import traceback
 import uuid
 
-import itertools
-import lxml.etree
-
 import jinja2
 import libvirt
+import lxml.etree
 
 try:
     import xen.lowlevel.xs  # pylint: disable=wrong-import-order
@@ -548,54 +546,6 @@ class VMCollection(object):
             'https://xkcd.com/221/',
             'http://dilbert.com/strip/2001-10-25')[random.randint(0, 1)])
 
-# pylint: disable=too-few-public-methods
-class RootThinPool:
-    '''The thin pool containing the rootfs device'''
-    _inited = False
-    _volume_group = None
-    _thin_pool = None
-
-    @classmethod
-    def _init(cls):
-        '''Find out the thin pool containing the root device'''
-        if not cls._inited:
-            cls._inited = True
-
-            try:
-                rootfs = os.stat('/')
-                root_major = (rootfs.st_dev & 0xff00) >> 8
-                root_minor = rootfs.st_dev & 0xff
-
-                root_table = subprocess.check_output(["dmsetup",
-                    "-j", str(root_major), "-m", str(root_minor),
-                    "table"])
-
-                _start, _sectors, target_type, target_args = \
-                    root_table.decode().split(" ", 3)
-                if target_type == "thin":
-                    thin_pool_devnum, _thin_pool_id = target_args.split(" ")
-                    with open("/sys/dev/block/{}/dm/name"
-                        .format(thin_pool_devnum), "r") as thin_pool_tpool_f:
-                        thin_pool_tpool = thin_pool_tpool_f.read().rstrip('\n')
-                    if thin_pool_tpool.endswith("-tpool"):
-                        volume_group, thin_pool, _tpool = \
-                            thin_pool_tpool.rsplit("-", 2)
-                        cls._volume_group = volume_group
-                        cls._thin_pool = thin_pool
-            except: # pylint: disable=bare-except
-                pass
-
-    @classmethod
-    def volume_group(cls):
-        '''Volume group of the thin pool containing the rootfs device'''
-        cls._init()
-        return cls._volume_group
-
-    @classmethod
-    def thin_pool(cls):
-        '''Thin pool name containing the rootfs device'''
-        cls._init()
-        return cls._thin_pool
 
 def _default_pool(app):
     ''' Default storage pool.
@@ -616,8 +566,8 @@ def _default_pool(app):
                 if pool.config['thin_pool'] == thin_pool:
                     return pool
         # no DEFAULT_LVM_POOL, or pool not defined
-        root_volume_group = RootThinPool.volume_group()
+        root_volume_group, root_thin_pool = \
-        root_thin_pool = RootThinPool.thin_pool()
+            qubes.storage.DirectoryThinPool.thin_pool('/')
         if root_thin_pool:
             for pool in app.pools.values():
                 if pool.config.get('driver', None) != 'lvm_thin':
@@ -1114,8 +1064,8 @@ class Qubes(qubes.PropertyHolder):
         }
         assert max(self.labels.keys()) == qubes.config.max_default_label
 
-        root_volume_group = RootThinPool.volume_group()
+        root_volume_group, root_thin_pool = \
-        root_thin_pool = RootThinPool.thin_pool()
+            qubes.storage.DirectoryThinPool.thin_pool('/')
 
         if root_thin_pool:
             self.add_pool(

qubes/storage/__init__.py

@@ -815,15 +815,26 @@ class Pool(object):
         '''
         raise self._not_implemented("get_volume")
 
+    def included_in(self, app):
+        ''' Check if this pool is physically included in another one
+
+        This works on best-effort basis, because one pool driver may not know
+        all the other drivers.
+
+        :param app: Qubes() object to lookup other pools in
+        :returns pool or None
+        '''
+        pass
+
     @property
     def size(self):
-        ''' Storage pool size in bytes '''
+        ''' Storage pool size in bytes, or None if unknown '''
-        raise self._not_implemented("size")
+        return None
 
     @property
     def usage(self):
-        ''' Space used in the pool, in bytes '''
+        ''' Space used in the pool in bytes, or None if unknown '''
-        raise self._not_implemented("usage")
+        return None
 
     def _not_implemented(self, method_name):
         ''' Helper for emitting helpful `NotImplementedError` exceptions '''
@@ -865,6 +876,27 @@ def isodate(seconds=time.time()):
     ''' Helper method which returns an iso date '''
     return datetime.utcfromtimestamp(seconds).isoformat("T")
 
+def search_pool_containing_dir(pools, dir_path):
+    ''' Helper function looking for a pool containing given directory.
+
+    This is useful for implementing Pool.included_in method
+    '''
+
+    # prefer filesystem pools
+    for pool in pools:
+        if hasattr(pool, 'dir_path'):
+            if dir_path.startswith(pool.dir_path):
+                return pool
+
+    # then look for lvm
+    for pool in pools:
+        if hasattr(pool, 'thin_pool') and hasattr(pool, 'volume_group'):
+            if (pool.volume_group, pool.thin_pool) == \
+                    DirectoryThinPool.thin_pool(dir_path):
+                return pool
+
+    return None
+
 
 class VmCreationManager(object):
     ''' A `ContextManager` which cleans up if volume creation fails.
@@ -883,3 +915,46 @@ class VmCreationManager(object):
                 except Exception:  # pylint: disable=broad-except
                     pass
             os.rmdir(self.vm.dir_path)
+
+# pylint: disable=too-few-public-methods
+class DirectoryThinPool:
+    '''The thin pool containing the device of given filesystem'''
+    _thin_pool = {}
+
+    @classmethod
+    def _init(cls, dir_path):
+        '''Find out the thin pool containing given filesystem'''
+        if dir_path not in cls._thin_pool:
+            cls._thin_pool[dir_path] = None, None
+
+            try:
+                fs_stat = os.stat(dir_path)
+                fs_major = (fs_stat.st_dev & 0xff00) >> 8
+                fs_minor = fs_stat.st_dev & 0xff
+
+                sudo = []
+                if os.getuid():
+                    sudo = ['sudo']
+                root_table = subprocess.check_output(sudo + ["dmsetup",
+                    "-j", str(fs_major), "-m", str(fs_minor),
+                    "table"], stderr=subprocess.DEVNULL)
+
+                _start, _sectors, target_type, target_args = \
+                    root_table.decode().split(" ", 3)
+                if target_type == "thin":
+                    thin_pool_devnum, _thin_pool_id = target_args.split(" ")
+                    with open("/sys/dev/block/{}/dm/name"
+                        .format(thin_pool_devnum), "r") as thin_pool_tpool_f:
+                        thin_pool_tpool = thin_pool_tpool_f.read().rstrip('\n')
+                    if thin_pool_tpool.endswith("-tpool"):
+                        volume_group, thin_pool, _tpool = \
+                            thin_pool_tpool.rsplit("-", 2)
+                        cls._thin_pool[dir_path] = volume_group, thin_pool
+            except:  # pylint: disable=bare-except
+                pass
+
+    @classmethod
+    def thin_pool(cls, dir_path):
+        '''Thin tuple (volume group, pool name) containing given filesystem'''
+        cls._init(dir_path)
+        return cls._thin_pool[dir_path]

qubes/storage/file.py

@@ -163,6 +163,12 @@ class FilePool(qubes.storage.Pool):
         statvfs = os.statvfs(self.dir_path)
         return statvfs.f_frsize * (statvfs.f_blocks - statvfs.f_bfree)
 
+    def included_in(self, app):
+        ''' Check if there is pool containing this one - either as a
+        filesystem or its LVM volume'''
+        return qubes.storage.search_pool_containing_dir(
+            [pool for pool in app.pools.values() if pool is not self],
+            self.dir_path)
 
 class FileVolume(qubes.storage.Volume):
     ''' Parent class for the xen volumes implementation which expects a

qubes/storage/kernels.py

@@ -23,18 +23,21 @@
 
 import os
 
+import qubes.exc
+import qubes.storage
 from qubes.storage import Pool, StoragePoolException, Volume
 
 
 class LinuxModules(Volume):
     ''' A volume representing a ro linux kernel '''
-    rw = False
 
     def __init__(self, target_dir, kernel_version, **kwargs):
         kwargs['vid'] = ''
         super(LinuxModules, self).__init__(**kwargs)
         self._kernel_version = kernel_version
         self.target_dir = target_dir
+        assert self.revisions_to_keep == 0
+        assert self.rw is False
 
     @property
     def vid(self):
@@ -104,6 +107,28 @@ class LinuxModules(Volume):
     def is_outdated(self):
         return False
 
+    @property
+    def revisions_to_keep(self):
+        return 0
+
+    @revisions_to_keep.setter
+    def revisions_to_keep(self, value):
+        # pylint: disable=no-self-use
+        if value:
+            raise qubes.exc.QubesValueError(
+                'LinuxModules supports only revisions_to_keep=0')
+
+    @property
+    def rw(self):
+        return False
+
+    @rw.setter
+    def rw(self, value):
+        # pylint: disable=no-self-use
+        if value:
+            raise qubes.exc.QubesValueError(
+                'LinuxModules supports only read-only volumes')
+
     def start(self):
         return self
 
@@ -131,7 +156,7 @@ class LinuxKernel(Pool):
 
     def __init__(self, name=None, dir_path=None):
         assert dir_path, 'Missing dir_path'
-        super(LinuxKernel, self).__init__(name=name)
+        super(LinuxKernel, self).__init__(name=name, revisions_to_keep=0)
         self.dir_path = dir_path
 
     def init_volume(self, vm, volume_config):
@@ -167,6 +192,23 @@ class LinuxKernel(Pool):
     def setup(self):
         pass
 
+    @property
+    def revisions_to_keep(self):
+        return 0
+
+    @revisions_to_keep.setter
+    def revisions_to_keep(self, value):
+        # pylint: disable=no-self-use
+        if value:
+            raise qubes.exc.QubesValueError(
+                'LinuxModules supports only revisions_to_keep=0')
+
+    def included_in(self, app):
+        ''' Check if there is pool containing /var/lib/qubes/vm-kernels '''
+        return qubes.storage.search_pool_containing_dir(
+            [pool for pool in app.pools.values() if pool is not self],
+            self.dir_path)
+
     @property
     def volumes(self):
         ''' Return all known kernel volumes '''

qubes/storage/reflink.py

@@ -108,6 +108,12 @@ class ReflinkPool(qubes.storage.Pool):
         statvfs = os.statvfs(self.dir_path)
         return statvfs.f_frsize * (statvfs.f_blocks - statvfs.f_bfree)
 
+    def included_in(self, app):
+        ''' Check if there is pool containing this one - either as a
+        filesystem or its LVM volume'''
+        return qubes.storage.search_pool_containing_dir(
+            [pool for pool in app.pools.values() if pool is not self],
+            self.dir_path)
 
 class ReflinkVolume(qubes.storage.Volume):
     def create(self):

qubes/tests/api_admin.py

@@ -40,7 +40,7 @@ import qubes.storage
 # properties defined in API
 volume_properties = [
     'pool', 'vid', 'size', 'usage', 'rw', 'source',
-    'save_on_stop', 'snap_on_start']
+    'save_on_stop', 'snap_on_start', 'revisions_to_keep', 'is_outdated']
 
 
 class AdminAPITestCase(qubes.tests.QubesTestCase):
@@ -557,6 +557,7 @@ class TC_00_VMs(AdminAPITestCase):
                 usage=102400,
                 size=204800)
         }
+        self.app.pools['pool1'].included_in.return_value = None
         value = self.call_mgmt_func(b'admin.pool.Info', b'dom0', b'pool1')
 
         self.assertEqual(value,
@@ -566,18 +567,34 @@ class TC_00_VMs(AdminAPITestCase):
     def test_151_pool_info_unsupported_size(self):
         self.app.pools = {
             'pool1': unittest.mock.Mock(config={
-                'param1': 'value1', 'param2': 'value2'})
+                'param1': 'value1', 'param2': 'value2'},
+                size=None, usage=None),
         }
-        type(self.app.pools['pool1']).size = unittest.mock.PropertyMock(
+        self.app.pools['pool1'].included_in.return_value = None
-            side_effect=NotImplementedError)
-        type(self.app.pools['pool1']).usage = unittest.mock.PropertyMock(
-            side_effect=NotImplementedError)
         value = self.call_mgmt_func(b'admin.pool.Info', b'dom0', b'pool1')
 
         self.assertEqual(value,
             'param1=value1\nparam2=value2\n')
         self.assertFalse(self.app.save.called)
 
+    def test_152_pool_info_included_in(self):
+        self.app.pools = {
+            'pool1': unittest.mock.MagicMock(config={
+                'param1': 'value1',
+                'param2': 'value2'},
+                usage=102400,
+                size=204800)
+        }
+        self.app.pools['pool1'].included_in.return_value = \
+            self.app.pools['pool1']
+        self.app.pools['pool1'].__str__.return_value = 'pool1'
+        value = self.call_mgmt_func(b'admin.pool.Info', b'dom0', b'pool1')
+
+        self.assertEqual(value,
+            'param1=value1\nparam2=value2\nsize=204800\nusage=102400'
+            '\nincluded_in=pool1\n')
+        self.assertFalse(self.app.save.called)
+
     @unittest.mock.patch('qubes.storage.pool_drivers')
     @unittest.mock.patch('qubes.storage.driver_parameters')
     def test_160_pool_add(self, mock_parameters, mock_drivers):
@@ -2373,6 +2390,34 @@ class TC_00_VMs(AdminAPITestCase):
             self.call_mgmt_func(b'admin.vm.volume.Set.revisions_to_keep',
                 b'test-vm1', b'private', b'abc')
 
+    def test_680_vm_volume_set_rw(self):
+        self.vm.volumes = unittest.mock.MagicMock()
+        volumes_conf = {
+            'keys.return_value': ['root', 'private', 'volatile', 'kernel'],
+        }
+        self.vm.volumes.configure_mock(**volumes_conf)
+        self.vm.storage = unittest.mock.Mock()
+        value = self.call_mgmt_func(b'admin.vm.volume.Set.rw',
+            b'test-vm1', b'private', b'True')
+        self.assertIsNone(value)
+        self.assertEqual(self.vm.volumes.mock_calls,
+            [unittest.mock.call.keys(),
+            ('__getitem__', ('private',), {})])
+        self.assertEqual(self.vm.volumes['private'].rw, True)
+        self.app.save.assert_called_once_with()
+
+    def test_681_vm_volume_set_rw_invalid(self):
+        self.vm.volumes = unittest.mock.MagicMock()
+        volumes_conf = {
+            'keys.return_value': ['root', 'private', 'volatile', 'kernel'],
+        }
+        self.vm.volumes.configure_mock(**volumes_conf)
+        self.vm.storage = unittest.mock.Mock()
+        with self.assertRaises(AssertionError):
+            self.call_mgmt_func(b'admin.vm.volume.Set.revisions_to_keep',
+                b'test-vm1', b'private', b'abc')
+        self.assertFalse(self.app.save.called)
+
     def test_990_vm_unexpected_payload(self):
         methods_with_no_payload = [
             b'admin.vm.List',

qubes/tests/storage_lvm.py

@@ -27,9 +27,11 @@
 
 import os
 import subprocess
+import tempfile
 import unittest
 
 import qubes.tests
+import qubes.storage
 from qubes.storage.lvm import ThinPool, ThinVolume
 
 if 'DEFAULT_LVM_POOL' in os.environ.keys():
@@ -267,3 +269,76 @@ class TC_01_ThinPool(ThinPoolBase, qubes.tests.SystemTestCase):
                 self.assertEqual(volume.path, expected)
         with self.assertNotRaises(qubes.exc.QubesException):
             vm.start()
+
+@skipUnlessLvmPoolExists
+class TC_02_StorageHelpers(ThinPoolBase):
+    def setUp(self):
+        xml_path = '/tmp/qubes-test.xml'
+        self.app = qubes.Qubes.create_empty_store(store=xml_path,
+            clockvm=None,
+            updatevm=None,
+            offline_mode=True,
+        )
+        os.environ['QUBES_XML_PATH'] = xml_path
+        super(TC_02_StorageHelpers, self).setUp()
+        # reset cache
+        qubes.storage.DirectoryThinPool._thin_pool = {}
+
+        self.thin_dir = tempfile.TemporaryDirectory()
+        subprocess.check_call(
+            ['sudo', 'lvcreate', '-q', '-V', '32M',
+                '-T', DEFAULT_LVM_POOL, '-n',
+                'test-file-pool'], stdout=subprocess.DEVNULL)
+        self.thin_dev = '/dev/{}/test-file-pool'.format(
+            DEFAULT_LVM_POOL.split('/')[0])
+        subprocess.check_call(
+            ['sudo', 'mkfs.ext4', '-q', self.thin_dev])
+        subprocess.check_call(['sudo', 'mount', self.thin_dev,
+            self.thin_dir.name])
+        subprocess.check_call(['sudo', 'chmod', '777',
+            self.thin_dir.name])
+
+    def tearDown(self):
+        subprocess.check_call(['sudo', 'umount', self.thin_dir.name])
+        subprocess.check_call(
+            ['sudo', 'lvremove', '-q', '-f', self.thin_dev],
+            stdout = subprocess.DEVNULL)
+        self.thin_dir.cleanup()
+        super(TC_02_StorageHelpers, self).tearDown()
+        os.unlink(self.app.store)
+        del self.app
+        for attr in dir(self):
+            if isinstance(getattr(self, attr), qubes.vm.BaseVM):
+                delattr(self, attr)
+
+    def test_000_search_thin_pool(self):
+        pool = qubes.storage.search_pool_containing_dir(
+            self.app.pools.values(), self.thin_dir.name)
+        self.assertEqual(pool, self.pool)
+
+    def test_001_search_none(self):
+        pool = qubes.storage.search_pool_containing_dir(
+            self.app.pools.values(), '/tmp')
+        self.assertIsNone(pool)
+
+    def test_002_search_subdir(self):
+        subdir = os.path.join(self.thin_dir.name, 'some-dir')
+        os.mkdir(subdir)
+        pool = qubes.storage.search_pool_containing_dir(
+            self.app.pools.values(), subdir)
+        self.assertEqual(pool, self.pool)
+
+    def test_003_search_file_pool(self):
+        subdir = os.path.join(self.thin_dir.name, 'some-dir')
+        file_pool_config = {
+            'name': 'test-file-pool',
+            'driver': 'file',
+            'dir_path': subdir
+        }
+        pool2 = self.app.add_pool(**file_pool_config)
+        pool = qubes.storage.search_pool_containing_dir(
+            self.app.pools.values(), subdir)
+        self.assertEqual(pool, pool2)
+        pool = qubes.storage.search_pool_containing_dir(
+            self.app.pools.values(), self.thin_dir.name)
+        self.assertEqual(pool, self.pool)