Merge "VMware: prevent race condition with VNC port allocation"

3 files changed, 8 insertions, 13 deletions

diff --git a/nova/tests/virt/vmwareapi/test_vmops.py b/nova/tests/virt/vmwareapi/test_vmops.py
index 45e21c19e4..b49adc8ab7 100644
--- a/nova/tests/virt/vmwareapi/test_vmops.py
+++ b/nova/tests/virt/vmwareapi/test_vmops.py
@@ -757,8 +757,7 @@ class VMwareVMOpsTestCase(test.NoDBTestCase):
     @mock.patch('nova.virt.vmwareapi.vmops.VMwareVMOps._set_machine_id')
     @mock.patch(
         'nova.virt.vmwareapi.imagecache.ImageCacheManager.enlist_image')
-    @mock.patch('nova.virt.vmwareapi.vm_util.get_vnc_port', return_value=5900)
-    @mock.patch('nova.virt.vmwareapi.vmops.VMwareVMOps._set_vnc_config')
+    @mock.patch.object(vmops.VMwareVMOps, '_get_and_set_vnc_config')
     @mock.patch('nova.virt.vmwareapi.vm_util.power_on_instance')
     @mock.patch('nova.virt.vmwareapi.vm_util.copy_virtual_disk')
     # TODO(dims): Need to add tests for create_virtual_disk after the
@@ -766,8 +765,7 @@ class VMwareVMOpsTestCase(test.NoDBTestCase):
     def _test_spawn(self,
                    mock_copy_virtual_disk,
                    mock_power_on_instance,
-                   mock_set_vnc_config,
-                   mock_get_vnc_port,
+                   mock_get_and_set_vnc_config,
                    mock_enlist_image,
                    mock_set_machine_id,
                    mock_mkdir,
@@ -821,7 +819,6 @@ class VMwareVMOpsTestCase(test.NoDBTestCase):
 
             self.assertEqual(expected_mkdir_calls, len(mock_mkdir.mock_calls))
 
-            mock_get_vnc_port.assert_called_once_with(self._session)
             mock_get_mo_id_for_instance.assert_called_once_with(self._instance)
             mock_get_res_pool_ref.assert_called_once_with(
                     self._session, None, 'fake_node_mo_id')
@@ -844,10 +841,9 @@ class VMwareVMOpsTestCase(test.NoDBTestCase):
                     'fake_vm_folder',
                     'fake_create_spec',
                     'fake_rp_ref')
-            mock_set_vnc_config.assert_called_once_with(
+            mock_get_and_set_vnc_config.assert_called_once_with(
                 self._session._get_vim().client.factory,
-                self._instance,
-                5900)
+                self._instance)
             mock_set_machine_id.assert_called_once_with(
                 self._session._get_vim().client.factory,
                 self._instance,
diff --git a/nova/virt/vmwareapi/vm_util.py b/nova/virt/vmwareapi/vm_util.py
index 7aaa9f1cfd..1f44e17098 100644
--- a/nova/virt/vmwareapi/vm_util.py
+++ b/nova/virt/vmwareapi/vm_util.py
@@ -29,7 +29,6 @@ from nova.i18n import _
 from nova.network import model as network_model
 from nova.openstack.common import log as logging
 from nova.openstack.common import units
-from nova import utils
 from nova.virt.vmwareapi import constants
 from nova.virt.vmwareapi import vim_util
 
@@ -717,7 +716,6 @@ def get_vnc_config_spec(client_factory, port):
     return virtual_machine_config_spec
 
 
-@utils.synchronized('vmware.get_vnc_port')
 def get_vnc_port(session):
     """Return VNC port for an VM or None if there is no available port."""
     min_port = CONF.vmware.vnc_port
diff --git a/nova/virt/vmwareapi/vmops.py b/nova/virt/vmwareapi/vmops.py
index 67ed6c32ad..2c5ecd467e 100644
--- a/nova/virt/vmwareapi/vmops.py
+++ b/nova/virt/vmwareapi/vmops.py
@@ -456,8 +456,7 @@ class VMwareVMOps(object):
 
         # Set the vnc configuration of the instance, vnc port starts from 5900
         if CONF.vnc_enabled:
-            vnc_port = vm_util.get_vnc_port(self._session)
-            self._set_vnc_config(client_factory, instance, vnc_port)
+            self._get_and_set_vnc_config(client_factory, instance)
 
         block_device_mapping = []
         if block_device_info is not None:
@@ -1247,8 +1246,10 @@ class VMwareVMOps(object):
         LOG.debug("Reconfigured VM instance to set the machine id",
                   instance=instance)
 
-    def _set_vnc_config(self, client_factory, instance, port):
+    @utils.synchronized('vmware.get_and_set_vnc_port')
+    def _get_and_set_vnc_config(self, client_factory, instance):
         """Set the vnc configuration of the VM."""
+        port = vm_util.get_vnc_port(self._session)
         vm_ref = vm_util.get_vm_ref(self._session, instance)
 
         vnc_config_spec = vm_util.get_vnc_config_spec(