From 9cc86b3be2a424e1bead99c59bbd2fd8ba19d2ec Mon Sep 17 00:00:00 2001 From: qubesuser Date: Thu, 9 Nov 2017 18:42:44 +0100 Subject: [PATCH 01/13] 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 7b09ff9fb..5eea268c4 100644 --- a/qubes/vm/mix/net.py +++ b/qubes/vm/mix/net.py @@ -123,13 +123,13 @@ def visible_ip(self): 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 02/13] 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 920527db4..33e3d03d5 100644 --- a/qubes/__init__.py +++ b/qubes/__init__.py @@ -200,6 +200,10 @@ def __init__(self, name, setter=None, saver=None, type=None, 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 @@ def get_default(self, instance): 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 03/13] 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 bdc2c904c..1c960d367 100644 --- a/qubes/app.py +++ b/qubes/app.py @@ -536,14 +536,6 @@ def get_new_unused_qid(self): 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 ded236aa0..ec6e16d27 100644 --- a/qubes/config.py +++ b/qubes/config.py @@ -100,7 +100,6 @@ } 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 3b3b7944a..ee3650efc 100644 --- a/qubes/tests/app.py +++ b/qubes/tests/app.py @@ -257,12 +257,6 @@ def test_100_get_new_unused_qid(self): 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 2b23aa0d9..a2d41de93 100644 --- a/qubes/tests/init.py +++ b/qubes/tests/init.py @@ -323,7 +323,6 @@ def test_010_property_require(self): 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 @@ def test_100_get_new_unused_qid(self): 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 43c9da4a6..faea5f33f 100644 --- a/qubes/vm/adminvm.py +++ b/qubes/vm/adminvm.py @@ -189,7 +189,7 @@ def untrusted_qdb(self): # 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 04/13] 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 33e3d03d5..c62642461 100644 --- a/qubes/__init__.py +++ b/qubes/__init__.py @@ -496,7 +496,7 @@ def __init__(self, xml, **kwargs): 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 @@ def __init__(self, xml, **kwargs): 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 @@ def __init__(self, xml, **kwargs): '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 @@ def property_list(cls, load_stage=None): :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 @@ def property_get_def(cls, prop): 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 05/13] 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 1c960d367..3da0e7886 100644 --- a/qubes/app.py +++ b/qubes/app.py @@ -545,6 +545,55 @@ def get_new_unused_dispid(self): '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 06/13] 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 3da0e7886..82f0a48e0 100644 --- a/qubes/app.py +++ b/qubes/app.py @@ -1040,10 +1040,13 @@ def load_initial_values(self): } 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) From 015ee9c9ba643519e5e296e9857f21624a3d7cf8 Mon Sep 17 00:00:00 2001 From: qubesuser Date: Thu, 9 Nov 2017 15:28:40 +0100 Subject: [PATCH 07/13] don't replace pdb debugger's SIGINT handler Otherwise qubesd will die if you try to break in the debugger --- qubes/tools/qubesd.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/qubes/tools/qubesd.py b/qubes/tools/qubesd.py index 77b748642..8ff23a3ce 100644 --- a/qubes/tools/qubesd.py +++ b/qubes/tools/qubesd.py @@ -51,9 +51,15 @@ def main(args=None): for sock in server.sockets: socknames.append(sock.getsockname()) - for signame in ('SIGINT', 'SIGTERM'): - loop.add_signal_handler(getattr(signal, signame), - sighandler, loop, signame, servers) + # don't replace pdb debugger's SIGINT handler + old_sigint = signal.getsignal(signal.SIGINT) + if (old_sigint == signal.SIG_DFL or old_sigint == signal.SIG_IGN or + old_sigint == signal.default_int_handler): + loop.add_signal_handler(signal.SIGINT, + sighandler, loop, "SIGINT", servers) + + loop.add_signal_handler(signal.SIGTERM, + sighandler, loop, "SIGTERM", servers) qubes.utils.systemd_notify() # make sure children will not inherit this From cf27e1079032f998e6a0a4a644fbbe6d83427f8e Mon Sep 17 00:00:00 2001 From: qubesuser Date: Thu, 9 Nov 2017 15:10:16 +0100 Subject: [PATCH 08/13] rework libvirt connection code - connect in a loop, starting libvirtd - reconnect on demand in a loop - register events on reconnection --- qubes/app.py | 78 ++++++++++++++++++++++++++++++++-------------------- 1 file changed, 48 insertions(+), 30 deletions(-) diff --git a/qubes/app.py b/qubes/app.py index 82f0a48e0..ac4b1e24f 100644 --- a/qubes/app.py +++ b/qubes/app.py @@ -90,27 +90,44 @@ def __getattr__(self, attrname): @functools.wraps(attr) def wrapper(*args, **kwargs): - try: - return attr(*args, **kwargs) - except libvirt.libvirtError: - if self._reconnect_if_dead(): + while True: + try: return getattr(self._vm, attrname)(*args, **kwargs) - raise + except libvirt.libvirtError: + if not self._reconnect_if_dead(): + raise return wrapper -class VirConnectWrapper(object): +class VirConnectWrapper(qubes.events.Emitter): # pylint: disable=too-few-public-methods def __init__(self, uri): - self._conn = libvirt.open(uri) + self.events_enabled = True + super(VirConnectWrapper, self).__init__() + self._conn = None + self._connect(uri) + + def _connect(self, uri): + if self._conn: + self._conn.close() + while True: + try: + self._conn = libvirt.open(uri) + self._conn.registerCloseCallback(self._close_callback, None) + break + except libvirt.libvirtError: + time.sleep(1.0) + self.fire_event("connected") def _reconnect_if_dead(self): - is_dead = not self._conn.isAlive() - if is_dead: - self._conn = libvirt.open(self._conn.getURI()) - # TODO: re-register event handlers - return is_dead + if self._conn.isAlive(): + return False + self._connect(self._conn.getURI()) + return True + + def _close_callback(self, _conn, _reason, _opaque): + self._reconnect_if_dead() def _wrap_domain(self, ret): if isinstance(ret, libvirt.virDomain): @@ -126,13 +143,13 @@ def __getattr__(self, attrname): @functools.wraps(attr) def wrapper(*args, **kwargs): - try: - return self._wrap_domain(attr(*args, **kwargs)) - except libvirt.libvirtError: - if self._reconnect_if_dead(): - return self._wrap_domain( - getattr(self._conn, attrname)(*args, **kwargs)) - raise + while True: + try: + func = getattr(self._conn, attrname) + return self._wrap_domain(func(*args, **kwargs)) + except libvirt.libvirtError: + if not self._reconnect_if_dead(): + raise return wrapper @@ -952,11 +969,6 @@ def close(self): super().close() - if self._domain_event_callback_id is not None: - self.vmm.libvirt_conn.domainEventDeregisterAny( - self._domain_event_callback_id) - self._domain_event_callback_id = None - # Only our Lord, The God Almighty, knows what references # are kept in extensions. del self._extensions @@ -1202,12 +1214,18 @@ def register_event_handlers(self): events into qubes.events. This function should be called only in 'qubesd' process and only when mainloop has been already set. ''' - self._domain_event_callback_id = ( - self.vmm.libvirt_conn.domainEventRegisterAny( - None, # any domain - libvirt.VIR_DOMAIN_EVENT_ID_LIFECYCLE, - self._domain_event_callback, - None)) + + if not self.vmm.offline_mode: + self.vmm.libvirt_conn.add_handler("connected", + self._libvirt_connected_handler) + self._libvirt_connected_handler() + + def _libvirt_connected_handler(self): + self.vmm.libvirt_conn.domainEventRegisterAny( + None, # any domain + libvirt.VIR_DOMAIN_EVENT_ID_LIFECYCLE, + self._domain_event_callback, + None) def _domain_event_callback(self, _conn, domain, event, _detail, _opaque): '''Generic libvirt event handler (virConnectDomainEventCallback), From 690a88b9c8abbd7f15e978da22f11ac1874b37cf Mon Sep 17 00:00:00 2001 From: qubesuser Date: Thu, 9 Nov 2017 15:22:33 +0100 Subject: [PATCH 09/13] store libvirt state and update on events instead of asking libvirtd every time --- qubes/app.py | 24 ++--- qubes/tests/api_admin.py | 4 +- qubes/tests/vm/appvm.py | 4 +- qubes/vm/__init__.py | 6 ++ qubes/vm/qubesvm.py | 201 ++++++++++++++++++++------------------- 5 files changed, 124 insertions(+), 115 deletions(-) diff --git a/qubes/app.py b/qubes/app.py index ac4b1e24f..30a5372c6 100644 --- a/qubes/app.py +++ b/qubes/app.py @@ -499,12 +499,13 @@ def __delitem__(self, key): if not vm.is_halted(): raise qubes.exc.QubesVMNotHaltedError(vm) self.app.fire_event('domain-pre-delete', pre_event=True, vm=vm) - try: - vm.libvirt_domain.undefine() - except libvirt.libvirtError as e: - if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN: - # already undefined - pass + if vm.libvirt_domain: + try: + vm.libvirt_domain.undefine() + except libvirt.libvirtError as e: + if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN: + # already undefined + pass del self._dict[vm.qid] self.app.fire_event('domain-delete', vm=vm) @@ -1227,21 +1228,20 @@ def _libvirt_connected_handler(self): self._domain_event_callback, None) - def _domain_event_callback(self, _conn, domain, event, _detail, _opaque): + for vm in self.domains: + vm.libvirt_connected() + + def _domain_event_callback(self, _conn, domain, event, detail, _opaque): '''Generic libvirt event handler (virConnectDomainEventCallback), translate libvirt event into qubes.events. ''' - if not self.events_enabled: - return - try: vm = self.domains[domain.name()] except KeyError: # ignore events for unknown domains return - if event == libvirt.VIR_DOMAIN_EVENT_STOPPED: - vm.fire_event('domain-shutdown') + vm.libvirt_lifecycle_event(event, detail) @qubes.events.handler('domain-pre-delete') def on_domain_pre_deleted(self, event, vm): diff --git a/qubes/tests/api_admin.py b/qubes/tests/api_admin.py index a3c4b0371..2c357a667 100644 --- a/qubes/tests/api_admin.py +++ b/qubes/tests/api_admin.py @@ -1566,7 +1566,7 @@ def test_500_vm_remove(self, mock_rmtree, mock_remove): def test_501_vm_remove_running(self, mock_rmtree, mock_remove): mock_remove.side_effect = self.dummy_coro with unittest.mock.patch.object( - self.vm, 'get_power_state', lambda: 'Running'): + self.vm, 'is_halted', lambda: False): with self.assertRaises(qubes.exc.QubesVMNotHaltedError): self.call_mgmt_func(b'admin.vm.Remove', b'test-vm1') self.assertFalse(mock_rmtree.called) @@ -1582,7 +1582,7 @@ def test_510_vm_volume_import(self): def test_511_vm_volume_import_running(self): with unittest.mock.patch.object( - self.vm, 'get_power_state', lambda: 'Running'): + self.vm, 'is_halted', lambda: False): with self.assertRaises(qubes.exc.QubesVMNotHaltedError): self.call_mgmt_func(b'admin.vm.volume.Import', b'test-vm1', b'private') diff --git a/qubes/tests/vm/appvm.py b/qubes/tests/vm/appvm.py index f3bc0979f..62b36c5a3 100644 --- a/qubes/tests/vm/appvm.py +++ b/qubes/tests/vm/appvm.py @@ -125,8 +125,8 @@ def test_002_storage_template_change(self): def test_003_template_change_running(self): vm = self.get_vm() - with mock.patch.object(vm, 'get_power_state') as mock_power: - mock_power.return_value = 'Running' + with mock.patch.object(vm, 'is_halted') as mock_power: + mock_power.return_value = False with self.assertRaises(qubes.exc.QubesVMNotHaltedError): vm.template = self.template diff --git a/qubes/vm/__init__.py b/qubes/vm/__init__.py index 1feffb354..8bce45c99 100644 --- a/qubes/vm/__init__.py +++ b/qubes/vm/__init__.py @@ -511,6 +511,12 @@ def klass(self): '''Domain class name''' return type(self).__name__ + def libvirt_connected(self): + pass + + def libvirt_lifecycle_event(self, event, detail): + pass + class VMProperty(qubes.property): '''Property that is referring to a VM diff --git a/qubes/vm/qubesvm.py b/qubes/vm/qubesvm.py index 241aaf877..97ddd4968 100644 --- a/qubes/vm/qubesvm.py +++ b/qubes/vm/qubesvm.py @@ -489,32 +489,11 @@ def xid(self): Or not Xen, but ID. ''' - if self.libvirt_domain is None: - return -1 - try: - return self.libvirt_domain.ID() - except libvirt.libvirtError as e: - if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN: - return -1 - else: - self.log.exception('libvirt error code: {!r}'.format( - e.get_error_code())) - raise + return self._xid @qubes.stateless_property def stubdom_xid(self): - if not self.is_running(): - return -1 - - if self.app.vmm.xs is None: - return -1 - - stubdom_xid_str = self.app.vmm.xs.read('', - '/local/domain/{}/image/device-model-domid'.format(self.xid)) - if stubdom_xid_str is None or not stubdom_xid_str.isdigit(): - return -1 - - return int(stubdom_xid_str) + return self._stubdom_xid @property def attached_volumes(self): @@ -539,19 +518,24 @@ def libvirt_domain(self): May be :py:obj:`None`, if libvirt knows nothing about this domain. ''' + return self._get_libvirt_domain(True) + def _get_libvirt_domain(self, update): if self._libvirt_domain is not None: return self._libvirt_domain - # XXX _update_libvirt_domain? - try: - self._libvirt_domain = self.app.vmm.libvirt_conn.lookupByUUID( - self.uuid.bytes) - except libvirt.libvirtError as e: - if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN: - self._update_libvirt_domain() - else: - raise + if not self.app.vmm.offline_mode: + # XXX _update_libvirt_domain? + try: + self._libvirt_domain = self.app.vmm.libvirt_conn.lookupByUUID( + self.uuid.bytes) + except libvirt.libvirtError as e: + if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN: + if update: + self._update_libvirt_domain() + else: + raise + return self._libvirt_domain @property @@ -642,6 +626,10 @@ def __init__(self, app, xml, volume_config=None, **kwargs): self._libvirt_domain = None self._qdb_connection = None + self._libvirt_state = libvirt.VIR_DOMAIN_SHUTOFF + self._libvirt_substate = 0 # libvirt.VIR_DOMAIN_SHUTOFF_UNKNOWN + self._xid = -1 + self._stubdom_xid = -1 if xml is None: # we are creating new VM and attributes came through kwargs @@ -695,6 +683,49 @@ def __xml__(self): return element + def libvirt_connected(self): + self._update_libvirt_state() + + def libvirt_lifecycle_event(self, event, detail): + # TODO: we could perhaps determine the new state from the event + # without asking libvirtd + self._update_libvirt_state() + + def _update_libvirt_state(self): + was_halted = self.is_halted() + + if self.libvirt_domain is not None: + try: + state = self.libvirt_domain.state() + self._libvirt_state = state[0] + self._libvirt_substate = state[1] + except libvirt.libvirtError as e: + if e.get_error_code() != libvirt.VIR_ERR_NO_DOMAIN: + self.log.exception('libvirt error code: {!r}'.format( + e.get_error_code())) + raise + + try: + self._xid = self.libvirt_domain.ID() + except libvirt.libvirtError as e: + if e.get_error_code() != libvirt.VIR_ERR_NO_DOMAIN: + self.log.exception('libvirt error code: {!r}'.format( + e.get_error_code())) + raise + + if self.is_running() and self.app.vmm.xs is not None: + stubdom_xid_str = self.app.vmm.xs.read('', + '/local/domain/{}/image/device-model-domid' + .format(self.xid)) + if stubdom_xid_str is not None and stubdom_xid_str.isdigit(): + self._stubdom_xid = int(stubdom_xid_str) + + if self.is_halted() and not was_halted: + self.fire_event('domain-shutdown') + + def _update_libvirt_state_after_op(self): + self._update_libvirt_state() + # # event handlers # @@ -710,6 +741,8 @@ def on_domain_init_loaded(self, event): if self.storage is None: self.storage = qubes.storage.Storage(self) + self._update_libvirt_state() + if not self.app.vmm.offline_mode and self.is_running(): self.start_qdb_watch() @@ -828,6 +861,7 @@ def start(self, start_guid=True, notify_function=None, self.libvirt_domain.createWithFlags( libvirt.VIR_DOMAIN_START_PAUSED) + self._update_libvirt_state() except Exception as exc: # let anyone receiving domain-pre-start know that startup failed @@ -850,6 +884,7 @@ def start(self, start_guid=True, notify_function=None, self.log.warning('Activating the {} VM'.format(self.name)) self.libvirt_domain.resume() + self._update_libvirt_state_after_op() yield from self.start_qrexec_daemon() @@ -905,6 +940,8 @@ def shutdown(self, force=False, wait=False): self.libvirt_domain.shutdown() + self._update_libvirt_state_after_op() + while wait and not self.is_halted(): yield from asyncio.sleep(0.25) @@ -923,6 +960,8 @@ def kill(self): self.libvirt_domain.destroy() + self._update_libvirt_state_after_op() + return self def force_shutdown(self, *args, **kwargs): @@ -951,6 +990,8 @@ def suspend(self): else: self.libvirt_domain.suspend() + self._update_libvirt_state_after_op() + return self @asyncio.coroutine @@ -962,6 +1003,8 @@ def pause(self): self.libvirt_domain.suspend() + self._update_libvirt_state_after_op() + return self @asyncio.coroutine @@ -975,6 +1018,9 @@ def resume(self): # pylint: disable=not-an-iterable if self.get_power_state() == "Suspended": self.libvirt_domain.pMWakeup() + + self._update_libvirt_state_after_op() + yield from self.run_service_for_stdio('qubes.SuspendPost', user='root') else: @@ -990,6 +1036,8 @@ def unpause(self): self.libvirt_domain.resume() + self._update_libvirt_state_after_op() + return self @asyncio.coroutine @@ -1444,52 +1492,24 @@ def get_power_state(self): Libvirt's enum describing precise state of a domain. ''' # pylint: disable=too-many-return-statements - # don't try to define libvirt domain, if it isn't there, VM surely - # isn't running - # reason for this "if": allow vm.is_running() in PCI (or other - # device) extension while constructing libvirt XML - if self.app.vmm.offline_mode: - return 'Halted' - if self._libvirt_domain is None: - try: - self._libvirt_domain = self.app.vmm.libvirt_conn.lookupByUUID( - self.uuid.bytes) - except libvirt.libvirtError as e: - if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN: - return 'Halted' - else: - raise - - libvirt_domain = self.libvirt_domain - if libvirt_domain is None: - return 'Halted' - - try: - if libvirt_domain.isActive(): - # pylint: disable=line-too-long - if libvirt_domain.state()[0] == libvirt.VIR_DOMAIN_PAUSED: - return "Paused" - elif libvirt_domain.state()[0] == libvirt.VIR_DOMAIN_CRASHED: - return "Crashed" - elif libvirt_domain.state()[0] == libvirt.VIR_DOMAIN_SHUTDOWN: - return "Halting" - elif libvirt_domain.state()[0] == libvirt.VIR_DOMAIN_SHUTOFF: - return "Dying" - elif libvirt_domain.state()[0] == libvirt.VIR_DOMAIN_PMSUSPENDED: # nopep8 - return "Suspended" - else: - if not self.is_fully_usable(): - return "Transient" - - return "Running" - - return 'Halted' - except libvirt.libvirtError as e: - if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN: - return 'Halted' - raise - - assert False + state = self._libvirt_state + if state == libvirt.VIR_DOMAIN_PAUSED: + return "Paused" + elif state == libvirt.VIR_DOMAIN_CRASHED: + return "Crashed" + elif state == libvirt.VIR_DOMAIN_SHUTDOWN: + return "Halting" + elif state == libvirt.VIR_DOMAIN_SHUTOFF: + return "Halted" + elif state == libvirt.VIR_DOMAIN_PMSUSPENDED: # nopep8 + return "Suspended" + elif state == libvirt.VIR_DOMAIN_RUNNING: + if not self.is_fully_usable(): + return "Transient" + + return "Running" + else: + assert False def is_halted(self): ''' Check whether this domain's state is 'Halted' @@ -1497,7 +1517,7 @@ def is_halted(self): :py:obj:`False` otherwise. :rtype: bool ''' - return self.get_power_state() == 'Halted' + return self._libvirt_state == libvirt.VIR_DOMAIN_SHUTOFF def is_running(self): '''Check whether this domain is running. @@ -1507,24 +1527,7 @@ def is_running(self): :rtype: bool ''' - if self.app.vmm.offline_mode: - return False - - # don't try to define libvirt domain, if it isn't there, VM surely - # isn't running - # reason for this "if": allow vm.is_running() in PCI (or other - # device) extension while constructing libvirt XML - if self._libvirt_domain is None: - try: - self._libvirt_domain = self.app.vmm.libvirt_conn.lookupByUUID( - self.uuid.bytes) - except libvirt.libvirtError as e: - if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN: - return False - else: - raise - - return self.libvirt_domain.isActive() + return self._libvirt_state != libvirt.VIR_DOMAIN_SHUTOFF def is_paused(self): '''Check whether this domain is paused. @@ -1534,8 +1537,7 @@ def is_paused(self): :rtype: bool ''' - return self.libvirt_domain \ - and self.libvirt_domain.state()[0] == libvirt.VIR_DOMAIN_PAUSED + return self._libvirt_state == libvirt.VIR_DOMAIN_PAUSED def is_qrexec_running(self): '''Check whether qrexec for this domain is available. @@ -1751,6 +1753,7 @@ def _update_libvirt_domain(self): try: self._libvirt_domain = self.app.vmm.libvirt_conn.defineXML( domain_config) + self._update_libvirt_state() except libvirt.libvirtError as e: if e.get_error_code() == libvirt.VIR_ERR_OS_TYPE \ and e.get_str2() == 'hvm': From 9ee73a2dbd0d1fd4ea9de54b6c3c6cb3a5fb0471 Mon Sep 17 00:00:00 2001 From: qubesuser Date: Thu, 9 Nov 2017 15:23:50 +0100 Subject: [PATCH 10/13] don't ask for domain state before asking for info info already contains domain state, so just use it, making a single libvirtd call --- qubes/vm/qubesvm.py | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/qubes/vm/qubesvm.py b/qubes/vm/qubesvm.py index 97ddd4968..3bdaa5b7e 100644 --- a/qubes/vm/qubesvm.py +++ b/qubes/vm/qubesvm.py @@ -1577,9 +1577,10 @@ def get_mem(self): return 0 try: - if not self.libvirt_domain.isActive(): + info = self.libvirt_domain.info() + if info[0] == libvirt.VIR_DOMAIN_SHUTOFF: return 0 - return self.libvirt_domain.info()[1] + return info[1] except libvirt.libvirtError as e: if e.get_error_code() in ( @@ -1632,20 +1633,16 @@ def get_cputime(self): if self.libvirt_domain is None: return 0 - if self.libvirt_domain is None: - return 0 - if not self.libvirt_domain.isActive(): - return 0 - try: - if not self.libvirt_domain.isActive(): - return 0 - # this does not work, because libvirt # return self.libvirt_domain.getCPUStats( # libvirt.VIR_NODE_CPU_STATS_ALL_CPUS, 0)[0]['cpu_time']/10**9 - return self.libvirt_domain.info()[4] + info = self.libvirt_domain.info() + if info[0] == libvirt.VIR_DOMAIN_SHUTOFF: + return 0 + + return info[4] except libvirt.libvirtError as e: if e.get_error_code() in ( From 041bfbb87d06ad41cd4ba31c7a0c9cbcba8e4489 Mon Sep 17 00:00:00 2001 From: qubesuser Date: Thu, 9 Nov 2017 15:23:17 +0100 Subject: [PATCH 11/13] take startup lock while starting services This should prevent getting "qrexec not connected" due to trying to start a service in the middle of VM startup --- qubes/vm/qubesvm.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/qubes/vm/qubesvm.py b/qubes/vm/qubesvm.py index 3bdaa5b7e..445deefeb 100644 --- a/qubes/vm/qubesvm.py +++ b/qubes/vm/qubesvm.py @@ -1074,6 +1074,10 @@ def run_service(self, service, source=None, user=None, if user is None: user = self.default_user + # wait for start to complete + with (yield from self.startup_lock): + pass + if self.is_paused(): # XXX what about autostart? raise qubes.exc.QubesVMNotRunningError( From 3ea172c31614f9617ecb824bba51ae60942a84b1 Mon Sep 17 00:00:00 2001 From: qubesuser Date: Wed, 8 Nov 2017 21:53:29 +0100 Subject: [PATCH 12/13] Add admin APIs to get all properties at once, and optimize the code This allows MUCH faster qvm-ls and qvm-prefs The code is optimized to be as fast as possible: - optimized property access - gather strings in a list and join them only at the end --- Makefile | 2 + qubes/__init__.py | 3 ++ qubes/api/admin.py | 117 ++++++++++++++++++++++++++++++++++++++------- 3 files changed, 106 insertions(+), 16 deletions(-) diff --git a/Makefile b/Makefile index 82ea9c2bd..7af2593a6 100644 --- a/Makefile +++ b/Makefile @@ -30,6 +30,7 @@ ADMIN_API_METHODS_SIMPLE = \ admin.pool.volume.Revert \ admin.pool.volume.Snapshot \ admin.property.Get \ + admin.property.GetAll \ admin.property.GetDefault \ admin.property.Help \ admin.property.HelpRst \ @@ -79,6 +80,7 @@ ADMIN_API_METHODS_SIMPLE = \ admin.vm.firewall.SetPolicy \ admin.vm.firewall.Reload \ admin.vm.property.Get \ + admin.vm.property.GetAll \ admin.vm.property.GetDefault \ admin.vm.property.Help \ admin.vm.property.HelpRst \ diff --git a/qubes/__init__.py b/qubes/__init__.py index c62642461..7cec976a7 100644 --- a/qubes/__init__.py +++ b/qubes/__init__.py @@ -227,6 +227,9 @@ def __get__(self, instance, owner): except AttributeError: return self.get_default(instance) + def has_default(self): + return self._default is not self._NO_DEFAULT + def get_default(self, instance): if self._default is self._NO_DEFAULT: raise AttributeError( diff --git a/qubes/api/admin.py b/qubes/api/admin.py index 25a542fd6..726fabbd2 100644 --- a/qubes/api/admin.py +++ b/qubes/api/admin.py @@ -82,6 +82,18 @@ def on_domain_delete(self, subject, event, vm): vm.remove_handler('*', self.vm_handler) +def escape(unescaped_string): + '''Escape a string for the Admin API''' + result = unescaped_string + # TODO: can we do this faster? + result = result.replace("\\", "\\\\") # this must be the first one + + result = result.replace("\r", "\\r") + result = result.replace("\n", "\\n") + result = result.replace("\t", "\\t") + result = result.replace("\0", "\\0") + return result + class QubesAdminAPI(qubes.api.AbstractQubesAPI): '''Implementation of Qubes Management API calls @@ -107,6 +119,15 @@ def vmclass_list(self): return ''.join('{}\n'.format(ep.name) for ep in entrypoints) + # pylint: disable=no-self-use + def _vm_line_strs(self, strs, vm): + strs.append(vm.name) + strs.append(" class=") + strs.append(vm.__class__.__name__) + strs.append(" state=") + strs.append(vm.get_power_state()) + strs.append("\n") + @qubes.api.method('admin.vm.List', no_payload=True, scope='global', read=True) @asyncio.coroutine @@ -119,11 +140,10 @@ def vm_list(self): else: domains = self.fire_event_for_filter([self.dest]) - return ''.join('{} class={} state={}\n'.format( - vm.name, - vm.__class__.__name__, - vm.get_power_state()) - for vm in sorted(domains)) + strs = [] + for vm in sorted(domains): + self._vm_line_strs(strs, vm) + return ''.join(strs) @qubes.api.method('admin.vm.property.List', no_payload=True, scope='local', read=True) @@ -154,6 +174,13 @@ def vm_property_get(self): '''Get a value of one property''' return self._property_get(self.dest) + @qubes.api.method('admin.vm.property.GetAll', no_payload=True, + scope='local', read=True) + @asyncio.coroutine + def vm_property_get_all(self): + '''Get the value of all properties''' + return self._property_get_all(self.dest) + @qubes.api.method('admin.property.Get', no_payload=True, scope='global', read=True) @asyncio.coroutine @@ -162,24 +189,39 @@ def property_get(self): assert self.dest.name == 'dom0' return self._property_get(self.app) - def _property_get(self, dest): - if self.arg not in dest.property_list(): - raise qubes.exc.QubesNoSuchPropertyError(dest, self.arg) - - self.fire_event_for_permission() + @qubes.api.method('admin.property.GetAll', no_payload=True, + scope='global', read=True) + @asyncio.coroutine + def property_get_all(self): + '''Get a value of all global properties''' + assert self.dest.name == 'dom0' + return self._property_get_all(self.app) - property_def = dest.property_get_def(self.arg) + # pylint: disable=no-self-use + def _property_type(self, property_def): # explicit list to be sure that it matches protocol spec - if isinstance(property_def, qubes.vm.VMProperty): - property_type = 'vm' - elif property_def.type is int: + property_def_type = property_def.type + if property_def_type is str: + property_type = 'str' + elif property_def_type is int: property_type = 'int' - elif property_def.type is bool: + elif property_def_type is bool: property_type = 'bool' - elif self.arg == 'label': + elif property_def.__name__ == 'label': property_type = 'label' + elif isinstance(property_def, qubes.vm.VMProperty): + property_type = 'vm' else: property_type = 'str' + return property_type + + def _property_get(self, dest): + if self.arg not in dest.property_list(): + raise qubes.exc.QubesNoSuchPropertyError(dest, self.arg) + + self.fire_event_for_permission() + property_def = dest.property_get_def(self.arg) + property_type = self._property_type(property_def) try: value = getattr(dest, self.arg) @@ -191,6 +233,49 @@ def _property_get(self, dest): property_type, str(value) if value is not None else '') + + # this is a performance critical function for qvm-ls + def _property_get_all_line_strs(self, strs, dest, property_def): + property_type = self._property_type(property_def) + property_attr = property_def._attr_name # pylint: disable=protected-access + dest_dict = dest.__dict__ + property_is_default = property_attr not in dest_dict + + if not property_is_default: + value = dest_dict[property_attr] + elif property_def.has_default(): + value = property_def.get_default(dest) + else: + value = None + + if value is None: + escaped_value = "" + elif property_type == "str": + escaped_value = escape(str(value)) + else: + escaped_value = str(value) + + strs.append(property_def.__name__) + strs.append("\tD\t" if property_is_default else "\t-\t") + strs.append(property_type) + strs.append("\t") + strs.append(escaped_value) + strs.append("\n") + + def _property_get_all_strs(self, strs, dest, prefix=None): + assert not self.arg + + properties = self.fire_event_for_filter(dest.property_list()) + for prop in properties: + if prefix: + strs.append(prefix) + self._property_get_all_line_strs(strs, dest, prop) + + def _property_get_all(self, dest): + strs = [] + self._property_get_all_strs(strs, dest) + return "".join(strs) + @qubes.api.method('admin.vm.property.GetDefault', no_payload=True, scope='local', read=True) @asyncio.coroutine From 0175ad68379895e153fe5fdf888ac656e9fff21f Mon Sep 17 00:00:00 2001 From: qubesuser Date: Fri, 10 Nov 2017 00:00:52 +0100 Subject: [PATCH 13/13] add GetAllData API to get data of all VMs Even faster qvm-ls! --- Makefile | 1 + qubes/api/admin.py | 27 +++++++++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/Makefile b/Makefile index 7af2593a6..e74662c0a 100644 --- a/Makefile +++ b/Makefile @@ -46,6 +46,7 @@ ADMIN_API_METHODS_SIMPLE = \ admin.vm.CreateInPool.StandaloneVM \ admin.vm.CreateInPool.TemplateVM \ admin.vm.CreateDisposable \ + admin.vm.GetAllData \ admin.vm.Kill \ admin.vm.List \ admin.vm.Pause \ diff --git a/qubes/api/admin.py b/qubes/api/admin.py index 726fabbd2..c550547a8 100644 --- a/qubes/api/admin.py +++ b/qubes/api/admin.py @@ -145,6 +145,33 @@ def vm_list(self): self._vm_line_strs(strs, vm) return ''.join(strs) + @qubes.api.method('admin.vm.GetAllData', no_payload=True, + scope='global', read=True) + @asyncio.coroutine + def vm_get_all_data(self): + '''List all VMs and return the value of all their properties''' + assert not self.arg + + if self.dest.name == 'dom0': + domains = self.fire_event_for_filter(self.app.domains) + else: + domains = self.fire_event_for_filter([self.dest]) + + strs = [] + orig_dest = self.dest + orig_method = self.method + try: + for vm in sorted(domains): + self.dest = vm + self.method = "admin.vm.property.GetAll" + self._vm_line_strs(strs, vm) + self._property_get_all_strs(strs, vm, "\tP\t") + finally: + self.dest = orig_dest + self.method = orig_method + + return ''.join(strs) + @qubes.api.method('admin.vm.property.List', no_payload=True, scope='local', read=True) @asyncio.coroutine