Merge branch '20171107-storage'

* 20171107-storage:
  api/admin: add API for changing revisions_to_keep dynamically
  storage/file: move revisions_to_keep restrictions to property setter
  api/admin: hide dd statistics in admin.vm.volume.Import call
  storage/lvm: fix importing different-sized volume from another pool
  storage/file: fix preserving spareness on volume clone
  api/admin: add pool size and usage to admin.pool.Info response
  storage: add size and usage properties to pool object

Makefile

@@ -23,11 +23,13 @@ ADMIN_API_METHODS_SIMPLE = \
 	admin.pool.List \
 	admin.pool.ListDrivers \
 	admin.pool.Remove \
+	admin.pool.Set.revisions_to_keep \
 	admin.pool.volume.Info \
 	admin.pool.volume.List \
 	admin.pool.volume.ListSnapshots \
 	admin.pool.volume.Resize \
 	admin.pool.volume.Revert \
+	admin.pool.volume.Set.revisions_to_keep \
 	admin.pool.volume.Snapshot \
 	admin.property.Get \
 	admin.property.GetDefault \
@@ -96,6 +98,7 @@ ADMIN_API_METHODS_SIMPLE = \
 	admin.vm.volume.ListSnapshots \
 	admin.vm.volume.Resize \
 	admin.vm.volume.Revert \
+	admin.vm.volume.Set.revisions_to_keep \
 	admin.vm.Stats \
 	$(null)
 

qubes-rpc/admin.vm.volume.Import

@@ -44,7 +44,7 @@ path=$(tail -c +3 "$tmpfile"|cut -d ' ' -f 2)
 
 # now process stdin into this path
 if dd bs=4k of="$path" count="$size" iflag=count_bytes,fullblock \
-        conv=sparse,notrunc,nocreat,fdatasync; then
+        conv=sparse,notrunc,nocreat,fdatasync status=none; then
     status="ok"
 else
     status="fail"

qubes/api/admin.py

@@ -476,6 +476,25 @@ class QubesAdminAPI(qubes.api.AbstractQubesAPI):
 
         return '{} {}'.format(size, path)
 
+    @qubes.api.method('admin.vm.volume.Set.revisions_to_keep',
+        scope='local', write=True)
+    @asyncio.coroutine
+    def vm_volume_set_revisions_to_keep(self, untrusted_payload):
+        assert self.arg in self.dest.volumes.keys()
+        try:
+            untrusted_value = int(untrusted_payload.decode('ascii'))
+        except (UnicodeDecodeError, ValueError):
+            raise qubes.api.ProtocolError('Invalid value')
+        del untrusted_payload
+        assert untrusted_value >= 0
+        newvalue = untrusted_value
+        del untrusted_value
+
+        self.fire_event_for_permission(newvalue=newvalue)
+
+        self.dest.volumes[self.arg].revisions_to_keep = newvalue
+        self.app.save()
+
     @qubes.api.method('admin.vm.tag.List', no_payload=True,
         scope='local', read=True)
     @asyncio.coroutine
@@ -559,8 +578,19 @@ class QubesAdminAPI(qubes.api.AbstractQubesAPI):
 
         self.fire_event_for_permission(pool=pool)
 
+        size_info = ''
+        try:
+            size_info += 'size={}\n'.format(pool.size)
+        except NotImplementedError:
+            pass
+        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()))
+            for prop, val in sorted(pool.config.items())) + \
+            size_info
 
     @qubes.api.method('admin.pool.Add',
         scope='global', write=True)
@@ -610,6 +640,27 @@ class QubesAdminAPI(qubes.api.AbstractQubesAPI):
         self.app.remove_pool(self.arg)
         self.app.save()
 
+    @qubes.api.method('admin.pool.Set.revisions_to_keep',
+        scope='global', write=True)
+    @asyncio.coroutine
+    def pool_set_revisions_to_keep(self, untrusted_payload):
+        assert self.dest.name == 'dom0'
+        assert self.arg in self.app.pools.keys()
+        pool = self.app.pools[self.arg]
+        try:
+            untrusted_value = int(untrusted_payload.decode('ascii'))
+        except (UnicodeDecodeError, ValueError):
+            raise qubes.api.ProtocolError('Invalid value')
+        del untrusted_payload
+        assert untrusted_value >= 0
+        newvalue = untrusted_value
+        del untrusted_value
+
+        self.fire_event_for_permission(newvalue=newvalue)
+
+        pool.revisions_to_keep = newvalue
+        self.app.save()
+
     @qubes.api.method('admin.label.List', no_payload=True,
         scope='global', read=True)
     @asyncio.coroutine

