Change in vdsm[master]: vdsm hooks: report hook stderr to Engine if it fails an action
fromani at redhat.com
fromani at redhat.com
Wed Jul 1 13:02:35 UTC 2015
Francesco Romani has posted comments on this change.
Change subject: vdsm hooks: report hook stderr to Engine if it fails an action
......................................................................
Patch Set 10:
(5 comments)
https://gerrit.ovirt.org/#/c/42592/10/vdsm/API.py
File vdsm/API.py:
Line 360: # check for errors
Line 361: if status and 'status' in status and 'code' in status['status'] and \
Line 362: status['status']['code'] != 0:
Line 363: return status
Line 364: # if no error, return progress
this deserve a separate change. Moreover, this patch can be of further assistence: https://gerrit.ovirt.org/#/c/42795/
Line 365: return {'status': doneCode, 'migrationStats': status}
Line 366:
Line 367: def getStats(self):
Line 368: """
Line 379: try:
Line 380: hooks.before_get_vm_stats()
Line 381: except hooks.HookError as e:
Line 382: self.status = response.error('unexpected', 'Hook error ' +
Line 383: 'during migration: ' + str(e))
why assign to self.status?!
VERY minor: this formatting is IMHO a little nicer:
response.error(
'unexpected',
'Hook error during migration: ' + str(e))
Line 384:
Line 385: stats = v.getStats().copy()
Line 386: if runHooks:
Line 387: stats = hooks.after_get_vm_stats([stats])[0]
Line 586: params['vmId'] = self._UUID
Line 587: result = self.create(params)
Line 588: if result['status']['code']:
Line 589: self.log.debug('Migration create - Failed')
Line 590: return result
good change, but deserves separate patch (which I have somewehere stuck on gerrit...)
Line 591:
Line 592: v = self._cif.vmContainer.get(self._UUID)
Line 593:
Line 594: try:
Line 592: v = self._cif.vmContainer.get(self._UUID)
Line 593:
Line 594: try:
Line 595: if not v.waitForMigrationDestinationPrepare():
Line 596: return errCode['createErr']
better:
return response.error('createErr')
Line 597: except hooks.HookError as e:
Line 598: self.log.debug('Destination VM creation failed due to hook' +
Line 599: ' error:' + str(e))
Line 600: return response.error('migrateErr', 'Destination hook failed: ' +
https://gerrit.ovirt.org/#/c/42592/10/vdsm/virt/migration.py
File vdsm/virt/migration.py:
Line 300: self.status = response.error('migrateErr', 'Hook error ' +
Line 301: 'during migration: ' + str(e))
Line 302: self.status = {'status': {'code': 12,
Line 303: 'message': 'Hook error during migration: ' +
Line 304: str(e)}}
Please use name for errors, and response.error.
If you need to use the straight code (it can rarely happen when translating errors) please use response.error_raw.
In general, this is the wrong direction, I'd rather get rid of self.status entirely.
Line 305: self._recover(str(e))
Line 306: self.log.error('Hook error during migration: ' + str(e))
Line 307:
Line 308: except Exception as e:
--
To view, visit https://gerrit.ovirt.org/42592
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe2d5eb52cf2c8855d9d7d5e0ff1628a6cf1dc51
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Marcin Mirecki <mmirecki at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Feng Yang <yangfeng at cloud-times.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Marcin Mirecki <mmirecki at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list