ShaoHe Feng has uploaded a new change for review.
Change subject: dump the core of a domain ......................................................................
dump the core of a domain
libvirt support an API to dump the core of a domain on a given file for analysis when guest OS crash.
There are two kind of dump files. one is QEMU suspend to disk image. the other is core file which like kdump file butcontains registers' value.
It's helpful to support by VDSM to find root cause if a guest gets hang and kdump isn't set up in it. This would be a good RAS feature.
Here's the definition of the new API: coreDump: This method will dump the core of a domain on a given file for analysis.
Input parameter: vmId - VM UUID to - the core file named by the user flags - defined in libvirt.py VIR_DUMP_CRASH VIR_DUMP_LIVE VIR_DUMP_BYPASS_CACHE VIR_DUMP_RESET VIR_DUMP_MEMORY_ONLY
Return value: success: return doneCode failure: return errCode including underlying libvirt error message.
Change-Id: If4aac9e747dc7aa64a6ff5ef256a7a4375aa2bb5 Signed-off-by: ShaoHe Feng shaohef@linux.vnet.ibm.com --- M vdsm/API.py M vdsm/BindingXMLRPC.py M vdsm/define.py M vdsm/libvirtvm.py M vdsm_cli/vdsClient.py 5 files changed, 80 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/29/7329/1
diff --git a/vdsm/API.py b/vdsm/API.py index 19cbb42..e2b24cb 100644 --- a/vdsm/API.py +++ b/vdsm/API.py @@ -244,6 +244,12 @@ self.log.debug("Error creating VM", exc_info=True) return errCode['unexpected']
+ def coreDump(self, to, flags): + v = self._cif.vmContainer.get(self._UUID) + if not v: + return errCode['noVM'] + return v.coreDump(to, flags) + def desktopLock(self): """ Lock user session in guest operating system using guest agent. diff --git a/vdsm/BindingXMLRPC.py b/vdsm/BindingXMLRPC.py index cc5300f..be71e6a 100644 --- a/vdsm/BindingXMLRPC.py +++ b/vdsm/BindingXMLRPC.py @@ -208,6 +208,10 @@ vm = API.VM(vmId) return vm.cont()
+ def vmCoreDump(self, vmId, to, flags): + vm = API.VM(vmId) + return vm.coreDump(to, flags) + def vmReset(self, vmId): vm = API.VM(vmId) return vm.reset() @@ -725,6 +729,7 @@ (self.getVMList, 'list'), (self.vmPause, 'pause'), (self.vmCont, 'cont'), + (self.vmCoreDump, 'coreDump'), (self.vmSnapshot, 'snapshot'), (self.vmMerge, 'merge'), (self.vmMergeStatus, 'mergeStatus'), diff --git a/vdsm/define.py b/vdsm/define.py index 31deb4f..1fedac5 100644 --- a/vdsm/define.py +++ b/vdsm/define.py @@ -114,6 +114,10 @@ 'mergeErr': {'status': {'code': 52, 'message': 'Merge failed'}}, + 'coreDumpErr': {'status': + {'code': 54, + 'message': + 'Failed to get coreDump file'}}, 'recovery': {'status': {'code': 99, 'message': diff --git a/vdsm/libvirtvm.py b/vdsm/libvirtvm.py index 4554fee..cbd9f96 100644 --- a/vdsm/libvirtvm.py +++ b/vdsm/libvirtvm.py @@ -1904,6 +1904,27 @@
self.saveState()
+ def coreDump(self, to, flags): + + def reportError(key='coreDumpErr', msg=None): + self.log.error("get coreDump failed", exc_info=True) + if msg == None: + error = errCode[key] + else: + error = {'status' : {'code': errCode[key] \ + ['status']['code'], 'message': msg}} + return error + + if self._dom == None: + return reportError() + try: + self._dom.coreDump(to, flags) + except libvirt.libvirtError, e: + if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN: + return reportError(key='noVM') + return reportError(msg=e.message) + return {'status': doneCode} + def changeCD(self, drivespec): return self._changeBlockDev('cdrom', 'hdc', drivespec)
diff --git a/vdsm_cli/vdsClient.py b/vdsm_cli/vdsClient.py index eeb7c95..cdcd3a8 100644 --- a/vdsm_cli/vdsClient.py +++ b/vdsm_cli/vdsClient.py @@ -1589,6 +1589,33 @@
return status['status']['code'], status['status']['message']
+ def coreDump(self, args): + DUMPFLAGS = {'crash': 1 << 0, + 'live': 1 << 1, + 'bypass-cache': 1 << 2, + 'reset': 1 << 3, + 'memory-only': 1 << 4} + flags = 0 + vmId = args[0] + coreFile = args[1] + params = {} + if len(args) > 2: + for arg in args[2:]: + kv = arg.split('=', 1) + if len(kv) < 2: + params[kv[0]] = "True" + else: + params[kv[0]] = kv[1] + for k, v in params.items(): + if v.lower() == "true" or not v: + try: + flags = flags + DUMPFLAGS[k] + except KeyError: + print "unrecognized optoin %s for cormDump command" % k + response = self.s.coreDump(vmId, coreFile, flags) + return response['status']['code'], response['status']['message'] + + if __name__ == '__main__': if _glusterEnabled: serv = ge.GlusterService() @@ -2239,6 +2266,23 @@ ('<vmId> <sdUUID> <imgUUID> <baseVolUUID> <volUUID>', "Take a live snapshot" )), + 'coreDump': (serv.coreDump, + ('<vmId> <file> [live=<True>] ' + '[crash=<True>] [bypass-cache=<True>] ' + '[reset=<True>] [memory-only=<True>]', + "get memeory dump or migration file" + 'optional params:', + 'crash: crash the domain after core dump' + 'default False', + 'live: perform a live core dump if supported, ' + 'default False', + 'bypass-cache: avoid file system cache when saving' + 'default False', + 'reset: reset the domain after core dump' + 'default False', + "memory-only: dump domain's memory only" + 'default False' + )), } if _glusterEnabled: commands.update(ge.getGlusterCmdDict(serv))
-- To view, visit http://gerrit.ovirt.org/7329 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange Gerrit-Change-Id: If4aac9e747dc7aa64a6ff5ef256a7a4375aa2bb5 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a domain ......................................................................
Patch Set 1:
Build Successful
http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/527/ : SUCCESS
-- 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: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
ShaoHe Feng has posted comments on this change.
Change subject: dump the core of a domain ......................................................................
Patch Set 1:
VIR_DUMP_MEMORY_ONLY was support recently buy libvirt and qemu.
So to test DUMP_MEMORY_ONLY, should update libvirt above v0.9.13 version and QEMU above v1.1.0 version.
-- 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: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Zhou Zheng Sheng has posted comments on this change.
Change subject: dump the core of a domain ......................................................................
Patch Set 1:
There are two considerations.
1. How do users download the dump file on the host storage? For example, we can configure a data domain to store, then mount it on the host and put the dump files there. Do we need to provide the corresponding web UI in Engine?
2. Dump is time consuming, the server does not return until finishes the dump. Can we leverage the vdsm/storage/task.py to do asynchronous dump?
-- 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: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
ShaoHe Feng has posted comments on this change.
Change subject: dump the core of a domain ......................................................................
Patch Set 1:
Zhou Zheng Sheng
two considerations.
1. dump file path
I think Engine has better support an data domain for dump file. it can be nfs or others.
2. Dump is time consuming, the server does not return until finishes the dump. Can we leverage the vdsm/storage/task.py to do asynchronous dump?
for VIR_DUMP_MEMORY_ONLY it is synchronous dump. it will blocked until the qemu-kvm finished this dump memory.
but the default dump is asynchronous. it is a good way to consider vdsm/storage/task.py.
actually this dumo file is generated by migration. if I can also make this VDSM API coreDump as synchronous dump . which is same with run attribute of MigrationMonitorThread class in vdsm/libvirtvm.py. but I think it is not a good way to support synchronous remote call for xmlrpc. so should I support another command in order to query the process of this dump?
-- 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: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Ryan Harper has posted comments on this change.
Change subject: dump the core of a domain ......................................................................
Patch Set 2: I would prefer that you didn't submit this
If you're touching API.py, you need to also update vdsm_api/vdsmapi-schema.json
-- 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: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Zhou Zheng Sheng has posted comments on this change.
Change subject: dump the core of a domain ......................................................................
Patch Set 4: (2 inline comments)
.................................................... Commit Message Line 3: AuthorDate: 2012-08-07 17:04:58 +0800 Line 4: Commit: ShaoHe Feng shaohef@linux.vnet.ibm.com Line 5: CommitDate: 2012-09-24 19:58:06 +0800 Line 6: Line 7: dump the core of a domain Is it better to use VM instead of domain? Because in VDSM we have storage domains, this message will make other people confused. Line 8: Line 9: libvirt support an API to dump the core of a domain on a given file for Line 10: analysis when guest OS crash. Line 11:
.................................................... File vdsm/vm.py Line 303: Line 304: def _finishSuccessfully(self): Line 305: self.status = {'status': {'code': 0, 'message': Line 306: 'Dump finished sucessfully'}, Line 307: 'progress': "done"} self.status can be written from the worker thread, and can be read from the main thread. Could race condition happen on "self.status"? It seems the race condition is not a big problem here, it just affects the accuracy of the status report. And Status transitions here is only to one direction, so if the client does not get the latest status, it can get a accurate one in the next call. Line 308: Line 309: def run(self): Line 310: Line 311: def reportError(key='coreDumpErr', msg=None):
-- 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: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
ShaoHe Feng has posted comments on this change.
Change subject: dump the core of a domain ......................................................................
Patch Set 4: (2 inline comments)
.................................................... Commit Message Line 3: AuthorDate: 2012-08-07 17:04:58 +0800 Line 4: Commit: ShaoHe Feng shaohef@linux.vnet.ibm.com Line 5: CommitDate: 2012-09-24 19:58:06 +0800 Line 6: Line 7: dump the core of a domain yes, VM is clear than domain Line 8: Line 9: libvirt support an API to dump the core of a domain on a given file for Line 10: analysis when guest OS crash. Line 11:
.................................................... File vdsm/vm.py Line 303: Line 304: def _finishSuccessfully(self): Line 305: self.status = {'status': {'code': 0, 'message': Line 306: 'Dump finished sucessfully'}, Line 307: 'progress': "done"} surely, it can, so does it need a lock? only in the worker thread can write the self.status, so I use _coreDumpOngoingEvt indicate when to read self.status for main thread. Line 308: Line 309: def run(self): Line 310: Line 311: def reportError(key='coreDumpErr', msg=None):
-- 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: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Zhou Zheng Sheng has posted comments on this change.
Change subject: dump the core of a domain ......................................................................
Patch Set 4: (1 inline comment)
.................................................... File vdsm/vm.py Line 303: Line 304: def _finishSuccessfully(self): Line 305: self.status = {'status': {'code': 0, 'message': Line 306: 'Dump finished sucessfully'}, Line 307: 'progress': "done"} But _coreDumpOngoingEvt itself can be read by main thread and written by worker thread..
I tried to write a small script to simulate the race here. Seems race very rare and even it occurs, it does no harm. So lock is not need here. Line 308: Line 309: def run(self): Line 310: Line 311: def reportError(key='coreDumpErr', msg=None):
-- 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: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Antoni Segura Puimedon has posted comments on this change.
Change subject: dump the core of a domain ......................................................................
Patch Set 4: (7 inline comments)
.................................................... File vdsm_api/vdsmapi-schema.json Line 4856: # @reset: #optional Reset the domain after core dump Line 4857: # Line 4858: # @memory-only: #optional Dump domain's memory only Line 4859: # Line 4860: # Since: 4.10.0 It will probably be 4.10.4 Line 4861: # Line 4862: ## Line 4863: {'type': 'DumpParams', Line 4864: 'data': {'*crash': 'bool', '*live:': 'bool',
Line 4873: # @to: the coreDump file Line 4874: # Line 4875: # @params: coreDump parameters Line 4876: # Line 4877: # Since: 4.10.0 It will probably be 4.10.4 Line 4878: # Line 4879: # XXX: Split 'params' into direct parameters Line 4880: ## Line 4881: {'command': {'class': 'VM', 'name': 'coreDump'},
Line 4875: # @params: coreDump parameters Line 4876: # Line 4877: # Since: 4.10.0 Line 4878: # Line 4879: # XXX: Split 'params' into direct parameters As we discussed, you will probably want to drop this. Line 4880: ## Line 4881: {'command': {'class': 'VM', 'name': 'coreDump'}, Line 4882: 'data': {'to': 'str', 'params': 'DumpParams'}} Line 4883:
Line 4885: # @VM.dumpCancel: Line 4886: # Line 4887: # Cancel the currently outgoing core dump process. Line 4888: # Line 4889: # Since: 4.10.0 It will probably be 4.10.4 Line 4890: ## Line 4891: {'command': {'class': 'VM', 'name': 'dumpCancel'}} Line 4892: Line 4893: ##
Line 4894: # @VM.dumpStatus: Line 4895: # Line 4896: # Reports the state of the currently core dump process Line 4897: # Line 4898: # Since: 4.10.0 It will probably be 4.10.4 Line 4899: # Line 4900: ## Line 4901: {'command': {'class': 'VM', 'name': 'dumpStatus'}} Line 4902:
.................................................... File vdsm_cli/vdsClient.py Line 1633: vmId = args[0] Line 1634: coreFile = args[1] Line 1635: params = {} Line 1636: if len(args) > 2: Line 1637: for arg in args[2:]: You could replace this with _eqsplit if you would accept the "lazy parameters with '='. Otherwise it is fine as it is. Line 1638: kv = arg.split('=', 1) Line 1639: kv.append('') Line 1640: k, v = kv[:2] Line 1641: params[k] = v
Line 1643: params[k] = v = v.lower() Line 1644: if not v: Line 1645: params[k] = v = "true" Line 1646: if k not in dumpParams: Line 1647: raise ValueError("Invalid optoin %s" % k) typo. It should say option. Line 1648: if v not in ["true", "false"]: Line 1649: raise ValueError("Invalid argument value: %s. " Line 1650: "It should be:%s=<True|False>" Line 1651: % (v, k))
-- 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: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
ShaoHe Feng has posted comments on this change.
Change subject: dump the core of a domain ......................................................................
Patch Set 4: (4 inline comments)
.................................................... File vdsm_api/vdsmapi-schema.json Line 4873: # @to: the coreDump file Line 4874: # Line 4875: # @params: coreDump parameters Line 4876: # Line 4877: # Since: 4.10.0 agree Line 4878: # Line 4879: # XXX: Split 'params' into direct parameters Line 4880: ## Line 4881: {'command': {'class': 'VM', 'name': 'coreDump'},
Line 4875: # @params: coreDump parameters Line 4876: # Line 4877: # Since: 4.10.0 Line 4878: # Line 4879: # XXX: Split 'params' into direct parameters agree Line 4880: ## Line 4881: {'command': {'class': 'VM', 'name': 'coreDump'}, Line 4882: 'data': {'to': 'str', 'params': 'DumpParams'}} Line 4883:
.................................................... File vdsm_cli/vdsClient.py Line 1633: vmId = args[0] Line 1634: coreFile = args[1] Line 1635: params = {} Line 1636: if len(args) > 2: Line 1637: for arg in args[2:]: yes, I can accept. when I write this patch, I call _eqsplit, but there is a bug in it, submit a patch to fix _eqsplit.
but it need time to merged. so I re-write this code(line 1637-1642). Line 1638: kv = arg.split('=', 1) Line 1639: kv.append('') Line 1640: k, v = kv[:2] Line 1641: params[k] = v
Line 1643: params[k] = v = v.lower() Line 1644: if not v: Line 1645: params[k] = v = "true" Line 1646: if k not in dumpParams: Line 1647: raise ValueError("Invalid optoin %s" % k) good catch Line 1648: if v not in ["true", "false"]: Line 1649: raise ValueError("Invalid argument value: %s. " Line 1650: "It should be:%s=<True|False>" Line 1651: % (v, k))
-- 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: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 5:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/741/ (1/2)
-- 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: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 5:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/776/ (2/2)
-- 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: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 5:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/741/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/776/ : SUCCESS
-- 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: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
ShaoHe Feng has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 5: (6 inline comments)
test this patch.
http://www.ovirt.org/Vdsm_Standalone
after the vm is create and start.
dumpPath = '/tmp/test.dump'
dumpPara = {"crash": False, "live": False, "bypass-cache": False, "reset": False, "memory-only": False}
then call s.coreDump(vmId, dumpPath, dumpPara)
.................................................... File vdsm_api/vdsmapi-schema.json Line 5449: # @reset: #optional Reset the domain after core dump Line 5450: # Line 5451: # @memory-only: #optional Dump domain's memory only Line 5452: # Line 5453: # Since: 4.10.4 "return" is expected Line 5454: # Line 5455: ## Line 5456: {'type': 'DumpParams', Line 5457: 'data': {'*crash': 'bool', '*live': 'bool',
.................................................... File vdsm/libvirtvm.py Line 519: Line 520: class DoCoreDumpThread(vm.DoCoreDumpThread): Line 521: Line 522: def _setupDumpParams(self): Line 523: vm.DoCoreDumpThread._setupDumpParams(self) should this be removed? Line 524: Line 525: def _startUnderlyingDump(self): Line 526: try: Line 527: self._vm._dom.coreDump(self.dumpfile, self.dumpflag)
.................................................... File vdsm/vm.py Line 312: fileTotal, fileProcessed, _) = self._vm._dom.jobInfo() Line 313: Line 314: dataProgress = (100 - 100 * dataRemaining / dataTotal Line 315: if dataTotal else 0) Line 316: self._vm.log.info('dump status: job type: %d, dump Progress: ' this can use "self.log" directly. Line 317: 'dataRemaining = %d, dataTotal=%d, %s%% processed' Line 318: % (jobType, dataRemaining, dataTotal, dataProgress)) Line 319: if self.isAlive(): Line 320: return self.status
Line 314: dataProgress = (100 - 100 * dataRemaining / dataTotal Line 315: if dataTotal else 0) Line 316: self._vm.log.info('dump status: job type: %d, dump Progress: ' Line 317: 'dataRemaining = %d, dataTotal=%d, %s%% processed' Line 318: % (jobType, dataRemaining, dataTotal, dataProgress)) should the log be removed? Line 319: if self.isAlive(): Line 320: return self.status Line 321: else: Line 322: status = self.status
Line 319: if self.isAlive(): Line 320: return self.status Line 321: else: Line 322: status = self.status Line 323: self.status = {'status': {'code': 0, 'message': 'noJob'}} if self.status "finished", after getStat is called, it should be change to "noJob".
noJob: there is no core dump job.
finished: the core dump job is finished.
ongoing: the core dump is in process.
"memory-only" dump can not be expressed as a percentage. Line 324: return status Line 325: Line 326: def _setupDumpParams(self): Line 327: params = {"crash": libvirt.VIR_DUMP_CRASH,
Line 1278: Line 1279: def migrate(self, params): Line 1280: self._acquireCpuLockWithTimeout() Line 1281: try: Line 1282: if self.isDoingDump(): for the coredump also call migrate, so conflict with migrating. Line 1283: self.log.warning('vm is doing coredump, ' Line 1284: 'conflict with migrating') Line 1285: return errCode['exist'] Line 1286: if self.isMigrating():
-- 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: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 6:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/791/ (2/2)
-- 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: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 6:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/756/ (1/2)
-- 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: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 6:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/756/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/791/ : SUCCESS
-- 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: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Vinzenz Feenstra has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 6: I would prefer that you didn't submit this
(12 inline comments)
.................................................... File vdsm/API.py Line 276: 'reset': False, Line 277: 'memory-only': False} Line 278: for k, v in params.items(): Line 279: try: Line 280: if k not in dumpParams or v not in [False, True]: if k not in dumpParams or type(v) is not bool: Line 281: raise ValueError("Core dump: Invalid argument: %s=%s" % Line 282: (k, v)) Line 283: except ValueError, e: Line 284: return reportError(msg=e.message)
Line 283: except ValueError, e: Line 284: return reportError(msg=e.message) Line 285: Line 286: dumpParams.update(params) Line 287: if dumpParams["crash"] is True and dumpParams["live"] is True: exclusiveParams = ("live", "crash", "reset") if [dumpParams[x] for x in exclusiveParams].count(True) > 1: reportError(msg="'reset', 'crash', and 'live' are mutually exclusive.")
Less prone to errors on extension. You simply add another item to the tuple. Line 288: return reportError(msg='crash and live are mutually exclusive') Line 289: if dumpParams["crash"] is True and dumpParams["reset"] is True: Line 290: return reportError(msg='crash and reset are mutually exclusive') Line 291: if dumpParams["reset"] is True and dumpParams["live"] is True:
.................................................... File vdsm/libvirtvm.py Line 522: def _startUnderlyingDump(self): Line 523: try: Line 524: self._vm._dom.coreDump(self.dumpfile, self.dumpflag) Line 525: except libvirt.libvirtError: Line 526: raise what is the try/except block for? This doing nothing at all Line 527: Line 528: def stop(self): Line 529: try: Line 530: self._vm._dom.abortJob()
Line 528: def stop(self): Line 529: try: Line 530: self._vm._dom.abortJob() Line 531: except libvirt.libvirtError: Line 532: raise what is the try/except block for? This doing nothing at all Line 533: Line 534: Line 535: class TimeoutError(libvirt.libvirtError): Line 536: pass
.................................................... File vdsm/vm.py Line 314: dataProgress = (100 - 100 * dataRemaining / dataTotal Line 315: if dataTotal else 0) Line 316: self.log.info('dump status: job type: %d, dump Progress: ' Line 317: 'dataRemaining = %d, dataTotal=%d, %s%% processed' Line 318: % (jobType, dataRemaining, dataTotal, dataProgress)) Not sure if this is really necessary. For debug it might be relevant. Please also do not use % for formatting, but the logging supported way.
self.log.info("%d %s", 100, "foobar")
Thanks Line 319: if self.isAlive(): Line 320: return self.status Line 321: else: Line 322: status = self.status
Line 331: "memory-only": libvirt.VIR_DUMP_MEMORY_ONLY} Line 332: self.dumpflag = 0 Line 333: for k, v in self.dumpParams.items(): Line 334: if v is True: Line 335: self.dumpflag += params[k] This should use |= for flag values You never know if a new item will contain something like VIR_DUMP_COOL = VIR_DUMP_LIVE | VIR_DUMP_NEW Line 336: if self.dumpParams.get('memory-only') is True: Line 337: self._mode = "memory" Line 338: else: Line 339: self._mode = "core"
Line 344: else: Line 345: error = {'status': Line 346: {'code': errCode[key]['status']['code'], Line 347: 'message': msg}} Line 348: self.log.error("Failed to do dump. " + msg, exc_info=True) Failed to perform core dump Line 349: Line 350: return error Line 351: Line 352: def _status(self, msg=''):
Line 353: return {'status': {'code': 0, 'message': msg}} Line 354: Line 355: def run(self): Line 356: try: Line 357: self.log.debug("begine to do coredump") begin to perform core dump Line 358: if self._vm._dom is None: Line 359: self.status = self._error(key='noVM') Line 360: raise RuntimeError('do core dump error') Line 361: self._setupDumpParams()
Line 356: try: Line 357: self.log.debug("begine to do coredump") Line 358: if self._vm._dom is None: Line 359: self.status = self._error(key='noVM') Line 360: raise RuntimeError('do core dump error') This exception message is not saying anything why it failed., not sure if this is helpful. Line 361: self._setupDumpParams() Line 362: self.status = self._status(msg="ongoing") Line 363: self._startUnderlyingDump() Line 364: self.status = self._status(msg="finished")
Line 1275: self._acquireCpuLockWithTimeout() Line 1276: try: Line 1277: if self.isDoingDump(): Line 1278: self.log.warning('vm is doing coredump, ' Line 1279: 'conflict with migrating') VM is performing a core dump, cannot perform migration. Line 1280: return errCode['exist'] Line 1281: if self.isMigrating(): Line 1282: self.log.warning('vm already migrating') Line 1283: return errCode['exist']
Line 1328: if self.isDoingDump(): Line 1329: self.log.warning('vm already coredump') Line 1330: return errCode['exist'] Line 1331: if self.isMigrating(): Line 1332: self.log.warning('vm is do migrating, conflict with coredump') VM is being migrated, cannot perform core dump Line 1333: return errCode['exist'] Line 1334: Line 1335: # even if the vm status is "Down", we still need to do core dump Line 1336: # we do not care "Down"
Line 1340: check = self._doCoredumpThread.getStat() Line 1341: return check Line 1342: except Exception: Line 1343: check = self._doCoredumpThread.getStat() Line 1344: return check You're not doing anything with that exception? Not even logging it?
check = self._doCoredumpThread.getStat() return check Line 1345: finally:
-- 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: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 7:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/759/ (1/2)
-- 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: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 7:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/794/ (2/2)
-- 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: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 7:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/759/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/794/ : SUCCESS
-- 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: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
ShaoHe Feng has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 6: (12 inline comments)
.................................................... File vdsm/API.py Line 276: 'reset': False, Line 277: 'memory-only': False} Line 278: for k, v in params.items(): Line 279: try: Line 280: if k not in dumpParams or v not in [False, True]: done. Line 281: raise ValueError("Core dump: Invalid argument: %s=%s" % Line 282: (k, v)) Line 283: except ValueError, e: Line 284: return reportError(msg=e.message)
Line 283: except ValueError, e: Line 284: return reportError(msg=e.message) Line 285: Line 286: dumpParams.update(params) Line 287: if dumpParams["crash"] is True and dumpParams["live"] is True: good idea. Line 288: return reportError(msg='crash and live are mutually exclusive') Line 289: if dumpParams["crash"] is True and dumpParams["reset"] is True: Line 290: return reportError(msg='crash and reset are mutually exclusive') Line 291: if dumpParams["reset"] is True and dumpParams["live"] is True:
.................................................... File vdsm/libvirtvm.py Line 522: def _startUnderlyingDump(self): Line 523: try: Line 524: self._vm._dom.coreDump(self.dumpfile, self.dumpflag) Line 525: except libvirt.libvirtError: Line 526: raise done. remove the try. Line 527: Line 528: def stop(self): Line 529: try: Line 530: self._vm._dom.abortJob()
Line 528: def stop(self): Line 529: try: Line 530: self._vm._dom.abortJob() Line 531: except libvirt.libvirtError: Line 532: raise done. remove the try. Line 533: Line 534: Line 535: class TimeoutError(libvirt.libvirtError): Line 536: pass
.................................................... File vdsm/vm.py Line 314: dataProgress = (100 - 100 * dataRemaining / dataTotal Line 315: if dataTotal else 0) Line 316: self.log.info('dump status: job type: %d, dump Progress: ' Line 317: 'dataRemaining = %d, dataTotal=%d, %s%% processed' Line 318: % (jobType, dataRemaining, dataTotal, dataProgress)) done Line 319: if self.isAlive(): Line 320: return self.status Line 321: else: Line 322: status = self.status
Line 331: "memory-only": libvirt.VIR_DUMP_MEMORY_ONLY} Line 332: self.dumpflag = 0 Line 333: for k, v in self.dumpParams.items(): Line 334: if v is True: Line 335: self.dumpflag += params[k] done Line 336: if self.dumpParams.get('memory-only') is True: Line 337: self._mode = "memory" Line 338: else: Line 339: self._mode = "core"
Line 344: else: Line 345: error = {'status': Line 346: {'code': errCode[key]['status']['code'], Line 347: 'message': msg}} Line 348: self.log.error("Failed to do dump. " + msg, exc_info=True) done Line 349: Line 350: return error Line 351: Line 352: def _status(self, msg=''):
Line 353: return {'status': {'code': 0, 'message': msg}} Line 354: Line 355: def run(self): Line 356: try: Line 357: self.log.debug("begine to do coredump") done Line 358: if self._vm._dom is None: Line 359: self.status = self._error(key='noVM') Line 360: raise RuntimeError('do core dump error') Line 361: self._setupDumpParams()
Line 356: try: Line 357: self.log.debug("begine to do coredump") Line 358: if self._vm._dom is None: Line 359: self.status = self._error(key='noVM') Line 360: raise RuntimeError('do core dump error') done Line 361: self._setupDumpParams() Line 362: self.status = self._status(msg="ongoing") Line 363: self._startUnderlyingDump() Line 364: self.status = self._status(msg="finished")
Line 1275: self._acquireCpuLockWithTimeout() Line 1276: try: Line 1277: if self.isDoingDump(): Line 1278: self.log.warning('vm is doing coredump, ' Line 1279: 'conflict with migrating') done Line 1280: return errCode['exist'] Line 1281: if self.isMigrating(): Line 1282: self.log.warning('vm already migrating') Line 1283: return errCode['exist']
Line 1328: if self.isDoingDump(): Line 1329: self.log.warning('vm already coredump') Line 1330: return errCode['exist'] Line 1331: if self.isMigrating(): Line 1332: self.log.warning('vm is do migrating, conflict with coredump') done Line 1333: return errCode['exist'] Line 1334: Line 1335: # even if the vm status is "Down", we still need to do core dump Line 1336: # we do not care "Down"
Line 1340: check = self._doCoredumpThread.getStat() Line 1341: return check Line 1342: except Exception: Line 1343: check = self._doCoredumpThread.getStat() Line 1344: return check add a log Line 1345: finally:
-- 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: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Vinzenz Feenstra has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 7: I would prefer that you didn't submit this
(6 inline comments)
.................................................... File vdsm/API.py Line 263: *reset* - reset the domain after core dump Line 264: *memory-only* - dump domain's memory only Line 265: """ Line 266: def reportError(key='coreDumpErr', msg=None): Line 267: self.log.error("Core dump failed. " + msg, exc_info=True) self.log.exception for exception logging Line 268: error = {'status': Line 269: {'code': errCode[key]['status']['code'], Line 270: 'message': msg}} Line 271: return error
Line 276: 'reset': False, Line 277: 'memory-only': False} Line 278: for k, v in params.items(): Line 279: try: Line 280: if k not in dumpParams or v is not bool: type(v) is not bool
v is not bool would be always True even if v is True or False
Try it. Line 281: raise ValueError("Core dump: Invalid argument: %s=%s" % Line 282: (k, v)) Line 283: except ValueError, e: Line 284: return reportError(msg=e.message)
Line 286: dumpParams.update(params) Line 287: exclusiveParams = ("live", "crash", "reset") Line 288: if [dumpParams[x] for x in exclusiveParams].count(True) > 1: Line 289: reportError(msg="'reset', 'crash', and 'live' " Line 290: "are mutually exclusive.") return missing? Line 291: v = self._cif.vmContainer.get(self._UUID) Line 292: if not v: Line 293: return errCode['noVM'] Line 294: return v.coreDump(to, dumpParams)
.................................................... File vdsm_api/vdsmapi-schema.json Line 5448: # Line 5449: # @reset: #optional Reset the domain after core dump Line 5450: # Line 5451: # @memory-only: #optional Dump domain's memory only Line 5452: # I am missing the documentation that only one of crash, live and reset are allowed Line 5453: # Since: 4.10.4 Line 5454: # Line 5455: ## Line 5456: {'type': 'DumpParams',
.................................................... File vdsm/vm.py Line 344: else: Line 345: error = {'status': Line 346: {'code': errCode[key]['status']['code'], Line 347: 'message': msg}} Line 348: self.log.error("Failed to perform core dump. " + msg, why exc_info=True? I don't see a exception handler here, if this is called from the exception handlers only, then you should log it with self.log.exception
Also you're using msg which might be "None" You probably wanted to use error['status']['message'] instead. Line 349: exc_info=True) Line 350: Line 351: return error Line 352:
Line 1339: check = self._doCoredumpThread.getStat() Line 1340: return check Line 1341: except Exception, e: Line 1342: self.log.error("Failed to perform core dump. " + e.message, Line 1343: exc_info=True) self.log.exception Line 1344: check = self._doCoredumpThread.getStat() Line 1345: return check Line 1346: finally:
-- 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: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Michal Skrivanek michal.skrivanek@redhat.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 8:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/764/ (2/2)
-- 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@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Michal Skrivanek michal.skrivanek@redhat.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 8:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/799/ (1/2)
-- 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@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Michal Skrivanek michal.skrivanek@redhat.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 8:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/764/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/799/ : SUCCESS
-- 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@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Michal Skrivanek michal.skrivanek@redhat.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
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@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Michal Skrivanek michal.skrivanek@redhat.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
ShaoHe Feng has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 8: (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: agree. 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) agree. 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() yes.
it is is volatile.
so getStat need to check whether the thread is alive.
getStat only can clear self.status when thread is no alive. 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@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Michal Skrivanek michal.skrivanek@redhat.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 9:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/778/ (1/2)
-- 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: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Michal Skrivanek michal.skrivanek@redhat.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 9:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/813/ (2/2)
-- 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: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Michal Skrivanek michal.skrivanek@redhat.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 9: Fails
Build Failed
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/778/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/813/ : FAILURE
-- 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: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Michal Skrivanek michal.skrivanek@redhat.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 10:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/816/ (1/2)
-- 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: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Michal Skrivanek michal.skrivanek@redhat.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 10:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/781/ (2/2)
-- 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: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Michal Skrivanek michal.skrivanek@redhat.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 10:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/781/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/816/ : SUCCESS
-- 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: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Michal Skrivanek michal.skrivanek@redhat.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Michal Skrivanek has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 10: Looks good to me, but someone else must approve
(2 inline comments)
.................................................... File vdsm/API.py Line 283: Line 284: dumpParams.update(params) Line 285: exclusiveParams = ("live", "crash", "reset") Line 286: if [dumpParams[x] for x in exclusiveParams].count(True) > 1: Line 287: msgstr = "'reset', 'crash', and 'live' are mutually exclusive." why is the "core dump failed" prefix not here, considering line 279 Line 288: self.log.debug("Core dump failed. " + msgstr) Line 289: return reportError(msg=msgstr) Line 290: Line 291: v = self._cif.vmContainer.get(self._UUID)
.................................................... File vdsm_cli/vdsClient.py Line 2416: 'coreDump': (serv.coreDump, Line 2417: ('<vmId> <file> [live=<True|False>] ' Line 2418: '[crash=<True|False>] [bypass-cache=<True|False>] ' Line 2419: '[reset=<True|False>] [memory-only=<True|False>]', Line 2420: "get memeory dump or migration file" typo. Also you should mention the mutual exclusivity Line 2421: 'optional params:', Line 2422: 'crash: crash the domain after core dump' Line 2423: 'default False', Line 2424: 'live: perform a live core dump if supported, '
-- 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: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Michal Skrivanek michal.skrivanek@redhat.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
ShaoHe Feng has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 10: (2 inline comments)
.................................................... File vdsm/API.py Line 283: Line 284: dumpParams.update(params) Line 285: exclusiveParams = ("live", "crash", "reset") Line 286: if [dumpParams[x] for x in exclusiveParams].count(True) > 1: Line 287: msgstr = "'reset', 'crash', and 'live' are mutually exclusive." agree. "core dump failed" can indicate failure explicitly. Line 288: self.log.debug("Core dump failed. " + msgstr) Line 289: return reportError(msg=msgstr) Line 290: Line 291: v = self._cif.vmContainer.get(self._UUID)
.................................................... File vdsm_cli/vdsClient.py Line 2416: 'coreDump': (serv.coreDump, Line 2417: ('<vmId> <file> [live=<True|False>] ' Line 2418: '[crash=<True|False>] [bypass-cache=<True|False>] ' Line 2419: '[reset=<True|False>] [memory-only=<True|False>]', Line 2420: "get memeory dump or migration file" surely. It does be helpful for user to avoid the mutual exclusivity operation. Line 2421: 'optional params:', Line 2422: 'crash: crash the domain after core dump' Line 2423: 'default False', Line 2424: 'live: perform a live core dump if supported, '
-- 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: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Michal Skrivanek michal.skrivanek@redhat.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Mark Wu has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 10: I would prefer that you didn't submit this
(18 inline comments)
most of the comments are just style issues.
.................................................... File vdsm/API.py Line 257: :param to: a string designates the dump file Line 258: Line 259: :param params: a dictionary containing: Line 260: *crash* - crash the domain after core dump Line 261: *live* - perform a live core dump if supported qemu-kvm definitely supports live core dump, so you could removed 'if supported' Line 262: *bypass-cache* - avoid file system cache when saving Line 263: *reset* - reset the domain after core dump Line 264: *memory-only* - dump domain's memory only Line 265: """
Line 273: 'live': False, Line 274: 'bypass-cache': False, Line 275: 'reset': False, Line 276: 'memory-only': False} Line 277: for k, v in params.items(): iteritems() is better. Line 278: if k not in dumpParams or type(v) is not bool: Line 279: msgstr = ("Core dump failed. Invalid argument: " Line 280: "%s=%s" % (k, v)) Line 281: self.log.error(msgstr)
Line 274: 'bypass-cache': False, Line 275: 'reset': False, Line 276: 'memory-only': False} Line 277: for k, v in params.items(): Line 278: if k not in dumpParams or type(v) is not bool: better to change to isinstance(v, bool) since it supports old style class and class inheritance. Line 279: msgstr = ("Core dump failed. Invalid argument: " Line 280: "%s=%s" % (k, v)) Line 281: self.log.error(msgstr) Line 282: return reportError(msg=msgstr)
Line 281: self.log.error(msgstr) Line 282: return reportError(msg=msgstr) Line 283: Line 284: dumpParams.update(params) Line 285: exclusiveParams = ("live", "crash", "reset") for the exclusive options, you could use them as the value for a new option, like 'operation', then only of them could be given. You don't need validate the exclusivity. Line 286: if [dumpParams[x] for x in exclusiveParams].count(True) > 1: Line 287: msgstr = "'reset', 'crash', and 'live' are mutually exclusive." Line 288: self.log.debug("Core dump failed. " + msgstr) Line 289: return reportError(msg=msgstr)
.................................................... File vdsm/libvirtvm.py Line 516: if not self._preparingMigrationEvt: Line 517: raise Line 518: Line 519: Line 520: class DoCoreDumpThread(vm.DoCoreDumpThread): How about calling it CoreDumpThread? Line 521: Line 522: def dumpInfo(self): Line 523: return self._vm._dom.jobInfo() Line 524:
.................................................... File vdsm/vm.py Line 297: self._vm = vm Line 298: self.dumpfile = to Line 299: self.dumpParams = kwargs Line 300: self._mode = "memory" \ Line 301: if kwargs.get('memory-only') is True else "core" I don't understand why you need differentiate the modes and record it in vdsm? The options is specified by the client, so it should know which mode of dump file it will generate. Line 302: self.dumpflag = 0 Line 303: self.status = {'status': {'code': 0, 'message': 'noJob'}} Line 304: Line 305: def getStat(self):
Line 303: self.status = {'status': {'code': 0, 'message': 'noJob'}} Line 304: Line 305: def getStat(self): Line 306: """ Line 307: Get the status of the dump. s/bump/core dump/ Line 308: """ Line 309: (jobType, timeElapsed, _, Line 310: dataTotal, dataProcessed, dataRemaining, Line 311: memTotal, memProcessed, memRemaining,
Line 314: dataProgress = (100 - 100 * dataRemaining / dataTotal Line 315: if dataTotal else 0) Line 316: self.log.info('dump status: job type: %d, dump Progress: ' Line 317: 'dataRemaining = %d, dataTotal=%d, %s%% processed', Line 318: jobType, dataRemaining, dataTotal, dataProgress) if you don't want to add api to query the progress, then you don't need fetch the jobinfo and log it. Line 319: if self.isAlive(): Line 320: return self.status Line 321: else: Line 322: status = self.status
Line 329: "bypass-cache": libvirt.VIR_DUMP_BYPASS_CACHE, Line 330: "reset": libvirt.VIR_DUMP_RESET, Line 331: "memory-only": libvirt.VIR_DUMP_MEMORY_ONLY} Line 332: self.dumpflag = 0 Line 333: for k, v in self.dumpParams.items(): the same as before. iteritems() could be better. Line 334: if v is True: Line 335: self.dumpflag |= params[k] Line 336: if self.dumpParams.get('memory-only') is True: Line 337: self._mode = "memory"
Line 330: "reset": libvirt.VIR_DUMP_RESET, Line 331: "memory-only": libvirt.VIR_DUMP_MEMORY_ONLY} Line 332: self.dumpflag = 0 Line 333: for k, v in self.dumpParams.items(): Line 334: if v is True: you have validated the parameters in api level. So you could just use 'if v:' Line 335: self.dumpflag |= params[k] Line 336: if self.dumpParams.get('memory-only') is True: Line 337: self._mode = "memory" Line 338: else:
Line 338: else: Line 339: self._mode = "core" Line 340: Line 341: def _status(self, msg=''): Line 342: return {'status': {'code': 0, 'message': msg}} I think you don't need add this function, and you can just update the status as this: self.status['status']['message'] = msg or change the self.status = {'code': 0, 'message': 'noJob'} and update it like self.status['message'] = msg Line 343: Line 344: def run(self): Line 345: def reportError(self, key='coreDumpErr', msg=None): Line 346: if msg is None:
Line 348: else: Line 349: error = {'status': Line 350: {'code': errCode[key]['status']['code'], Line 351: 'message': msg}} Line 352: self.log.exception("Failed to perform core dump. " + msg, self.log.error? Line 353: exc_info=True) Line 354: Line 355: return error Line 356:
Line 357: try: Line 358: self.log.debug("begin to perform core dump") Line 359: if self._vm._dom is None: Line 360: raise RuntimeError('noVM') Line 361: self._setupDumpParams() you could change to all it in the __init__ , and then you don't need keep dumpParams any more. Line 362: self.status = self._status(msg="ongoing") Line 363: self._startUnderlyingDump() Line 364: self.status = self._status(msg="finished") Line 365: except libvirt.libvirtError, e:
Line 361: self._setupDumpParams() Line 362: self.status = self._status(msg="ongoing") Line 363: self._startUnderlyingDump() Line 364: self.status = self._status(msg="finished") Line 365: except libvirt.libvirtError, e: libvirt.libvirtError as e: Line 366: if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN: Line 367: self.status = reportError(key='noVM') Line 368: elif e.get_error_code() == libvirt.VIR_ERR_OPERATION_ABORTED: Line 369: self.status = self._status(msg='canceled')
Line 1317: def isDoingDump(self): Line 1318: return self._doCoredumpThread.isAlive() Line 1319: Line 1320: def dumpMode(self): Line 1321: return self._doCoredumpThread._mode why do you need this function? I don't see it's used by any function. Line 1322: Line 1323: def coreDump(self, to, params): Line 1324: self._acquireCpuLockWithTimeout() Line 1325: try:
Line 1335: # we do not care "Down" Line 1336: self._doCoredumpThread = self.DoCoreDumpThreadClass(self, to, Line 1337: **params) Line 1338: self._doCoredumpThread.start() Line 1339: check = self._doCoredumpThread.getStat() status could be a better name. and you could move this call and the call in exception into the finally block. Line 1340: return check Line 1341: except Exception, e: Line 1342: self.log.exception("Failed to perform core dump. " + e.message, Line 1343: exc_info=True)
Line 1336: self._doCoredumpThread = self.DoCoreDumpThreadClass(self, to, Line 1337: **params) Line 1338: self._doCoredumpThread.start() Line 1339: check = self._doCoredumpThread.getStat() Line 1340: return check move 'return' outside the try block, because it could not raise any exception Line 1341: except Exception, e: Line 1342: self.log.exception("Failed to perform core dump. " + e.message, Line 1343: exc_info=True) Line 1344: check = self._doCoredumpThread.getStat()
Line 1338: self._doCoredumpThread.start() 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, the same log.error? Line 1343: exc_info=True) Line 1344: check = self._doCoredumpThread.getStat() Line 1345: return check Line 1346: finally:
-- 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: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Michal Skrivanek michal.skrivanek@redhat.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
ShaoHe Feng has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 10: (16 inline comments)
.................................................... File vdsm/API.py Line 257: :param to: a string designates the dump file Line 258: Line 259: :param params: a dictionary containing: Line 260: *crash* - crash the domain after core dump Line 261: *live* - perform a live core dump if supported ok. I will check it. Line 262: *bypass-cache* - avoid file system cache when saving Line 263: *reset* - reset the domain after core dump Line 264: *memory-only* - dump domain's memory only Line 265: """
Line 273: 'live': False, Line 274: 'bypass-cache': False, Line 275: 'reset': False, Line 276: 'memory-only': False} Line 277: for k, v in params.items(): iteritems is more efficient than items for a big dict.
will use iteritems. Line 278: if k not in dumpParams or type(v) is not bool: Line 279: msgstr = ("Core dump failed. Invalid argument: " Line 280: "%s=%s" % (k, v)) Line 281: self.log.error(msgstr)
Line 274: 'bypass-cache': False, Line 275: 'reset': False, Line 276: 'memory-only': False} Line 277: for k, v in params.items(): Line 278: if k not in dumpParams or type(v) is not bool: ok. will change it. Line 279: msgstr = ("Core dump failed. Invalid argument: " Line 280: "%s=%s" % (k, v)) Line 281: self.log.error(msgstr) Line 282: return reportError(msg=msgstr)
Line 281: self.log.error(msgstr) Line 282: return reportError(msg=msgstr) Line 283: Line 284: dumpParams.update(params) Line 285: exclusiveParams = ("live", "crash", "reset") a good idea. Line 286: if [dumpParams[x] for x in exclusiveParams].count(True) > 1: Line 287: msgstr = "'reset', 'crash', and 'live' are mutually exclusive." Line 288: self.log.debug("Core dump failed. " + msgstr) Line 289: return reportError(msg=msgstr)
.................................................... File vdsm/libvirtvm.py Line 516: if not self._preparingMigrationEvt: Line 517: raise Line 518: Line 519: Line 520: class DoCoreDumpThread(vm.DoCoreDumpThread): agree Line 521: Line 522: def dumpInfo(self): Line 523: return self._vm._dom.jobInfo() Line 524:
.................................................... File vdsm/vm.py Line 297: self._vm = vm Line 298: self.dumpfile = to Line 299: self.dumpParams = kwargs Line 300: self._mode = "memory" \ Line 301: if kwargs.get('memory-only') is True else "core" it is used in the following patch. http://gerrit.ovirt.org/#/c/11130/4/vdsm/vm.py Line 302: self.dumpflag = 0 Line 303: self.status = {'status': {'code': 0, 'message': 'noJob'}} Line 304: Line 305: def getStat(self):
Line 303: self.status = {'status': {'code': 0, 'message': 'noJob'}} Line 304: Line 305: def getStat(self): Line 306: """ Line 307: Get the status of the dump. yes. Line 308: """ Line 309: (jobType, timeElapsed, _, Line 310: dataTotal, dataProcessed, dataRemaining, Line 311: memTotal, memProcessed, memRemaining,
Line 314: dataProgress = (100 - 100 * dataRemaining / dataTotal Line 315: if dataTotal else 0) Line 316: self.log.info('dump status: job type: %d, dump Progress: ' Line 317: 'dataRemaining = %d, dataTotal=%d, %s%% processed', Line 318: jobType, dataRemaining, dataTotal, dataProgress) OK I will remove it. Line 319: if self.isAlive(): Line 320: return self.status Line 321: else: Line 322: status = self.status
Line 329: "bypass-cache": libvirt.VIR_DUMP_BYPASS_CACHE, Line 330: "reset": libvirt.VIR_DUMP_RESET, Line 331: "memory-only": libvirt.VIR_DUMP_MEMORY_ONLY} Line 332: self.dumpflag = 0 Line 333: for k, v in self.dumpParams.items(): iteritems is more efficient than items for a big dict.
will use iteritems. Line 334: if v is True: Line 335: self.dumpflag |= params[k] Line 336: if self.dumpParams.get('memory-only') is True: Line 337: self._mode = "memory"
Line 338: else: Line 339: self._mode = "core" Line 340: Line 341: def _status(self, msg=''): Line 342: return {'status': {'code': 0, 'message': msg}} agree Line 343: Line 344: def run(self): Line 345: def reportError(self, key='coreDumpErr', msg=None): Line 346: if msg is None:
Line 348: else: Line 349: error = {'status': Line 350: {'code': errCode[key]['status']['code'], Line 351: 'message': msg}} Line 352: self.log.exception("Failed to perform core dump. " + msg, I'm not sure whether error or exception.
only exception, will call reportError. Line 353: exc_info=True) Line 354: Line 355: return error Line 356:
Line 357: try: Line 358: self.log.debug("begin to perform core dump") Line 359: if self._vm._dom is None: Line 360: raise RuntimeError('noVM') Line 361: self._setupDumpParams() agree. Line 362: self.status = self._status(msg="ongoing") Line 363: self._startUnderlyingDump() Line 364: self.status = self._status(msg="finished") Line 365: except libvirt.libvirtError, e:
Line 361: self._setupDumpParams() Line 362: self.status = self._status(msg="ongoing") Line 363: self._startUnderlyingDump() Line 364: self.status = self._status(msg="finished") Line 365: except libvirt.libvirtError, e: yes. Line 366: if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN: Line 367: self.status = reportError(key='noVM') Line 368: elif e.get_error_code() == libvirt.VIR_ERR_OPERATION_ABORTED: Line 369: self.status = self._status(msg='canceled')
Line 1317: def isDoingDump(self): Line 1318: return self._doCoredumpThread.isAlive() Line 1319: Line 1320: def dumpMode(self): Line 1321: return self._doCoredumpThread._mode it is used in the following patch. http://gerrit.ovirt.org/#/c/11130/4/vdsm/vm.py Line 1322: Line 1323: def coreDump(self, to, params): Line 1324: self._acquireCpuLockWithTimeout() Line 1325: try:
Line 1335: # we do not care "Down" Line 1336: self._doCoredumpThread = self.DoCoreDumpThreadClass(self, to, Line 1337: **params) Line 1338: self._doCoredumpThread.start() Line 1339: check = self._doCoredumpThread.getStat() agree. Line 1340: return check Line 1341: except Exception, e: Line 1342: self.log.exception("Failed to perform core dump. " + e.message, Line 1343: exc_info=True)
Line 1338: self._doCoredumpThread.start() 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, not sure, I need to check. Line 1343: exc_info=True) Line 1344: check = self._doCoredumpThread.getStat() Line 1345: return check Line 1346: finally:
-- 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: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Michal Skrivanek michal.skrivanek@redhat.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 11:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/973/ (2/3)
-- 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: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Michal Skrivanek michal.skrivanek@redhat.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 11:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/84/ (3/3)
-- 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: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Michal Skrivanek michal.skrivanek@redhat.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 11:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/938/ (1/3)
-- 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: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Michal Skrivanek michal.skrivanek@redhat.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 11: Fails
Build Failed
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/938/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/973/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/84/ : FAILURE
-- 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: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Michal Skrivanek michal.skrivanek@redhat.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
ShaoHe Feng has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 10: (3 inline comments)
.................................................... File vdsm/vm.py Line 357: try: Line 358: self.log.debug("begin to perform core dump") Line 359: if self._vm._dom is None: Line 360: raise RuntimeError('noVM') Line 361: self._setupDumpParams() see line 432.
It make no sense to call dumpParams in the __init__ when we instance a DoCoreDumpThread class without kwargs. Line 362: self.status = self._status(msg="ongoing") Line 363: self._startUnderlyingDump() Line 364: self.status = self._status(msg="finished") Line 365: except libvirt.libvirtError, e:
Line 1335: # we do not care "Down" Line 1336: self._doCoredumpThread = self.DoCoreDumpThreadClass(self, to, Line 1337: **params) Line 1338: self._doCoredumpThread.start() Line 1339: check = self._doCoredumpThread.getStat() about status or check, I have no prefer. same with line 1289
I have changed it to status.
about the move this call into the finally block.
I have tried. But not successfully.
same with 1294 or 1315 Line 1340: return check Line 1341: except Exception, e: Line 1342: self.log.exception("Failed to perform core dump. " + e.message, Line 1343: exc_info=True)
Line 1336: self._doCoredumpThread = self.DoCoreDumpThreadClass(self, to, Line 1337: **params) Line 1338: self._doCoredumpThread.start() Line 1339: check = self._doCoredumpThread.getStat() Line 1340: return check I have tried, it can cause exception.
How did you try?
same with line 1315 Line 1341: except Exception, e: Line 1342: self.log.exception("Failed to perform core dump. " + e.message, Line 1343: exc_info=True) Line 1344: check = self._doCoredumpThread.getStat()
-- 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: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Michal Skrivanek michal.skrivanek@redhat.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 12:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/987/ (1/3)
-- 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: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Michal Skrivanek michal.skrivanek@redhat.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 12:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/98/ (2/3)
-- 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: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Michal Skrivanek michal.skrivanek@redhat.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 12:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/952/ (3/3)
-- 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: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Michal Skrivanek michal.skrivanek@redhat.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 12: Fails
Build Failed
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/952/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/987/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/98/ : FAILURE
-- 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: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Michal Skrivanek michal.skrivanek@redhat.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 13:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/955/ (1/3)
-- 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: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Michal Skrivanek michal.skrivanek@redhat.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 13:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/990/ (2/3)
-- 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: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Michal Skrivanek michal.skrivanek@redhat.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 13:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/101/ (3/3)
-- 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: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Michal Skrivanek michal.skrivanek@redhat.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 13: Fails
Build Failed
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/955/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/990/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/101/ : FAILURE
-- 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: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Michal Skrivanek michal.skrivanek@redhat.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 14:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1324/ (1/3)
-- 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: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Michal Skrivanek michal.skrivanek@redhat.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 14:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/471/ (2/3)
-- 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: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Michal Skrivanek michal.skrivanek@redhat.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 14:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1362/ (3/3)
-- 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: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Michal Skrivanek michal.skrivanek@redhat.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 14:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1324/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1362/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/471/ : SUCCESS
-- 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: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Michal Skrivanek michal.skrivanek@redhat.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Doron Fediuck has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 14:
ShaoHe, one thing I'd like to understand; The current functionality added to vdsm of watchdog[1] already includes an option to create core dump. In what way is it different and why should we have both?
[1] http://www.ovirt.org/Sla/Watchdog_Device
-- 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: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Michal Skrivanek michal.skrivanek@redhat.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
ShaoHe Feng has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 14:
watchdog dump will generate the dump file at auto_dump_path defined in /etc/libvirt/qemu.conf.
auto_dump_path is defined in configure file, it can not be changed after the vdsm start.
And name format of dump file is vmName-EpochTime, e.g. vm1-1346088786. The name was define by libvirt, it can not be changed.
This coreDump patch, it can assign any valid dump path and name for coreDump file. It is flexible.
If the action of watchdog was not set 'dump', we still can generate coreDump file by this patch.
For this patch can generate coreDump file at any time.
-- 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: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Michal Skrivanek michal.skrivanek@redhat.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Saggi Mizrahi has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 14:
Should this be abandoned?
Doron Fediuck has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 14:
No.
I'd like to resume the work on this patch and make sure it is merged.
Doron Fediuck has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 14:
Adding some more info;
The previously mentioned watchdog is missing drivers in some Windows versions. This feature is more generic so I'd like to have it merged.
Itamar Heim has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 14:
ping
Itamar Heim has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 14:
ping
Adam Litke has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 14: Code-Review-1
(1 comment)
I propose some changes to the vdsm API for this command.
.................................................... File vdsm_api/vdsmapi-schema.json Line 5549: # Line 5550: ## Line 5551: {'command': {'class': 'VM', 'name': 'coreDump'}, Line 5552: 'data': {'to': 'str', 'params': 'DumpParams'}} Line 5553: I don't see any value in wrapping some parameters into a params dict while leaving the 'to' param on the command line. How about specifying all arguments to the command directly:
{'command': {'class': 'VM', 'name': 'coreDump'}, 5552 + 'data': {'to': 'str', 'post-action': 'CoreDumpPostAction', 'flags': 'CoreDumpFlags'}} Line 5554: ## Line 5555: # @VM.monitorCommand: Line 5556: # Line 5557: # Send a command to the qemu monitor.
Adam Litke has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 14:
I'd like to pick this up and work on it. I will need to rebase as there have been quite a few major changes since this was updated (removal of libvirtvm, etc.). Shao He: since you're pretty busy these days I am guessing you won't mind?
ShaoHe Feng has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 14:
Adam, I'm very glad that you can pick this up. This patch pending so long time.
Adam Litke has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 15: Verified+1
Vinzenz Feenstra has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 15: Code-Review-1
(6 comments)
http://gerrit.ovirt.org/#/c/7329/15/vdsm/API.py File vdsm/API.py:
Line 272: :param postAction: An action to take after the core dump Line 273: :param bypassCache: Avoid filesystem cache when saving. Line 274: :param memoryOnly: dump VM memory only (skip disks) Line 275: """ Line 276: def reportError(key='coreDumpErr', msg=None): key as a parameter here has no use, it's always coreDumpErr here and msg=None should not be by default None, as this is also always passed. And if it is None, you're passing msg as None which is not allowed (IIRC) Line 277: error = {'status': Line 278: {'code': errCode[key]['status']['code'], Line 279: 'message': msg}} Line 280: self.log.error(msg)
http://gerrit.ovirt.org/#/c/7329/15/vdsm/vm.py File vdsm/vm.py:
Line 2984: raise Line 2985: finally: Line 2986: self._guestCpuLock.release() Line 2987: Line 2988: def isDoingDump(self): isCoreDumpActive/Alive? The language implications here are a bit ambiguous, if you know what I am talking about Line 2989: return self._coredumpThread.isAlive() Line 2990: Line 2991: def coreDump(self, to, postAction, bypassCache, memoryOnly): Line 2992: self._acquireCpuLockWithTimeout()
Line 3004: bypassCache, memoryOnly) Line 3005: self._coredumpThread.dumpStart() Line 3006: return {'status': doneCode} Line 3007: except Exception, e: Line 3008: self.log.error("Failed to perform core dump. " + e.message) ("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'], Line 3012: 'message': status['message']}}
http://gerrit.ovirt.org/#/c/7329/15/vdsm_api/vdsmapi-schema.json File vdsm_api/vdsmapi-schema.json:
Line 5891: # @live: Do not interrupt the running VM Line 5892: # Line 5893: # @reset: Reboot the VM after gathering core dump Line 5894: # Line 5895: # Since: 4.10.4 bump please Line 5896: ## Line 5897: {'enum': 'CoreDumpPostAction', 'data': ['crash', 'live', 'reset']} Line 5898: Line 5899: ##
Line 5902: # dump a VM to a designated file Line 5903: # Line 5904: # @to: the coreDump file Line 5905: # Line 5906: # @postAction: #optional An action to take after the dump has been taken what's the default action if not specified? I guess this would be the right place to document this Line 5907: # Line 5908: # @bypassCache: #optional Avoid file system cache when saving Line 5909: # Line 5910: # @memoryOnly: #optional Dump VM's memory only
Line 5908: # @bypassCache: #optional Avoid file system cache when saving Line 5909: # Line 5910: # @memoryOnly: #optional Dump VM's memory only Line 5911: # Line 5912: # Since: 4.10.4 same Line 5913: # Line 5914: ## Line 5915: {'command': {'class': 'VM', 'name': 'coreDump'}, Line 5916: 'data': {'to': 'str', 'postAction': 'CoreDumpPostAction',
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 15:
Build Successful
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/6723/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/6636/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/5831/ : SUCCESS
Adam Litke has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 15:
(6 comments)
http://gerrit.ovirt.org/#/c/7329/15/vdsm/API.py File vdsm/API.py:
Line 272: :param postAction: An action to take after the core dump Line 273: :param bypassCache: Avoid filesystem cache when saving. Line 274: :param memoryOnly: dump VM memory only (skip disks) Line 275: """ Line 276: def reportError(key='coreDumpErr', msg=None):
key as a parameter here has no use, it's always coreDumpErr here and msg=No
Done Line 277: error = {'status': Line 278: {'code': errCode[key]['status']['code'], Line 279: 'message': msg}} Line 280: self.log.error(msg)
http://gerrit.ovirt.org/#/c/7329/15/vdsm/vm.py File vdsm/vm.py:
Line 2984: raise Line 2985: finally: Line 2986: self._guestCpuLock.release() Line 2987: Line 2988: def isDoingDump(self):
isCoreDumpActive/Alive? The language implications here are a bit ambiguous,
Done Line 2989: return self._coredumpThread.isAlive() Line 2990: Line 2991: def coreDump(self, to, postAction, bypassCache, memoryOnly): Line 2992: self._acquireCpuLockWithTimeout()
Line 3004: bypassCache, memoryOnly) Line 3005: self._coredumpThread.dumpStart() Line 3006: return {'status': doneCode} Line 3007: except Exception, e: Line 3008: self.log.error("Failed to perform core dump. " + e.message)
("Failed to perform core dump. %s", e.message)
Done Line 3009: status = self._coredumpThread.getStatus() Line 3010: return {'status': Line 3011: {'code': errCode['coreDumpErr']['status']['code'], Line 3012: 'message': status['message']}}
http://gerrit.ovirt.org/#/c/7329/15/vdsm_api/vdsmapi-schema.json File vdsm_api/vdsmapi-schema.json:
Line 5891: # @live: Do not interrupt the running VM Line 5892: # Line 5893: # @reset: Reboot the VM after gathering core dump Line 5894: # Line 5895: # Since: 4.10.4
bump please
Done Line 5896: ## Line 5897: {'enum': 'CoreDumpPostAction', 'data': ['crash', 'live', 'reset']} Line 5898: Line 5899: ##
Line 5902: # dump a VM to a designated file Line 5903: # Line 5904: # @to: the coreDump file Line 5905: # Line 5906: # @postAction: #optional An action to take after the dump has been taken
what's the default action if not specified? I guess this would be the right
The default action is to suspend the VM before the dump and resume it afterwords. Will document. Line 5907: # Line 5908: # @bypassCache: #optional Avoid file system cache when saving Line 5909: # Line 5910: # @memoryOnly: #optional Dump VM's memory only
Line 5908: # @bypassCache: #optional Avoid file system cache when saving Line 5909: # Line 5910: # @memoryOnly: #optional Dump VM's memory only Line 5911: # Line 5912: # Since: 4.10.4
same
Done Line 5913: # Line 5914: ## Line 5915: {'command': {'class': 'VM', 'name': 'coreDump'}, Line 5916: 'data': {'to': 'str', 'postAction': 'CoreDumpPostAction',
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 16:
Build Successful
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/6733/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/6646/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/5841/ : SUCCESS
Adam Litke has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 16: Verified+1
Jiří Moskovčák has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 16: Code-Review-1
(1 comment)
Just a minor thing, but I'd rather see it fixed, so it won't hit as later. The rest looks ok.
http://gerrit.ovirt.org/#/c/7329/16/vdsm/vm.py File vdsm/vm.py:
Line 494: self.status = {'code': CoreDumpThread.Status.CANCELED, Line 495: 'message': 'The operation has been aborted'} Line 496: else: Line 497: self.status = reportError(msg=e.message) Line 498: except Exception, e: please use "<ExceptionType> as e" everywhere Line 499: self.status = reportError(msg=e.message) Line 500: Line 501: Line 502: class VolumeError(RuntimeError):
Adam Litke has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 17: Verified+1
Rebase and fixed minor comment related to Exceptions from last patchset.
Adam Litke has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 17:
Rebase and fixed minor comment related to Exceptions from last patchset.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 17: Verified-1
Build Failed
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6014/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/6800/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/6908/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_storage_functional_tests/44/ : FAILURE
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 17:
Build Failed
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6014/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/6800/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/6908/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_storage_functional_tests_localfs/47/ : ABORTED
Francesco Romani has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 17:
(11 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. 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 bool value and we must ensure bool type? 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 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 433: ) super(CoreDumpThread, self).__init__()
would be nicer
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 here. Line 456: self.status = {'code': CoreDumpThread.Status.NONE, Line 457: 'message': 'No core dump operation'} Line 458: return status Line 459:
Line 2953: '] I think this errCode is misleading here. errCode 61 (added by this change) may be slightly better, but I think we need another explicit error code here.
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. 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. 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 except: or except Exception: in the codebase and I would *really* like to not increment the number... :\ 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'],
http://gerrit.ovirt.org/#/c/7329/17/vdsm_api/vdsmapi-schema.json File vdsm_api/vdsmapi-schema.json:
Line 5898: # @live: Do not interrupt the running VM Line 5899: # Line 5900: # @reset: Reboot the VM after gathering core dump Line 5901: # Line 5902: # Since: 4.14.2 4.15.0? Line 5903: ## Line 5904: {'enum': 'CoreDumpPostAction', 'data': ['crash', 'live', 'reset']} Line 5905: Line 5906: ##
Line 5917: # @bypassCache: #optional Avoid file system cache when saving Line 5918: # Line 5919: # @memoryOnly: #optional Dump VM's memory only Line 5920: # Line 5921: # Since: 4.14.2 same here Line 5922: # Line 5923: ## Line 5924: {'command': {'class': 'VM', 'name': 'coreDump'}, Line 5925: 'data': {'to': 'str', 'postAction': 'CoreDumpPostAction',
Francesco Romani has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 17:
(2 comments)
http://gerrit.ovirt.org/#/c/7329/17/vdsm_api/vdsmapi-schema.json File vdsm_api/vdsmapi-schema.json:
Line 5898: # @live: Do not interrupt the running VM Line 5899: # Line 5900: # @reset: Reboot the VM after gathering core dump Line 5901: # Line 5902: # Since: 4.14.2
4.15.0?
Drop this, I overlooked the tag list. Line 5903: ## Line 5904: {'enum': 'CoreDumpPostAction', 'data': ['crash', 'live', 'reset']} Line 5905: Line 5906: ##
Line 5917: # @bypassCache: #optional Avoid file system cache when saving Line 5918: # Line 5919: # @memoryOnly: #optional Dump VM's memory only Line 5920: # Line 5921: # Since: 4.14.2
same here
Of course drop this one as well. Line 5922: # Line 5923: ## Line 5924: {'command': {'class': 'VM', 'name': 'coreDump'}, Line 5925: 'data': {'to': 'str', 'postAction': 'CoreDumpPostAction',
Francesco Romani has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 17: Code-Review-1
sorry forgot to vote in the previous comment
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'],
oVirt Jenkins CI Server has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 18: Verified-1
Build Failed
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7291/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6389/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7173/ : SUCCESS
Francesco Romani has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 18:
(2 comments)
http://gerrit.ovirt.org/#/c/7329/18/vdsm/API.py File vdsm/API.py:
Line 286: 'message': msg}} Line 287: self.log.error(msg) Line 288: return error Line 289: Line 290: if postAction and postAction not in ("live", "crash", "reset"): Some constants would be better here instead of plain strings. Moreover, just checking
if postAction not in ("live", ...):
should be enough Line 291: msg = "Invalid value '%s' for postAction" % postAction Line 292: return reportError(msg) Line 293: Line 294: v = self._cif.vmContainer.get(self._UUID)
http://gerrit.ovirt.org/#/c/7329/18/vdsm/vm.py File vdsm/vm.py:
Line 481: return error Line 482: Line 483: try: Line 484: self.log.debug("About to perform core dump") Line 485: if self._vm._dom is None: How can we end up with self._vm._dom set to None? Is this just defensive programming or there is another reason? Line 486: raise RuntimeError('noVM') Line 487: self._vm._dom.coreDump(self.dumpfile, self.flags) Line 488: self.status = {'code': CoreDumpThread.Status.FINISHED, Line 489: 'message': 'The operation is finished'}
Itamar Heim has posted comments on this change.
Change subject: dump the core of a VM ......................................................................
Patch Set 18:
ping
oVirt Jenkins CI Server has posted comments on this change.
Change subject: vm: add support to dump the core of a VM ......................................................................
Patch Set 19:
Build Successful
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9030/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/721/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9814/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/738/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_storage_functional_tests_localfs_ge... : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9970/ : SUCCESS
Vinzenz Feenstra has posted comments on this change.
Change subject: vm: add support to dump the core of a VM ......................................................................
Patch Set 19: Code-Review+1
Francesco Romani has posted comments on this change.
Change subject: vm: add support to dump the core of a VM ......................................................................
Patch Set 20:
changes: - shed duplicate code by leveraging the new Vm._report{Error,Exception} methods - update version information in schema
oVirt Jenkins CI Server has posted comments on this change.
Change subject: vm: add support to dump the core of a VM ......................................................................
Patch Set 20:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9136/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/734/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9921/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/802/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/10076/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5003/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/3160/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_storage_functional_tests_localfs_ge... : There was an infra issue, please contact infra@ovirt.org
Adam Litke has posted comments on this change.
Change subject: vm: add support to dump the core of a VM ......................................................................
Patch Set 20:
(1 comment)
Just a small fix for vdsClient. Otherwise looks good and tested.
http://gerrit.ovirt.org/#/c/7329/20/client/vdsClient.py File client/vdsClient.py:
Line 1806: Line 1807: def coreDump(self, args): Line 1808: vmId = args[0] Line 1809: coreFile = args[1] Line 1810: postAction = None Should this be:
postAction = 'live'
Otherwise the code below will fail if postAction is not specified with the following error:
Error using command: cannot marshal None unless allow_none is enabled Line 1811: bypassCache = False Line 1812: memoryOnly = False Line 1813: params = {} Line 1814: if len(args) > 2:
Adam Litke has posted comments on this change.
Change subject: vm: add support to dump the core of a VM ......................................................................
Patch Set 20: Code-Review-1
Just to get attention to the previous comment.
Francesco Romani has posted comments on this change.
Change subject: vm: add support to dump the core of a VM ......................................................................
Patch Set 20:
(1 comment)
http://gerrit.ovirt.org/#/c/7329/20/client/vdsClient.py File client/vdsClient.py:
Line 1806: Line 1807: def coreDump(self, args): Line 1808: vmId = args[0] Line 1809: coreFile = args[1] Line 1810: postAction = None
Should this be:
Thanks, fixed. Line 1811: bypassCache = False Line 1812: memoryOnly = False Line 1813: params = {} Line 1814: if len(args) > 2:
Francesco Romani has posted comments on this change.
Change subject: virt: add support to dump the core of a VM ......................................................................
Patch Set 21:
changes: - fixes vdsClient as Adam pointed out - since coredump.py is a new module, make it compliant with http://wiki.ovirt.org/Vdsm_Coding_Guidelines
oVirt Jenkins CI Server has posted comments on this change.
Change subject: virt: add support to dump the core of a VM ......................................................................
Patch Set 21:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9346/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/752/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10130/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/885/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/10286/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5212/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/3370/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_storage_functional_tests_localfs_ge... : There was an infra issue, please contact infra@ovirt.org
Itamar Heim has posted comments on this change.
Change subject: virt: add support to dump the core of a VM ......................................................................
Patch Set 21:
ping
Jenkins CI RO has posted comments on this change.
Change subject: virt: add support to dump the core of a VM ......................................................................
Patch Set 21:
Abandoned due to no activity - please restore if still relevant
Jenkins CI RO has abandoned this change.
Change subject: virt: add support to dump the core of a VM ......................................................................
Abandoned
Abandoned due to no activity - please restore if still relevant
automation@ovirt.org has posted comments on this change.
Change subject: virt: add support to dump the core of a VM ......................................................................
Patch Set 21:
* Update tracker::IGNORE, no Bug-Url found
vdsm-patches@lists.fedorahosted.org