diff mbox

[3/4] KVM test: Refine image_check function

Message ID 1302239305-15786-4-git-send-email-lmr@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Lucas Meneghel Rodrigues April 8, 2011, 5:08 a.m. UTC
With the accumulated experience running the KVM test to
perform quality control on our KVM branches, we noticed
that qemu-img check might return exit code != 0, but
not all failures mean some data integrity problem happend.

After checking qemu-img check code, we found out that:

Exit code 1: Check error. Problem on the check itself,
most of the time harmless, however there is some risk
of data corruption.

Exit code 2: Data corruption error. This means for sure
that disk corruption happened. Bad, bad.

Exit code 3: Leaked clusters error. This means some
leaked clusters were found on the image, which is not
a data corruption condition, it only means that some
space is going to be lost on that image.

So, refine the logic of the image_check function, executing
qemu-img check and verifying its exit code

Exit code 1: Raise error.TestWarn
Exit code 2: Raise VMImageError
Exit code 3: Raise error.TestWarn

This change, together with the new logic of the KVM
run_tests() utility function, will make it possible
to still fail tests in cases 1) and 3), but letting
the dependencies to be executed.

Signed-off-by: Lucas Meneghel Rodrigues <lmr@redhat.com>
---
 client/tests/kvm/kvm_vm.py |   19 ++++++++++++++++---
 1 files changed, 16 insertions(+), 3 deletions(-)

Comments

Lucas Meneghel Rodrigues April 8, 2011, 5:17 a.m. UTC | #1
On Fri, Apr 8, 2011 at 2:08 AM, Lucas Meneghel Rodrigues <lmr@redhat.com> wrote:
> With the accumulated experience running the KVM test to
> perform quality control on our KVM branches, we noticed
> that qemu-img check might return exit code != 0, but
> not all failures mean some data integrity problem happend.
>
> After checking qemu-img check code, we found out that:
>
> Exit code 1: Check error. Problem on the check itself,
> most of the time harmless, however there is some risk
> of data corruption.
>
> Exit code 2: Data corruption error. This means for sure
> that disk corruption happened. Bad, bad.
>
> Exit code 3: Leaked clusters error. This means some
> leaked clusters were found on the image, which is not
> a data corruption condition, it only means that some
> space is going to be lost on that image.
>
> So, refine the logic of the image_check function, executing
> qemu-img check and verifying its exit code
>
> Exit code 1: Raise error.TestWarn
> Exit code 2: Raise VMImageError
> Exit code 3: Raise error.TestWarn
>
> This change, together with the new logic of the KVM
> run_tests() utility function, will make it possible
> to still fail tests in cases 1) and 3), but letting
> the dependencies to be executed.
>
> Signed-off-by: Lucas Meneghel Rodrigues <lmr@redhat.com>
> ---
>  client/tests/kvm/kvm_vm.py |   19 ++++++++++++++++---
>  1 files changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
> index 8114670..445ba6b 100755
> --- a/client/tests/kvm/kvm_vm.py
> +++ b/client/tests/kvm/kvm_vm.py
> @@ -294,10 +294,23 @@ def check_image(params, root_dir):
>             except error.CmdError:
>                 logging.error("Error getting info from image %s",
>                               image_filename)
> -            try:
> -                utils.system("%s check %s" % (qemu_img_cmd, image_filename))
> -            except error.CmdError:
> +
> +            cmd_result = utils.run("%s check %s" %
> +                                   (qemu_img_cmd, image_filename),
> +                                   ignore_status=True)
> +            # Error check, large chances of a non-fatal problem.
> +            # There are chances that bad data was skipped though
> +            if cmd_result.exit_status == 1:

^ I think here I could've printed the stdout and stderr of the
qemu-img check command to help debugging things. Another option is to
pass the appropriate run params to make stdout and stderr to be
printed to logs. However, if we do that we'll have not overly useful
and generally enormous 'leaked cluster' messages on case 3.

> +                raise error.TestWarn("qemu-img check error. Some bad data in "
> +                                     "the image may have gone unnoticed")
> +            # Exit status 2 is data corruption for sure, so fail the test
> +            elif cmd_result.exit_status == 2:

^ Same here. I'm commenting on my own patches so I remember to make
those improvements :)

>                 raise VMImageCheckError(image_filename)
> +            # Leaked clusters, they are known to be harmless to data integrity
> +            elif cmd_result.exit_status == 3:
> +                raise error.TestWarn("Leaked clusters were noticed during "
> +                                     "image check. No data integrity problem "
> +                                     "was found though.")
>
>     else:
>         if not os.path.exists(image_filename):
> --
> 1.7.4.2
>
> _______________________________________________
> Autotest mailing list
> Autotest@test.kernel.org
> http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
>
diff mbox

Patch

diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index 8114670..445ba6b 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -294,10 +294,23 @@  def check_image(params, root_dir):
             except error.CmdError:
                 logging.error("Error getting info from image %s",
                               image_filename)
-            try:
-                utils.system("%s check %s" % (qemu_img_cmd, image_filename))
-            except error.CmdError:
+
+            cmd_result = utils.run("%s check %s" %
+                                   (qemu_img_cmd, image_filename),
+                                   ignore_status=True)
+            # Error check, large chances of a non-fatal problem.
+            # There are chances that bad data was skipped though
+            if cmd_result.exit_status == 1:
+                raise error.TestWarn("qemu-img check error. Some bad data in "
+                                     "the image may have gone unnoticed")
+            # Exit status 2 is data corruption for sure, so fail the test
+            elif cmd_result.exit_status == 2:
                 raise VMImageCheckError(image_filename)
+            # Leaked clusters, they are known to be harmless to data integrity
+            elif cmd_result.exit_status == 3:
+                raise error.TestWarn("Leaked clusters were noticed during "
+                                     "image check. No data integrity problem "
+                                     "was found though.")
 
     else:
         if not os.path.exists(image_filename):