Adam Litke has posted comments on this change.
Change subject: dump the core of a VM
......................................................................
Patch Set 17:
(9 comments)
http://gerrit.ovirt.org/#/c/7329/17/client/vdsClient.py
File client/vdsClient.py:
Line 1760: try:
Line 1761: return {"true": True, "false": False}[v]
Line 1762: except KeyError:
Line 1763: raise ValueError("Option '%s' must be
'true' or 'false'" % k)
Line 1764:
I think we can use utils.tobool and replace most if not all of this
helper.
Done
Line 1765: vmId = args[0]
Line 1766: coreFile = args[1]
Line 1767: postAction = None
Line 1768: bypassCache = False
http://gerrit.ovirt.org/#/c/7329/17/vdsm/API.py
File vdsm/API.py:
Line 283: if postAction and postAction not in ("live",
"crash", "reset"):
Line 284: msg = "Invalid value '%s' for postAction" %
postAction
Line 285: return reportError(msg)
Line 286: if not isinstance(bypassCache, bool):
Line 287: return reportError("boolean value expected for
bypassCache")
Do we really need type checking here? I mean, why isn't enough to
check for
Done
Line 288: if not isinstance(memoryOnly, bool):
Line 289: return reportError("boolean value expected for
memoryOnly")
Line 290:
Line 291: v = self._cif.vmContainer.get(self._UUID)
Line 285: return reportError(msg)
Line 286: if not isinstance(bypassCache, bool):
Line 287: return reportError("boolean value expected for
bypassCache")
Line 288: if not isinstance(memoryOnly, bool):
Line 289: return reportError("boolean value expected for
memoryOnly")
ditto
Done
Line 290:
Line 291: v = self._cif.vmContainer.get(self._UUID)
Line 292: if not v:
Line 293: return errCode['noVM']
http://gerrit.ovirt.org/#/c/7329/17/vdsm/vm.py
File vdsm/vm.py:
Line 429: ERROR = "error"
Line 430:
Line 431: def __init__(self, vm, to='', postAction=None, bypassCache=False,
Line 432: memoryOnly=False):
Line 433: threading.Thread.__init__(self)
super(CoreDumpThread, self).__init__()
Done
Line 434: self.log = vm.log
Line 435: self._vm = vm
Line 436: self.dumpfile = to
Line 437: self.flags = 0
Line 451: """
Line 452: if self.isAlive():
Line 453: return self.status
Line 454: else:
Line 455: status = self.status
Maybe a little helper which resets the status would led to clearer
code her
Done
Line 456: self.status = {'code': CoreDumpThread.Status.NONE,
Line 457: 'message': 'No core dump operation'}
Line 458: return status
Line 459:
Line 2949: return errCode['exist']
Line 2950: if self.isCoreDumpActive():
Line 2951: self.log.warning('VM is performing a core dump, '
Line 2952: 'cannot perform migration.')
Line 2953: return errCode['exist']
I think this errCode is misleading here. errCode 61 (added by this
change)
Done
Line 2954: if self.hasTransientDisks():
Line 2955: return errCode['transientErr']
Line 2956: # while we were blocking, another migrationSourceThread could have
Line 2957: # taken self Down
Line 2992: self._acquireCpuLockWithTimeout()
Line 2993: try:
Line 2994: if self.isCoreDumpActive():
Line 2995: self.log.warning('Core dump is already in progress')
Line 2996: return errCode['exist']
if above applies, same here.
Done
Line 2997: if self.isMigrating():
Line 2998: self.log.warning('VM is being migrated, '
Line 2999: 'cannot perform core dump')
Line 3000: return errCode['exist']
Line 2996: return errCode['exist']
Line 2997: if self.isMigrating():
Line 2998: self.log.warning('VM is being migrated, '
Line 2999: 'cannot perform core dump')
Line 3000: return errCode['exist']
and here.
here errCode['migInProgress'] is more
appropriate.
Line 3001:
Line 3002: # Perform the core dump even if VM status is "Down"
Line 3003: self._coredumpThread = CoreDumpThread(self, to, postAction,
Line 3004: bypassCache, memoryOnly)
Line 3003: self._coredumpThread = CoreDumpThread(self, to, postAction,
Line 3004: bypassCache, memoryOnly)
Line 3005: self._coredumpThread.dumpStart()
Line 3006: return {'status': doneCode}
Line 3007: except Exception as e:
Any chance to narrow down this? We already have a lot of
Well
there shouldn't be any errors possible. The only one I see is if postAction is
invalid, but API.py should filter those out for us. I'll delete the except block
altogether.
Line 3008: self.log.error("Failed to perform core dump. %s",
e.message)
Line 3009: status = self._coredumpThread.getStatus()
Line 3010: return {'status':
Line 3011: {'code':
errCode['coreDumpErr']['status']['code'],
--
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: 17
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Better Saggi <bettersaggi(a)gmail.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfediuck(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com>
Gerrit-Reviewer: Jiří Moskovčák <jmoskovc(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
Gerrit-HasComments: Yes