| Message ID | 63bacaa214ccd95c18fb644056855acd72757ac4.1243179847.git.mgoldish@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, May 24, 2009 at 9:16 PM, Michael Goldish <mgoldish@redhat.com> 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. > > Signed-off-by: Michael Goldish <mgoldish@redhat.com> > --- >  client/tests/kvm_runtest_2/kvm_vm.py |  85 +++++++++++++++++++--------------- >  1 files changed, 48 insertions(+), 37 deletions(-) > > diff --git a/client/tests/kvm_runtest_2/kvm_vm.py b/client/tests/kvm_runtest_2/kvm_vm.py > index 3ce2003..af06693 100644 > --- a/client/tests/kvm_runtest_2/kvm_vm.py > +++ b/client/tests/kvm_runtest_2/kvm_vm.py > @@ -3,6 +3,7 @@ >  import time >  import socket >  import os > +import fcntl > >  import kvm_utils >  import kvm_log > @@ -289,48 +290,58 @@ class VM: >           kvm_log.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+") How do you handle an open failure? > +     fcntl.lockf(lockfile, fcntl.LOCK_EX) What if other instance has locked the file at the moment. Definitely you would not like to fail. You may want to wait for a while and try agian. I feel the default behaviour should be a blocking one but still you want to print the debug message kvm_log.debug("Trying to acquire lock for port selection") before getting the lock. > > -     # 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 > +     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 > + > +       kvm_log.debug("Running qemu command:\n%s" % qemu_command) > +       (status, pid, output) = kvm_utils.run_bg(qemu_command, None, kvm_log.debug, "(qemu) ") > + > +       if status: > +         kvm_log.debug("qemu exited with status %d" % status) > +         kvm_log.error("VM could not be created -- qemu command failed:\n%s" % qemu_command) > +         return False > > -     kvm_log.debug("Running qemu command:\n%s" % qemu_command) > -     (status, pid, output) = kvm_utils.run_bg(qemu_command, None, kvm_log.debug, "(qemu) ") > +       self.pid = pid > > -     if status: > -       kvm_log.debug("qemu exited with status %d" % status) > -       kvm_log.error("VM could not be created -- qemu command failed:\n%s" % qemu_command) > -       return False > - > -     self.pid = pid > +       if not kvm_utils.wait_for(self.is_alive, timeout, 0, 1): > +         kvm_log.debug("VM is not alive for some reason") > +         kvm_log.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): > -       kvm_log.debug("VM is not alive for some reason") > -       kvm_log.error("VM could not be created with command:\n%s" % qemu_command) > -       self.destroy() > -       return False > +       kvm_log.debug("VM appears to be alive with PID %d" % self.pid) > > -     kvm_log.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): >     """Send command to the QEMU monitor. > -- > 1.5.4.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 --git a/client/tests/kvm_runtest_2/kvm_vm.py b/client/tests/kvm_runtest_2/kvm_vm.py index 3ce2003..af06693 100644 --- a/client/tests/kvm_runtest_2/kvm_vm.py +++ b/client/tests/kvm_runtest_2/kvm_vm.py @@ -3,6 +3,7 @@ import time import socket import os +import fcntl import kvm_utils import kvm_log @@ -289,48 +290,58 @@ class VM: kvm_log.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 + 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 + + kvm_log.debug("Running qemu command:\n%s" % qemu_command) + (status, pid, output) = kvm_utils.run_bg(qemu_command, None, kvm_log.debug, "(qemu) ") + + if status: + kvm_log.debug("qemu exited with status %d" % status) + kvm_log.error("VM could not be created -- qemu command failed:\n%s" % qemu_command) + return False - kvm_log.debug("Running qemu command:\n%s" % qemu_command) - (status, pid, output) = kvm_utils.run_bg(qemu_command, None, kvm_log.debug, "(qemu) ") + self.pid = pid - if status: - kvm_log.debug("qemu exited with status %d" % status) - kvm_log.error("VM could not be created -- qemu command failed:\n%s" % qemu_command) - return False - - self.pid = pid + if not kvm_utils.wait_for(self.is_alive, timeout, 0, 1): + kvm_log.debug("VM is not alive for some reason") + kvm_log.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): - kvm_log.debug("VM is not alive for some reason") - kvm_log.error("VM could not be created with command:\n%s" % qemu_command) - self.destroy() - return False + kvm_log.debug("VM appears to be alive with PID %d" % self.pid) - kvm_log.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): """Send command to the QEMU monitor.
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_runtest_2/kvm_vm.py | 85 +++++++++++++++++++--------------- 1 files changed, 48 insertions(+), 37 deletions(-)