From c6eb4c49a3efc3d6361f3d3feff773415c866a2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Sun, 25 Jun 2017 21:32:39 +0200 Subject: [PATCH] storage: rework clone as two-stage operation Split clone to two Admin API calls - one to the source volume, then other to destination. --- qubesadmin/storage.py | 11 ++++------- qubesadmin/tests/app.py | 16 ++++++++++++---- qubesadmin/tests/storage.py | 29 +++++++++++------------------ 3 files changed, 27 insertions(+), 29 deletions(-) diff --git a/qubesadmin/storage.py b/qubesadmin/storage.py index 5a72719..520f80d 100644 --- a/qubesadmin/storage.py +++ b/qubesadmin/storage.py @@ -221,14 +221,11 @@ class Volume(object): ''' # pylint: disable=protected-access - if source._vm_name is None or self._vm_name is None: - raise NotImplementedError( - 'Operation implemented only for VM volumes') - if source._vm_name != self._vm_name: - raise ValueError('Source and target volume must have the same type') - # this call is to *source* volume, because we extract data from there - source._qubesd_call('Clone', payload=str(self._vm).encode()) + # get a token from source volume + token = source._qubesd_call('CloneFrom') + # and use it to actually clone volume data + self._qubesd_call('CloneTo', payload=token) class Pool(object): diff --git a/qubesadmin/tests/app.py b/qubesadmin/tests/app.py index f82b39f..be450b8 100644 --- a/qubesadmin/tests/app.py +++ b/qubesadmin/tests/app.py @@ -284,7 +284,10 @@ class TC_10_QubesBase(qubesadmin.tests.QubesTestCase): (src, 'admin.vm.volume.List', None, None)] = \ b'0\x00root\nprivate\nvolatile\nkernel\n' self.app.expected_calls[ - (src, 'admin.vm.volume.Clone', 'private', dst.encode())] = \ + (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' def test_030_clone(self): @@ -364,7 +367,10 @@ class TC_10_QubesBase(qubesadmin.tests.QubesTestCase): b'save_on_stop=True\n' \ b'snap_on_start=False\n' self.app.expected_calls[ - ('test-vm', 'admin.vm.volume.Clone', 'root', b'new-name')] = \ + ('test-vm', 'admin.vm.volume.CloneFrom', 'root', None)] = \ + b'0\x00token-root' + self.app.expected_calls[ + ('new-name', 'admin.vm.volume.CloneTo', 'root', b'token-root')] = \ b'0\x00' new_vm = self.app.clone_vm('test-vm', 'new-name', new_cls='StandaloneVM') @@ -482,7 +488,8 @@ class TC_10_QubesBase(qubesadmin.tests.QubesTestCase): self.app.expected_calls[('dom0', 'admin.vm.Create.AppVM', 'test-template', b'name=new-name label=red')] = b'0\x00' self.app.expected_calls[ - ('test-vm', 'admin.vm.volume.Clone', 'private', b'new-name')] = \ + ('new-name', 'admin.vm.volume.CloneTo', 'private', + b'token-private')] = \ b'2\0QubesException\0\0something happened\0' self.app.expected_calls[('new-name', 'admin.vm.Remove', None, None)] = \ b'0\x00' @@ -504,7 +511,8 @@ class TC_10_QubesBase(qubesadmin.tests.QubesTestCase): self.app.expected_calls[('dom0', 'admin.vm.Create.AppVM', 'test-template', b'name=new-name label=red')] = b'0\x00' self.app.expected_calls[ - ('test-vm', 'admin.vm.volume.Clone', 'private', b'new-name')] = \ + ('new-name', 'admin.vm.volume.CloneTo', 'private', + b'token-private')] = \ b'2\0QubesException\0\0something happened\0' self.app.expected_calls[('new-name', 'admin.vm.Remove', None, None)] = \ b'0\x00' diff --git a/qubesadmin/tests/storage.py b/qubesadmin/tests/storage.py index 76d3134..df4243a 100644 --- a/qubesadmin/tests/storage.py +++ b/qubesadmin/tests/storage.py @@ -163,7 +163,10 @@ class TestVMVolume(qubesadmin.tests.QubesTestCase): def test_050_clone(self): self.app.expected_calls[ - ('source-vm', 'admin.vm.volume.Clone', 'volname', b'test-vm')] = \ + ('source-vm', 'admin.vm.volume.CloneFrom', 'volname', None)] = \ + b'0\x00abcdef' + self.app.expected_calls[ + ('test-vm', 'admin.vm.volume.CloneTo', 'volname', b'abcdef')] = \ b'0\x00' self.app.expected_calls[ ('dom0', 'admin.vm.List', None, None)] = \ @@ -174,17 +177,6 @@ class TestVMVolume(qubesadmin.tests.QubesTestCase): self.vol.clone(self.app.domains['source-vm'].volumes['volname']) self.assertAllCalled() - def test_050_clone_wrong_volume(self): - self.app.expected_calls[ - ('dom0', 'admin.vm.List', None, None)] = \ - b'0\x00source-vm class=AppVM state=Halted\n' - self.app.expected_calls[ - ('source-vm', 'admin.vm.volume.List', None, None)] = \ - b'0\x00volname\nother\n' - with self.assertRaises(ValueError): - self.vol.clone(self.app.domains['source-vm'].volumes['other']) - self.assertAllCalled() - class TestPoolVolume(TestVMVolume): def setUp(self): @@ -273,13 +265,14 @@ class TestPoolVolume(TestVMVolume): def test_050_clone(self): self.app.expected_calls[ - ('dom0', 'admin.vm.List', None, None)] = \ - b'0\x00source-vm class=AppVM state=Halted\n' + ('dom0', 'admin.pool.volume.CloneFrom', 'test-pool', + b'volid')] = b'0\x00abcdef' self.app.expected_calls[ - ('source-vm', 'admin.vm.volume.List', None, None)] = \ - b'0\x00volname\nother\n' - with self.assertRaises(NotImplementedError): - self.vol.clone(self.app.domains['source-vm'].volumes['volname']) + ('dom0', 'admin.pool.volume.CloneTo', 'test-pool', + b'some-id abcdef')] = b'0\x00' + source_vol = qubesadmin.storage.Volume(self.app, pool='test-pool', + vid='volid') + self.vol.clone(source_vol) self.assertAllCalled() def test_050_clone_wrong_volume(self):