qubes/storage/__init__.py

@@ -815,6 +815,16 @@ class Pool(object):
         '''
         raise self._not_implemented("get_volume")
 
+    @property
+    def size(self):
+        ''' Storage pool size in bytes '''
+        raise self._not_implemented("size")
+
+    @property
+    def usage(self):
+        ''' Space used in the pool, in bytes '''
+        raise self._not_implemented("usage")
+
     def _not_implemented(self, method_name):
         ''' Helper for emitting helpful `NotImplementedError` exceptions '''
         msg = "Pool driver {!s} has {!s}() not implemented"

qubes/storage/file.py

@@ -54,6 +54,7 @@ class FilePool(qubes.storage.Pool):
     driver = 'file'
 
     def __init__(self, revisions_to_keep=1, dir_path=None, **kwargs):
+        self._revisions_to_keep = 0
         super(FilePool, self).__init__(revisions_to_keep=revisions_to_keep,
                                        **kwargs)
         assert dir_path, "No pool dir_path specified"
@@ -85,19 +86,27 @@ class FilePool(qubes.storage.Pool):
                 volume_config['revisions_to_keep'] = 0
         except KeyError:
             pass
-        finally:
-            if 'revisions_to_keep' not in volume_config:
-                volume_config['revisions_to_keep'] = self.revisions_to_keep
 
-        if int(volume_config['revisions_to_keep']) > 1:
+        if 'revisions_to_keep' not in volume_config:
-            raise NotImplementedError(
+            volume_config['revisions_to_keep'] = self.revisions_to_keep
-                'FilePool supports maximum 1 volume revision to keep')
 
         volume_config['pool'] = self
         volume = FileVolume(**volume_config)
         self._volumes += [volume]
         return volume
 
+    @property
+    def revisions_to_keep(self):
+        return self._revisions_to_keep
+
+    @revisions_to_keep.setter
+    def revisions_to_keep(self, value):
+        value = int(value)
+        if value > 1:
+            raise NotImplementedError(
+                'FilePool supports maximum 1 volume revision to keep')
+        self._revisions_to_keep = value
+
     def destroy(self):
         pass
 
@@ -144,6 +153,16 @@ class FilePool(qubes.storage.Pool):
     def list_volumes(self):
         return self._volumes
 
+    @property
+    def size(self):
+        statvfs = os.statvfs(self.dir_path)
+        return statvfs.f_frsize * statvfs.f_blocks
+
+    @property
+    def usage(self):
+        statvfs = os.statvfs(self.dir_path)
+        return statvfs.f_frsize * (statvfs.f_blocks - statvfs.f_bfree)
+
 
 class FileVolume(qubes.storage.Volume):
     ''' Parent class for the xen volumes implementation which expects a
@@ -152,12 +171,24 @@ class FileVolume(qubes.storage.Volume):
     def __init__(self, dir_path, **kwargs):
         self.dir_path = dir_path
         assert self.dir_path, "dir_path not specified"
+        self._revisions_to_keep = 0
         super(FileVolume, self).__init__(**kwargs)
 
         if self.snap_on_start:
             img_name = self.source.vid + '-cow.img'
             self.path_source_cow = os.path.join(self.dir_path, img_name)
 
+    @property
+    def revisions_to_keep(self):
+        return self._revisions_to_keep
+
+    @revisions_to_keep.setter
+    def revisions_to_keep(self, value):
+        if int(value) > 1:
+            raise NotImplementedError(
+                'FileVolume supports maximum 1 volume revision to keep')
+        self._revisions_to_keep = int(value)
+
     def create(self):
         assert isinstance(self.size, int) and self.size > 0, \
             'Volume size must be > 0'
@@ -419,7 +450,7 @@ def copy_file(source, destination):
         os.makedirs(parent_dir)
 
     try:
