Browse Source

storage: rework clone as two-stage operation

Split clone to two Admin API calls - one to the source volume, then
other to destination.
Marek Marczykowski-Górecki 7 years ago
parent
commit
c6eb4c49a3
3 changed files with 28 additions and 30 deletions
  1. 5 8
      qubesadmin/storage.py
  2. 12 4
      qubesadmin/tests/app.py
  3. 11 18
      qubesadmin/tests/storage.py

+ 5 - 8
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):

+ 12 - 4
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'

+ 11 - 18
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):