Re: [KVM-AUTOTEST PATCH] stress_boot - Boot VMs until one of them becomes unresponsive - Version2

[Lucas Meneghel Rodrigues <lmr@redhat.com>]

On Fri, 2009-06-12 at 21:27 +0800, Yolkfull Chow wrote:
> Following are the differences between version 1:
> 
> 1) use framework to destroy VMs except the main_vm
> 2) use snapshot to boot other VMs except the first one
> 
> 
> Regards,
> Yolkfull

Hi Yolkfull, Michael and Uri already made a thorough first comment about
your test, and I have a minor thing to note (and I admit I'm being picky
here):

+            # check whether all previous ssh sessions are responsive
+            for i, vm_session in enumerate(sessions):
+                if vm_session.get_command_status(params.get("alive_test_cmd")):
+                    raise error.TestFail("Session #%d is not responsive" % i)
+            num += 1
+
+        except (error.TestFail, OSError):
+            for se in sessions:
+                se.close()
+            logging.info("Total number booted: %d" % num)
+            raise
+    else:
+        for se in sessions:
+            se.close()
+        logging.info("Total number booted: %d" % num)

When the test finishes successfuly, the counter num will be incremented
by one, will break the while condition and later will be used to print
the number of vms successfuly booted. In the end the total number of vms
booted that the test will report is the actual number of vms booted plus
1. To fix this we can either:

 * Just subtract 1 from num at the last info logging call;
 * Remove num initialization and replace the while loop by a

for num in range(1, int(params.get("max_vms")):

this way we don't even need to increment num manually.

It's up to you which one you're going to implement. I have tested your
code and it works fine (aside from the minor cosmetic issue). Once you
send me an updated version, I am going to apply it. 

Thanks for your work!

-- 
Lucas Meneghel Rodrigues
Software Engineer (QE)
Red Hat - Emerging Technologies