Adam Litke has posted comments on this change.

Change subject: get the status of core dump
......................................................................


Patch Set 8: Code-Review-1

(2 comments)

....................................................
File vdsm/vm.py
Line 1349:         finally:
Line 1350:             self._guestCpuLock.release()
Line 1351: 
Line 1352:     def coreDumpStatus(self):
Line 1353:         return self._coredumpThread.getStat()
This function should really be named self._coredumpThread.getStatus


....................................................
File vdsm_cli/vdsClient.py
Line 1683: 
Line 1684:     def do_coreDumpStat(self, args):
Line 1685:         vmId = args[0]
Line 1686:         response = self.s.coreDumpStatus(vmId)
Line 1687:         return response['status']['code'], 
response['status']['message']
It looks like you are overriding the 'status' member to return the dump status. 
 In general you should not do this.  This command's job is to retrieve the 
status of an asynchronous operation and return it.  So the 'status' member 
should indicate whether the thread status could be retrieved and another field 
is needed to deliver the actual retrieved core dump status.

For example, this command should return success even if the retrieved core dump 
status is a failure.
Line 1688: 
Line 1689:     def coreDump(self, args):
Line 1690:         dumpParams = {'bypass-cache': False,
Line 1691:                       'memory-only': False}


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5d552db4dbd88762950ec5a113a25c13b73319c8
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: ShaoHe Feng <[email protected]>
Gerrit-Reviewer: Adam Litke <[email protected]>
Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Better Saggi <[email protected]>
Gerrit-Reviewer: Itamar Heim <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
Gerrit-Reviewer: ShaoHe Feng <[email protected]>
Gerrit-Reviewer: Shu Ming <[email protected]>
Gerrit-Reviewer: Vinzenz Feenstra <[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