[Kimchi-devel] [PATCH] Fixed the comments for debugreports in report tool and removed the check for platform s390x and introduced wok_log errors in log_errors.
Daniel Henrique Barboza
dhbarboza82 at gmail.com
Tue Dec 29 11:20:52 UTC 2015
I have a few comments of my own:
On 12/29/2015 07:41 AM, Megha Smriti wrote:
> Hi Aline,
>
> Please find my comments below.
>
> Regards,
> Megha
>
>> Subject: Re: [Kimchi-devel] [PATCH] Fixed the comments for
>> debugreports in report tool and removed the check for platform s390x
>> and introduced wok_log errors in log_errors.
>> Date: Mon, 30 Nov 2015 12:03:46 -0200
>> From: Aline Manera <alinefm at linux.vnet.ibm.com>
>> To: mesmriti at linux.vnet.ibm.com, Kimchi Devel <kimchi-devel at ovirt.org>
>>
>>
>>
>> Almost there! =)
>>
>> Some comments below
>>
>> On 19/11/2015 02:56,mesmriti at linux.vnet.ibm.com wrote:
>> > From: root<root at localhost.localdomain>
>> >
>> > Signed-off-by:mesmriti at linux.vnet.ibm.com
Please remember to config git global settings in your host with your
email and name. Otherwise it'll generate patches using
root@<localhost_name>.
>> > ---
>> > src/wok/plugins/gingerbase/i18n.py | 4 +-
>> > src/wok/plugins/gingerbase/model/debugreports.py | 181 ++++++++++++++++++-----
>> > 2 files changed, 143 insertions(+), 42 deletions(-)
>> >
>> > diff --git a/src/wok/plugins/gingerbase/i18n.py b/src/wok/plugins/gingerbase/i18n.py
>> > index af75c70..6ff783b 100644
>> > --- a/src/wok/plugins/gingerbase/i18n.py
>> > +++ b/src/wok/plugins/gingerbase/i18n.py
>> > @@ -36,7 +36,9 @@ 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 compress the final debug report tar file with %(retcode)s. Details: %(error)s"),
>> > + "GGBDR0011E": _("Unable to generate final debug report %(name)s. Details: %(err)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"),
>> > "GGBHOST0003E": _("There may be virtual machines running on the host"),
>> > diff --git a/src/wok/plugins/gingerbase/model/debugreports.py b/src/wok/plugins/gingerbase/model/debugreports.py
>> > index 94ab7fe..c6b60e8 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
Import order is wrong here ^
>> >
>> > from wok.exception import InvalidParameter, NotFoundError, OperationFailed
>> > from wok.exception import WokException
>> > @@ -71,54 +71,107 @@ class DebugReportsModel(object):
>> > raise OperationFailed("GGBDR0002E")
>> >
>> > @staticmethod
>> > - def sosreport_generate(cb, name):
>> > + def debugreport_generate(cb, name):
>> > def log_error(e):
>> > - log = logging.getLogger('Model')
>> > - log.warning('Exception in generating debug file: %s', e)
>> > + wok_log = logging.getLogger('Model')
>> > + wok_log.warning('Exception in generating debug file: %s', e)
>> >
>> > try:
>> > - command = ['sosreport', '--batch', '--name=%s' % name]
>> > + # Sosreport generation
>> > + sosreport_file = sosreport_collection(name)
>> > + md5_report_file = sosreport_file + '.md5'
>> > + report_file_extension = '.' + sosreport_file.split('.', 1)[1]
>> > + # If the platform is a system Z machine.
>> > + path_debugreport = '/var/tmp/'
>> > + dbgreport_regex = '(\S+\s+)(' + path_debugreport + \
>> > + 'DBGINFO-[\d+]{4}-[\d+]{2}' \
>> > + '-[\d+]{2}-[\d+]{2}-[\d+]{2}' \
>> > + '-[\d+]{2}-\w+-\d+\S+)(\s+\S+)'
>> > + command = ['/usr/sbin/dbginfo.sh', '-d', path_debugreport]
>> > 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
>> > + raise OperationFailed("GGBDR0009E",
>> > + {'retcode': retcode, 'err': error})
>>
>> > + output = output.splitlines()
>> > + dbginfo_report = None
>> > + for line in output:
>> > + line = line.strip()
>> > + n = re.match(dbgreport_regex, line)
>> > + if n:
>> > + dbginfo_report = n.groups()[1]
>> > break
>>
>> As you know where dbg report will be created, you can change the
>> dbgreport_regex to be a string pattern and use glob.
>>
>> import glob
>>
>> file = glob.glob(dgbreport_pattern)
>> if len(file) == 0:
>> # error
> [Megha:] Will incorporate this in my next patch.
>> I included I more detailed example on how use glob below.
>>
>> > - # 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]
>> > + final_tar_report_name = name + report_file_extension
>> > + sosreport_tar = sosreport_file.split('/', 3)[3]
>> > + dbginfo_tar = dbginfo_report.split('/', 3)[3]
>> > + msg = 'Compressing the sosreport and debug info files into ' \
>> > + 'final report file'
>> > + wok_log.info(msg)
>> > + # Compressing the sosreport and dbginfo reports into one
>> > + # tar file
>> > + command = ['tar', '-cvzf', '%s' % final_tar_report_name,
>> > + '-C', path_debugreport, dbginfo_tar,
>> > + sosreport_tar]
>> > + output, error, retcode = run_command(command)
>> > + if retcode != 0:
>> > + raise OperationFailed("GGBDR0010E",
>> > + {'retcode': retcode,
>> > + 'error': error})
>> > 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)
>> > + dbg_target = os.path.join(path,
>> > + name + report_file_extension)
>> > + # Moving final tar file to debugreports path
>> > + 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)
>> > + # Deleting the sosreport md5 file
>> > + delete_the_sosreport_md5_file(md5_report_file)
>> > + # Deleting the dbingo report file
>> > + msg = 'Deleting the dbginfo file "%s" ' \
>> > + % dbginfo_report
>> > wok_log.info(msg)
>> > - shutil.move(reportFile, target)
>> > - # Deleting md5
>> > - msg = 'Deleting report md5 file: "%s"' % (md5_report_file)
>> > + os.remove(dbginfo_report)
>> > + # Deleting the sosreport file
>> > + msg = 'Deleting the sosreport file "%s" ' % sosreport_file
>> > 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)
>> > + os.remove(sosreport_file)
>> > + wok_log.info('The debug report file has been moved')
>> > + cb('OK', True)
>> > + return
>> > +
>>
>> > + except WokException as e:
>> > + log_error(e)
>> > + raise
>> > +
>> > + except OSError as e:
>> > + log_error(e)
>> > + raise
>> > +
>>
>> Please, raise an appropriated exception. Otherwise, the user will not be
>> able to understand what went wrong.
>> [Megha]: As far as exceptions are concerned These are the exception I think can be thrown back from the try block and
> it is also similar to what is done in
> sosreport. Please correct me if I any suggestions.
>> > + except Exception, e:
>> > + # No need to call cb to update the task status here.
>> > + # The task object will catch the exception raised here
>> > + # and update the task status there
>> > + log_error(e)
>> > + raise OperationFailed("GGBDR0011E", {'name': name, 'err': e})
>> > +
>> > + @staticmethod
>> > + def sosreport_generate(cb, name):
>> > + def log_error(e):
>> > + wok_log = logging.getLogger('Model')
>> > + wok_log.warning('Exception in generating debug file: %s', e)
>> > + try:
>> > + # 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()
>> > + 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
>> >
>> > @@ -142,7 +195,9 @@ class DebugReportsModel(object):
>> > # Please add new possible debug report command here
>> > # and implement the report generating function
>> > # based on the new report command
>> > - report_tools = ({'cmd': 'sosreport --help',
>> > + report_tools = ({'cmd': '/usr/sbin/dbginfo.sh --help',
>> > + 'fn': DebugReportsModel.debugreport_generate},
>> > + {'cmd': 'sosreport --help',
>> > 'fn': DebugReportsModel.sosreport_generate},)
>> >
>> > # check if the command can be found by shell one by one
>> > @@ -213,3 +268,47 @@ 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.
>> > + """
>> > + 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_name_regex = '(\s+)(' + path_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:
>> > + sosreport_file = matched_name.groups()[1]
>> > + break
>> > + # 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
>>
>> You know the sosreport file was created under /var/tmp so you don't need
>> to query the command output to know the file was created or not.
>>
>> If you want to confirm the file was created you can use 'glob'
>>
>> import glob
>>
>> sosreport_pattern = path_sosreport + 'sosreport-' + name + '.tar.xz'
>> sosreport_file = glob.glob(sosreport_pattern)
>>
>> if len(sosreport_file) == 0:
>> raise OperationFailed()
>>
>> return sosreport_file[0]
>>
>> [Megha]: Will incorporate in my next patch
>> _______________________________________________
>> Kimchi-devel mailing list
>> Kimchi-devel at ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>>
>>
>>
>
>
>
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ovirt.org/pipermail/kimchi-devel/attachments/20151229/76aaee68/attachment.html>
More information about the Kimchi-devel
mailing list