From e827e47926fec968dc6a902dc8d176a2b87b29d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Fri, 7 Dec 2018 22:20:01 +0100 Subject: [PATCH] Clone VM's volume into the same pool, unless overridden specifically When cloning VM, create it in the same pool as the source one. Previously it always used default pool, which means for example renaming a VM in non-default pool moved it back to the default one. Fixes QubesOS/qubes-issues#4145 Fixes QubesOS/qubes-issues#4523 --- qubesadmin/app.py | 15 +++++ qubesadmin/tests/app.py | 141 ++++++++++++++++++++++++++-------------- 2 files changed, 107 insertions(+), 49 deletions(-) diff --git a/qubesadmin/app.py b/qubesadmin/app.py index 073b8ad..bf3ec06 100644 --- a/qubesadmin/app.py +++ b/qubesadmin/app.py @@ -329,6 +329,21 @@ class QubesBase(qubesadmin.base.PropertyHolder): label = src_vm.label + if pool is None and pools is None: + # use the same pools as the source - check if non default is used + for volume in sorted(src_vm.volumes.values()): + if not volume.save_on_stop: + # clone only persistent volumes + continue + if ignore_volumes and volume.name in ignore_volumes: + continue + default_pool = getattr(self.app, 'default_pool_' + volume.name, + volume.pool) + if default_pool != volume.pool: + if pools is None: + pools = {} + pools[volume.name] = volume.pool + method_prefix = 'admin.vm.Create.' payload = 'name={} label={}'.format(new_name, label) if pool: diff --git a/qubesadmin/tests/app.py b/qubesadmin/tests/app.py index f6375a4..4a19cb1 100644 --- a/qubesadmin/tests/app.py +++ b/qubesadmin/tests/app.py @@ -298,61 +298,62 @@ class TC_10_QubesBase(qubesadmin.tests.QubesTestCase): b'0\x00' # storage - self.app.expected_calls[ - (dst, 'admin.vm.volume.List', None, None)] = \ - b'0\x00root\nprivate\nvolatile\nkernel\n' - self.app.expected_calls[ - (dst, 'admin.vm.volume.Info', 'root', None)] = \ - b'0\x00pool=lvm\n' \ - b'vid=vm-test-vm/root\n' \ - b'size=10737418240\n' \ - b'usage=2147483648\n' \ - b'rw=False\n' \ - b'internal=True\n' \ - b'source=vm-test-template/root\n' \ - b'save_on_stop=False\n' \ - b'snap_on_start=True\n' - self.app.expected_calls[ - (dst, 'admin.vm.volume.Info', 'private', None)] = \ - b'0\x00pool=lvm\n' \ - b'vid=vm-test-vm/private\n' \ - b'size=2147483648\n' \ - b'usage=214748364\n' \ - b'rw=True\n' \ - b'internal=True\n' \ - b'save_on_stop=True\n' \ - b'snap_on_start=False\n' - self.app.expected_calls[ - (dst, 'admin.vm.volume.Info', 'volatile', None)] = \ - b'0\x00pool=lvm\n' \ - b'vid=vm-test-vm/volatile\n' \ - b'size=10737418240\n' \ - b'usage=0\n' \ - b'rw=True\n' \ - b'internal=True\n' \ - b'source=None\n' \ - b'save_on_stop=False\n' \ - b'snap_on_start=False\n' - self.app.expected_calls[ - (dst, 'admin.vm.volume.Info', 'kernel', None)] = \ - b'0\x00pool=linux-kernel\n' \ - b'vid=\n' \ - b'size=0\n' \ - b'usage=0\n' \ - b'rw=False\n' \ - b'internal=True\n' \ - b'source=None\n' \ - b'save_on_stop=False\n' \ - b'snap_on_start=False\n' - self.app.expected_calls[ - (src, 'admin.vm.volume.List', None, None)] = \ - b'0\x00root\nprivate\nvolatile\nkernel\n' + for vm in (src, dst): + self.app.expected_calls[ + (vm, 'admin.vm.volume.Info', 'root', None)] = \ + b'0\x00pool=lvm\n' \ + b'vid=vm-' + vm.encode() + b'/root\n' \ + b'size=10737418240\n' \ + b'usage=2147483648\n' \ + b'rw=False\n' \ + b'internal=True\n' \ + b'source=vm-test-template/root\n' \ + b'save_on_stop=False\n' \ + b'snap_on_start=True\n' + self.app.expected_calls[ + (vm, 'admin.vm.volume.Info', 'private', None)] = \ + b'0\x00pool=lvm\n' \ + b'vid=vm-' + vm.encode() + b'/private\n' \ + b'size=2147483648\n' \ + b'usage=214748364\n' \ + b'rw=True\n' \ + b'internal=True\n' \ + b'save_on_stop=True\n' \ + b'snap_on_start=False\n' + self.app.expected_calls[ + (vm, 'admin.vm.volume.Info', 'volatile', None)] = \ + b'0\x00pool=lvm\n' \ + b'vid=vm-' + vm.encode() + b'/volatile\n' \ + b'size=10737418240\n' \ + b'usage=0\n' \ + b'rw=True\n' \ + b'internal=True\n' \ + b'source=None\n' \ + b'save_on_stop=False\n' \ + b'snap_on_start=False\n' + self.app.expected_calls[ + (vm, 'admin.vm.volume.Info', 'kernel', None)] = \ + b'0\x00pool=linux-kernel\n' \ + b'vid=\n' \ + b'size=0\n' \ + b'usage=0\n' \ + b'rw=False\n' \ + b'internal=True\n' \ + b'source=None\n' \ + b'save_on_stop=False\n' \ + b'snap_on_start=False\n' + self.app.expected_calls[ + (vm, 'admin.vm.volume.List', None, None)] = \ + b'0\x00root\nprivate\nvolatile\nkernel\n' self.app.expected_calls[ (src, 'admin.vm.volume.CloneFrom', 'private', None)] = \ b'0\x00token-private' self.app.expected_calls[ (dst, 'admin.vm.volume.CloneTo', 'private', b'token-private')] = \ b'0\x00' + self.app.expected_calls[ + ('dom0', 'admin.property.Get', 'default_pool_private', None)] = \ + b'0\0default=True type=str lvm' def test_030_clone(self): self.clone_setup_common_calls('test-vm', 'new-name') @@ -387,6 +388,11 @@ class TC_10_QubesBase(qubesadmin.tests.QubesTestCase): def test_032_clone_pool(self): self.clone_setup_common_calls('test-vm', 'new-name') + for volume in ('root', 'private', 'volatile', 'kernel'): + del self.app.expected_calls[ + 'test-vm', 'admin.vm.volume.Info', volume, None] + del self.app.expected_calls[ + 'dom0', 'admin.property.Get', 'default_pool_private', None] self.app.expected_calls[('dom0', 'admin.vm.CreateInPool.AppVM', 'test-template', b'name=new-name label=red pool=some-pool')] = b'0\x00' @@ -401,6 +407,11 @@ class TC_10_QubesBase(qubesadmin.tests.QubesTestCase): def test_033_clone_pools(self): self.clone_setup_common_calls('test-vm', 'new-name') + for volume in ('root', 'private', 'volatile', 'kernel'): + del self.app.expected_calls[ + 'test-vm', 'admin.vm.volume.Info', volume, None] + del self.app.expected_calls[ + 'dom0', 'admin.property.Get', 'default_pool_private', None] self.app.expected_calls[('dom0', 'admin.vm.CreateInPool.AppVM', 'test-template', b'name=new-name label=red pool:private=some-pool ' @@ -451,6 +462,10 @@ class TC_10_QubesBase(qubesadmin.tests.QubesTestCase): self.app.expected_calls[ ('test-vm', 'admin.vm.property.List', None, None)] = \ b'0\0qid\nname\ntemplate\nlabel\nmemory\n' + # simplify it a little, shouldn't get this far anyway + self.app.expected_calls[ + ('test-vm', 'admin.vm.volume.List', None, None)] = \ + b'0\x00' self.app.expected_calls[ ('test-vm', 'admin.vm.property.Get', 'label', None)] = \ b'0\0default=False type=label red' @@ -587,6 +602,34 @@ class TC_10_QubesBase(qubesadmin.tests.QubesTestCase): self.app.clone_vm('test-vm', 'new-name') self.assertAllCalled() + def test_042_clone_nondefault_pool(self): + self.clone_setup_common_calls('test-vm', 'new-name') + self.app.expected_calls[ + ('test-vm', 'admin.vm.volume.Info', 'private', None)] = \ + b'0\x00pool=another\n' \ + b'vid=vm-test-vm/private\n' \ + b'size=2147483648\n' \ + b'usage=214748364\n' \ + b'rw=True\n' \ + b'internal=True\n' \ + b'save_on_stop=True\n' \ + b'snap_on_start=False\n' + self.app.expected_calls[('dom0', 'admin.vm.List', None, None)] = \ + b'0\x00new-name class=AppVM state=Halted\n' \ + b'test-vm class=AppVM state=Halted\n' \ + b'test-template class=TemplateVM state=Halted\n' \ + b'test-net class=AppVM state=Halted\n' + self.app.expected_calls[('dom0', 'admin.vm.CreateInPool.AppVM', + 'test-template', b'name=new-name label=red pool:private=another')]\ + = b'0\x00' + new_vm = self.app.clone_vm('test-vm', 'new-name') + self.assertEqual(new_vm.name, 'new-name') + self.check_output_mock.assert_called_once_with( + ['qvm-appmenus', '--init', '--update', + '--source', 'test-vm', 'new-name'], + stderr=subprocess.STDOUT + ) + self.assertAllCalled() class TC_20_QubesLocal(unittest.TestCase):