WIP: Refactoring

Signed-off-by: Antoine Bartuccio <antoine.bartuccio@vates.tech>

Author: klmp200

drivers/FileSR.py

@@ -257,6 +257,18 @@ def _update(self, sr_uuid, virt_alloc_delta):
         self.physical_utilisation = self._getutilisation()
         self._db_update()
 
+    def get_children_of(self, uuid: str) -> List[str]:
+        if not self.vdis:
+            self._loadvdis()
+
+        children = []
+
+        for vdi in self.vdis.values():
+            if vdi.parent == uuid:
+                children.append(vdi.uuid)
+
+        return children
+
     @override
     def content_type(self, sr_uuid) -> str:
         return super(FileSR, self).content_type(sr_uuid)
@@ -755,54 +767,32 @@ def _tap_pause(self, secondary=None):
         finally:
             blktap2.VDI.tap_unpause(self.session, self.sr.uuid, self.uuid, secondary)
 
-
     @override
-    def revert(self, sr_uuid, src_uuid, dest_uuid):
-        util.SMlog("---------------------------------------------------------------->")
-        util.SMlog(f"in: {src_uuid} - {self.uuid}")
-
-        if self.label == "base copy":
-            raise xs_errors.XenError(f"{self.uuid} is a base copy.")
-
-        dest = VDI.VDI.from_uuid(self.sr.session, dest_uuid)
-        util.SMlog(f"out: {dest_uuid} - {dest.uuid}")
-
-        if self.vdi_type != dest.vdi_type:
-            raise xs_errors.XenError(f"{self.uuid} and {dest.uuid} has incompatible types {self.vdi_type} != {dest.vdi_type}")
-
-
+    def _do_revert(self, dest: "FileVDI", cbtlog: Optional[str] = None):
+        # Sanity checks
         if not self._checkpath(self.path):
             raise xs_errors.XenError(f"Could not find {self.path}")
 
         if not dest._checkpath(dest.path):
             raise xs_errors.XenError(f"Could not find {dest.path}")
 
-        def get_child_of(sr: FileSR, uuid: str, exclude: Optional[List[str]] = None) -> Optional[str]:
-            if not exclude:
-                exclude = []
-
-            excluded = set(exclude)
-            sr._loadvdis()
-
-            children = []
-
-            for vdi in sr.vdis.values():
-                if vdi.uuid == uuid:
-                    continue
-                if vdi.parent == uuid and vdi.uuid not in excluded:
-                    children.append(vdi.uuid)
-
-            if len(children) != 1:
-                return None
-
-            return children[0]
+        self._ensure_not_max_depth()
 
-        common_parent_uuid = self.parent if self.parent == dest.parent else get_child_of(self.sr, self.parent, [self.uuid])
-        if not common_parent_uuid:
-            raise xs_errors.XenError(f"{self.uuid} and {dest.uuid} aren't on the same snapshot tree")
+        # Get first common parent
+        if self.parent == dest.parent:
+            downward_child = None
+        else:
+            children = [child for child in self.sr.get_children_of(self.parent) if child != self.uuid]
+            if not children:
+                downward_child = None
+            else:
+                downward_child = VDI.VDI.from_uuid(self.sr.session, children[0])
 
-        self._ensure_not_max_depth()
+        with self._tap_pause(), dest._tap_pause():
+            self._revert(dest, downward_child, cbtlog)
 
+    def _revert(self, dest: "FileVDI", downward_child: Optional["FileVDI"], cbtlog: Optional[str]):
+        """This assumes that self and dest VDIs has been paused"""
         dest_tmp_uuid = util.gen_uuid() # Will be replaced by dest.uuid at the end
         dest_tmp_path = os.path.join(dest.sr.path, "%s%s" % (dest_tmp_uuid, VDI_TYPE_TO_EXTENSION[dest.vdi_type]))
 
@@ -817,90 +807,74 @@ def update_vdi_from_file(vdi: FileVDI, path: str):
             if vdi.parent:
                 vdi.sm_config['vhd-parent'] = vdi.parent
 
