fix(LVMSR): set QCOW2 chain RW from the master on vdi_attach (#155)

Changing LVs to RW on coalesce on a remote host can clash with the master doing
LVM commands.  It might even lead to LVM metadata corruption.  With this
change, the `vdi_attach` will check if the chain is coalesceable on remote, if
so it will ask the master to change the status before continuing.  It's copying
the behavior of changing LV sizes in `_prepareThin`.

Signed-off-by: Damien Thenot <damien.thenot@vates.tech>

Author: Nambrok

drivers/LVMSR.py

@@ -1534,6 +1534,10 @@ def attach(self, sr_uuid, vdi_uuid) -> str:
                 util.logException("attach")
                 raise xs_errors.XenError('LVMProvisionAttach')
 
+        if self.cowutil.isCoalesceableOnRemote():
+            # We make the chain RW before it's used so it can be coalesced online
+            self._make_chain_rw()
+
         try:
             return self._attach()
         finally:
@@ -2259,6 +2263,33 @@ def _prepareThin(self, attach, vdiType):
             self.session.xenapi.SR.set_physical_utilisation(self.sr.sr_ref,
                     str(sr_utilisation))
 
+    def _make_chain_rw(self):
+        """
+        In the case of QCOW2, we need the chain to be RW for the coalesce to be done by tapdisk.
+        Setting the chain to be RW early avoids having to stop the tapdisk so LVM can acknowledge the chain RW on the slave.
+        And while making the change to RW from the slave works without the refresh, it can cause LVM metadata corruption.
+        """
+        # TODO(XCPNG-3689): We could check if any VDI on the chain is RO before doing the call to master to change this
+        if self.sr.isMaster:
+            lvmcache = self.sr.lvmCache
+            for uuid, lvName in self.cowutil.getParentChain(self.lvname, LvmCowUtil.extractUuid, self.sr.vgname).items():
+                lvmcache.setReadonly(lvName, False)
+        else:
+            master = util.get_master_ref(self.session)
+            response = self.session.xenapi.host.call_plugin(
+                master,
+                self.sr.PLUGIN_ON_SLAVE,
+                "make_chain_rw",
+                {
+                    "vgName": self.sr.vgname,
+                    "lvName": self.lvname,
+                    "vdiType": self.vdi_type
+                }
+            )
+            util.SMlog(f"call-plugin returned: {response}")
+            if not response:
+                raise Exception(f"plugin {self.sr.PLUGIN_ON_SLAVE} failed")
+
     @override
     def update(self, sr_uuid, vdi_uuid) -> None:
         if self.sr.legacyMode:

drivers/on_slave.py

@@ -162,22 +162,6 @@ def refresh_lun_size_by_SCSIid(session, args):
         util.SMlog("on-slave.refresh_lun_size_by_SCSIid with %s failed" % args)
         return "False"
 
-def _make_chain_RW(cowutil, leaf_path, write: bool):
-    def set_RW(path):
-        try:
-            util.pread2(["lvchange", "-p", "rw", path])
-        except:
-            pass
-
-    # if path.startswith("/dev/"):
-    #     set_RW(leaf_path) # Not needed since it's a leaf
-
-    parent = cowutil.getParentNoCheck(leaf_path)
-    while parent:
-        if parent.startswith("/dev/"):
-            set_RW(parent)
-        parent = cowutil.getParentNoCheck(parent)
-
 def commit_tapdisk(session, args):
     path: str = args["path"]
     vdi_type = args["vdi_type"]
@@ -186,12 +170,9 @@ def commit_tapdisk(session, args):
     from cowutil import getCowUtil
     cowutil = getCowUtil(vdi_type)
     try:
-        if path.startswith("/dev/"):
-            # We need to make children RW or tapdisk will crash trying to do it
-            _make_chain_RW(cowutil, leaf_path, True)
         return str(cowutil.coalesceOnline(path))
     except:
-        util.logException("Couldn't coalesce online")
+        util.logException(f"Couldn't coalesce online: `{path}`")
         raise
 
 def commit_cancel(session, args):
@@ -249,6 +230,29 @@ def is_openers(session, args):
     openers_pid= util.get_openers_pid(path)
     return str(bool(openers_pid))
 
+def make_chain_rw(session, args):
+    from cowutil import getCowUtil
+    from lvmcowutil import LvmCowUtil
+    from lvutil import MASTER_LVM_CONF
+
+    if util.is_master(session):
+        os.environ['LVM_SYSTEM_DIR'] = MASTER_LVM_CONF
+
+    vgName = args["vgName"]
+    lvName = args["lvName"]
+    vdi_type = args["vdiType"]
+
+    cowutil = getCowUtil(vdi_type)
+    lvmcache = LVMCache(vgName)
+
+    for uuid, lvName in cowutil.getParentChain(lvName, LvmCowUtil.extractUuid, vgName).items():
+        try:
+            lvmcache.setReadonly(lvName, False)
+        except util.CommandException as e:
+            util.SMlog(f"on-slave:make_chain_rw: {e}")
+
+    return "True"
+
 if __name__ == "__main__":
     import XenAPIPlugin
     XenAPIPlugin.dispatch({
@@ -259,4 +263,5 @@ def is_openers(session, args):
         "commit_tapdisk": commit_tapdisk,
         "commit_cancel": commit_cancel,
         "cancel_coalesce_master": cancel_coalesce_master,
+        "make_chain_rw": make_chain_rw,
         })