<html>
  <head>
    <meta content="text/html; charset=windows-1252"
      http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <br>
    Thanks for the answers, Megha!<br>
    <br>
    I know have a better background to provide feedbacks and
    suggestions.<br>
    <br>
    1) I'd say to keep compressing the sosreport + dbginfo into one
    tar.gz file<br>
        Otherwise we would need to update the UI and I don't think it is
    a good idea right now.<br>
    <br>
    2) For the implementation, I'd suggest to keep the function very
    well self contained, ie, the sosreport function will only deal with
    sosreport, dbginfo will only deal with that command and so.<br>
    <br>
    So update get_system_report_tool() to use dbginfo. As the final
    result would be dbginfo + sosreport<br>
    <br>
    <font face="monospace">   
      @staticmethod                                                              
      <br>
          def
      get_system_report_tool():                                              
      <br>
              # Please add new possible debug report command
      here                     <br>
              # and implement the report generating
      function                          <br>
              # based on the new report
      command                                       <br>
              report_tools = (<b>{'cmd': '/usr/sbin/dbginfo.sh --help',
        'fn': DebugReportsModel.dbginfo_generate},</b><br>
                              {'cmd': 'sosreport --help', 'fn':
      DebugReportsModel.sosreport_generate},)<br>
    </font><br>
    Create a new function ddginfo_generate:<br>
    <br>
    <font face="monospace">@staticmethod<br>
      def dbginfo_generate(cb, name):<br>
          # first of all let's generate the sosreport<br>
          DebugReportsModel.sosreport_generate(cb, name)<br>
      <br>
          # After it, generated the dbginfo tar file<br>
          &lt;do all the work here&gt;<br>
      <br>
          # Merge sosreport + dbginfo<br>
          &lt;...&gt;</font><br>
    <br>
    And it is done. That way we make sure to have the functions self
    contained. It is better for maintenance.<br>
    <br>
    Let me know your thoughts on it.<br>
    <br>
    Regards,<br>
    Aline Manera <br>
    <br>
    <div class="moz-cite-prefix">On 04/11/2015 05:08, Megha Smriti
      wrote:<br>
    </div>
    <blockquote cite="mid:5639AEF7.3060509@linux.vnet.ibm.com"
      type="cite">Hello Aline,
      <br>
      <br>
      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.
      <br>
      <br>
      Thanks,
      <br>
      Megha
      <br>
      <br>
      On 11/3/2015 10:49 PM, Aline Manera wrote:
      <br>
      <blockquote type="cite">
        <br>
        The same I commented regarding the commit message.
        <br>
        <br>
        On 02/11/2015 10:03, <a class="moz-txt-link-abbreviated" href="mailto:mesmriti@linux.vnet.ibm.com">mesmriti@linux.vnet.ibm.com</a> wrote:
        <br>
        <blockquote type="cite">From: Megha Smriti
          <a class="moz-txt-link-rfc2396E" href="mailto:mesmriti@linux.vnet.ibm.com">&lt;mesmriti@linux.vnet.ibm.com&gt;</a>
          <br>
          <br>
          ---
          <br>
            src/wok/plugins/gingerbase/i18n.py               |  2 +
          <br>
            src/wok/plugins/gingerbase/model/debugreports.py | 80
          +++++++++++++++++++++---
          <br>
            2 files changed, 72 insertions(+), 10 deletions(-)
          <br>
          <br>
          diff --git a/src/wok/plugins/gingerbase/i18n.py
          b/src/wok/plugins/gingerbase/i18n.py
          <br>
          index 01573b0..e19b997 100644
          <br>
          --- a/src/wok/plugins/gingerbase/i18n.py
          <br>
          +++ b/src/wok/plugins/gingerbase/i18n.py
          <br>
          @@ -36,6 +36,8 @@ messages = {
          <br>
                "GGBDR0007E": _("Debug report name must be a string.
          Only letters, digits, underscore ('_') and "
          <br>
                                "hyphen ('-') are allowed."),
          <br>
                "GGBDR0008E": _("The debug report with specified name
          \"%(name)s\" already exists. Please use another one."),
          <br>
          +    "GGBDR0009E": _("Unable to create dbginfo report with
          %(retcode)s. Details: %(err)s"),
          <br>
        </blockquote>
        <br>
        <blockquote type="cite">+    "GGBDR0010E": _("Unable to tar the
          final debug report tar file with %(retcode)s. Details:
          %(error)s"),
          <br>
        </blockquote>
        <br>
        "Unable to compress the final debug report data due error
        %(retcode)s. Details: %(error)s"
        <br>
      </blockquote>
      Will take care of this in next patch version.
      <br>
      <blockquote type="cite">
        <br>
        <blockquote type="cite">      "GGBHOST0001E": _("Unable to
          shutdown host machine as there are running virtual machines"),
          <br>
                "GGBHOST0002E": _("Unable to reboot host machine as
          there are running virtual machines"),
          <br>
                "GGBHOST0005E": _("When specifying CPU topology, each
          element must be an integer greater than zero."),
          <br>
          diff --git a/src/wok/plugins/gingerbase/model/debugreports.py
          b/src/wok/plugins/gingerbase/model/debugreports.py
          <br>
          index 0bb36fe..719e460 100644
          <br>
          --- a/src/wok/plugins/gingerbase/model/debugreports.py
          <br>
          +++ b/src/wok/plugins/gingerbase/model/debugreports.py
          <br>
          @@ -25,6 +25,7 @@ import os
          <br>
            import shutil
          <br>
            import subprocess
          <br>
            import time
          <br>
          +import platform
          <br>
            import re
          <br>
          <br>
            from wok.exception import InvalidParameter, NotFoundError,
          OperationFailed
          <br>
          @@ -108,16 +109,75 @@ class DebugReportsModel(object):
          <br>
                            raise OperationFailed('GGBDR0004E', {'name':
          name})
          <br>
                        md5_report_file = sosreport_file + '.md5'
          <br>
                        report_file_extension = '.' +
          sosreport_file.split('.', 1)[1]
          <br>
          -            path = config.get_debugreports_path()
          <br>
          -            sosreport_target = os.path.join(path,
          <br>
          -                                            name +
          report_file_extension)
          <br>
          -            msg = 'Moving debug report file "%s" to "%s"' \
          <br>
          -                  % (sosreport_file, sosreport_target)
          <br>
          -            wok_log.info(msg)
          <br>
          -            shutil.move(sosreport_file, sosreport_target)
          <br>
          -            delete_the_sosreport_md5_file(md5_report_file)
          <br>
          -            cb('OK', True)
          <br>
          -            return
          <br>
          +            # If the platform is a system Z machine.
          <br>
          +            if platform.machine().startswith('s390'):
          <br>
        </blockquote>
        <br>
        I suggest to create a new function to handle the sosreport
        generation for s390x. Just to make the code easier to read.
        <br>
      </blockquote>
      Please let me know you input regarding the separation of function
      mention in my previous mail [PATCH 1/2].
      <br>
      <blockquote type="cite">
        <br>
        <blockquote type="cite">+                dbgreport_regex =
          '(\S+\s+)(\/\w+\/\w+-[\d+]{4}-[\d+]{2}' \
          <br>
          +                                 
          '-[\d+]{2}-[\d+]{2}-[\d+]{2}' \
          <br>
          + '-[\d+]{2}-\w+-\d+\S+)(\s+\S+)'
          <br>
          +                command = ['/usr/sbin/dbginfo.sh']
          <br>
        </blockquote>
        <br>
        Is there a parameter like '--tmp-dir' from sosreport to use with
        dbginfo.sh?
        <br>
      </blockquote>
      I will use that option to generate the dbginfo at config path.
      <br>
      <blockquote type="cite">
        <br>
        What about the report name? Is it auto generated?
        <br>
      </blockquote>
      Yes, the name is auto generated with format  DBGINFO-*.tgz.
      <br>
      <blockquote type="cite">
        <br>
        <blockquote type="cite">+                output, error, retcode
          = run_command(command)
          <br>
          +                if retcode != 0:
          <br>
          +                    raise OperationFailed("GGBDR0009E",
          <br>
          +                                          {'retcode':
          retcode, 'err': error})
          <br>
          +                output = output.splitlines()
          <br>
          +                for line in output:
          <br>
          +                    line = line.strip()
          <br>
          +                    n = re.match(dbgreport_regex, line)
          <br>
          +                    if n:
          <br>
          +                        dbginfo_report = n.groups()[1]
          <br>
          +                        break
          <br>
        </blockquote>
        <br>
        <blockquote type="cite">+                pathis = '/var/tmp/'
          <br>
          +                file_extension_dbginfo =
          dbginfo_report.split('/', 2)[2]
          <br>
          +                target_report_file = os.path.join(pathis,
          <br>
          + file_extension_dbginfo)
          <br>
          +                msg = 'Moving the "%s" to "%s"'\
          <br>
          +                      % (dbginfo_report, target_report_file)
          <br>
          +                shutil.move(dbginfo_report,
          target_report_file)
          <br>
        </blockquote>
        <br>
        So the report is generated somewhere and moved to /var/tmp. Is
        that correct? Why do you need to move it to /var/tmp?
        <br>
      </blockquote>
      Since sosreport was getting generated at this path and we though
      to compress both together if platform is s390x.
      <br>
      Let me investigate if it is good idea to keep both tar separately
      in config path or should be compressed into one file.
      <br>
      <blockquote type="cite">
        <br>
        <blockquote type="cite">+                wok_log.info(msg)
          <br>
        </blockquote>
        <br>
        <blockquote type="cite">+                final_tar_report_name =
          name + report_file_extension
          <br>
        </blockquote>
        <br>
        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.
        <br>
      </blockquote>
      This was done for moving the combined (sosreport compressed file+
      dbginfo compressed file).
      <br>
      <blockquote type="cite">
        <br>
        <blockquote type="cite">+                if dbginfo_report is
          not None:
          <br>
          +                    sosreport_tar = sosreport_file.split('/',
          3)[3]
          <br>
          +                    dbginfo_tar = dbginfo_report.split('/',
          2)[2]
          <br>
          +                    msg = 'Zipping the sosreport and debug
          info files into ' \
          <br>
          +                          'final report file'
          <br>
          +                    wok_log.info(msg)
          <br>
          +                    command = ['tar', '-cvzf', '%s' %
          final_tar_report_name,
          <br>
          +                               '-C', '/var/tmp/',
          dbginfo_tar, sosreport_tar]
          <br>
        </blockquote>
        <br>
        Are you mixing sosreport with dbginfo? Is sosreport still
        present in s390x systems? Is the debug report a collection of
        sosreport + dbginfo?
        <br>
      </blockquote>
      Yes, I will investigate if it is good idea to keep both the report
      separately or compressed together.
      <br>
      <blockquote type="cite">
        <br>
        Sorry, I am newbie with s390x tools, so it'd be good if you
        explain what you are going to do here.
        <br>
      </blockquote>
      That is fine. I will add comments in my next patch explaining what
      it is doing.
      <br>
      <blockquote type="cite">
        <br>
        <blockquote type="cite">+                    output, error,
          retcode = run_command(command)
          <br>
          +                    if retcode != 0:
          <br>
          +                        raise OperationFailed("GGBDR0010E",
          <br>
          +                                              {'retcode':
          retcode,
          <br>
          +                                               'error':
          error})
          <br>
          +                    path = config.get_debugreports_path()
          <br>
          +                    dbg_target = os.path.join(path,
          <br>
          +                                              name +
          report_file_extension)
          <br>
          +                    # Moving report
          <br>
          +                    msg = 'Moving final debug  report file
          "%s" to "%s"' % \
          <br>
          +                          (final_tar_report_name, dbg_target)
          <br>
          +                    wok_log.info(msg)
          <br>
          +                    shutil.move(final_tar_report_name,
          dbg_target)
          <br>
          + delete_the_sosreport_md5_file(md5_report_file)
          <br>
          +                    msg = 'Deleting the dbginfo file "%s" ' \
          <br>
          +                          % target_report_file
          <br>
          +                    wok_log.info(msg)
          <br>
          +                    os.remove(target_report_file)
          <br>
          +                    msg = 'Deleting the sosreport file "%s" '
          % sosreport_file
          <br>
          +                    wok_log.info(msg)
          <br>
          +                    os.remove(sosreport_file)
          <br>
          +                    wok_log.info('The debug report file has
          been moved')
          <br>
          +                    cb('OK', True)
          <br>
          +                    return
          <br>
        </blockquote>
        <br>
        <br>
        <br>
        <blockquote type="cite">+            else:
          <br>
          +                path = config.get_debugreports_path()
          <br>
          +                sosreport_target = os.path.join(path,
          <br>
          +                                                name +
          report_file_extension)
          <br>
          +                msg = 'Moving debug report file "%s" to "%s"'
          \
          <br>
          +                      % (sosreport_file, sosreport_target)
          <br>
          +                wok_log.info(msg)
          <br>
          +                shutil.move(sosreport_file, sosreport_target)
          <br>
          +               
          delete_the_sosreport_md5_file(md5_report_file)
          <br>
          +                cb('OK', True)
          <br>
          +                return
          <br>
          <br>
                    except WokException as e:
          <br>
                        log_error(e)
          <br>
        </blockquote>
        <br>
      </blockquote>
      <br>
    </blockquote>
    <br>
  </body>
</html>