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

2010-05-17 Thread Lucas Meneghel Rodrigues 
On Mon, 2010-05-17 at 15:28 +0800, Jason Wang wrote:
> 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 
> >> ---
> >>  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 000..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.

^ Typo here, search.

> >> +"""
> >> +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.

When I made the first pre command script, I opted to use a class to
represent the unattended install setup, and other pre commands followed
this style. Since the code is small, I see no problem in putting it
under __main__ altogether. So Jason, you can remove the code from the
class and put it under __main__, please.

> >> 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

^ typo, should be search_pattern

> >> +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?

Other than the issues pointed out, looks good.

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2010-05-17 Thread Jason Wang 

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 
---
 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 000..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 majord...@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 majord...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2010-05-12 Thread Michael Goldish 
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 
> ---
>  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 000..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?

> 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.

>  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 majord...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2010-05-11 Thread Jason Wang 
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 
---
 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 000..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" \
+ % 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" \
+ " check the log")
+
+
+if __name__ == "__main__":
+checker = SerialChecker()
+checker.check()
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"
 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html