-        cmd = ['cp', '--sparse=auto',
+        cmd = ['cp', '--sparse=always',
                '--reflink=auto', source, destination]
         subprocess.check_call(cmd)
     except subprocess.CalledProcessError:

qubes/storage/lvm.py

@@ -137,6 +137,22 @@ class ThinPool(qubes.storage.Pool):
             volumes += [ThinVolume(**config)]
         return volumes
 
+    @property
+    def size(self):
+        try:
+            return qubes.storage.lvm.size_cache[
+                self.volume_group + '/' + self.thin_pool]['size']
+        except KeyError:
+            return 0
+
+    @property
+    def usage(self):
+        try:
+            return qubes.storage.lvm.size_cache[
+                self.volume_group + '/' + self.thin_pool]['usage']
+        except KeyError:
+            return 0
+
 
 def init_cache(log=logging.getLogger('qubes.storage.lvm')):
     cmd = ['lvs', '--noheadings', '-o',
@@ -159,7 +175,7 @@ def init_cache(log=logging.getLogger('qubes.storage.lvm')):
         line = line.decode().strip()
         pool_name, pool_lv, name, size, usage_percent, attr, \
             origin = line.split(';', 6)
-        if '' in [pool_name, pool_lv, name, size, usage_percent]:
+        if '' in [pool_name, name, size, usage_percent]:
             continue
         name = pool_name + "/" + name
         size = int(size[:-1])  # Remove 'B' suffix
@@ -342,6 +358,8 @@ class ThinVolume(qubes.storage.Volume):
             cmd = ['clone', str(src_volume), str(self)]
             qubes_lvm(cmd, self.log)
         else:
+            if src_volume.size != self.size:
+                self.resize(src_volume.size)
             src_path = src_volume.export()
             cmd = ['dd', 'if=' + src_path, 'of=/dev/' + self.vid,
                 'conv=sparse']

qubes/tests/api_admin.py

@@ -545,11 +545,29 @@ class TC_00_VMs(AdminAPITestCase):
     def test_150_pool_info(self):
         self.app.pools = {
             'pool1': unittest.mock.Mock(config={
-                'param1': 'value1', 'param2': 'value2'})
+                'param1': 'value1', 'param2': 'value2'},
+                usage=102400,
+                size=204800)
         }
         value = self.call_mgmt_func(b'admin.pool.Info', b'dom0', b'pool1')
 
-        self.assertEqual(value, 'param1=value1\nparam2=value2\n')
+        self.assertEqual(value,
+            'param1=value1\nparam2=value2\nsize=204800\nusage=102400\n')
+        self.assertFalse(self.app.save.called)
+
+    def test_151_pool_info_unsupported_size(self):
+        self.app.pools = {
+            'pool1': unittest.mock.Mock(config={
+                'param1': 'value1', 'param2': 'value2'})
+        }
+        type(self.app.pools['pool1']).size = unittest.mock.PropertyMock(
+            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)
 
     @unittest.mock.patch('qubes.storage.pool_drivers')
@@ -2257,6 +2275,69 @@ class TC_00_VMs(AdminAPITestCase):
         self.assertNotIn(dev, self.vm.devices['testclass'].persistent())
         self.assertFalse(self.app.save.called)
 
