The same I commented regarding the commit message. On 02/11/2015 10:03, mesmriti@linux.vnet.ibm.com wrote:
From: Megha Smriti <mesmriti@linux.vnet.ibm.com>
--- src/wok/plugins/gingerbase/i18n.py | 2 + src/wok/plugins/gingerbase/model/debugreports.py | 80 +++++++++++++++++++++--- 2 files changed, 72 insertions(+), 10 deletions(-)
diff --git a/src/wok/plugins/gingerbase/i18n.py b/src/wok/plugins/gingerbase/i18n.py index 01573b0..e19b997 100644 --- a/src/wok/plugins/gingerbase/i18n.py +++ b/src/wok/plugins/gingerbase/i18n.py @@ -36,6 +36,8 @@ messages = { "GGBDR0007E": _("Debug report name must be a string. Only letters, digits, underscore ('_') and " "hyphen ('-') are allowed."), "GGBDR0008E": _("The debug report with specified name \"%(name)s\" already exists. Please use another one."), + "GGBDR0009E": _("Unable to create dbginfo report with %(retcode)s. Details: %(err)s"),
+ "GGBDR0010E": _("Unable to tar the final debug report tar file with %(retcode)s. Details: %(error)s"),
"Unable to compress the final debug report data due error %(retcode)s. Details: %(error)s"
"GGBHOST0001E": _("Unable to shutdown host machine as there are running virtual machines"), "GGBHOST0002E": _("Unable to reboot host machine as there are running virtual machines"), "GGBHOST0005E": _("When specifying CPU topology, each element must be an integer greater than zero."), diff --git a/src/wok/plugins/gingerbase/model/debugreports.py b/src/wok/plugins/gingerbase/model/debugreports.py index 0bb36fe..719e460 100644 --- a/src/wok/plugins/gingerbase/model/debugreports.py +++ b/src/wok/plugins/gingerbase/model/debugreports.py @@ -25,6 +25,7 @@ import os import shutil import subprocess import time +import platform import re
from wok.exception import InvalidParameter, NotFoundError, OperationFailed @@ -108,16 +109,75 @@ class DebugReportsModel(object): raise OperationFailed('GGBDR0004E', {'name': name}) md5_report_file = sosreport_file + '.md5' report_file_extension = '.' + sosreport_file.split('.', 1)[1] - path = config.get_debugreports_path() - sosreport_target = os.path.join(path, - name + report_file_extension) - msg = 'Moving debug report file "%s" to "%s"' \ - % (sosreport_file, sosreport_target) - wok_log.info(msg) - shutil.move(sosreport_file, sosreport_target) - delete_the_sosreport_md5_file(md5_report_file) - cb('OK', True) - return + # If the platform is a system Z machine. + if platform.machine().startswith('s390'):
I suggest to create a new function to handle the sosreport generation for s390x. Just to make the code easier to read.
+ dbgreport_regex = '(\S+\s+)(\/\w+\/\w+-[\d+]{4}-[\d+]{2}' \ + '-[\d+]{2}-[\d+]{2}-[\d+]{2}' \ + '-[\d+]{2}-\w+-\d+\S+)(\s+\S+)' + command = ['/usr/sbin/dbginfo.sh']
Is there a parameter like '--tmp-dir' from sosreport to use with dbginfo.sh? What about the report name? Is it auto generated?
+ output, error, retcode = run_command(command) + if retcode != 0: + raise OperationFailed("GGBDR0009E", + {'retcode': retcode, 'err': error}) + output = output.splitlines() + for line in output: + line = line.strip() + n = re.match(dbgreport_regex, line) + if n: + dbginfo_report = n.groups()[1] + break
+ pathis = '/var/tmp/' + file_extension_dbginfo = dbginfo_report.split('/', 2)[2] + target_report_file = os.path.join(pathis, + file_extension_dbginfo) + msg = 'Moving the "%s" to "%s"'\ + % (dbginfo_report, target_report_file) + shutil.move(dbginfo_report, target_report_file)
So the report is generated somewhere and moved to /var/tmp. Is that correct? Why do you need to move it to /var/tmp?
+ wok_log.info(msg)
+ final_tar_report_name = name + report_file_extension
In the code below you are creating the tar.gz file. So why are you using an extension variable? It should correspond to the tar command you are using below.
+ if dbginfo_report is not None: + sosreport_tar = sosreport_file.split('/', 3)[3] + dbginfo_tar = dbginfo_report.split('/', 2)[2] + msg = 'Zipping the sosreport and debug info files into ' \ + 'final report file' + wok_log.info(msg) + command = ['tar', '-cvzf', '%s' % final_tar_report_name, + '-C', '/var/tmp/', dbginfo_tar, sosreport_tar]
Are you mixing sosreport with dbginfo? Is sosreport still present in s390x systems? Is the debug report a collection of sosreport + dbginfo? Sorry, I am newbie with s390x tools, so it'd be good if you explain what you are going to do here.
+ output, error, retcode = run_command(command) + if retcode != 0: + raise OperationFailed("GGBDR0010E", + {'retcode': retcode, + 'error': error}) + path = config.get_debugreports_path() + dbg_target = os.path.join(path, + name + report_file_extension) + # Moving report + msg = 'Moving final debug report file "%s" to "%s"' % \ + (final_tar_report_name, dbg_target) + wok_log.info(msg) + shutil.move(final_tar_report_name, dbg_target) + delete_the_sosreport_md5_file(md5_report_file) + msg = 'Deleting the dbginfo file "%s" ' \ + % target_report_file + wok_log.info(msg) + os.remove(target_report_file) + msg = 'Deleting the sosreport file "%s" ' % sosreport_file + wok_log.info(msg) + os.remove(sosreport_file) + wok_log.info('The debug report file has been moved') + cb('OK', True) + return
+ else: + path = config.get_debugreports_path() + sosreport_target = os.path.join(path, + name + report_file_extension) + msg = 'Moving debug report file "%s" to "%s"' \ + % (sosreport_file, sosreport_target) + wok_log.info(msg) + shutil.move(sosreport_file, sosreport_target) + delete_the_sosreport_md5_file(md5_report_file) + cb('OK', True) + return
except WokException as e: log_error(e)