From 9cc86b3be2a424e1bead99c59bbd2fd8ba19d2ec Mon Sep 17 00:00:00 2001 From: qubesuser Date: Thu, 9 Nov 2017 18:42:44 +0100 Subject: [PATCH 1/6] don't access netvm if it's None in visible_gateway/netmask Causes an unnecessary exception --- qubes/vm/mix/net.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/qubes/vm/mix/net.py b/qubes/vm/mix/net.py index 7b09ff9f..5eea268c 100644 --- a/qubes/vm/mix/net.py +++ b/qubes/vm/mix/net.py @@ -123,13 +123,13 @@ class NetVMMixin(qubes.events.Emitter): def visible_gateway(self): '''Default gateway of this domain as seen by the domain.''' return self.features.check_with_template('net.fake-gateway', None) or \ - self.netvm.gateway + (self.netvm.gateway if self.netvm else None) @qubes.stateless_property def visible_netmask(self): '''Netmask as seen by the domain.''' return self.features.check_with_template('net.fake-netmask', None) or \ - self.netvm.netmask + (self.netvm.netmask if self.netvm else None) # # used in netvms (provides_network=True) From b297ecbcf156254fde8f21d2df3edd409c055da4 Mon Sep 17 00:00:00 2001 From: qubesuser Date: Thu, 9 Nov 2017 18:17:58 +0100 Subject: [PATCH 2/6] cache isinstance(default, collections.Callable) --- qubes/__init__.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/qubes/__init__.py b/qubes/__init__.py index 920527db..33e3d03d 100644 --- a/qubes/__init__.py +++ b/qubes/__init__.py @@ -200,6 +200,10 @@ class property(object): # pylint: disable=redefined-builtin,invalid-name lambda self, prop, value: str(value)) self.type = type self._default = default + self._default_function = None + if isinstance(default, collections.Callable): + self._default_function = default + self._write_once = write_once self.order = order self.load_stage = load_stage @@ -227,8 +231,8 @@ class property(object): # pylint: disable=redefined-builtin,invalid-name if self._default is self._NO_DEFAULT: raise AttributeError( 'property {!r} have no default'.format(self.__name__)) - elif isinstance(self._default, collections.Callable): - return self._default(instance) + elif self._default_function: + return self._default_function(instance) else: return self._default From f2b8ad7d38ab17447f588cb9f46e11205e9aa87a Mon Sep 17 00:00:00 2001 From: qubesuser Date: Sat, 11 Nov 2017 02:36:17 +0100 Subject: [PATCH 3/6] remove unused netid code it's unused and has a netid property with name different than key that would cause issues in the next commit --- qubes/app.py | 8 -------- qubes/config.py | 1 - qubes/tests/app.py | 6 ------ qubes/tests/init.py | 7 ------- qubes/vm/adminvm.py | 2 +- 5 files changed, 1 insertion(+), 23 deletions(-) diff --git a/qubes/app.py b/qubes/app.py index bdc2c904..1c960d36 100644 --- a/qubes/app.py +++ b/qubes/app.py @@ -536,14 +536,6 @@ class VMCollection(object): raise LookupError("Cannot find unused qid!") - def get_new_unused_netid(self): - used_ids = set([vm.netid for vm in self]) # if vm.is_netvm()]) - for i in range(1, qubes.config.max_netid): - if i not in used_ids: - return i - raise LookupError("Cannot find unused netid!") - - def get_new_unused_dispid(self): for _ in range(int(qubes.config.max_dispid ** 0.5)): dispid = random.SystemRandom().randrange(qubes.config.max_dispid) diff --git a/qubes/config.py b/qubes/config.py index ded236aa..ec6e16d2 100644 --- a/qubes/config.py +++ b/qubes/config.py @@ -100,7 +100,6 @@ defaults = { } max_qid = 254 -max_netid = 254 max_dispid = 10000 #: built-in standard labels, if creating new one, allocate them above this # number, at least until label index is removed from API diff --git a/qubes/tests/app.py b/qubes/tests/app.py index 3b3b7944..ee3650ef 100644 --- a/qubes/tests/app.py +++ b/qubes/tests/app.py @@ -257,12 +257,6 @@ class TC_30_VMCollection(qubes.tests.QubesTestCase): self.vms.get_new_unused_qid() - def test_101_get_new_unused_netid(self): - self.vms.add(self.testvm1) - self.vms.add(self.testvm2) - - self.vms.get_new_unused_netid() - # def test_200_get_vms_based_on(self): # pass diff --git a/qubes/tests/init.py b/qubes/tests/init.py index 2b23aa0d..a2d41de9 100644 --- a/qubes/tests/init.py +++ b/qubes/tests/init.py @@ -323,7 +323,6 @@ class TC_20_PropertyHolder(qubes.tests.QubesTestCase): class TestVM(qubes.vm.BaseVM): qid = qubes.property('qid', type=int) name = qubes.property('name') - netid = qid uuid = uuid.uuid5(uuid.NAMESPACE_DNS, 'testvm') def __lt__(self, other): @@ -452,12 +451,6 @@ class TC_30_VMCollection(qubes.tests.QubesTestCase): self.vms.get_new_unused_qid() - def test_101_get_new_unused_netid(self): - self.vms.add(self.testvm1) - self.vms.add(self.testvm2) - - self.vms.get_new_unused_netid() - # def test_200_get_vms_based_on(self): # pass diff --git a/qubes/vm/adminvm.py b/qubes/vm/adminvm.py index 43c9da4a..faea5f33 100644 --- a/qubes/vm/adminvm.py +++ b/qubes/vm/adminvm.py @@ -189,7 +189,7 @@ class AdminVM(qubes.vm.BaseVM): # def __init__(self, **kwargs): -# super(QubesAdminVm, self).__init__(qid=0, name="dom0", netid=0, +# super(QubesAdminVm, self).__init__(qid=0, name="dom0", # dir_path=None, # private_img = None, # template = None, From d183ab1c183581e42223399f3644d27186900dd6 Mon Sep 17 00:00:00 2001 From: qubesuser Date: Thu, 9 Nov 2017 03:15:37 +0100 Subject: [PATCH 4/6] cache PropertyHolder.property_list and use O(1) property name lookups --- qubes/__init__.py | 60 ++++++++++++++++++++++++++++++++++++----------- 1 file changed, 46 insertions(+), 14 deletions(-) diff --git a/qubes/__init__.py b/qubes/__init__.py index 33e3d03d..c6264246 100644 --- a/qubes/__init__.py +++ b/qubes/__init__.py @@ -496,7 +496,7 @@ class PropertyHolder(qubes.events.Emitter): propvalues = {} - all_names = set(prop.__name__ for prop in self.property_list()) + all_names = self.property_dict() for key in list(kwargs): if not key in all_names: continue @@ -509,8 +509,6 @@ class PropertyHolder(qubes.events.Emitter): if self.xml is not None: # check if properties are appropriate - all_names = set(prop.__name__ for prop in self.property_list()) - for node in self.xml.xpath('./properties/property'): name = node.get('name') if name not in all_names: @@ -518,6 +516,39 @@ class PropertyHolder(qubes.events.Emitter): 'property {!r} not applicable to {!r}'.format( name, self.__class__.__name__)) + # pylint: disable=too-many-nested-blocks + @classmethod + def property_dict(cls, load_stage=None): + '''List all properties attached to this VM's class + + :param load_stage: Filter by load stage + :type load_stage: :py:func:`int` or :py:obj:`None` + ''' + + # use cls.__dict__ since we must not look at parent classes + if "_property_dict" not in cls.__dict__: + cls._property_dict = {} + memo = cls._property_dict + + if load_stage not in memo: + props = dict() + if load_stage is None: + for class_ in cls.__mro__: + for name in class_.__dict__: + # don't overwrite props with those from base classes + if name not in props: + prop = class_.__dict__[name] + if isinstance(prop, property): + assert name == prop.__name__ + props[name] = prop + else: + for prop in cls.property_dict().values(): + if prop.load_stage == load_stage: + props[prop.__name__] = prop + memo[load_stage] = props + + return memo[load_stage] + @classmethod def property_list(cls, load_stage=None): '''List all properties attached to this VM's class @@ -526,14 +557,15 @@ class PropertyHolder(qubes.events.Emitter): :type load_stage: :py:func:`int` or :py:obj:`None` ''' - props = set() - for class_ in cls.__mro__: - props.update(prop for prop in class_.__dict__.values() - if isinstance(prop, property)) - if load_stage is not None: - props = set(prop for prop in props - if prop.load_stage == load_stage) - return sorted(props) + # use cls.__dict__ since we must not look at parent classes + if "_property_list" not in cls.__dict__: + cls._property_list = {} + memo = cls._property_list + + if load_stage not in memo: + memo[load_stage] = sorted(cls.property_dict(load_stage).values()) + + return memo[load_stage] def _property_init(self, prop, value): '''Initialise property to a given value, without side effects. @@ -588,9 +620,9 @@ class PropertyHolder(qubes.events.Emitter): if isinstance(prop, qubes.property): return prop - for p in cls.property_list(): - if p.__name__ == prop: - return p + props = cls.property_dict() + if prop in props: + return props[prop] raise AttributeError('No property {!r} found in {!r}'.format( prop, cls)) From 92c452a6557727df162a4d796e8239b91a1e92ac Mon Sep 17 00:00:00 2001 From: qubesuser Date: Fri, 10 Nov 2017 02:40:38 +0100 Subject: [PATCH 5/6] change default pool code to be fast currently it takes 100ms+ to determine the default pool every time, including subprocess spawning (!), which is unacceptable since finding out the default value of properties should be instantaneous instead of checking every thin pool to see if the root device is in it, find and cache the name of the root device thin pool and see if there is a configured pool with that name --- qubes/app.py | 73 ++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 59 insertions(+), 14 deletions(-) diff --git a/qubes/app.py b/qubes/app.py index 1c960d36..3da0e788 100644 --- a/qubes/app.py +++ b/qubes/app.py @@ -545,6 +545,55 @@ 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. @@ -556,20 +605,16 @@ def _default_pool(app): 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']) - thin_volumes = thin_volumes.decode() - if any([str(root_major), str(root_minor)] == thin_vol.split() - for thin_vol in thin_volumes.splitlines()): - return pool + root_volume_group = RootThinPool.volume_group() + root_thin_pool = RootThinPool.thin_pool() + if root_thin_pool: + for pool in app.pools.values(): + if pool.config.get('driver', None) != 'lvm_thin': + continue + if (pool.config['volume_group'] == root_volume_group and + pool.config['thin_pool'] == root_thin_pool): + return pool + # not a thin volume? look for file pools for pool in app.pools.values(): if pool.config.get('driver', None) != 'file': From edae7a16b96702adb2e3c531ea563a23deda2ea8 Mon Sep 17 00:00:00 2001 From: qubesuser Date: Fri, 10 Nov 2017 19:55:12 +0100 Subject: [PATCH 6/6] create "lvm" pool using rootfs thin pool instead of hardcoding qubes_dom0-pool00 --- qubes/app.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/qubes/app.py b/qubes/app.py index 3da0e788..82f0a48e 100644 --- a/qubes/app.py +++ b/qubes/app.py @@ -1040,10 +1040,13 @@ class Qubes(qubes.PropertyHolder): } assert max(self.labels.keys()) == qubes.config.max_default_label - # check if the default LVM Thin pool qubes_dom0/pool00 exists - if os.path.exists('/dev/mapper/qubes_dom0-pool00-tpool'): - self.add_pool(volume_group='qubes_dom0', thin_pool='pool00', - name='lvm', driver='lvm_thin') + root_volume_group = RootThinPool.volume_group() + root_thin_pool = RootThinPool.thin_pool() + + if root_thin_pool: + self.add_pool( + volume_group=root_volume_group, thin_pool=root_thin_pool, + name='lvm', driver='lvm_thin') # pool based on /var/lib/qubes will be created here: for name, config in qubes.config.defaults['pool_configs'].items(): self.pools[name] = self._get_pool(**config)