Michal Skrivanek has posted comments on this change. Change subject: Live Merge: fix race between libvirt event and vm stats ......................................................................
Patch Set 2: (1 comment) keeping "0" as it already is an improvement anyway. ButI'd love to understand why we need to "trigger" stats from outside(API call) - hence why do we need this whole locking thing. http://gerrit.ovirt.org/#/c/29309/2/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 5456: jobs = {} Line 5457: # We need to take the jobs lock here to ensure that we don't race with Line 5458: # another call to merge() where the job has been recorded but not yet Line 5459: # started. Line 5460: with self._jobsLock: so there are only 2 places where the queryBlockJobs is called - stat thread and the end of merge(). Why do we have that "trigger" in merge()? Can't we live without it? If yes then we don't need any locking here Is the concern a possibly incorrect "view" of engine the first time stats return? Couldn't this be solved on the engine side? Line 5461: for job in self.conf['_blockJobs'].values()[:]: Line 5462: jobID = job['jobID'] Line 5463: drive = self._findDriveByUUIDs(job['disk']) Line 5464: -- To view, visit http://gerrit.ovirt.org/29309 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic0314a39428419bb21bc579f07a92227d5a7acff Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <[email protected]> Gerrit-Reviewer: Adam Litke <[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: Michal Skrivanek <[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
