diff mbox

KVM Test: Call postprocess_vm before postprocess_image.

Message ID 1312442310-6594-1-git-send-email-fyang@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Feng Yang Aug. 4, 2011, 7:18 a.m. UTC 
From: Feng Yang <fyang@redhat.com>

Current we call postprocess_image befor postprocess_vm.
If exception is thrown in postprocess_image, postprocess_vm will
be skipped. So vm could not be killed, it may fail following case
 in same loop.

Signed-off-by: Feng Yang <fyang@redhat.com>
---
 client/virt/virt_env_process.py |   25 ++++++++++++++++---------
 1 files changed, 16 insertions(+), 9 deletions(-)

Comments

Lucas Meneghel Rodrigues Aug. 4, 2011, 3:08 p.m. UTC | #1 
On 08/04/2011 04:18 AM, fyang@redhat.com wrote:
> From: Feng Yang<fyang@redhat.com>
>
> Current we call postprocess_image befor postprocess_vm.
> If exception is thrown in postprocess_image, postprocess_vm will
> be skipped. So vm could not be killed, it may fail following case
>   in same loop.

Looks good to me, applied, thanks!

http://autotest.kernel.org/changeset/5520

> Signed-off-by: Feng Yang<fyang@redhat.com>
> ---
>   client/virt/virt_env_process.py |   25 ++++++++++++++++---------
>   1 files changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/client/virt/virt_env_process.py b/client/virt/virt_env_process.py
> index 12918eb..8fd5c21 100644
> --- a/client/virt/virt_env_process.py
> +++ b/client/virt/virt_env_process.py
> @@ -163,7 +163,7 @@ def process_command(test, params, env, command, command_timeout,
>               raise
>
>
> -def process(test, params, env, image_func, vm_func):
> +def process(test, params, env, image_func, vm_func, vm_first=False):
>       """
>       Pre- or post-process VMs and images according to the instructions in params.
>       Call image_func for each image listed in params and vm_func for each VM.
> @@ -177,13 +177,20 @@ def process(test, params, env, image_func, vm_func):
>       # Get list of VMs specified for this test
>       for vm_name in params.objects("vms"):
>           vm_params = params.object_params(vm_name)
> -        # Get list of images specified for this VM
> -        for image_name in vm_params.objects("images"):
> -            image_params = vm_params.object_params(image_name)
> -            # Call image_func for each image
> -            image_func(test, image_params)
> -        # Call vm_func for each vm
> -        vm_func(test, vm_params, env, vm_name)
> +        if not vm_first:
> +            # Get list of images specified for this VM
> +            for image_name in vm_params.objects("images"):
> +                image_params = vm_params.object_params(image_name)
> +                # Call image_func for each image
> +                image_func(test, image_params)
> +            # Call vm_func for each vm
> +            vm_func(test, vm_params, env, vm_name)
> +        else:
> +            vm_func(test, vm_params, env, vm_name)
> +            for image_name in vm_params.objects("images"):
> +                image_params = vm_params.object_params(image_name)
> +                image_func(test, image_params)
> +
>
>
>   @error.context_aware
> @@ -293,7 +300,7 @@ def postprocess(test, params, env):
>       error.context("postprocessing")
>
>       # Postprocess all VMs and images
> -    process(test, params, env, postprocess_image, postprocess_vm)
> +    process(test, params, env, postprocess_image, postprocess_vm, vm_first=True)
>
>       # Terminate the screendump thread
>       global _screendump_thread, _screendump_thread_termination_event

--
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
Amos Kong Aug. 4, 2011, 3:34 p.m. UTC | #2 
On Thu, Aug 04, 2011 at 03:18:30PM +0800, fyang@redhat.com wrote:
> From: Feng Yang <fyang@redhat.com>
> 
> Current we call postprocess_image befor postprocess_vm.
> If exception is thrown in postprocess_image, postprocess_vm will
> be skipped. So vm could not be killed, it may fail following case
>  in same loop.
> 
> Signed-off-by: Feng Yang <fyang@redhat.com>
> ---
>  client/virt/virt_env_process.py |   25 ++++++++++++++++---------
>  1 files changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/client/virt/virt_env_process.py b/client/virt/virt_env_process.py
> index 12918eb..8fd5c21 100644
> --- a/client/virt/virt_env_process.py
> +++ b/client/virt/virt_env_process.py
> @@ -163,7 +163,7 @@ def process_command(test, params, env, command, command_timeout,
>              raise
>  
>  
> -def process(test, params, env, image_func, vm_func):
> +def process(test, params, env, image_func, vm_func, vm_first=False):
>      """
>      Pre- or post-process VMs and images according to the instructions in params.
>      Call image_func for each image listed in params and vm_func for each VM.
> @@ -177,13 +177,20 @@ def process(test, params, env, image_func, vm_func):
>      # Get list of VMs specified for this test
>      for vm_name in params.objects("vms"):
>          vm_params = params.object_params(vm_name)
> -        # Get list of images specified for this VM
> -        for image_name in vm_params.objects("images"):
> -            image_params = vm_params.object_params(image_name)
> -            # Call image_func for each image
> -            image_func(test, image_params)
> -        # Call vm_func for each vm
> -        vm_func(test, vm_params, env, vm_name)
> +        if not vm_first:
> +            # Get list of images specified for this VM
> +            for image_name in vm_params.objects("images"):
> +                image_params = vm_params.object_params(image_name)
> +                # Call image_func for each image
> +                image_func(test, image_params)
> +            # Call vm_func for each vm
> +            vm_func(test, vm_params, env, vm_name)
> +        else:
> +            vm_func(test, vm_params, env, vm_name)
> +            for image_name in vm_params.objects("images"):
> +                image_params = vm_params.object_params(image_name)
> +                image_func(test, image_params)
> +

         if vm_first:
             vm_func(test, vm_params, env, vm_name)

         # Get list of images specified for this VM
         for image_name in vm_params.objects("images"):
             image_params = vm_params.object_params(image_name)
             # Call image_func for each image
             image_func(test, image_params)

         if not vm_first:
             vm_func(test, vm_params, env, vm_name)


However, I'm ok for Feng's fix.
Acked-by: Amos Kong <akong@redhat.com>

>  @error.context_aware
> @@ -293,7 +300,7 @@ def postprocess(test, params, env):
>      error.context("postprocessing")
>  
>      # Postprocess all VMs and images
> -    process(test, params, env, postprocess_image, postprocess_vm)
> +    process(test, params, env, postprocess_image, postprocess_vm, vm_first=True)
>  
>      # Terminate the screendump thread
>      global _screendump_thread, _screendump_thread_termination_event
> -- 
> 1.7.1
--
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
diff mbox

Patch

diff --git a/client/virt/virt_env_process.py b/client/virt/virt_env_process.py
index 12918eb..8fd5c21 100644
--- a/client/virt/virt_env_process.py
+++ b/client/virt/virt_env_process.py
@@ -163,7 +163,7 @@  def process_command(test, params, env, command, command_timeout,
             raise
 
 
-def process(test, params, env, image_func, vm_func):
+def process(test, params, env, image_func, vm_func, vm_first=False):
     """
     Pre- or post-process VMs and images according to the instructions in params.
     Call image_func for each image listed in params and vm_func for each VM.
@@ -177,13 +177,20 @@  def process(test, params, env, image_func, vm_func):
     # Get list of VMs specified for this test
     for vm_name in params.objects("vms"):
         vm_params = params.object_params(vm_name)
-        # Get list of images specified for this VM
-        for image_name in vm_params.objects("images"):
-            image_params = vm_params.object_params(image_name)
-            # Call image_func for each image
-            image_func(test, image_params)
-        # Call vm_func for each vm
-        vm_func(test, vm_params, env, vm_name)
+        if not vm_first:
+            # Get list of images specified for this VM
+            for image_name in vm_params.objects("images"):
+                image_params = vm_params.object_params(image_name)
+                # Call image_func for each image
+                image_func(test, image_params)
+            # Call vm_func for each vm
+            vm_func(test, vm_params, env, vm_name)
+        else:
+            vm_func(test, vm_params, env, vm_name)
+            for image_name in vm_params.objects("images"):
+                image_params = vm_params.object_params(image_name)
+                image_func(test, image_params)
+
 
 
 @error.context_aware
@@ -293,7 +300,7 @@  def postprocess(test, params, env):
     error.context("postprocessing")
 
     # Postprocess all VMs and images
-    process(test, params, env, postprocess_image, postprocess_vm)
+    process(test, params, env, postprocess_image, postprocess_vm, vm_first=True)
 
     # Terminate the screendump thread
     global _screendump_thread, _screendump_thread_termination_event