From f48b1be669b5935b3663809abd6b34610b7296b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Mon, 19 Jun 2017 17:21:44 +0200 Subject: [PATCH 1/5] storage: extract single volume clone into clone_volume This will be useful for admin.vm.volume.Clone implementation. QubesOS/qubes-issues#2256 --- qubes/storage/__init__.py | 66 +++++++++++++++++++-------------------- 1 file changed, 32 insertions(+), 34 deletions(-) diff --git a/qubes/storage/__init__.py b/qubes/storage/__init__.py index b3e86e49..f5126c5d 100644 --- a/qubes/storage/__init__.py +++ b/qubes/storage/__init__.py @@ -431,46 +431,44 @@ class Storage(object): os.umask(old_umask) @asyncio.coroutine - def clone(self, src_vm): - ''' Clone volumes from the specified vm ''' + def clone_volume(self, src_vm, name): + ''' Clone single volume from the specified vm + + :param QubesVM src_vm: source VM + :param str name: name of volume to clone ('root', 'private' etc) + :return cloned volume object + ''' + config = self.vm.volume_config[name] + dst_pool = self.vm.app.get_pool(config['pool']) + dst = dst_pool.init_volume(self.vm, config) + src_volume = src_vm.volumes[name] + src_pool = src_volume.pool + if dst_pool == src_pool: + msg = "Cloning volume {!s} from vm {!s}" + self.vm.log.info(msg.format(src_volume.name, src_vm.name)) + clone_op_ret = dst_pool.clone(src_volume, dst) + else: + msg = "Importing volume {!s} from vm {!s}" + self.vm.log.info(msg.format(src_volume.name, src_vm.name)) + clone_op_ret = dst_pool.import_volume( + dst_pool, dst, src_pool, src_volume) # clone/import functions may be either synchronous or asynchronous # in the later case, we need to wait for them to finish - clone_op = {} + if asyncio.iscoroutine(clone_op_ret): + self.vm.volumes[name] = yield from clone_op_ret + else: + self.vm.volumes[name] = clone_op_ret + return self.vm.volumes[name] + + @asyncio.coroutine + def clone(self, src_vm): + ''' Clone volumes from the specified vm ''' self.vm.volumes = {} with VmCreationManager(self.vm): - for name, config in self.vm.volume_config.items(): - dst_pool = self.vm.app.get_pool(config['pool']) - dst = dst_pool.init_volume(self.vm, config) - src_volume = src_vm.volumes[name] - src_pool = src_volume.pool - if dst_pool == src_pool: - msg = "Cloning volume {!s} from vm {!s}" - self.vm.log.info(msg.format(src_volume.name, src_vm.name)) - clone_op_ret = dst_pool.clone(src_volume, dst) - else: - msg = "Importing volume {!s} from vm {!s}" - self.vm.log.info(msg.format(src_volume.name, src_vm.name)) - clone_op_ret = dst_pool.import_volume( - dst_pool, dst, src_pool, src_volume) - if asyncio.iscoroutine(clone_op_ret): - clone_op[name] = asyncio.ensure_future(clone_op_ret) - - yield from asyncio.wait(x for x in clone_op.values() - if inspect.isawaitable(x)) - - for name, clone_op_ret in clone_op.items(): - if inspect.isawaitable(clone_op_ret): - volume = clone_op_ret.result - else: - volume = clone_op_ret - - assert volume, "%s.clone() returned '%s'" % ( - self.vm.app.get_pool(self.vm.volume_config[name]['pool']). - __class__.__name__, volume) - - self.vm.volumes[name] = volume + yield from asyncio.wait(self.clone_volume(src_vm, vol_name) + for vol_name in self.vm.volume_config.keys()) @property def outdated_volumes(self): From aadbe223c3af6322d57d7f97bea1ff6d6e4b3218 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Mon, 19 Jun 2017 17:23:17 +0200 Subject: [PATCH 2/5] admin: add admin.vm.volume.Clone QubesOS/qubes-issues#2622 --- qubes/api/admin.py | 18 ++++++++ qubes/tests/api_admin.py | 89 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+) diff --git a/qubes/api/admin.py b/qubes/api/admin.py index be596696..9172d8e4 100644 --- a/qubes/api/admin.py +++ b/qubes/api/admin.py @@ -291,6 +291,24 @@ class QubesAdminAPI(qubes.api.AbstractQubesAPI): self.dest.storage.get_pool(volume).revert(revision) self.app.save() + @qubes.api.method('admin.vm.volume.Clone') + @asyncio.coroutine + def vm_volume_clone(self, untrusted_payload): + assert self.arg in self.dest.volumes.keys() + untrusted_target = untrusted_payload.decode('ascii').strip() + del untrusted_payload + qubes.vm.validate_name(None, None, untrusted_target) + target_vm = self.app.domains[untrusted_target] + del untrusted_target + assert self.arg in target_vm.volumes.keys() + + volume = self.dest.volumes[self.arg] + + self.fire_event_for_permission(target_vm=target_vm, volume=volume) + + yield from target_vm.storage.clone_volume(self.dest, self.arg) + self.app.save() + @qubes.api.method('admin.vm.volume.Resize') @asyncio.coroutine def vm_volume_resize(self, untrusted_payload): diff --git a/qubes/tests/api_admin.py b/qubes/tests/api_admin.py index 36c3d8ac..6a18e221 100644 --- a/qubes/tests/api_admin.py +++ b/qubes/tests/api_admin.py @@ -1598,6 +1598,95 @@ class TC_00_VMs(AdminAPITestCase): self.call_mgmt_func(b'admin.vm.volume.Import', b'test-vm1', b'private') + def test_520_vm_volume_clone(self): + self.vm2 = self.app.add_new_vm('AppVM', label='red', name='test-vm2', + template='test-template') + self.vm.volumes = unittest.mock.MagicMock() + self.vm2.volumes = unittest.mock.MagicMock() + volumes_conf = { + 'keys.return_value': ['root', 'private', 'volatile', 'kernel'], + } + self.vm.volumes.configure_mock(**volumes_conf) + self.vm2.volumes.configure_mock(**volumes_conf) + self.vm2.storage = unittest.mock.Mock() + func_mock = unittest.mock.Mock() + + @asyncio.coroutine + def coroutine_mock(*args, **kwargs): + return func_mock(*args, **kwargs) + self.vm2.storage.clone_volume = coroutine_mock + + self.call_mgmt_func(b'admin.vm.volume.Clone', + b'test-vm1', b'private', b'test-vm2') + self.assertEqual(self.vm.volumes.mock_calls, + [('keys', (), {}), + ('__getitem__', ('private', ), {}), + ('__getitem__().__hash__', (), {}), + ]) + self.assertEqual(self.vm2.volumes.mock_calls, + [unittest.mock.call.keys()]) + self.assertEqual(self.vm2.storage.mock_calls, []) + self.assertEqual(func_mock.mock_calls, [ + unittest.mock.call(self.vm, 'private') + ]) + self.app.save.assert_called_once_with() + + def test_521_vm_volume_clone_invalid_volume(self): + self.vm2 = self.app.add_new_vm('AppVM', label='red', name='test-vm2', + template='test-template') + self.vm.volumes = unittest.mock.MagicMock() + self.vm2.volumes = unittest.mock.MagicMock() + volumes_conf = { + 'keys.return_value': ['root', 'private', 'volatile', 'kernel'], + } + self.vm.volumes.configure_mock(**volumes_conf) + self.vm2.volumes.configure_mock(**volumes_conf) + self.vm2.storage = unittest.mock.Mock() + func_mock = unittest.mock.Mock() + + @asyncio.coroutine + def coroutine_mock(*args, **kwargs): + return func_mock(*args, **kwargs) + self.vm2.storage.clone_volume = coroutine_mock + + with self.assertRaises(AssertionError): + self.call_mgmt_func(b'admin.vm.volume.Clone', + b'test-vm1', b'private123', b'test-vm2') + self.assertEqual(self.vm.volumes.mock_calls, + [('keys', (), {})]) + self.assertEqual(self.vm2.volumes.mock_calls, []) + self.assertEqual(self.vm2.storage.mock_calls, []) + self.assertEqual(func_mock.mock_calls, []) + self.assertFalse(self.app.save.called) + + def test_522_vm_volume_clone_invalid_vm(self): + self.vm2 = self.app.add_new_vm('AppVM', label='red', name='test-vm2', + template='test-template') + self.vm.volumes = unittest.mock.MagicMock() + self.vm2.volumes = unittest.mock.MagicMock() + volumes_conf = { + 'keys.return_value': ['root', 'private', 'volatile', 'kernel'], + } + self.vm.volumes.configure_mock(**volumes_conf) + self.vm2.volumes.configure_mock(**volumes_conf) + self.vm2.storage = unittest.mock.Mock() + func_mock = unittest.mock.Mock() + + @asyncio.coroutine + def coroutine_mock(*args, **kwargs): + return func_mock(*args, **kwargs) + self.vm2.storage.clone_volume = coroutine_mock + + with self.assertRaises(AssertionError): + self.call_mgmt_func(b'admin.vm.volume.Clone', + b'test-vm1', b'private123', b'no-such-vm') + self.assertEqual(self.vm.volumes.mock_calls, + [('keys', (), {})]) + self.assertEqual(self.vm2.volumes.mock_calls, []) + self.assertEqual(self.vm2.storage.mock_calls, []) + self.assertEqual(func_mock.mock_calls, []) + self.assertFalse(self.app.save.called) + def test_990_vm_unexpected_payload(self): methods_with_no_payload = [ b'admin.vm.List', From 4a1a5fc24b74b7c781d092bc1b674d228eb21283 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Mon, 19 Jun 2017 17:24:11 +0200 Subject: [PATCH 3/5] exc: fix QubesNoTemplateError --- qubes/exc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qubes/exc.py b/qubes/exc.py index 157f24d2..5e708c98 100644 --- a/qubes/exc.py +++ b/qubes/exc.py @@ -101,7 +101,7 @@ class QubesVMNotHaltedError(QubesVMError): class QubesNoTemplateError(QubesVMError): '''Cannot start domain, because there is no template''' def __init__(self, vm, msg=None): - super(QubesNoTemplateError, self).__init__( + super(QubesNoTemplateError, self).__init__(vm, msg or 'Template for the domain {!r} not found'.format(vm.name)) From 9242202db2c11b8c7d8e7f9fdb9dcbf2123295d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Mon, 19 Jun 2017 19:57:15 +0200 Subject: [PATCH 4/5] admin: implement admin.vm.tag.* QubesOS/qubes-issues#2622 --- qubes/api/admin.py | 43 ++++++++++++++++++++++++++++++++++++ qubes/exc.py | 9 ++++++++ qubes/tests/api_admin.py | 47 ++++++++++++++++++++++++++++++++++++++++ qubes/vm/__init__.py | 5 +++++ 4 files changed, 104 insertions(+) diff --git a/qubes/api/admin.py b/qubes/api/admin.py index 9172d8e4..859c9806 100644 --- a/qubes/api/admin.py +++ b/qubes/api/admin.py @@ -354,6 +354,49 @@ class QubesAdminAPI(qubes.api.AbstractQubesAPI): return '{} {}'.format(size, path) + @qubes.api.method('admin.vm.tag.List', no_payload=True) + @asyncio.coroutine + def vm_tag_list(self): + assert not self.arg + + tags = self.dest.tags + + tags = self.fire_event_for_filter(tags) + + return ''.join('{}\n'.format(tag) for tag in sorted(tags)) + + @qubes.api.method('admin.vm.tag.Get', no_payload=True) + @asyncio.coroutine + def vm_tag_get(self): + qubes.vm.Tags.validate_tag(self.arg) + + self.fire_event_for_permission() + + return '1' if self.arg in self.dest.tags else '0' + + @qubes.api.method('admin.vm.tag.Set', no_payload=True) + @asyncio.coroutine + def vm_tag_set(self): + qubes.vm.Tags.validate_tag(self.arg) + + self.fire_event_for_permission() + + self.dest.tags.add(self.arg) + self.app.save() + + @qubes.api.method('admin.vm.tag.Remove', no_payload=True) + @asyncio.coroutine + def vm_tag_remove(self): + qubes.vm.Tags.validate_tag(self.arg) + + self.fire_event_for_permission() + + try: + self.dest.tags.remove(self.arg) + except KeyError: + raise qubes.exc.QubesTagNotFoundError(self.dest, self.arg) + self.app.save() + @qubes.api.method('admin.pool.List', no_payload=True) @asyncio.coroutine def pool_list(self): diff --git a/qubes/exc.py b/qubes/exc.py index 5e708c98..8b3fa68c 100644 --- a/qubes/exc.py +++ b/qubes/exc.py @@ -162,3 +162,12 @@ class QubesFeatureNotFoundError(QubesException, KeyError): 'Feature not set for domain {}: {}'.format(domain, feature)) self.feature = feature self.vm = domain + +class QubesTagNotFoundError(QubesException, KeyError): + '''Tag not set for a given domain''' + + def __init__(self, domain, tag): + super().__init__('Tag not set for domain {}: {}'.format( + domain, tag)) + self.vm = domain + self.tag = tag diff --git a/qubes/tests/api_admin.py b/qubes/tests/api_admin.py index 6a18e221..7eec7619 100644 --- a/qubes/tests/api_admin.py +++ b/qubes/tests/api_admin.py @@ -1687,6 +1687,53 @@ class TC_00_VMs(AdminAPITestCase): self.assertEqual(func_mock.mock_calls, []) self.assertFalse(self.app.save.called) + def test_530_tag_list(self): + self.vm.tags.add('tag1') + self.vm.tags.add('tag2') + value = self.call_mgmt_func(b'admin.vm.tag.List', b'test-vm1') + self.assertEqual(value, 'tag1\ntag2\n') + self.assertFalse(self.app.save.called) + + def test_540_tag_get(self): + self.vm.tags.add('tag1') + value = self.call_mgmt_func(b'admin.vm.tag.Get', b'test-vm1', + b'tag1') + self.assertEqual(value, '1') + self.assertFalse(self.app.save.called) + + def test_541_tag_get_absent(self): + value = self.call_mgmt_func(b'admin.vm.tag.Get', b'test-vm1', b'tag1') + self.assertEqual(value, '0') + self.assertFalse(self.app.save.called) + + def test_550_tag_remove(self): + self.vm.tags.add('tag1') + value = self.call_mgmt_func(b'admin.vm.tag.Remove', b'test-vm1', + b'tag1') + self.assertIsNone(value, None) + self.assertNotIn('tag1', self.vm.tags) + self.assertTrue(self.app.save.called) + + def test_551_tag_remove_absent(self): + with self.assertRaises(qubes.exc.QubesTagNotFoundError): + self.call_mgmt_func(b'admin.vm.tag.Remove', + b'test-vm1', b'tag1') + self.assertFalse(self.app.save.called) + + def test_560_tag_set(self): + value = self.call_mgmt_func(b'admin.vm.tag.Set', + b'test-vm1', b'tag1') + self.assertIsNone(value) + self.assertIn('tag1', self.vm.tags) + self.assertTrue(self.app.save.called) + + def test_561_tag_set_invalid(self): + with self.assertRaises(AssertionError): + self.call_mgmt_func(b'admin.vm.tag.Set', + b'test-vm1', b'+.some-tag') + self.assertNotIn('+.some-tag', self.vm.tags) + self.assertFalse(self.app.save.called) + def test_990_vm_unexpected_payload(self): methods_with_no_payload = [ b'admin.vm.List', diff --git a/qubes/vm/__init__.py b/qubes/vm/__init__.py index 44edbf85..885bd0e2 100644 --- a/qubes/vm/__init__.py +++ b/qubes/vm/__init__.py @@ -237,6 +237,11 @@ class Tags(set): # end of overriding # + @staticmethod + def validate_tag(tag): + safe_set = string.ascii_letters + string.digits + '-_' + assert all((x in safe_set) for x in tag) + class BaseVM(qubes.PropertyHolder): '''Base class for all VMs From f976f7ec6cccdfd626c53849d7e5c4826eb7da06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Fri, 23 Jun 2017 02:35:49 +0200 Subject: [PATCH 5/5] storage: simplify coroutine handling Suggested by @woju --- qubes/storage/__init__.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/qubes/storage/__init__.py b/qubes/storage/__init__.py index f5126c5d..1f80c2bb 100644 --- a/qubes/storage/__init__.py +++ b/qubes/storage/__init__.py @@ -456,9 +456,8 @@ class Storage(object): # clone/import functions may be either synchronous or asynchronous # in the later case, we need to wait for them to finish if asyncio.iscoroutine(clone_op_ret): - self.vm.volumes[name] = yield from clone_op_ret - else: - self.vm.volumes[name] = clone_op_ret + clone_op_ret = yield from clone_op_ret + self.vm.volumes[name] = clone_op_ret return self.vm.volumes[name] @asyncio.coroutine