+    def test_660_pool_set_revisions_to_keep(self):
+        self.app.pools['test-pool'] = unittest.mock.Mock()
+        value = self.call_mgmt_func(b'admin.pool.Set.revisions_to_keep',
+            b'dom0', b'test-pool', b'2')
+        self.assertIsNone(value)
+        self.assertEqual(self.app.pools['test-pool'].mock_calls, [])
+        self.assertEqual(self.app.pools['test-pool'].revisions_to_keep, 2)
+        self.app.save.assert_called_once_with()
+
+    def test_661_pool_set_revisions_to_keep_negative(self):
+        self.app.pools['test-pool'] = unittest.mock.Mock()
+        with self.assertRaises(AssertionError):
+            self.call_mgmt_func(b'admin.pool.Set.revisions_to_keep',
+                b'dom0', b'test-pool', b'-2')
+        self.assertEqual(self.app.pools['test-pool'].mock_calls, [])
+        self.assertFalse(self.app.save.called)
+
+    def test_662_pool_set_revisions_to_keep_not_a_number(self):
+        self.app.pools['test-pool'] = unittest.mock.Mock()
+        with self.assertRaises(AssertionError):
+            self.call_mgmt_func(b'admin.pool.Set.revisions_to_keep',
+                b'dom0', b'test-pool', b'abc')
+        self.assertEqual(self.app.pools['test-pool'].mock_calls, [])
+        self.assertFalse(self.app.save.called)
+
+    def test_670_vm_volume_set_revisions_to_keep(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.revisions_to_keep',
+            b'test-vm1', b'private', b'2')
+        self.assertIsNone(value)
+        self.assertEqual(self.vm.volumes.mock_calls,
+            [unittest.mock.call.keys(),
+            ('__getitem__', ('private',), {})])
+        self.assertEqual(self.vm.volumes['private'].revisions_to_keep, 2)
+        self.app.save.assert_called_once_with()
+
+    def test_671_vm_volume_set_revisions_to_keep_negative(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'-2')
+
+    def test_672_vm_volume_set_revisions_to_keep_not_a_number(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')
+
     def test_990_vm_unexpected_payload(self):
         methods_with_no_payload = [
             b'admin.vm.List',

qubes/tests/storage_file.py

@@ -296,6 +296,22 @@ class TC_01_FileVolumes(qubes.tests.QubesTestCase):
         expected = vm_dir + '/volatile.img'
         self.assertVolumePath(vm, 'volatile', expected, rw=True)
 
+    def test_010_revisions_to_keep_reject_invalid(self):
+        ''' Check if TemplateVM volumes are propertly initialized '''
+        config = {
+            'name': 'root',
+            'pool': self.POOL_NAME,
+            'save_on_stop': True,
+            'rw': True,
+            'size': defaults['root_img_size'],
+        }
+        vm = qubes.tests.storage.TestVM(self)
+        volume = self.app.get_pool(self.POOL_NAME).init_volume(vm, config)
+        self.assertEqual(volume.revisions_to_keep, 1)
+        with self.assertRaises((NotImplementedError, ValueError)):
+            volume.revisions_to_keep = 2
+        self.assertEqual(volume.revisions_to_keep, 1)
+
     def assertVolumePath(self, vm, dev_name, expected, rw=True):
         # :pylint: disable=invalid-name
         volumes = vm.volumes
@@ -369,6 +385,28 @@ class TC_03_FilePool(qubes.tests.QubesTestCase):
 
         shutil.rmtree(pool_dir, ignore_errors=True)
 
+    def test_003_size(self):
+        pool = self.app.get_pool(self.POOL_NAME)
+        with self.assertNotRaises(NotImplementedError):
+            size = pool.size
+        statvfs = os.statvfs(self.POOL_DIR)
+        self.assertEqual(size, statvfs.f_blocks * statvfs.f_frsize)
+
+    def test_004_usage(self):
+        pool = self.app.get_pool(self.POOL_NAME)
+        with self.assertNotRaises(NotImplementedError):
+            usage = pool.usage
+        statvfs = os.statvfs(self.POOL_DIR)
+        self.assertEqual(usage,
+            statvfs.f_frsize * (statvfs.f_blocks - statvfs.f_bfree))
+
+    def test_005_revisions_to_keep(self):
+        pool = self.app.get_pool(self.POOL_NAME)
+        self.assertEqual(pool.revisions_to_keep, 1)
+        with self.assertRaises((NotImplementedError, ValueError)):
+            pool.revisions_to_keep = 2
+        self.assertEqual(pool.revisions_to_keep, 1)
+
     def test_011_appvm_file_images(self):
         """ Check if all the needed image files are created for an AppVm"""
 

qubes/tests/storage_lvm.py

@@ -26,6 +26,7 @@
 '''
 
 import os
+import subprocess
 import unittest
 
 import qubes.tests
@@ -158,6 +159,26 @@ class TC_00_ThinPool(ThinPoolBase):
         self.assertTrue(os.path.exists(path))
         volume.remove()
 
+    def test_004_size(self):
+        with self.assertNotRaises(NotImplementedError):
+            size = self.pool.size
+        pool_size = subprocess.check_output(['sudo', 'lvs', '--noheadings',
+            '-o', 'lv_size',
+            '--units', 'b', self.pool.volume_group + '/' + self.pool.thin_pool])
+        self.assertEqual(size, int(pool_size.strip()[:-1]))
+
+    def test_005_usage(self):
+        with self.assertNotRaises(NotImplementedError):
+            usage = self.pool.usage
+        pool_info = subprocess.check_output(['sudo', 'lvs', '--noheadings',
+            '-o', 'lv_size,data_percent',
+            '--units', 'b', self.pool.volume_group + '/' + self.pool.thin_pool])
+        pool_size, pool_usage = pool_info.strip().split()
+        pool_size = int(pool_size[:-1])
+        pool_usage = float(pool_usage)
+        self.assertEqual(usage, int(pool_size * pool_usage / 100))
+
+
 @skipUnlessLvmPoolExists
 class TC_01_ThinPool(ThinPoolBase, qubes.tests.SystemTestCase):
     ''' Sanity tests for :py:class:`qubes.storage.lvm.ThinPool` '''