wip

Author: Nambrok

drivers/blktap2.py

@@ -1730,15 +1730,21 @@ def _check_journal_coalesce_chain(self, sr_uuid: str, vdi_uuid: str) -> bool:
         for entry in journal.getAll("coalesce").keys():
             if entry in vdi_chain:
                 vdi_to_cancel.append(entry)
-                util.SMlog("Coalescing VDI {} in chain".format(entry))
+                util.SMlog("VDI {} in chain is coalescing".format(entry))
+
+        for entry in journal.getAll("relink").keys():
+            if entry in vdi_chain:
+                util.SMlog("VDI {} is in relink".format(entry))
+                return False #TODO: need to do stop and retry the activating after the relink
 
         # Get the host_ref from the host doing the GC work
         host_ref = self._get_sr_master_host_ref()
         for vdi in vdi_to_cancel:
             args = {"sr_uuid": sr_uuid, "vdi_uuid": vdi}
             util.SMlog("Calling cancel_coalesce_master with args: {}".format(args))
             self._session.xenapi.host.call_plugin(\
-                host_ref, PLUGIN_ON_SLAVE, "cancel_coalesce_master", args)
+                host_ref, PLUGIN_ON_SLAVE, "cancel_coalesce_master", args)  #TODO: This can potentially never return if we send it after the coalesce is done and the relink is in process
+            # We could run it abortable and check regularly that the journal isn't here?
 
         return True
 
@@ -1806,6 +1812,8 @@ def _activate_locked(self, sr_uuid, vdi_uuid, options):
                     util.SMlog('Using key with hash {} for VDI {}'.format(key_hash, vdi_uuid))
             # Activate the physical node
             dev_path = self._activate(sr_uuid, vdi_uuid, options)
+            # if not dev_path:
+            #     return False
 
             if hasattr(self.target.vdi.sr, 'DRIVER_TYPE') and \
                self.target.vdi.sr.DRIVER_TYPE == 'lvhd' and \
@@ -1846,6 +1854,9 @@ def _activate_locked(self, sr_uuid, vdi_uuid, options):
     def _activate(self, sr_uuid, vdi_uuid, options):
         vdi_options = self.target.activate(sr_uuid, vdi_uuid)
 
+        # if not self._check_journal_coalesce_chain(sr_uuid, vdi_uuid):
+        #     return None
+
         dev_path = self.setup_cache(sr_uuid, vdi_uuid, options)
         if not dev_path:
             phy_path = self.PhyLink.from_uuid(sr_uuid, vdi_uuid).readlink()

drivers/cleanup.py

@@ -2528,7 +2528,7 @@ def _coalesce(self, vdi: VDI):
             try:
                 vdi.parent._tagChildrenForRelink()
                 self.scan()
-                vdi._relinkSkip()
+                vdi._relinkSkip() #TODO: here
             finally:
                 self.unlock()
                 # Reload the children to leave things consistent

drivers/lvmanager.py

@@ -20,6 +20,7 @@
 import util
 
 from constants import NS_PREFIX_LVM
+from lvmcache import LVMCache
 
 class LVManagerException(util.SMException):
     pass
@@ -41,7 +42,7 @@ class LVActivator:
     TEMPORARY = False
     PERSISTENT = True
 
-    def __init__(self, srUuid, lvmCache):
+    def __init__(self, srUuid, lvmCache: LVMCache):
         self.ns = NS_PREFIX_LVM + srUuid
         self.lvmCache = lvmCache
         self.lvActivations = dict()
@@ -63,9 +64,9 @@ def activate(self, uuid, lvName, binary, persistent=False):
 
     def activateEnforce(self, uuid, lvName, lvPath):
         """incrementing the refcount is not enough to keep an LV activated if
-        another party is unaware of refcounting. For example, blktap does 
+        another party is unaware of refcounting. For example, blktap does
         direct "lvchange -an, lvchange -ay" during VBD.attach/resume without
-        any knowledge of refcounts. Therefore, we need to keep the device open 
+        any knowledge of refcounts. Therefore, we need to keep the device open
         to prevent unwanted deactivations. Note that blktap can do "lvchange
         -an" the very moment we try to open the file, so retry on failure"""
         if self.lvActivations[self.TEMPORARY][self.NORMAL].get(uuid):

drivers/lvmcache.py

@@ -146,6 +146,7 @@ def activate(self, ns, ref, lvName, binary):
         lock.acquire()
         try:
             count = RefCounter.get(ref, binary, ns)
