Change in vdsm[ovirt-3.5]: Live Merge: add reconcileVolumeChain verb

alitke at redhat.com alitke at redhat.com
Tue Sep 2 17:20:44 UTC 2014


Hello Piotr Kliczewski, Federico Simoncelli, Francesco Romani,

I'd like you to do a code review.  Please visit

    http://gerrit.ovirt.org/32328

to review the following change.

Change subject: Live Merge: add reconcileVolumeChain verb
......................................................................

Live Merge: add reconcileVolumeChain verb

If a live merge operation is catastrophically interrupted (we completely
lose the host on which the VM was running during the merge) engine must
have a way to discover what happened by examining storage only.

When the active layer was being merged, it's critical to know whether
the the pivot completed or not.  Choosing the wrong leaf volume could
result in data corruption.  When an internal volume is being merged the
situation is less dire but providing a way for engine to resolve the
final status of the merge provides for a substantially better user
experience since the snapshot can be unlocked to allow a cold merge to
be attempted.

This new verb must run on SPM and is only valid for images which are not
in use by running VMs.  It will use qemu-img to examine the actual
volume chain and will correct the vdsm metadata if needed.  Finally, the
current volume chain is returned.  This chain can be used by engine to
understand what happened with the interrupted merge command.

Change-Id: Ia8927a42d2bc9adcf7c6b26babb327a00e91e49e
Signed-off-by: Adam Litke <alitke at redhat.com>
Reviewed-on: http://gerrit.ovirt.org/31788
Reviewed-by: Francesco Romani <fromani at redhat.com>
Reviewed-by: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Reviewed-by: Federico Simoncelli <fsimonce at redhat.com>
---
M client/vdsClient.py
M vdsm/API.py
M vdsm/rpc/BindingXMLRPC.py
M vdsm/rpc/Bridge.py
M vdsm/rpc/vdsmapi-schema.json
M vdsm/storage/hsm.py
M vdsm/storage/image.py
M vdsm/storage/sp.py
8 files changed, 163 insertions(+), 23 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/28/32328/1

diff --git a/client/vdsClient.py b/client/vdsClient.py
index 2c09b28..e06137d 100644
--- a/client/vdsClient.py
+++ b/client/vdsClient.py
@@ -1374,6 +1374,17 @@
 
         return ret['status']['code'], ret['status']['message']
 
+    def reconcileVolumeChain(self, args):
+        if len(args) != 4:
+            raise ValueError('Wrong number of parameters')
+
+        ret = self.s.reconcileVolumeChain(*args)
+        if 'volumes' in ret:
+            for v in ret['volumes']:
+                print v
+            return 0, ''
+        return ret['status']['code'], ret['status']['message']
+
     def moveMultiImage(self, args):
         spUUID = args[0]
         srcDomUUID = args[1]
@@ -2465,6 +2476,10 @@
             '<spUUID> <sdUUID> <imgUUID> [<volUUID>]',
             'Teardown an image, releasing the prepared volumes.'
         )),
