[Kimchi-devel] [PATCH V2] Fix host debug report for Fedora 20

[Rodrigo Trujillo]

On 02/10/2014 12:39 PM, Aline Manera wrote:
> On 02/06/2014 12:27 AM, Rodrigo Trujillo wrote:
>> The tool sosreport does not save the report file in /tmp in F20, this
>> causes a error in Kimchi, which has /tmp hardcoded. This fixes this
>> problem searching report files in /var/tmp too.
>> Notice that is possible to pass --tmp-dir as the directory to save the
>> report file, but this option is crashing in Fedora 20, so the code is
>> not using it.
>> This patch also improves the logging.
>>
>> Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo at linux.vnet.ibm.com>
>> ---
>>   src/kimchi/model/debugreports.py | 61 
>> ++++++++++++++++++++++++++--------------
>>   1 file changed, 40 insertions(+), 21 deletions(-)
>>
>> diff --git a/src/kimchi/model/debugreports.py 
>> b/src/kimchi/model/debugreports.py
>> index a1cb19c..8337709 100644
>> --- a/src/kimchi/model/debugreports.py
>> +++ b/src/kimchi/model/debugreports.py
>> @@ -32,6 +32,7 @@ from kimchi import config
>>   from kimchi.exception import NotFoundError, OperationFailed
>>   from kimchi.model.tasks import TaskModel
>>   from kimchi.utils import add_task, kimchi_log
>> +from kimchi.utils import run_command
>>
>>
>>   class DebugReportsModel(object):
>> @@ -63,36 +64,54 @@ class DebugReportsModel(object):
>>
>>       @staticmethod
>>       def sosreport_generate(cb, name):
>> -        command = 'sosreport --batch --name "%s"' % name
>>           try:
>> -            retcode = subprocess.call(command, shell=True,
>> -                                      stdout=subprocess.PIPE)
>> +            command = ['sosreport', '--batch', '--name=%s' % name]
>
> why did you remove the stdout redirection?
> That way we will display a lot of useless messages to the user.
No, no, I just moved from subprocess to run_command ... which in turns 
also uses subprocess and handles the stdout and stderr.
See the line below:

>> +            output, error, retcode = run_command(command)
>> +
>>               if retcode < 0:
>>                   raise OperationFailed('Command terminated with 
>> signal')
>>               elif retcode > 0:
>>                   raise OperationFailed('Command failed: rc = %i' % 
>> 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
>>               pattern = '/tmp/sosreport-%s-*' % name
>
> Let's avoid duplicating code.
> How about:
>
> patterns = ['/tmp/sosreport-%s-*', '/var/tmp/sosreport-%s-*']
> reports = []
> for p in patterns:
>     reports.append(glob.glob(p % name))
>
> for f in reports:
>     ...
>
ok

>> -            for reportFile in glob.glob(pattern):
>> -                if not fnmatch.fnmatch(reportFile, '*.md5'):
>> -                    output = reportFile
>> +            reports = glob.glob(pattern)
>> +            reportFile = None
>> +            for f in reports:
>> +                if not fnmatch.fnmatch(f, '*.md5'):
>> +                    reportFile = f
>>                       break
>> -            else:
>> -                # sosreport tends to change the name mangling rule and
>> -                # compression file format between different releases.
>> -                # It's possible to fail to match a report file even 
>> sosreport
>> -                # runs successfully. In future we might have a 
>> general name
>> -                # mangling function in kimchi to format the name 
>> before passing
>> -                # it to sosreport. Then we can delete this exception.
>> -                raise OperationFailed('Can not find generated debug 
>> report '
>> -                                      'named by %s' % pattern)
>> -            ext = output.split('.', 1)[1]
>> +            if reportFile is None:
>> +                # Search in /var/tmp
>> +                reports = glob.glob('/var' + pattern)
>> +                for f in reports:
>> +                    if not fnmatch.fnmatch(f, '*.md5'):
>> +                        reportFile = f
>> +                        break
>> +
>> +            # Some error in sosreport happened
>> +            if reportFile is None:
>> +                kimchi_log.error('Debug report file not found. See 
>> sosreport '
>> +                                 'output for detail:\n%s', output)
>> +                raise OperationFailed('Debug report file not found.')
>> +
>> +            md5_report_file = reportFile + '.md5'
>> +            report_file_extension = '.' + reportFile.split('.', 1)[1]
>>               path = config.get_debugreports_path()
>> -            target = os.path.join(path, name)
>> -            target_file = '%s.%s' % (target, ext)
>> -            shutil.move(output, target_file)
>> -            os.remove('%s.md5' % output)
>> +            target = os.path.join(path, name + report_file_extension)
>> +            # Moving report
>> +            msg = 'Moving debug report file "%s" to "%s"' % 
>> (reportFile, target)
>> +            kimchi_log.info(msg)
>> +            shutil.move(reportFile, target)
>> +            # Deleting md5
>> +            msg = 'Deleting report md5 file: "%s"' % (md5_report_file)
>> +            kimchi_log.info(msg)
>> +            md5 = open(md5_report_file).read().strip()
>> +            kimchi_log.info('Md5 file content: "%s"', md5)
>> +            os.remove(md5_report_file)
>>               cb('OK', True)
>> -
>>               return
>>
>>           except OSError:
>
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>