-        with self._tap_pause(), dest._tap_pause():
-            # try:
-            util.fistpoint.activate_custom_fn(
-                "FileSR_fail_snap1",
-                self.__fist_enospace)
-            util.ioretry(lambda: self._snap(dest_tmp_path, src_parent.path, False))
-            self.cowutil.setHidden(dest_tmp_path, False)
-
-            if common_parent_uuid != dest.parent:
-                # Create a new parent VDI based on src parent
-                src_parent_clone_uuid = util.gen_uuid()
-                src_parent_clone_path = os.path.join(
-                    dest.sr.path,
-                    "%s%s" % (src_parent_clone_uuid, VDI_TYPE_TO_EXTENSION[dest.vdi_type])
-                )
+        # try:
+        util.fistpoint.activate_custom_fn(
+            "FileSR_fail_snap1",
+            self.__fist_enospace)
+        util.ioretry(lambda: self._snap(dest_tmp_path, src_parent.path, False))
+        self.cowutil.setHidden(dest_tmp_path, False)
+
+        if downward_child:
+            # Create a new parent VDI based on src parent and attach the rest of the snapshoot tree to it
+            src_parent_clone_uuid = util.gen_uuid()
+            src_parent_clone_path = os.path.join(
+                dest.sr.path,
+                "%s%s" % (src_parent_clone_uuid, VDI_TYPE_TO_EXTENSION[dest.vdi_type])
+            )
 
-                common_parent = VDI.VDI.from_uuid(self.sr.session, common_parent_uuid)
-                common_parent.sm_config = common_parent.session.xenapi.VDI.get_sm_config(common_parent.sr.srcmd.params['vdi_ref'])
+            downward_child.sm_config = downward_child.session.xenapi.VDI.get_sm_config(downward_child.sr.srcmd.params['vdi_ref'])
 
-                util.fistpoint.activate_custom_fn(
-                    "FileSR_fail_snap2",
-                    self.__fist_enospace)
-                util.ioretry(lambda: self._snap(src_parent_clone_path, src_parent.path, False))
-                self.cowutil.setHidden(src_parent_clone_path, False)
-
-                is_raw = self.VDI_TYPE == VdiType.RAW
-                self.cowutil.setParent(src_parent_clone_path, src_parent.path, is_raw)
-                self.cowutil.setParent(common_parent.path, src_parent_clone_path, is_raw)
-                self.cowutil.setParent(self.path, src_parent_clone_path, is_raw)
-
-                # Introduce new readonly vdi to db
-                src_parent_clone = VDI.VDI(self.sr, src_parent_clone_uuid)
-
-                # FileVDI emulation for update_vdi_from_file
-                src_parent_clone.path = src_parent_clone_path
-                src_parent_clone.parent = src_parent.uuid
-                src_parent_clone.cowutil = self.cowutil
-
-                src_parent_clone.label = "base copy"
-                src_parent_clone.read_only = True
-                src_parent_clone.location = src_parent_clone_uuid
-                src_parent_clone.sm_config = {}
-                # TODO: fix the raw snapshot case 
-                src_parent_clone.sm_config["image-format"] = getImageStringFromVdiType(self.vdi_type)
-                if "key_hash" in common_parent.sm_config:
-                    src_parent_clone.sm_config['key_hash'] = common_parent.sm_config['key_hash']
-                update_vdi_from_file(src_parent_clone, src_parent_clone_uuid)
-
-                self.sm_config = self.session.xenapi.VDI.get_sm_config(self.sr.srcmd.params['vdi_ref'])
-                update_vdi_from_file(self, self.path)
+            util.fistpoint.activate_custom_fn(
+                "FileSR_fail_snap2",
+                self.__fist_enospace)
+            util.ioretry(lambda: self._snap(src_parent_clone_path, src_parent.path, False))
+            self.cowutil.setHidden(src_parent_clone_path, False)
 
-                update_vdi_from_file(common_parent, common_parent.path)
+            is_raw = self.VDI_TYPE == VdiType.RAW
+            self.cowutil.setParent(src_parent_clone_path, src_parent.path, is_raw)
+            self.cowutil.setParent(downward_child.path, src_parent_clone_path, is_raw)
+            self.cowutil.setParent(self.path, src_parent_clone_path, is_raw)
 
-                src_parent_clone._db_introduce()
+            # Introduce new readonly vdi to db
+            src_parent_clone = VDI.VDI(self.sr, src_parent_clone_uuid)
 
-                common_parent._db_update()
+            # FileVDI emulation for update_vdi_from_file
+            src_parent_clone.path = src_parent_clone_path
+            src_parent_clone.parent = src_parent.uuid
+            src_parent_clone.cowutil = self.cowutil
 
