Change in vdsm[master]: dump the core of a VM

shuming at linux.vnet.ibm.com shuming at linux.vnet.ibm.com
Fri Jan 18 06:18:14 UTC 2013


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 <shaohef at linux.vnet.ibm.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw at linux.vnet.ibm.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek at redhat.com>
Gerrit-Reviewer: Ryan Harper <ryanh at us.ibm.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef at linux.vnet.ibm.com>
Gerrit-Reviewer: Shu Ming <shuming at linux.vnet.ibm.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr at redhat.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list