[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.
Megha Smriti
mesmriti at linux.vnet.ibm.com
Tue Dec 29 09:41:11 UTC 2015
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
> > ---
> > 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
> >
> > 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
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ovirt.org/pipermail/kimchi-devel/attachments/20151229/30a20478/attachment.html>
More information about the Kimchi-devel
mailing list