From 2a962c54dbbad7aa074dee8f171a81b5ef80e864 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Sun, 18 Mar 2018 21:45:43 +0100 Subject: [PATCH 1/9] api/admin: include 'revisions_to_keep' and 'is_outdated' in volume info Since Volume.is_outdated() is a method, not a property, add a function for handling serialization. And at the same time, fix None serialization (applicable to 'source' property). QubesOS/qubes-issues#3256 --- qubes/api/admin.py | 13 ++++++++++--- qubes/tests/api_admin.py | 2 +- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/qubes/api/admin.py b/qubes/api/admin.py index b42d6479..f03560f3 100644 --- a/qubes/api/admin.py +++ b/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'] - return ''.join('{}={}\n'.format(key, getattr(volume, key)) for key in - volume_properties) + 'save_on_stop', 'snap_on_start', 'revisions_to_keep', 'is_outdated'] + + 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) diff --git a/qubes/tests/api_admin.py b/qubes/tests/api_admin.py index dcbe5522..ec59fd49 100644 --- a/qubes/tests/api_admin.py +++ b/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): From 376c8ec00d80bfdc09b5cd8c9dc1540dfb97cb19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Sun, 18 Mar 2018 23:07:40 +0100 Subject: [PATCH 2/9] api/admin: implement admin.vm.volume.Set.rw method Allow setting 'rw' volume property. --- Makefile | 2 ++ qubes/api/admin.py | 20 ++++++++++++++++++++ qubes/tests/api_admin.py | 28 ++++++++++++++++++++++++++++ 3 files changed, 50 insertions(+) diff --git a/Makefile b/Makefile index 54ebaa76..eb951965 100644 --- a/Makefile +++ b/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) diff --git a/qubes/api/admin.py b/qubes/api/admin.py index f03560f3..c5bdb997 100644 --- a/qubes/api/admin.py +++ b/qubes/api/admin.py @@ -503,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 diff --git a/qubes/tests/api_admin.py b/qubes/tests/api_admin.py index ec59fd49..9df2df9c 100644 --- a/qubes/tests/api_admin.py +++ b/qubes/tests/api_admin.py @@ -2371,6 +2371,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', From a0723a9e3222f740c505536d240330e806c7104e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Mon, 19 Mar 2018 21:51:20 +0100 Subject: [PATCH 3/9] storage/kernels: refuse changes to 'rw' and 'revisions_to_keep' This pool driver support only rw=False and revisions_to_keep=0 volumes. Since there is API for changing those properties dynamically, block it at pool driver level, instead of silently ignoring them. --- qubes/storage/kernels.py | 39 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/qubes/storage/kernels.py b/qubes/storage/kernels.py index b39d8b3d..e60ac70e 100644 --- a/qubes/storage/kernels.py +++ b/qubes/storage/kernels.py @@ -23,18 +23,20 @@ import os +import qubes.exc 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 +106,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 +155,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 +191,17 @@ 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') + @property def volumes(self): ''' Return all known kernel volumes ''' From 99f430511a66c371b7f009c9d0a893609fbe85b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Mon, 19 Mar 2018 22:26:54 +0100 Subject: [PATCH 4/9] storage: move and generalize RootThinPool helper class This is a class for finding thin pool containing root filesytem. Generalize it to work for other filesystems too and rename to DirectoryThinPool. --- qubes/app.py | 62 ++++----------------------------------- qubes/storage/__init__.py | 40 +++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 56 deletions(-) diff --git a/qubes/app.py b/qubes/app.py index a993c94d..0b50f6a0 100644 --- a/qubes/app.py +++ b/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_thin_pool = RootThinPool.thin_pool() + root_volume_group, root_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_thin_pool = RootThinPool.thin_pool() + root_volume_group, root_thin_pool = \ + qubes.storage.DirectoryThinPool.thin_pool('/') if root_thin_pool: self.add_pool( diff --git a/qubes/storage/__init__.py b/qubes/storage/__init__.py index e5997e7d..c081fcb4 100644 --- a/qubes/storage/__init__.py +++ b/qubes/storage/__init__.py @@ -883,3 +883,43 @@ 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 + + root_table = subprocess.check_output(["dmsetup", + "-j", str(fs_major), "-m", str(fs_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._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] From d40fae975687c52c946156a861f83222a0f9c965 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Mon, 19 Mar 2018 22:36:45 +0100 Subject: [PATCH 5/9] storage: add Pool.included_in() method for checking nested pools It may happen that one pool is inside a volume of other pool. This is the case for example for varlibqubes pool (file driver, dir_path=/var/lib/qubes) and default lvm pool (lvm_thin driver). The latter include whole root filesystem, so /var/lib/qubes too. This is relevant for proper disk space calculation - to not count some space twice. QubesOS/qubes-issues#3240 QubesOS/qubes-issues#3241 --- qubes/storage/__init__.py | 32 ++++++++++++++++++++++++++++++++ qubes/storage/file.py | 6 ++++++ qubes/storage/kernels.py | 7 +++++++ qubes/storage/reflink.py | 6 ++++++ 4 files changed, 51 insertions(+) diff --git a/qubes/storage/__init__.py b/qubes/storage/__init__.py index c081fcb4..f373740a 100644 --- a/qubes/storage/__init__.py +++ b/qubes/storage/__init__.py @@ -815,6 +815,17 @@ 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 ''' @@ -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. diff --git a/qubes/storage/file.py b/qubes/storage/file.py index 10c1b304..f598f406 100644 --- a/qubes/storage/file.py +++ b/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 diff --git a/qubes/storage/kernels.py b/qubes/storage/kernels.py index e60ac70e..448057b1 100644 --- a/qubes/storage/kernels.py +++ b/qubes/storage/kernels.py @@ -24,6 +24,7 @@ import os import qubes.exc +import qubes.storage from qubes.storage import Pool, StoragePoolException, Volume @@ -202,6 +203,12 @@ class LinuxKernel(Pool): 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 ''' diff --git a/qubes/storage/reflink.py b/qubes/storage/reflink.py index 7cb9ca10..a11e6d3a 100644 --- a/qubes/storage/reflink.py +++ b/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): From 1bc640f3e095555e48eca7815aaa0aed131c7d8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Mon, 19 Mar 2018 22:43:02 +0100 Subject: [PATCH 6/9] api/admin: add 'included_in' to admin.pool.Info call QubesOS/qubes-issues#3240 QubesOS/qubes-issues#3241 --- qubes/api/admin.py | 15 +++++++++++---- qubes/tests/api_admin.py | 20 ++++++++++++++++++++ 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/qubes/api/admin.py b/qubes/api/admin.py index c5bdb997..80408dff 100644 --- a/qubes/api/admin.py +++ b/qubes/api/admin.py @@ -606,19 +606,26 @@ class QubesAdminAPI(qubes.api.AbstractQubesAPI): self.fire_event_for_permission(pool=pool) - size_info = '' + other_info = '' try: - size_info += 'size={}\n'.format(pool.size) + other_info += 'size={}\n'.format(pool.size) except NotImplementedError: pass try: - size_info += 'usage={}\n'.format(pool.usage) + other_info += 'usage={}\n'.format(pool.usage) + except NotImplementedError: + pass + + try: + included_in = pool.included_in(self.app) + if included_in: + other_info += 'included_in={}\n'.format(str(included_in)) 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) diff --git a/qubes/tests/api_admin.py b/qubes/tests/api_admin.py index 9df2df9c..998587ce 100644 --- a/qubes/tests/api_admin.py +++ b/qubes/tests/api_admin.py @@ -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, @@ -568,6 +569,7 @@ class TC_00_VMs(AdminAPITestCase): 'pool1': unittest.mock.Mock(config={ 'param1': 'value1', 'param2': 'value2'}) } + self.app.pools['pool1'].included_in.return_value = None type(self.app.pools['pool1']).size = unittest.mock.PropertyMock( side_effect=NotImplementedError) type(self.app.pools['pool1']).usage = unittest.mock.PropertyMock( @@ -578,6 +580,24 @@ class TC_00_VMs(AdminAPITestCase): '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): From 825de497675df7040452e1f03b8c3bf74a237682 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Tue, 20 Mar 2018 15:54:23 +0100 Subject: [PATCH 7/9] storage: make DirectoryThinPool helper less verbose, add sudo Don't print scary messages when given pool cannot be found. Also, add sudo to make it work from non-root user (tests) --- qubes/storage/__init__.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/qubes/storage/__init__.py b/qubes/storage/__init__.py index f373740a..45eb79ae 100644 --- a/qubes/storage/__init__.py +++ b/qubes/storage/__init__.py @@ -932,9 +932,12 @@ class DirectoryThinPool: fs_major = (fs_stat.st_dev & 0xff00) >> 8 fs_minor = fs_stat.st_dev & 0xff - root_table = subprocess.check_output(["dmsetup", + sudo = [] + if os.getuid(): + sudo = ['sudo'] + root_table = subprocess.check_output(sudo + ["dmsetup", "-j", str(fs_major), "-m", str(fs_minor), - "table"]) + "table"], stderr=subprocess.DEVNULL) _start, _sectors, target_type, target_args = \ root_table.decode().split(" ", 3) From 05c80c4531d252970a8fa566a7c70493b849446b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Tue, 20 Mar 2018 16:12:14 +0100 Subject: [PATCH 8/9] tests: call search_pool_containing_dir with various dirs and pools QubesOS/qubes-issues#3241 --- qubes/tests/storage_lvm.py | 75 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/qubes/tests/storage_lvm.py b/qubes/tests/storage_lvm.py index 2672a280..e8679aa7 100644 --- a/qubes/tests/storage_lvm.py +++ b/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) From 03dc3e315e3fb0b2c3ff8f5733aa71a5f818ff23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Tue, 20 Mar 2018 17:19:50 +0100 Subject: [PATCH 9/9] storage: use None for size/usage properties if unknown Raising NotImplementedError in a _property_ is weird behaviour, better suited for actions (methods). Use None instead. QubesOS/qubes-issues#3241 --- qubes/api/admin.py | 15 +++++++-------- qubes/storage/__init__.py | 8 ++++---- qubes/tests/api_admin.py | 7 ++----- 3 files changed, 13 insertions(+), 17 deletions(-) diff --git a/qubes/api/admin.py b/qubes/api/admin.py index 80408dff..ddf620ae 100644 --- a/qubes/api/admin.py +++ b/qubes/api/admin.py @@ -607,14 +607,13 @@ class QubesAdminAPI(qubes.api.AbstractQubesAPI): self.fire_event_for_permission(pool=pool) other_info = '' - try: - other_info += 'size={}\n'.format(pool.size) - except NotImplementedError: - pass - try: - other_info += 'usage={}\n'.format(pool.usage) - except NotImplementedError: - pass + 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: included_in = pool.included_in(self.app) diff --git a/qubes/storage/__init__.py b/qubes/storage/__init__.py index 45eb79ae..fdcdceb5 100644 --- a/qubes/storage/__init__.py +++ b/qubes/storage/__init__.py @@ -828,13 +828,13 @@ class Pool(object): @property def size(self): - ''' Storage pool size in bytes ''' - raise self._not_implemented("size") + ''' Storage pool size in bytes, or None if unknown ''' + return None @property def usage(self): - ''' Space used in the pool, in bytes ''' - raise self._not_implemented("usage") + ''' Space used in the pool in bytes, or None if unknown ''' + return None def _not_implemented(self, method_name): ''' Helper for emitting helpful `NotImplementedError` exceptions ''' diff --git a/qubes/tests/api_admin.py b/qubes/tests/api_admin.py index 998587ce..1a658880 100644 --- a/qubes/tests/api_admin.py +++ b/qubes/tests/api_admin.py @@ -567,13 +567,10 @@ 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), } self.app.pools['pool1'].included_in.return_value = None - 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,