Shu Ming has posted comments on this change.
Change subject: dump the core of a VM
......................................................................
Patch Set 8: I would prefer that you didn't submit this
(3 inline comments)
....................................................
File vdsm/API.py
Line 278: try:
Line 279: if k not in dumpParams or type(v) is not bool:
Line 280: raise ValueError("Core dump: Invalid argument:
%s=%s" %
Line 281: (k, v))
Line 282: except ValueError, e:
I don't think you need a exception handling flow here. It is more clear to
have:
if k not in dumpParams or type(v) is not bool:
self.log.exception("Core dump failed. " + e.message,
exc_info=True)
return reportError(msg=e.message)
Line 283: self.log.exception("Core dump failed. " + e.message,
Line 284: exc_info=True)
Line 285: return reportError(msg=e.message)
Line 286:
....................................................
File vdsm/vm.py
Line 299: self._mode = "memory" \
Line 300: if kwargs.get('memory-only') is True else "core"
Line 301: self.dumpflag = 0
Line 302: self.status = {'status': {'code': 0, 'message': 'noJob'}}
Line 303: threading.Thread.__init__(self)
It is better to move the threading.Thread.__init__() to the beginning of
DoCoreDumpThread.__init__()
Line 304:
Line 305: def getStat(self):
Line 306: """
Line 307: Get the status of the dump.
Line 1334: # even if the vm status is "Down", we still need to do
core dump
Line 1335: # we do not care "Down"
Line 1336: self._doCoredumpThread =
self.DoCoreDumpThreadClass(self, to,
Line 1337:
**params)
Line 1338: self._doCoredumpThread.start()
If I understand correctly, _doCoredumpThread.start() will launch another thread
to do the core dump. So the result of the following getStat() call is
volatile. ie., the result can be any of the state in "ongoing", "finished",
"cancelled" &etc. The result depends on the timing.
Line 1339: check = self._doCoredumpThread.getStat()
Line 1340: return check
Line 1341: except Exception, e:
Line 1342: self.log.exception("Failed to perform core dump. " +
e.message,
--
To view, visit http://gerrit.ovirt.org/7329
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If4aac9e747dc7aa64a6ff5ef256a7a4375aa2bb5
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: ShaoHe Feng <[email protected]>
Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Mark Wu <[email protected]>
Gerrit-Reviewer: Michal Skrivanek <[email protected]>
Gerrit-Reviewer: Ryan Harper <[email protected]>
Gerrit-Reviewer: ShaoHe Feng <[email protected]>
Gerrit-Reviewer: Shu Ming <[email protected]>
Gerrit-Reviewer: Vinzenz Feenstra <[email protected]>
Gerrit-Reviewer: Zhou Zheng Sheng <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
