Adam Litke has posted comments on this change. Change subject: Live Merge: work around racy libvirt pivot ......................................................................
Patch Set 1: (6 comments) https://gerrit.ovirt.org/#/c/39303/1/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 5090 Line 5091 Line 5092 Line 5093 Line 5094 > Here we know that the pivot was successful - can we update our meta data at I'd prefer not to since we could then be out of sync with libvirt for awhile. Libvirt must always be the master of this information. Line 5060: self.drive = drive Line 5061: self.doPivot = doPivot Line 5062: self.success = False Line 5063: Line 5064: def _waitForBuggyLibvirt(self): > How about _waitForUpdatedXML() of waitUntilXMLIsUpdated() Yes to moving it and I can create a tamer name for the function. I do want the name to reflect our desire to remove the function ASAP. Line 5065: # Libvirt version 1.2.8-16.el7_1.2 introduced a bug where the Line 5066: # synchronous call to blockJobAbort will return before the domain XML Line 5067: # has been updated. This makes it look like the pivot failed when it Line 5068: # actually succeeded. This means that vdsm state will not be properly Line 5072: # Line 5073: # TODO: Remove once we depend on a libvirt with this bug fixed. Line 5074: origVols = [x['volumeID'] for x in self.drive.volumeChain] Line 5075: alias = self.drive['alias'] Line 5076: while True: > Agreed. The pivot has already been reported as successful by libvirt. I am not sure what real failure scenarios are here. Also, what should we do if we bail out of here? Pause the VM? If we do nothing, we'll have lots of _highWrite failures and other unknown problems. Line 5077: chains = self.vm._driveGetActualVolumeChain([self.drive]) Line 5078: try: Line 5079: curVols = chains[alias] Line 5080: except KeyError: Line 5073: # TODO: Remove once we depend on a libvirt with this bug fixed. Line 5074: origVols = [x['volumeID'] for x in self.drive.volumeChain] Line 5075: alias = self.drive['alias'] Line 5076: while True: Line 5077: chains = self.vm._driveGetActualVolumeChain([self.drive]) > Does it return many chains or one? An array with one element is the positive case. The plural signifies it's an array. Line 5078: try: Line 5079: curVols = chains[alias] Line 5080: except KeyError: Line 5081: raise RuntimeError("Failed to retrieve volume chain for " Line 5080: except KeyError: Line 5081: raise RuntimeError("Failed to retrieve volume chain for " Line 5082: "drive:%s. Pivot failed.") Line 5083: if len(curVols) < len(origVols): Line 5084: # The merged volume has now been removed from the chain > Lets log here that xml was updated? ok. Line 5085: break Line 5086: Line 5087: self.vm.log.debug("Waiting for libvirt to update the XML after " Line 5088: "pivot of drive %s completed", alias) Line 5084: # The merged volume has now been removed from the chain Line 5085: break Line 5086: Line 5087: self.vm.log.debug("Waiting for libvirt to update the XML after " Line 5088: "pivot of drive %s completed", alias) > Lets log this once at the start ok Line 5089: time.sleep(1) Line 5090: Line 5091: def tryPivot(self): Line 5092: # We call imageSyncVolumeChain which will mark the current leaf -- To view, visit https://gerrit.ovirt.org/39303 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1e794622baf66c75cbe583be03a7b9a4a7e4883d Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <[email protected]> Gerrit-Reviewer: Adam Litke <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Greg Padgett <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