+            util.SMlog(f"DAMS: LVMCache Activating: {lvName}, binary={binary}, count={count}")
             if count == 1:
                 try:
                     self.activateNoRefcount(lvName)
@@ -161,6 +162,7 @@ def deactivate(self, ns, ref, lvName, binary):
         lock.acquire()
         try:
             count = RefCounter.put(ref, binary, ns)
+            util.SMlog(f"DAMS: LVMCache Deactivating: {lvName}, count={count}")
             if count > 0:
                 return
             refreshed = False

drivers/on_slave.py

@@ -166,21 +166,22 @@ def commit_tapdisk(session, args):
     vdi_type = args["vdi_type"]
     #TODO: naming should reflect that it does more than coalesceing, like setting volume RW
 
-    def set_RW(path):
-        try:
-            util.pread2(["lvchange", "-p", "rw", path])
-        except:
-            pass
+    # def set_RW(path):
+    #     try:
+    #         util.pread2(["lvchange", "-p", "rw", path]) #This will disable the LV see: https://github.com/xcp-ng/sm/commit/8bc81723c87b2b30c28422be75b86c0f76b95d18
+    #     except Exception:
+    #         pass
     #TODO: need to make children RW. Or we let the relink happen with a refresh on master and hope it doesn't corrupt the disk
-    if path.startswith("/dev/"):
-        set_RW(path)
+    #TODO: we might want to make the chain RW for tapdisk in blktap2.py instead
+    # if path.startswith("/dev/"):
+    #     set_RW(path)
 
     from cowutil import getCowUtil
     cowutil = getCowUtil(vdi_type)
     try:
-        parent = cowutil.getParentNoCheck(path)
-        if parent.startswith("/dev/"):
-            set_RW(parent)
+        # parent = cowutil.getParentNoCheck(path) #TODO: Same things as above, should be done in blktap2.py
+        # if parent.startswith("/dev/"):
+        #     set_RW(parent)
         return str(cowutil.coalesceOnline(path))
     except:
         util.logException("Couldn't coalesce online")
@@ -193,7 +194,7 @@ def commit_cancel(session, args):
     cowutil = getCowUtil(vdi_type)
     try:
         cowutil.cancelCoalesceOnline(path)
-    except:
+    except Exception:
         return "False"
     return "True"
 
@@ -232,7 +233,7 @@ def cancel_coalesce_master(session, args):
         raise
 
     while os.path.exists(path):
-        time.sleep(1)
+        time.sleep(1) #TODO: We can get stuck here, we need a way to check we are in a legitimate wait
 
     return "True"
 

drivers/qcow2util.py

@@ -496,11 +496,12 @@ def _getInfoLV(
 
         vdiUuid = extractUuidFunction(lvPath)
         srUuid = extractUuidFunction(vgName)
-        lvcache.activate(NS_PREFIX_LVM + srUuid, vdiUuid, lvName, False)
+        ns = NS_PREFIX_LVM + srUuid
+        lvcache.activate(ns, vdiUuid, lvName, False)
         try:
             cowinfo = self.getInfo(lvPath, extractUuidFunction)
         finally:
-            lvcache.deactivate(NS_PREFIX_LVM + srUuid, vdiUuid, lvName, False)
+            lvcache.deactivate(ns, vdiUuid, lvName, False)
         return cowinfo
 
     @override
@@ -678,7 +679,7 @@ def killData(self, path: str) -> None:
         Returns:
             nothing.
         """
-        self._read_qcow2(path)
+        self._read_qcow2(path, read_clusters=True)
         # We need to reset L1 entries and then just truncate the file right
         # after L1 entries
         with open(self.filename, "r+b") as file:
@@ -783,7 +784,9 @@ def snapshot(
         parent_type = QCOW2_TYPE
         if parentRaw:
             parent_type = RAW_TYPE
+        # TODO: msize, it's use to preallocate metadata, could we honor this too?
         # TODO: checkEmpty? If it is False, then the parent could be empty and should still be used for snapshot
+        # But if True, if the parent is empty, we do what? vhd would just use the parent of parent as base, should we emulate this behavior?
         cmd = [QEMU_IMG, "create", "-f", QCOW2_TYPE, "-b", parent, "-F", parent_type, path]
         self._ioretry(cmd)
         self.setHidden(path, False) #We add hidden header at creation