+        'reconcileVolumeChain': (serv.reconcileVolumeChain, (
+            '<spUUID> <sdUUID> <imgUUID> <leafVolUUID>',
+            'Reconcile an image volume chain and return the current chain.'
+        )),
         'moveMultiImage': (serv.moveMultiImage,
                            ('<spUUID> <srcDomUUID> <dstDomUUID> '
                             '<imgList>({imgUUID=postzero,'
diff --git a/vdsm/API.py b/vdsm/API.py
index fc02b01..6ea76a5 100644
--- a/vdsm/API.py
+++ b/vdsm/API.py
@@ -888,6 +888,10 @@
             methodArgs, callback, self._spUUID, self._sdUUID, self._UUID,
             volUUID)
 
+    def reconcileVolumeChain(self, leafVolUUID):
+        return self._irs.reconcileVolumeChain(self._spUUID, self._sdUUID,
+                                              self._UUID, leafVolUUID)
+
 
 class LVMVolumeGroup(APIBase):
     ctorArgs = ['lvmvolumegroupID']
diff --git a/vdsm/rpc/BindingXMLRPC.py b/vdsm/rpc/BindingXMLRPC.py
index 12a1b49..dc85fe8 100644
--- a/vdsm/rpc/BindingXMLRPC.py
+++ b/vdsm/rpc/BindingXMLRPC.py
@@ -696,6 +696,10 @@
         image = API.Image(imgUUID, spUUID, sdUUID)
         return image.teardown(volUUID)
 
+    def imageReconcileVolumeChain(self, spUUID, sdUUID, imgUUID, leafUUID):
+        image = API.Image(imgUUID, spUUID, sdUUID)
+        return image.reconcileVolumeChain(leafUUID)
+
     def poolConnect(self, spUUID, hostID, scsiKey, msdUUID, masterVersion,
                     domainsMap=None, options=None):
         pool = API.StoragePool(spUUID)
@@ -1038,6 +1042,7 @@
                 (self.imageDownload, 'downloadImage'),
                 (self.imagePrepare, 'prepareImage'),
                 (self.imageTeardown, 'teardownImage'),
+                (self.imageReconcileVolumeChain, 'reconcileVolumeChain'),
                 (self.poolConnect, 'connectStoragePool'),
                 (self.poolConnectStorageServer, 'connectStorageServer'),
                 (self.poolCreate, 'createStoragePool'),
diff --git a/vdsm/rpc/Bridge.py b/vdsm/rpc/Bridge.py
index 4c5204d..7a7494b 100644
--- a/vdsm/rpc/Bridge.py
+++ b/vdsm/rpc/Bridge.py
@@ -397,6 +397,7 @@
     'Image_download': {'ret': 'uuid'},
     'Image_mergeSnapshots': {'ret': 'uuid'},
     'Image_move': {'ret': 'uuid'},
+    'Image_reconcileVolumeChain': {'ret': 'volumes'},
     'ISCSIConnection_discoverSendTargets': {'ret': 'fullTargets'},
     'LVMVolumeGroup_create': {'ret': 'uuid'},
     'LVMVolumeGroup_getInfo': {'ret': 'info'},
diff --git a/vdsm/rpc/vdsmapi-schema.json b/vdsm/rpc/vdsmapi-schema.json
index c62513f..db441f5 100644
--- a/vdsm/rpc/vdsmapi-schema.json
+++ b/vdsm/rpc/vdsmapi-schema.json
@@ -4267,6 +4267,29 @@
  'data': {'storagepoolID': 'UUID', 'storagedomainID': 'UUID',
           'imageID': 'UUID', '*volumeID': 'UUID'}}
 
+##
+# @Image.reconcileVolumeChain:
+#
+# Reconcile an image volume chain and return the current chain.
+#
+# @storagepoolID:    The UUID of the Storage Pool associated with the Image
+#
+# @storagedomainID:  The UUID of the Storage Domain associated with the Image
+#
+# @imageID:          The UUID of the Image
+#
+# @leafVolID:        The UUID of the original leaf volume
+#
+# Returns:
+# A list of volume UUIDs representing the current volume chain
+#
+# Since: 4.16.0
+##
+{'command': {'class': 'Image', 'name': 'reconcileVolumeChain'},
+ 'data': {'storagepoolID': 'UUID', 'storagedomainID': 'UUID',
+          'imageID': 'UUID', 'leafVolID': 'UUID'}
+ 'returns': ['UUID']}
+
 ## Category: @LVMVolumeGroup ###################################################
 ##
 # @LVMVolumeGroup:
diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py
index b6972af..5d0f1b6 100644
--- a/vdsm/storage/hsm.py
+++ b/vdsm/storage/hsm.py
@@ -1812,36 +1812,34 @@
         volumes.  In this case we update the chain in the metadata LV only.
         The LV tag will be fixed when the unlinked volume is deleted by an SPM.
         """
+        argsStr = ("sdUUID=%s, imgUUID=%s, volUUID=%s, newChain=%s" %
+                   (sdUUID, imgUUID, volUUID, newChain))
+        vars.task.setDefaultException(se.StorageException("%s" % argsStr))
         sdDom = sdCache.produce(sdUUID=sdUUID)
         repoPath = os.path.join(self.storage_repository, sdDom.getPools()[0])
-        img = image.Image(repoPath)
 
         imageResourcesNamespace = sd.getNamespace(sdUUID, IMAGE_NAMESPACE)
         with rmanager.acquireResource(imageResourcesNamespace, imgUUID,
                                       rm.LockType.shared):
-            curChain = img.getChain(sdUUID, imgUUID, volUUID)
-            subChain = []
-            for vol in curChain:
-                if vol.volUUID not in newChain:
-                    subChain.insert(0, vol.volUUID)
-                elif len(subChain) > 0:
-                    break
-            self.log.debug("unlinking subchain: %s" % subChain)
-            if len(subChain) == 0:
-                return
+            image.Image(repoPath).syncVolumeChain(sdUUID, imgUUID, volUUID,
+                                                  newChain)
 
-            dstParent = sdDom.produceVolume(imgUUID, subChain[0]).getParent()
-            subChainTailVol = sdDom.produceVolume(imgUUID, subChain[-1])
-            if subChainTailVol.isLeaf():
-                self.log.debug("Leaf volume is being removed from the chain. "
-                               "Marking it ILLEGAL to prevent data corruption")
-                subChainTailVol.setLegality(volume.ILLEGAL_VOL)
-            else:
-                for childID in subChainTailVol.getChildren():
-                    self.log.debug("Setting parent of volume %s to %s",
-                                   childID, dstParent)
-                    sdDom.produceVolume(imgUUID, childID). \
-                        setParentMeta(dstParent)
+    @public
+    def reconcileVolumeChain(self, spUUID, sdUUID, imgUUID, leafVolUUID):
+        """
+        In some situations (such as when a live merge is interrupted), the
+        vdsm volume chain could become out of sync with the actual chain as
+        understood by qemu.  This API uses qemu-img to determine the correct
+        chain and synchronizes vdsm metadata accordingly.  Returns the correct
+        volume chain.  NOT for use on images of running VMs.
+        """
+        argsStr = ("spUUID=%s, sdUUID=%s, imgUUID=%s, leafVolUUID=%s" %
+                   (spUUID, sdUUID, imgUUID, leafVolUUID))
+        vars.task.setDefaultException(se.StorageException("%s" % argsStr))
+        pool = self.getPool(spUUID)
+        sdCache.produce(sdUUID=sdUUID)
+        vars.task.getSharedLock(STORAGE, sdUUID)
+        return pool.reconcileVolumeChain(sdUUID, imgUUID, leafVolUUID)
 
     @public
     def mergeSnapshots(self, sdUUID, spUUID, vmUUID, imgUUID, ancestor,
diff --git a/vdsm/storage/image.py b/vdsm/storage/image.py
index 37ee135..70f59a6 100644
--- a/vdsm/storage/image.py
+++ b/vdsm/storage/image.py
@@ -1056,6 +1056,78 @@
 
         return accumulatedChainSize, chain
 
+    def syncVolumeChain(self, sdUUID, imgUUID, volUUID, actualChain):
+        """
+        Fix volume metadata to reflect the given actual chain.  This function
+        is used to correct the volume chain linkage after a live merge.
+        """
+        curChain = self.getChain(sdUUID, imgUUID, volUUID)
+        subChain = []
+        for vol in curChain:
+            if vol.volUUID not in actualChain:
+                subChain.insert(0, vol.volUUID)
+            elif len(subChain) > 0:
+                break
+        if len(subChain) == 0:
+            return
+        self.log.debug("unlinking subchain: %s" % subChain)
+
+        sdDom = sdCache.produce(sdUUID=sdUUID)
+        dstParent = sdDom.produceVolume(imgUUID, subChain[0]).getParent()
+        subChainTailVol = sdDom.produceVolume(imgUUID, subChain[-1])
+        if subChainTailVol.isLeaf():
+            self.log.debug("Leaf volume is being removed from the chain. "
+                           "Marking it ILLEGAL to prevent data corruption")
+            subChainTailVol.setLegality(volume.ILLEGAL_VOL)
+        else:
+            for childID in subChainTailVol.getChildren():
+                self.log.debug("Setting parent of volume %s to %s",
+                               childID, dstParent)
+                sdDom.produceVolume(imgUUID, childID). \
+                    setParentMeta(dstParent)
+
+    def reconcileVolumeChain(self, sdUUID, imgUUID, leafVolUUID):
+        """
+        Discover and return the actual volume chain of an offline image
+        according to the qemu-img info command and synchronize volume metadata.
+        """
+        # Prepare volumes
+        dom = sdCache.produce(sdUUID)
+        allVols = dom.getAllVolumes()
+        imgVolumes = sd.getVolsOfImage(allVols, imgUUID).keys()
+        dom.activateVolumes(imgUUID, imgVolumes)
+
+        # Walk the volume chain using qemu-img.  Not safe for running VMs
+        actualVolumes = []
+        volUUID = leafVolUUID
+        while volUUID is not None:
+            actualVolumes.insert(0, volUUID)
+            vol = dom.produceVolume(imgUUID, volUUID)
+            qemuImgFormat = volume.fmt2str(vol.getFormat())
+            imgInfo = qemuimg.info(vol.volumePath, qemuImgFormat)
+            backingFile = imgInfo.get('backingfile')
+            if backingFile is not None:
+                volUUID = os.path.basename(backingFile)
+            else:
+                volUUID = None
+
+        # A merge of the active layer has copy and pivot phases.
+        # During copy, data is copied from the leaf into its parent.  Writes
+        # are mirrored to both volumes.  So even after copying is complete the
+        # volumes will remain consistent.  Finally, the VM is pivoted from the
+        # old leaf to the new leaf and mirroring to the old leaf ceases. During
+        # mirroring and before pivoting, we mark the old leaf ILLEGAL so we
+        # know it's safe to delete in case the operation is interrupted.
+        vol = dom.produceVolume(imgUUID, leafVolUUID)
+        if vol.getLegality() == volume.ILLEGAL_VOL:
+            actualVolumes.remove(leafVolUUID)
+
+        # Now that we know the correct volume chain, sync the storge metadata
+        self.syncVolumeChain(sdUUID, imgUUID, actualVolumes[-1], actualVolumes)
+
+        dom.deactivateImage(imgUUID)
+        return actualVolumes
+
     def merge(self, sdUUID, vmUUID, imgUUID, ancestor, successor, postZero):
         """Merge source volume to the destination volume.
             'successor' - source volume UUID
