
On 17/11/2015 11:54, mesmriti@linux.vnet.ibm.com wrote:
From: Megha Smriti <mesmriti@linux.vnet.ibm.com>
Please, add an appropriated commit message explaining what this patch is for.
--- src/wok/plugins/gingerbase/model/debugreports.py | 103 ++++++++++++++--------- 1 file changed, 61 insertions(+), 42 deletions(-)
diff --git a/src/wok/plugins/gingerbase/model/debugreports.py b/src/wok/plugins/gingerbase/model/debugreports.py index 94ab7fe..9c325ef 100644 --- a/src/wok/plugins/gingerbase/model/debugreports.py +++ b/src/wok/plugins/gingerbase/model/debugreports.py @@ -19,13 +19,13 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
-import fnmatch import glob import logging import os import shutil import subprocess import time +import re
from wok.exception import InvalidParameter, NotFoundError, OperationFailed from wok.exception import WokException @@ -75,50 +75,19 @@ class DebugReportsModel(object): def log_error(e): log = logging.getLogger('Model') log.warning('Exception in generating debug file: %s', e) - try: - command = ['sosreport', '--batch', '--name=%s' % name] - output, error, retcode = run_command(command) - - if retcode != 0: - raise OperationFailed("GGBDR0003E", {'name': name, - 'err': retcode}) - - # SOSREPORT might create file in /tmp or /var/tmp - # FIXME: The right way should be passing the tar.xz file directory - # though the parameter '--tmp-dir', but it is failing in Fedora 20 - patterns = ['/tmp/sosreport-%s-*', '/var/tmp/sosreport-%s-*'] - reports = [] - reportFile = None - for p in patterns: - reports = reports + [f for f in glob.glob(p % name)] - for f in reports: - if not fnmatch.fnmatch(f, '*.md5'): - reportFile = f - break - # Some error in sosreport happened - if reportFile is None: - wok_log.error('Debug report file not found. See sosreport ' - 'output for detail:\n%s', output) - fname = (patterns[0] % name).split('/')[-1] - raise OperationFailed('GGBDR0004E', {'name': fname}) - - md5_report_file = reportFile + '.md5' - report_file_extension = '.' + reportFile.split('.', 1)[1] + # Sosreport collection + sosreport_file = sosreport_collection(name) + md5_report_file = sosreport_file + '.md5'
+ report_file_extension = '.' + sosreport_file.split('.', 1)[1] path = config.get_debugreports_path() - target = os.path.join(path, name + report_file_extension) - # Moving report - msg = 'Moving debug report file "%s" to "%s"' % (reportFile, - target) - wok_log.info(msg) - shutil.move(reportFile, target) - # Deleting md5 - msg = 'Deleting report md5 file: "%s"' % (md5_report_file) + sosreport_target = os.path.join(path, + name + report_file_extension) + msg = 'Moving debug report file "%s" to "%s"' \ + % (sosreport_file, sosreport_target)
Why do you need to move the file if you use the --tmp-dir to collect and save the sosreport in the right location?
wok_log.info(msg) - with open(md5_report_file) as f: - md5 = f.read().strip() - wok_log.info('Md5 file content: "%s"', md5) - os.remove(md5_report_file)
+ shutil.move(sosreport_file, sosreport_target)
This is not needed anymore, right? As we are using the --tmp-dir from now on.
+ delete_the_sosreport_md5_file(md5_report_file) cb('OK', True) return
@@ -213,3 +182,53 @@ class DebugReportContentModel(object):
def lookup(self, name): return self._debugreport.lookup(name) + + +def delete_the_sosreport_md5_file(md5_file): + """ + Deleting md5 file and displaying the contents of the same. + """ + msg = 'Deleting report md5 file: "%s"' % md5_file + wok_log.info(msg) + with open(md5_file) as f: + md5 = f.read().strip() + wok_log.info('Md5 file content: "%s"', md5) + os.remove(md5_file) + + +def sosreport_collection(name):
+ """ + Code for the collection of sosreport n the path + /var/tmp as specified in the command. + """
This comment is not up to date
+ path_sosreport = '/var/tmp/' + command = ['sosreport', '--batch', '--name=%s' % name, + '--tmp-dir=%s' % path_sosreport] + output, error, retcode = run_command(command) + if retcode != 0: + raise OperationFailed("GGBDR0003E", {'name': name, + 'err': retcode}) +
+ # SOSREPORT might create file in /tmp or /var/tmp + # FIXME: The right way should be passing the tar.xz file directory + # though the parameter '--tmp-dir', but it is failing in Fedora 20
You can remove this comment.
+ sosreport_name = name.replace('_', '')
Do not change the user input without letting the user know about it. As I said before, we should not allow user to enter underscore in the debug report name.
+ sosreport_name_regex = '(\s+)(' + path_sosreport + \ + ')(sosreport-' +\ + sosreport_name + '-\d+.tar.xz)' + sosreport_file = None + output = output.splitlines() + for line in output: + if line: + matched_name = re.match(sosreport_name_regex, line) + if matched_name: + path = matched_name.groups()[1] + fname = matched_name.groups()[2] + sosreport_file = path + fname + break
You don't need that code anymore while using --tmp-dir The file will be <tmp-dir>/sosreport-<name>.tar.gz
+ # Some error in sosreport happened + if sosreport_file is None: + wok_log.error('Debug report file not found. See sosreport ' + 'output for detail:\n%s', output) + raise OperationFailed('GGBDR0004E', {'name': name}) + return sosreport_file