diff mbox

[KVM-AUTOTEST,01/06] kvm_vm.py: create a lock file to avoid VM creation in parallel

Message ID 1244507691-9575-1-git-send-email-lmr@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Lucas Meneghel Rodrigues June 9, 2009, 12:34 a.m. UTC
VM.create() does a few things (such as finding free ports) which
are not safe to execute in parallel. Use a lock file to make
sure this doesn't happen. The lock is released only after the
VM is started or fails to start.

Signed-off-by: Michael Goldish <mgoldish@redhat.com>
---
 client/tests/kvm/kvm_vm.py |   91 ++++++++++++++++++++++++--------------------
 1 files changed, 50 insertions(+), 41 deletions(-)

Comments

Lucas Meneghel Rodrigues June 10, 2009, 7:29 p.m. UTC | #1
On Mon, 2009-06-08 at 21:34 -0300, Lucas Meneghel Rodrigues wrote:
> VM.create() does a few things (such as finding free ports) which
> are not safe to execute in parallel. Use a lock file to make
> sure this doesn't happen. The lock is released only after the
> VM is started or fails to start.

Applied, thanks!

> Signed-off-by: Michael Goldish <mgoldish@redhat.com>
> ---
>  client/tests/kvm/kvm_vm.py |   91 ++++++++++++++++++++++++--------------------
>  1 files changed, 50 insertions(+), 41 deletions(-)
> 
> diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
> index eb9717b..e2b684c 100644
> --- a/client/tests/kvm/kvm_vm.py
> +++ b/client/tests/kvm/kvm_vm.py
> @@ -335,51 +335,60 @@ class VM:
>                      logging.error("Actual MD5 sum differs from expected one")
>                      return False
>  
> -        # Handle port redirections
> -        redir_names = kvm_utils.get_sub_dict_names(params, "redirs")
> -        host_ports = kvm_utils.find_free_ports(5000, 6000, len(redir_names))
> -        self.redirs = {}
> -        for i in range(len(redir_names)):
> -            redir_params = kvm_utils.get_sub_dict(params, redir_names[i])
> -            guest_port = int(redir_params.get("guest_port"))
> -            self.redirs[guest_port] = host_ports[i]
> -
> -        # Find available VNC port, if needed
> -        if params.get("display") == "vnc":
> -            self.vnc_port = kvm_utils.find_free_port(5900, 6000)
> -
> -        # Make qemu command
> -        qemu_command = self.make_qemu_command()
> +        # Make sure the following code is not executed by more than one thread
> +        # at the same time
> +        lockfile = open("/tmp/kvm-autotest-vm-create.lock", "w+")
> +        fcntl.lockf(lockfile, fcntl.LOCK_EX)
>  
> -        # Is this VM supposed to accept incoming migrations?
> -        if for_migration:
> -            # Find available migration port
> -            self.migration_port = kvm_utils.find_free_port(5200, 6000)
> -            # Add -incoming option to the qemu command
> -            qemu_command += " -incoming tcp:0:%d" % self.migration_port
> -
> -        logging.debug("Running qemu command:\n%s" % qemu_command)
> -        (status, pid, output) = kvm_utils.run_bg(qemu_command, None,
> -                                                 logging.debug, "(qemu) ")
> -
> -        if status:
> -            logging.debug("qemu exited with status %d" % status)
> -            logging.error("VM could not be created -- qemu command"
> -                          " failed:\n%s" % qemu_command)
> -            return False
> +        try:
> +            # Handle port redirections
> +            redir_names = kvm_utils.get_sub_dict_names(params, "redirs")
> +            host_ports = kvm_utils.find_free_ports(5000, 6000, len(redir_names))
> +            self.redirs = {}
> +            for i in range(len(redir_names)):
> +                redir_params = kvm_utils.get_sub_dict(params, redir_names[i])
> +                guest_port = int(redir_params.get("guest_port"))
> +                self.redirs[guest_port] = host_ports[i]
> +
> +            # Find available VNC port, if needed
> +            if params.get("display") == "vnc":
> +                self.vnc_port = kvm_utils.find_free_port(5900, 6000)
> +
> +            # Make qemu command
> +            qemu_command = self.make_qemu_command()
> +
> +            # Is this VM supposed to accept incoming migrations?
> +            if for_migration:
> +                # Find available migration port
> +                self.migration_port = kvm_utils.find_free_port(5200, 6000)
> +                # Add -incoming option to the qemu command
> +                qemu_command += " -incoming tcp:0:%d" % self.migration_port
> +
> +            logging.debug("Running qemu command:\n%s", qemu_command)
> +            (status, pid, output) = kvm_utils.run_bg(qemu_command, None,
> +                                                     logging.debug, "(qemu) ")
> +
> +            if status:
> +                logging.debug("qemu exited with status %d", status)
> +                logging.error("VM could not be created -- qemu command"
> +                              " failed:\n%s", qemu_command)
> +                return False
>  
> -        self.pid = pid
> +            self.pid = pid
>  
> -        if not kvm_utils.wait_for(self.is_alive, timeout, 0, 1):
> -            logging.debug("VM is not alive for some reason")
> -            logging.error("VM could not be created with"
> -                          " command:\n%s" % qemu_command)
> -            self.destroy()
> -            return False
> +            if not kvm_utils.wait_for(self.is_alive, timeout, 0, 1):
> +                logging.debug("VM is not alive for some reason")
> +                logging.error("VM could not be created with"
> +                              " command:\n%s", qemu_command)
> +                self.destroy()
> +                return False
>  
> -        logging.debug("VM appears to be alive with PID %d" % self.pid)
> +            logging.debug("VM appears to be alive with PID %d", self.pid)
> +            return True
>  
> -        return True
> +        finally:
> +            fcntl.lockf(lockfile, fcntl.LOCK_UN)
> +            lockfile.close()
>  
> 
>      def send_monitor_cmd(self, command, block=True, timeout=20.0):
diff mbox

