Adam Litke has posted comments on this change.

Change subject: Live Merge: Ignore libvirt block job events
......................................................................


Patch Set 3:

(3 comments)

http://gerrit.ovirt.org/#/c/30046/3/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 4524:                 if response['status']['code']:
Line 4525:                     return response
Line 4526: 
Line 4527:             # Remove any Live Merge cleanup threads
Line 4528:             for jobInfo in self.conf['_blockJobs'].values():
> I don't think that self.conf is the place where to save this information (t
It seemed like a good place to me because this is where the rest of the job 
stuff is stored.  Open to hear other suggestions though.
Line 4529:                 if 'cleanupThread' in jobInfo:
Line 4530:                     jobInfo['cleanupThread'].stop()
Line 4531:                     jobInfo['cleanupThread'].join()
Line 4532: 


Line 5517:         self.vm = vm
Line 5518:         self.jobId = jobId
Line 5519:         self.drive = drive
Line 5520:         self.mode = mode
Line 5521:         self.running = True
> Please take a look at domain monitor. Using an event is better.
Done
Line 5522: 
Line 5523:     def run(self):
Line 5524:         while self.running:
Line 5525:             self.vm.log.info("Starting live merge cleanup for job 
%s",


Line 5532:                 break
Line 5533:             else:
Line 5534:                 self.vm.log.warning("live merge cleanup failed for 
job %s "
Line 5535:                                     "will retry", self.jobId)
Line 5536:                 time.sleep(10)
> If you use an event you can use wait here.
Done
Line 5537:         self.vm.log.debug("LiveMergeCleanupThread ending for job %s",
Line 5538:                           self.jobId)
Line 5539: 
Line 5540:     def stop(self):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibbcdca4c0c0e45e9323ecfef9ce2fce10d8451a5
Gerrit-PatchSet: 3
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: [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

Reply via email to