VMware: Fix memory leaks caused by caches

Using suds objects as keys for the DatastoreBrowser cache is incorrect
because they don't implement __eq__ and __hash__ for the VIM types.
This always results in cache miss and the cache grows with every spawn()
operation.

This patch fix this by using the 'value' property (which is string) of
the MoRef as key.

Closes-Bug: 1316433
(cherry picked from commit bceb3f96b0)

Conflicts:

	nova/tests/virt/vmwareapi/test_imagecache.py
	nova/tests/virt/vmwareapi/test_vmops.py

Change-Id: I2bcaf87e733d51055566aee41bb0a7e254027ba9

nova/tests/virt/vmwareapi/test_imagecache.py

@@ -104,26 +104,18 @@ class ImageCacheManagerTestCase(test.NoDBTestCase):
         self.assertEqual(self._time, t)
 
     def test_get_ds_browser(self):
-        def fake_get_dynamic_property(vim, mobj, type, property_name):
-            self.assertEqual('Datastore', type)
-            self.assertEqual('browser', property_name)
-            self.fake_called += 1
-            return 'fake-ds-browser'
-
-        with contextlib.nested(
-            mock.patch.object(vim_util, 'get_dynamic_property',
-                              fake_get_dynamic_property)
-        ) as _get_dynamic:
-            self.fake_called = 0
-            self.assertEqual({}, self._imagecache._ds_browser)
-            browser = self._imagecache._get_ds_browser('fake-ds-ref')
-            self.assertEqual('fake-ds-browser', browser)
-            self.assertEqual({'fake-ds-ref': 'fake-ds-browser'},
-                             self._imagecache._ds_browser)
-            self.assertEqual(1, self.fake_called)
-            browser = self._imagecache._get_ds_browser('fake-ds-ref')
-            self.assertEqual('fake-ds-browser', browser)
-            self.assertEqual(1, self.fake_called)
+        cache = self._imagecache._ds_browser
+        ds_browser = mock.Mock()
+        moref = fake.ManagedObjectReference('datastore-100')
+        self.assertIsNone(cache.get(moref.value))
+        mock_get_method = mock.Mock(return_value=ds_browser)
+        with mock.patch.object(vim_util, 'get_dynamic_property',
+                               mock_get_method):
+            ret = self._imagecache._get_ds_browser(moref)
+            mock_get_method.assert_called_once_with(mock.ANY, moref,
+                                                    'Datastore', 'browser')
+            self.assertIs(ds_browser, ret)
+            self.assertIs(ds_browser, cache.get(moref.value))
 
     def test_list_base_images(self):
         def fake_get_dynamic_property(vim, mobj, type, property_name):
@@ -140,7 +132,8 @@ class ImageCacheManagerTestCase(test.NoDBTestCase):
             mock.patch.object(ds_util, 'get_sub_folders',
                               fake_get_sub_folders)
         ) as (_get_dynamic, _get_sub_folders):
-            datastore = {'name': 'ds', 'ref': 'fake-ds-ref'}
+            fake_ds_ref = fake.ManagedObjectReference('fake-ds-ref')
+            datastore = {'name': 'ds', 'ref': fake_ds_ref}
             ds_path = ds_util.build_datastore_path(datastore['name'],
                                                    'base_folder')
             images = self._imagecache._list_datastore_images(

nova/tests/virt/vmwareapi/test_vmops.py

@@ -26,6 +26,7 @@ from nova.virt.vmwareapi import driver
 from nova.virt.vmwareapi import ds_util
 from nova.virt.vmwareapi import error_util
 from nova.virt.vmwareapi import fake as vmwareapi_fake
+from nova.virt.vmwareapi import vim_util
 from nova.virt.vmwareapi import vmops
 
 
@@ -262,3 +263,17 @@ class VMwareVMOpsTestCase(test.NoDBTestCase):
             self.assertEqual("fake_snapshot_ref", snap)
             _wait_for_task.assert_has_calls([
                    mock.call('fake_snapshot_task')])
+
+    def test_get_ds_browser(self):
+        cache = self._vmops._datastore_browser_mapping
+        ds_browser = mock.Mock()
+        moref = vmwareapi_fake.ManagedObjectReference('datastore-100')
+        self.assertIsNone(cache.get(moref.value))
+        mock_call_method = mock.Mock(return_value=ds_browser)
+        with mock.patch.object(self._session, '_call_method',
+                               mock_call_method):
+            ret = self._vmops._get_ds_browser(moref)
+            mock_call_method.assert_called_once_with(vim_util,
+                'get_dynamic_property', moref, 'Datastore', 'browser')
+            self.assertIs(ds_browser, ret)
+            self.assertIs(ds_browser, cache.get(moref.value))

nova/virt/vmwareapi/imagecache.py

@@ -105,12 +105,12 @@ class ImageCacheManager(imagecache.ImageCacheManager):
         return timeutils.parse_strtime(ts, fmt=TIMESTAMP_FORMAT)
 
     def _get_ds_browser(self, ds_ref):
-        ds_browser = self._ds_browser.get(ds_ref)
+        ds_browser = self._ds_browser.get(ds_ref.value)
         if not ds_browser:
             ds_browser = vim_util.get_dynamic_property(
                     self._session._get_vim(), ds_ref,
                     "Datastore", "browser")
-            self._ds_browser[ds_ref] = ds_browser
+            self._ds_browser[ds_ref.value] = ds_browser
         return ds_browser
 
     def _list_datastore_images(self, ds_path, datastore):

nova/virt/vmwareapi/vmops.py

@@ -1550,12 +1550,12 @@ class VMwareVMOps(object):
                   instance=instance)
 
     def _get_ds_browser(self, ds_ref):
-        ds_browser = self._datastore_browser_mapping.get(ds_ref)
+        ds_browser = self._datastore_browser_mapping.get(ds_ref.value)
         if not ds_browser:
             ds_browser = self._session._call_method(
                 vim_util, "get_dynamic_property", ds_ref, "Datastore",
                 "browser")
-            self._datastore_browser_mapping[ds_ref] = ds_browser
+            self._datastore_browser_mapping[ds_ref.value] = ds_browser
         return ds_browser
 
     def get_datacenter_ref_and_name(self, ds_ref):