Patch

diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index eb9717b..e2b684c 100644
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -335,51 +335,60 @@  class VM:
                     logging.error("Actual MD5 sum differs from expected one")
                     return False
 
-        # Handle port redirections
-        redir_names = kvm_utils.get_sub_dict_names(params, "redirs")
-        host_ports = kvm_utils.find_free_ports(5000, 6000, len(redir_names))
-        self.redirs = {}
-        for i in range(len(redir_names)):
-            redir_params = kvm_utils.get_sub_dict(params, redir_names[i])
-            guest_port = int(redir_params.get("guest_port"))
-            self.redirs[guest_port] = host_ports[i]
-
-        # Find available VNC port, if needed
-        if params.get("display") == "vnc":
-            self.vnc_port = kvm_utils.find_free_port(5900, 6000)
-
-        # Make qemu command
-        qemu_command = self.make_qemu_command()
+        # Make sure the following code is not executed by more than one thread
+        # at the same time
+        lockfile = open("/tmp/kvm-autotest-vm-create.lock", "w+")
+        fcntl.lockf(lockfile, fcntl.LOCK_EX)
 
-        # Is this VM supposed to accept incoming migrations?
-        if for_migration:
-            # Find available migration port
-            self.migration_port = kvm_utils.find_free_port(5200, 6000)
-            # Add -incoming option to the qemu command
-            qemu_command += " -incoming tcp:0:%d" % self.migration_port
-
-        logging.debug("Running qemu command:\n%s" % qemu_command)
-        (status, pid, output) = kvm_utils.run_bg(qemu_command, None,
-                                                 logging.debug, "(qemu) ")
-
-        if status:
-            logging.debug("qemu exited with status %d" % status)
-            logging.error("VM could not be created -- qemu command"
-                          " failed:\n%s" % qemu_command)
-            return False
+        try:
+            # Handle port redirections
+            redir_names = kvm_utils.get_sub_dict_names(params, "redirs")
+            host_ports = kvm_utils.find_free_ports(5000, 6000, len(redir_names))
+            self.redirs = {}
+            for i in range(len(redir_names)):
+                redir_params = kvm_utils.get_sub_dict(params, redir_names[i])
+                guest_port = int(redir_params.get("guest_port"))
+                self.redirs[guest_port] = host_ports[i]
+
+            # Find available VNC port, if needed
+            if params.get("display") == "vnc":
+                self.vnc_port = kvm_utils.find_free_port(5900, 6000)
+
+            # Make qemu command
+            qemu_command = self.make_qemu_command()
+
+            # Is this VM supposed to accept incoming migrations?
+            if for_migration:
+                # Find available migration port
+                self.migration_port = kvm_utils.find_free_port(5200, 6000)
+                # Add -incoming option to the qemu command
+                qemu_command += " -incoming tcp:0:%d" % self.migration_port
+
+            logging.debug("Running qemu command:\n%s", qemu_command)
+            (status, pid, output) = kvm_utils.run_bg(qemu_command, None,
+                                                     logging.debug, "(qemu) ")
+
+            if status:
+                logging.debug("qemu exited with status %d", status)
+                logging.error("VM could not be created -- qemu command"
+                              " failed:\n%s", qemu_command)
+                return False
 
-        self.pid = pid
+            self.pid = pid
 
-        if not kvm_utils.wait_for(self.is_alive, timeout, 0, 1):
-            logging.debug("VM is not alive for some reason")
-            logging.error("VM could not be created with"
-                          " command:\n%s" % qemu_command)
-            self.destroy()
-            return False
+            if not kvm_utils.wait_for(self.is_alive, timeout, 0, 1):
+                logging.debug("VM is not alive for some reason")
+                logging.error("VM could not be created with"
+                              " command:\n%s", qemu_command)
+                self.destroy()
+                return False
 
-        logging.debug("VM appears to be alive with PID %d" % self.pid)
+            logging.debug("VM appears to be alive with PID %d", self.pid)
+            return True
 
-        return True
+        finally:
+            fcntl.lockf(lockfile, fcntl.LOCK_UN)
+            lockfile.close()
 
 
     def send_monitor_cmd(self, command, block=True, timeout=20.0):