-                self._db_update()
+            src_parent_clone.label = "base copy"
+            src_parent_clone.read_only = True
+            src_parent_clone.location = src_parent_clone_uuid
+            src_parent_clone.sm_config = {}
+            # TODO: fix the raw snapshot case 
+            src_parent_clone.sm_config["image-format"] = getImageStringFromVdiType(self.vdi_type)
+            if "key_hash" in downward_child.sm_config:
+                src_parent_clone.sm_config['key_hash'] = downward_child.sm_config['key_hash']
+            update_vdi_from_file(src_parent_clone, src_parent_clone_uuid)
 
-                self.cowutil.setHidden(src_parent_clone_path, True)
+            self.sm_config = self.session.xenapi.VDI.get_sm_config(self.sr.srcmd.params['vdi_ref'])
+            update_vdi_from_file(self, self.path)
 
-            util.ioretry(lambda: self._rename(dest_tmp_path, dest.path),
-                         errlist=[errno.EIO, errno.EACCES])
+            update_vdi_from_file(downward_child, downward_child.path)
 
-            dest.sm_config = dest.session.xenapi.VDI.get_sm_config(dest.sr.srcmd.params['vdi_ref'])
-            update_vdi_from_file(dest, dest.path)
+            src_parent_clone._db_introduce()
 
-            dest._db_update()
+            downward_child._db_update()
 
-        return
+            self._db_update()
 
+            self.cowutil.setHidden(src_parent_clone_path, True)
 
-            # except util.CommandException as inst:
-            #     # TODO: cleanup
-            #     # XXX: it might be too late if the base disk has been marked as deleted!
-            #     # self._clonecleanup(src, dst, newsrc)
-            #     # util.end_log_entry(self.sr.path, self.path, ["error"])
-            #     # raise xs_errors.XenError('VDIClone',
-            #     #       opterr='VDI clone failed error %d' % inst.code)
-            #     pass
+        util.ioretry(lambda: self._rename(dest_tmp_path, dest.path),
+                     errlist=[errno.EIO, errno.EACCES])
 
-        util.SMlog("<---------------------------------------------------------------")
-        raise xs_errors.XenError('Heloiastrnie')
+        dest.sm_config = dest.session.xenapi.VDI.get_sm_config(dest.sr.srcmd.params['vdi_ref'])
+        update_vdi_from_file(dest, dest.path)
 
+        dest._db_update()
+        # except:
+        #cleanup
 
     @override
     def compose(self, sr_uuid, vdi1, vdi2) -> None:

drivers/VDI.py

@@ -217,6 +217,9 @@ def _do_snapshot(self, sr_uuid, vdi_uuid, snapType,
                      cloneOp=False, secondary=None, cbtlog=None, is_mirror_destination=False) -> str:
         raise xs_errors.XenError('Unimplemented')
 
+    def _do_revert(self, dest, cbtlog: Optional[str] = None) -> None:
+        raise xs_errors.XenError('Unimplemented')
+
     def _delete_cbt_log(self) -> None:
         raise xs_errors.XenError('Unimplemented')
 
@@ -249,14 +252,28 @@ def resize(self, sr_uuid, vdi_uuid, size) -> str:
         """
         raise xs_errors.XenError('Unimplemented')
 
-    def revert(self, sr_uuid, vdi_uuid, target_uuid):
+    def revert(self, sr_uuid: str, vdi_uuid: str, target_uuid: str) -> None:
         """Replaces the contents of the target_uuid VDI with the contents of the vdi_uuid
         without changing the identitity of the target (i.e. name-label, uuid and location
         are guaranteed to remain the same)..
 
         This operation IS idempotent and the vdi pointed by vdi_uuid is preserved.
         """
-        raise xs_errors.XenError('Unimplemented')
+        if self.label == "base copy":
+            raise xs_errors.XenError(f"{self.uuid} is a base copy.")
+
+        dest = self.__class__.from_uuid(self.sr.session, target_uuid)
+
+        if self.vdi_type != dest.vdi_type:
+            raise xs_errors.XenError(f"{self.uuid} and {dest.uuid} has incompatible types {self.vdi_type} != {dest.vdi_type}")
+
+        if self._get_blocktracking_status():
+            cbtlog = self._get_cbt_logpath(self.uuid)
+        else:
+            cbtlog = None
+
+        self._do_revert(dest, cbtlog=cbtlog)
+
 
     def resize_cbt(self, sr_uuid, vdi_uuid, size):
         """Resize the given VDI to size <size> MB. Size can