[Kimchi-devel] [PATCH 2/2] Modified code to check platform and if its s390x then collect dbginfo tar along with sosreport and create new tar having both the above tar and move to /var/lib/kimchi/debugreports. Also added i18n error messages required for dbginfo.
Megha Smriti
mesmriti at linux.vnet.ibm.com
Wed Nov 4 07:08:39 UTC 2015
Hello Aline,
Please find my comment inline. Once I will get confirmation about the
method separation mention in my previous mail [PATCH 1/2]. I will send
you the next version patch.
Thanks,
Megha
On 11/3/2015 10:49 PM, Aline Manera wrote:
>
> The same I commented regarding the commit message.
>
> On 02/11/2015 10:03, mesmriti at linux.vnet.ibm.com wrote:
>> From: Megha Smriti <mesmriti at 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"
Will take care of this in next patch version.
>
>> "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.
Please let me know you input regarding the separation of function
mention in my previous mail [PATCH 1/2].
>
>> + 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?
I will use that option to generate the dbginfo at config path.
>
> What about the report name? Is it auto generated?
Yes, the name is auto generated with format DBGINFO-*.tgz.
>
>> + 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?
Since sosreport was getting generated at this path and we though to
compress both together if platform is s390x.
Let me investigate if it is good idea to keep both tar separately in
config path or should be compressed into one file.
>
>> + 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.
This was done for moving the combined (sosreport compressed file+
dbginfo compressed file).
>
>> + 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?
Yes, I will investigate if it is good idea to keep both the report
separately or compressed together.
>
> Sorry, I am newbie with s390x tools, so it'd be good if you explain
> what you are going to do here.
That is fine. I will add comments in my next patch explaining what it is
doing.
>
>> + 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)
>
More information about the Kimchi-devel
mailing list