diff --git a/vdsm/storage/sp.py b/vdsm/storage/sp.py
index 01fcc5f..323f878 100644
--- a/vdsm/storage/sp.py
+++ b/vdsm/storage/sp.py
@@ -1707,6 +1707,28 @@
             image.Image(self.poolPath).multiMove(
                 srcDomUUID, dstDomUUID, imgDict, vmUUID, force)
 
+    def reconcileVolumeChain(self, sdUUID, imgUUID, leafVolUUID):
+        """
+        Determines the actual volume chain for an offline image and returns it.
+        If the actual chain differs from storage metadata, the metadata is
+        corrected to reflect the actual chain.
+
+        :param sdUUID: The UUID of the storage domain that contains the image.
+        :type sdUUID: UUID
+        :param imgUUID: The UUID of the image to be checked.
+        :type imgUUID: UUID
+        :param leafVolUUID: The UUID of the last known leaf volume.
+        :type leafVolUUID: UUID
+        :returns: A dict with a list of volume UUIDs in the corrected chain
+        :rtype: dict
+        """
+        imageResourcesNamespace = sd.getNamespace(sdUUID, IMAGE_NAMESPACE)
+        with rmanager.acquireResource(imageResourcesNamespace, imgUUID,
+                                      rm.LockType.exclusive):
+            img = image.Image(self.poolPath)
+            chain = img.reconcileVolumeChain(sdUUID, imgUUID, leafVolUUID)
+        return dict(volumes=chain)
+
     def mergeSnapshots(self, sdUUID, vmUUID, imgUUID, ancestor, successor,
                        postZero):
         """


-- 
To view, visit http://gerrit.ovirt.org/32328
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia8927a42d2bc9adcf7c6b26babb327a00e91e49e
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>


More information about the vdsm-patches mailing list