Federico Simoncelli has posted comments on this change.

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


Patch Set 2: Code-Review-1

(1 comment)

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

Line 5627:                 liveInfo = None
Line 5628:                 try:
Line 5629:                     liveInfo = self._dom.blockJobInfo(drive.name, 0)
Line 5630:                 except libvirt.libvirtError:
Line 5631:                     self.log.exception("Error getting block job 
info")
> You're right.  I tend to miss the comments when not accompanied by a -1.  I
1) then you should probably rise and not continue?

2) you can still have a class but use target= (check the domain monitor), 
anyway this is not really urgent

3) how do you stop the thread if the vm crashes/disappears? could it be that we 
keep having this thread that tries to modify the metadata even after the vm 
died and maybe it was even restarted on another host? (that it's really 
problematic)
Line 5632: 
Line 5633:                 if liveInfo:
Line 5634:                     entry['bandwidth'] = liveInfo['bandwidth']
Line 5635:                     entry['cur'] = str(liveInfo['cur'])


-- 
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: 2
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