diff --git a/Makefile b/Makefile index 64492ec4a..eaf706b2a 100644 --- a/Makefile +++ b/Makefile @@ -35,7 +35,6 @@ ADMIN_API_METHODS_SIMPLE = \ admin.property.List \ admin.property.Reset \ admin.property.Set \ - admin.vm.Clone \ admin.vm.Create.AppVM \ admin.vm.Create.DispVM \ admin.vm.Create.StandaloneVM \ diff --git a/qubes/api/admin.py b/qubes/api/admin.py index 108e45557..6fab01668 100644 --- a/qubes/api/admin.py +++ b/qubes/api/admin.py @@ -293,22 +293,57 @@ def vm_volume_revert(self, untrusted_payload): self.dest.storage.get_pool(volume).revert(revision) self.app.save() - @qubes.api.method('admin.vm.volume.Clone') + @qubes.api.method('admin.vm.volume.CloneFrom', no_payload=True) @asyncio.coroutine - def vm_volume_clone(self, untrusted_payload): + def vm_volume_clone_from(self): 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) + self.fire_event_for_permission(volume=volume) + + token = qubes.utils.random_string(32) + # save token on self.app, as self is not persistent + if not hasattr(self.app, 'api_admin_pending_clone'): + self.app.api_admin_pending_clone = {} + # don't handle collisions any better - if someone is so much out of + # luck, can try again anyway + assert token not in self.app.api_admin_pending_clone + + self.app.api_admin_pending_clone[token] = volume + return token + + @qubes.api.method('admin.vm.volume.CloneTo') + @asyncio.coroutine + def vm_volume_clone_to(self, untrusted_payload): + assert self.arg in self.dest.volumes.keys() + untrusted_token = untrusted_payload.decode('ascii').strip() + del untrusted_payload + assert untrusted_token in getattr(self.app, + 'api_admin_pending_clone', {}) + token = untrusted_token + del untrusted_token + + src_volume = self.app.api_admin_pending_clone[token] + del self.app.api_admin_pending_clone[token] + + # make sure the volume still exists, but invalidate token anyway + assert str(src_volume.pool) in self.app.pools + assert src_volume in self.app.pools[str(src_volume.pool)].volumes + + dst_volume = self.dest.volumes[self.arg] + + self.fire_event_for_permission(src_volume=src_volume, + dst_volume=dst_volume) - yield from target_vm.storage.clone_volume(self.dest, self.arg) + op_retval = dst_volume.import_volume(src_volume) + + # clone/import functions may be either synchronous or asynchronous + # in the later case, we need to wait for them to finish + if asyncio.iscoroutine(op_retval): + op_retval = yield from op_retval + + self.dest.volumes[self.arg] = op_retval self.app.save() @qubes.api.method('admin.vm.volume.Resize') @@ -829,40 +864,6 @@ def vm_remove(self): self.app.save() - @qubes.api.method('admin.vm.Clone') - @asyncio.coroutine - def vm_clone(self, untrusted_payload): - assert not self.arg - - assert untrusted_payload.startswith(b'name=') - untrusted_name = untrusted_payload[5:].decode('ascii') - qubes.vm.validate_name(None, None, untrusted_name) - new_name = untrusted_name - - del untrusted_payload - - if new_name in self.app.domains: - raise qubes.exc.QubesValueError('Already exists') - - self.fire_event_for_permission(new_name=new_name) - - src_vm = self.dest - - dst_vm = self.app.add_new_vm(src_vm.__class__, name=new_name) - try: - dst_vm.clone_properties(src_vm) - dst_vm.tags.update(src_vm.tags) - dst_vm.features.update(src_vm.features) - dst_vm.firewall.clone(src_vm.firewall) - for devclass in src_vm.devices: - for device_assignment in src_vm.devices[devclass].assignments(): - dst_vm.devices[devclass].attach(device_assignment.clone()) - yield from dst_vm.clone_disk_files(src_vm) - except: - del self.app.domains[dst_vm] - raise - self.app.save() - @qubes.api.method('admin.vm.device.{endpoint}.Available', endpoints=(ep.name for ep in pkg_resources.iter_entry_points('qubes.devices')), no_payload=True) diff --git a/qubes/storage/__init__.py b/qubes/storage/__init__.py index 1f80c2bb4..22123b81d 100644 --- a/qubes/storage/__init__.py +++ b/qubes/storage/__init__.py @@ -146,6 +146,12 @@ def create(self): ''' raise self._not_implemented("create") + def remove(self): + ''' Remove volume. + + This can be implemented as a coroutine.''' + raise self._not_implemented("remove") + def commit(self): ''' Write the snapshot to disk @@ -171,7 +177,11 @@ def import_data_end(self, success): def import_volume(self, src_volume): ''' Imports data from a different volume (possibly in a different - pool ''' + pool. + + The needs to be create()d first. + + This can be implemented as a coroutine. ''' # pylint: disable=unused-argument raise self._not_implemented("import_volume") @@ -442,16 +452,9 @@ def clone_volume(self, src_vm, 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) + msg = "Importing volume {!s} from vm {!s}" + self.vm.log.info(msg.format(src_volume.name, src_vm.name)) + clone_op_ret = dst.import_volume(src_volume) # clone/import functions may be either synchronous or asynchronous # in the later case, we need to wait for them to finish @@ -520,7 +523,7 @@ def remove(self): for name, volume in self.vm.volumes.items(): self.log.info('Removing volume %s: %s' % (name, volume.vid)) try: - ret = volume.pool.remove(volume) + ret = volume.remove() if asyncio.iscoroutine(ret): futures.append(ret) except (IOError, OSError) as e: @@ -614,6 +617,58 @@ def import_data_end(self, volume, success): return self.vm.volumes[volume].import_data_end(success=success) +class VolumesCollection(object): + '''Convenient collection wrapper for pool.get_volume and + pool.list_volumes + ''' + def __init__(self, pool): + self._pool = pool + + def __getitem__(self, item): + ''' Get a single volume with given Volume ID. + + You can also a Volume instance to get the same Volume or KeyError if + Volume no longer exists. + + :param item: a Volume ID (str) or a Volume instance + ''' + if isinstance(item, Volume): + if item.pool == self._pool: + return self[item.vid] + else: + raise KeyError(item) + try: + return self._pool.get_volume(item) + except NotImplementedError: + for vol in self: + if vol.vid == item: + return vol + # if list_volumes is not implemented too, it will raise + # NotImplementedError again earlier + raise KeyError(item) + + def __iter__(self): + ''' Get iterator over pool's volumes ''' + return iter(self._pool.list_volumes()) + + def __contains__(self, item): + ''' Check if given volume (either Volume ID or Volume instance) is + present in the pool + ''' + try: + return self[item] is not None + except KeyError: + return False + + def keys(self): + ''' Return list of volume IDs ''' + return [vol.vid for vol in self] + + def values(self): + ''' Return list of Volumes''' + return [vol for vol in self] + + class Pool(object): ''' A Pool is used to manage different kind of volumes (File based/LVM/Btrfs/...). @@ -626,6 +681,7 @@ class Pool(object): def __init__(self, name, revisions_to_keep=1, **kwargs): super(Pool, self).__init__(**kwargs) + self._volumes_collection = VolumesCollection(self) self.name = name self.revisions_to_keep = revisions_to_keep kwargs['name'] = self.name @@ -666,12 +722,6 @@ def init_volume(self, vm, volume_config): ''' raise self._not_implemented("init_volume") - def remove(self, volume): - ''' Remove volume. - - This can be implemented as a coroutine.''' - raise self._not_implemented("remove") - def rename(self, volume, old_name, new_name): ''' Called when the domain changes its name ''' raise self._not_implemented("rename") @@ -684,8 +734,19 @@ def setup(self): @property def volumes(self): + ''' Return a collection of volumes managed by this pool ''' + return self._volumes_collection + + def list_volumes(self): ''' Return a list of volumes managed by this pool ''' - raise self._not_implemented("volumes") + raise self._not_implemented("list_volumes") + + def get_volume(self, vid): + ''' Return a volume with *vid* from this pool + + :raise KeyError: if no volume is found + ''' + raise self._not_implemented("get_volume") def _not_implemented(self, method_name): ''' Helper for emitting helpful `NotImplementedError` exceptions ''' @@ -740,8 +801,7 @@ def __exit__(self, type, value, tb): # pylint: disable=redefined-builtin if type is not None and value is not None and tb is not None: for volume in self.vm.volumes.values(): try: - pool = volume.pool - pool.remove(volume) + volume.remove() except Exception: # pylint: disable=broad-except pass os.rmdir(self.vm.dir_path) diff --git a/qubes/storage/file.py b/qubes/storage/file.py index dd2165082..5b3522545 100644 --- a/qubes/storage/file.py +++ b/qubes/storage/file.py @@ -79,16 +79,6 @@ def init_volume(self, vm, volume_config): self._volumes += [volume] return volume - def remove(self, volume): - if not volume.internal: - return # do not remove random attached file volumes - elif volume._is_snapshot: - return # no need to remove, because it's just a snapshot - else: - _remove_if_exists(volume.path) - if volume._is_origin: - _remove_if_exists(volume.path_cow) - def rename(self, volume, old_name, new_name): assert issubclass(volume.__class__, FileVolume) subdir, _, volume_path = volume.vid.split('/', 2) @@ -152,8 +142,7 @@ def target_dir(self, vm): return os.path.join(self.dir_path, self._vid_prefix(vm)) - @property - def volumes(self): + def list_volumes(self): return self._volumes @@ -202,6 +191,16 @@ def create(self): else: create_sparse_file(self.path, self.size) + def remove(self): + if not self.internal: + return # do not remove random attached file volumes + elif self._is_snapshot: + return # no need to remove, because it's just a snapshot + else: + _remove_if_exists(self.path) + if self._is_origin: + _remove_if_exists(self.path_cow) + def is_dirty(self): return False # TODO: How to implement this? @@ -262,6 +261,7 @@ def import_volume(self, src_volume): msg = msg.format(src_volume, self) assert not src_volume.snap_on_start, msg if self.save_on_stop: + _remove_if_exists(self.path) copy_file(src_volume.export(), self.path) return self diff --git a/qubes/storage/kernels.py b/qubes/storage/kernels.py index 3637dea5a..1f965e92e 100644 --- a/qubes/storage/kernels.py +++ b/qubes/storage/kernels.py @@ -93,6 +93,9 @@ def clone(self, source): def create(self): return self + def remove(self): + pass + def commit(self): return self @@ -154,9 +157,6 @@ def destroy(self): def import_volume(self, dst_pool, dst_volume, src_pool, src_volume): pass - def remove(self, volume): - pass - def rename(self, volume, old_name, new_name): return volume diff --git a/qubes/storage/lvm.py b/qubes/storage/lvm.py index 37ae08379..77f94d0c8 100644 --- a/qubes/storage/lvm.py +++ b/qubes/storage/lvm.py @@ -88,18 +88,6 @@ def init_volume(self, vm, volume_config): volume_config['pool'] = self return ThinVolume(**volume_config) - def remove(self, volume): - assert volume.vid - if volume.is_dirty(): - cmd = ['remove', volume._vid_snap] - qubes_lvm(cmd, self.log) - - if not os.path.exists(volume.path): - return - cmd = ['remove', volume.vid] - qubes_lvm(cmd, self.log) - reset_cache() - def rename(self, volume, old_name, new_name): ''' Called when the domain changes its name ''' new_vid = "{!s}/vm-{!s}-{!s}".format(self.volume_group, new_name, @@ -120,8 +108,7 @@ def rename(self, volume, old_name, new_name): def setup(self): pass # TODO Should we create a non existing pool? - @property - def volumes(self): + def list_volumes(self): ''' Return a list of volumes managed by this pool ''' volumes = [] for vid, vol_info in size_cache.items(): @@ -133,7 +120,7 @@ def volumes(self): # implementation detail volume continue config = { - 'pool': self.name, + 'pool': self, 'vid': vid, 'name': vid, 'volume_group': self.volume_group, @@ -297,6 +284,18 @@ def create(self): reset_cache() return self + def remove(self): + assert self.vid + if self.is_dirty(): + cmd = ['remove', self._vid_snap] + qubes_lvm(cmd, self.log) + + if not os.path.exists(self.path): + return + cmd = ['remove', self.vid] + qubes_lvm(cmd, self.log) + reset_cache() + def export(self): ''' Returns an object that can be `open()`. ''' devpath = '/dev/' + self.vid @@ -306,18 +305,17 @@ def import_volume(self, src_volume): if not src_volume.save_on_stop: return self - src_path = src_volume.export() - # HACK: neat trick to speed up testing if you have same physical thin # pool assigned to two qubes-pools i.e: qubes_dom0 and test-lvm # pylint: disable=line-too-long if isinstance(src_volume.pool, ThinPool) and \ src_volume.pool.thin_pool == self.pool.thin_pool: # NOQA + cmd = ['remove', self.vid] + qubes_lvm(cmd, self.log) cmd = ['clone', str(src_volume), str(self)] qubes_lvm(cmd, self.log) else: - self.create() - + src_path = src_volume.export() cmd = ['sudo', 'dd', 'if=' + src_path, 'of=/dev/' + self.vid, 'conv=sparse'] subprocess.check_call(cmd) diff --git a/qubes/tests/api_admin.py b/qubes/tests/api_admin.py index 7eec7619f..8a33eae97 100644 --- a/qubes/tests/api_admin.py +++ b/qubes/tests/api_admin.py @@ -21,6 +21,7 @@ ''' Tests for management calls endpoints ''' import asyncio +import operator import os import shutil import unittest.mock @@ -31,6 +32,7 @@ import qubes.devices import qubes.api.admin import qubes.tests +import qubes.storage # properties defined in API volume_properties = [ @@ -1236,72 +1238,6 @@ def test_346_vm_create_in_pool_duplicate_pool(self, storage_mock): self.assertNotIn('test-vm2', self.app.domains) self.assertFalse(self.app.save.called) - @unittest.mock.patch('qubes.storage.Storage.clone') - @unittest.mock.patch('qubes.storage.Storage.verify') - def test_350_vm_clone(self, mock_verify, mock_clone): - mock_clone.side_effect = self.dummy_coro - mock_verify.side_effect = self.dummy_coro - self.call_mgmt_func(b'admin.vm.Clone', - b'test-vm1', b'', b'name=test-vm2') - - self.assertIn('test-vm2', self.app.domains) - vm = self.app.domains['test-vm2'] - self.assertEqual(vm.label, self.app.get_label('red')) - self.assertEqual(vm.template, self.app.domains['test-template']) - self.assertEqual(vm.tags, self.vm.tags) - self.assertEqual(vm.features, self.vm.features) - self.assertEqual(vm.firewall, self.vm.firewall) - self.assertEqual(mock_clone.mock_calls, - [unittest.mock.call(self.app.domains['test-vm2']).clone( - self.app.domains['test-vm1'])]) - self.assertTrue(os.path.exists(os.path.join( - self.test_base_dir, 'appvms', 'test-vm2'))) - - self.assertTrue(self.app.save.called) - - @unittest.mock.patch('qubes.storage.Storage.clone') - @unittest.mock.patch('qubes.storage.Storage.verify') - def test_351_vm_clone_extra_params(self, mock_verify, mock_clone): - mock_clone.side_effect = self.dummy_coro - mock_verify.side_effect = self.dummy_coro - with self.assertRaises(qubes.exc.QubesException): - self.call_mgmt_func(b'admin.vm.Clone', - b'test-vm1', b'', b'name=test-vm2 label=red') - - self.assertNotIn('test-vm2', self.app.domains) - self.assertEqual(mock_clone.mock_calls, []) - self.assertFalse(os.path.exists(os.path.join( - self.test_base_dir, 'appvms', 'test-vm2'))) - - self.assertFalse(self.app.save.called) - - @unittest.mock.patch('qubes.storage.Storage.clone') - @unittest.mock.patch('qubes.storage.Storage.verify') - def test_352_vm_clone_duplicate_name(self, mock_verify, mock_clone): - mock_clone.side_effect = self.dummy_coro - mock_verify.side_effect = self.dummy_coro - with self.assertRaises(qubes.exc.QubesException): - self.call_mgmt_func(b'admin.vm.Clone', - b'test-vm1', b'', b'name=test-vm1') - - self.assertFalse(self.app.save.called) - - @unittest.mock.patch('qubes.storage.Storage.clone') - @unittest.mock.patch('qubes.storage.Storage.verify') - def test_353_vm_clone_invalid_name(self, mock_verify, mock_clone): - mock_clone.side_effect = self.dummy_coro - mock_verify.side_effect = self.dummy_coro - with self.assertRaises(qubes.exc.QubesException): - self.call_mgmt_func(b'admin.vm.Clone', - b'test-vm1', b'', b'name=test-vm2/..') - - self.assertNotIn('test-vm2/..', self.app.domains) - self.assertEqual(mock_clone.mock_calls, []) - self.assertFalse(os.path.exists(os.path.join( - self.test_base_dir, 'appvms', 'test-vm2/..'))) - - self.assertFalse(self.app.save.called) - def test_400_property_list(self): # actual function tested for admin.vm.property.* already @@ -1598,93 +1534,100 @@ def test_511_vm_volume_import_running(self): 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 + def setup_for_clone(self): + self.pool = unittest.mock.MagicMock() + self.app.pools['test'] = self.pool + self.vm2 = self.app.add_new_vm('AppVM', label='red', + name='test-vm2', + template='test-template', kernel='') + self.pool.configure_mock(**{ + 'volumes': qubes.storage.VolumesCollection(self.pool), + 'init_volume.return_value.pool': self.pool, + '__str__.return_value': 'test', + 'get_volume.side_effect': (lambda vid: + self.vm.volumes['private'] + if vid is self.vm.volumes['private'].vid + else self.vm2.volumes['private'] + ), + }) + self.loop.run_until_complete( + self.vm.create_on_disk(pool='test')) + self.loop.run_until_complete( + self.vm2.create_on_disk(pool='test')) + + # the call replaces self.vm.volumes[...] with result of import + # operation - make sure it stays as the same object + self.vm.volumes['private'].import_volume.return_value = \ + self.vm.volumes['private'] + self.vm2.volumes['private'].import_volume.return_value = \ + self.vm2.volumes['private'] - 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') - ]) + def test_520_vm_volume_clone(self): + self.setup_for_clone() + token = self.call_mgmt_func(b'admin.vm.volume.CloneFrom', + b'test-vm1', b'private', b'') + # token + self.assertEqual(len(token), 32) + self.assertFalse(self.app.save.called) + value = self.call_mgmt_func(b'admin.vm.volume.CloneTo', + b'test-vm2', b'private', token.encode()) + self.assertIsNone(value) + self.vm2.volumes['private'].import_volume.assert_called_once_with( + self.vm.volumes['private'] + ) + self.vm2.volumes['private'].import_volume.assert_called_once_with( + self.vm2.volumes['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 + self.setup_for_clone() 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.call_mgmt_func(b'admin.vm.volume.CloneFrom', + b'test-vm1', b'private123', None) + self.assertNotIn('init_volume().import_volume', + map(operator.itemgetter(0), self.pool.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() + def test_522_vm_volume_clone_invalid_volume2(self): + self.setup_for_clone() - @asyncio.coroutine - def coroutine_mock(*args, **kwargs): - return func_mock(*args, **kwargs) - self.vm2.storage.clone_volume = coroutine_mock + token = self.call_mgmt_func(b'admin.vm.volume.CloneFrom', + b'test-vm1', b'private', b'') + with self.assertRaises(AssertionError): + self.call_mgmt_func(b'admin.vm.volume.CloneTo', + b'test-vm1', b'private123', token.encode()) + self.assertNotIn('init_volume().import_volume', + map(operator.itemgetter(0), self.pool.mock_calls)) + self.assertFalse(self.app.save.called) + + def test_523_vm_volume_clone_removed_volume(self): + self.setup_for_clone() + + token = self.call_mgmt_func(b'admin.vm.volume.CloneFrom', + b'test-vm1', b'private', b'') + def get_volume(vid): + if vid == self.vm.volumes['private']: + raise KeyError(vid) + else: + return unittest.mock.DEFAULT + self.pool.get_volume.side_effect = get_volume + with self.assertRaises(AssertionError): + self.call_mgmt_func(b'admin.vm.volume.CloneTo', + b'test-vm1', b'private', token.encode()) + self.assertNotIn('init_volume().import_volume', + map(operator.itemgetter(0), self.pool.mock_calls)) + self.assertFalse(self.app.save.called) + + def test_524_vm_volume_clone_invlid_token(self): + self.setup_for_clone() 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.call_mgmt_func(b'admin.vm.volume.CloneTo', + b'test-vm1', b'private', b'no-such-token') + self.assertNotIn('init_volume().import_volume', + map(operator.itemgetter(0), self.pool.mock_calls)) self.assertFalse(self.app.save.called) def test_530_tag_list(self): diff --git a/qubes/tests/storage_lvm.py b/qubes/tests/storage_lvm.py index 4d1d90cee..1426e9323 100644 --- a/qubes/tests/storage_lvm.py +++ b/qubes/tests/storage_lvm.py @@ -137,7 +137,7 @@ def test_001_origin_volume(self): volume.create() path = "/dev/%s" % volume.vid self.assertTrue(os.path.exists(path)) - self.pool.remove(volume) + volume.remove() def test_003_read_write_volume(self): ''' Test read-write volume creation ''' @@ -157,7 +157,7 @@ def test_003_read_write_volume(self): volume.create() path = "/dev/%s" % volume.vid self.assertTrue(os.path.exists(path)) - self.pool.remove(volume) + volume.remove() @skipUnlessLvmPoolExists class TC_01_ThinPool(qubes.tests.SystemTestsMixin, ThinPoolBase):