Re: [PATCH v2 10/10] KVM test: Add a helper to search the panic in the log

[Jason Wang <jasowang@redhat.com>]

Michael Goldish wrote:
> On 05/11/2010 12:04 PM, Jason Wang wrote:
>   
>> This checker serves as the post_command to find the panic information
>> in the file which contains the content of guest serial console.
>>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>>  client/tests/kvm/scripts/check_serial.py |   41 ++++++++++++++++++++++++++++++
>>  client/tests/kvm/tests_base.cfg.sample   |    7 ++++-
>>  2 files changed, 46 insertions(+), 2 deletions(-)
>>  create mode 100644 client/tests/kvm/scripts/check_serial.py
>>
>> diff --git a/client/tests/kvm/scripts/check_serial.py b/client/tests/kvm/scripts/check_serial.py
>> new file mode 100644
>> index 0000000..969bbe3
>> --- /dev/null
>> +++ b/client/tests/kvm/scripts/check_serial.py
>> @@ -0,0 +1,41 @@
>> +import os, sys, glob, re
>> +
>> +
>> +class SerialCheckerError(Exception):
>> +    """
>> +    Simple wrapper for the builtin Exception class.
>> +    """
>> +    pass
>> +
>> +
>> +class SerialChecker(object):
>> +    """
>> +    Serach the panic or other keywords in the guest serial.
>> +    """
>> +    def __init__(self):
>> +        """
>> +        Gets params from environment variables and sets class attributes.
>> +        """
>> +        client_dir =  os.environ['AUTODIR']
>> +        self.pattern = os.environ['KVM_TEST_search_pattern']
>> +        self.shortname = os.environ['KVM_TEST_shortname']
>> +        self.debugdir = os.path.join(client_dir, "results/default/kvm.%s/debug" \
>>     
>
> I think the final backslash is unnecessary.
>
>   
>> +                                     % self.shortname)
>> +        self.serial_files = glob.glob(os.path.join(self.debugdir, 'serial*'))
>> +
>> +
>> +    def check(self):
>> +        """
>> +        Check whether the pattern were found in the serial files
>> +        """
>> +        fail = [ f for f in self.serial_files if
>> +                 re.findall(self.pattern, file(f).read(), re.I) ]
>> +        if fail:
>> +            print "%s is found in %s" % (self.pattern, fail)
>> +            raise SerialCheckerError("Error found during the check, Please" \
>>     
>
> Same here.
>
>   
>> +                                     " check the log")
>> +
>> +
>> +if __name__ == "__main__":
>> +    checker = SerialChecker()
>> +    checker.check()
>>     
>
> I wonder why we need a class.  Why not just put all the code here?
>
>   
Just follow the style of other pre_command, maybe Lucas like it.
>> diff --git a/client/tests/kvm/tests_base.cfg.sample b/client/tests/kvm/tests_base.cfg.sample
>> index 3c0933e..3ac8f0d 100644
>> --- a/client/tests/kvm/tests_base.cfg.sample
>> +++ b/client/tests/kvm/tests_base.cfg.sample
>> @@ -52,6 +52,10 @@ address_index = 0
>>  # Misc
>>  profilers = kvm_stat
>>  
>> +# pattern serach in guest serial console
>> +serach_pattern = panic
>> +post_command = "python scripts/check_serial.py"
>> +post_command_noncritical = no
>>  
>>  # Tests
>>  variants:
>> @@ -1314,10 +1318,9 @@ virtio|virtio_blk|e1000|balloon_check:
>>  variants:
>>      - @qcow2:
>>          image_format = qcow2
>> -        post_command = " python scripts/check_image.py;"
>> +        post_command = " python scripts/check_image.py; python scripts/check_serial.py"
>>     
>                         ^
> This should be +=, because post_command may have been previously
> assigned some value.
>
>   
Would change it, and do you have any other comments about this patchset?
>>          remove_image = no
>>          post_command_timeout = 600
>> -        post_command_noncritical = yes
>>      - vmdk:
>>          only Fedora Ubuntu Windows
>>          only smp2
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe kvm" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>     
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>