diff --git a/nova/tests/unit/virt/libvirt/test_imagecache.py b/nova/tests/unit/virt/libvirt/test_imagecache.py index 58682613d9cc..89fbccf1f41d 100644 --- a/nova/tests/unit/virt/libvirt/test_imagecache.py +++ b/nova/tests/unit/virt/libvirt/test_imagecache.py @@ -674,12 +674,12 @@ class ImageCacheManagerTestCase(test.NoDBTestCase): lock_path=lock_path) @mock.patch('os.stat') - def test_cache_dir_is_on_same_dev_as_instances_dir(self, mock_stat): + def test_get_disk_usage_cache_is_on_different_disk(self, mock_stat): mock_stat.side_effect = [mock.Mock(st_dev=0), mock.Mock(st_dev=1)] - manager = imagecache.ImageCacheManager() - self.assertFalse(manager.cache_dir_is_on_same_dev_as_instances_dir) + self.assertEqual(0, manager.get_disk_usage()) + mock_stat.assert_has_calls([ mock.call(CONF.instances_path), mock.call( @@ -687,37 +687,34 @@ class ImageCacheManagerTestCase(test.NoDBTestCase): CONF.instances_path, CONF.image_cache_subdirectory_name))]) - mock_stat.reset_mock() - mock_stat.side_effect = [mock.Mock(st_dev=0), mock.Mock(st_dev=0)] - self.assertTrue(manager.cache_dir_is_on_same_dev_as_instances_dir) - - @mock.patch('nova.virt.libvirt.imagecache.ImageCacheManager.' - 'cache_dir_is_on_same_dev_as_instances_dir', - new=mock.PropertyMock(return_value=False)) - def test_get_disk_usage_cache_is_on_different_disk(self): - manager = imagecache.ImageCacheManager() - - self.assertEqual(0, manager.get_disk_usage()) - @mock.patch('os.listdir') - @mock.patch('nova.virt.libvirt.imagecache.ImageCacheManager.' - 'cache_dir_is_on_same_dev_as_instances_dir', - new=mock.PropertyMock(return_value=True)) - def test_get_disk_usage_empty(self, mock_listdir): + @mock.patch('os.stat') + def test_get_disk_usage_empty(self, mock_stat, mock_listdir): + mock_stat.side_effect = [mock.Mock(st_dev=0), mock.Mock(st_dev=0)] mock_listdir.return_value = [] manager = imagecache.ImageCacheManager() self.assertEqual(0, manager.get_disk_usage()) + @mock.patch('os.stat') + @mock.patch('os.listdir') + def test_get_disk_usage_cache_dir_missing(self, mock_listdir, mock_stat): + mock_stat.side_effect = [mock.Mock(st_dev=0), OSError] + + manager = imagecache.ImageCacheManager() + + self.assertEqual(0, manager.get_disk_usage()) + mock_listdir.assert_not_called() + @mock.patch('os.path.isfile') @mock.patch('os.listdir') @mock.patch('os.stat') - @mock.patch('nova.virt.libvirt.imagecache.ImageCacheManager.' - 'cache_dir_is_on_same_dev_as_instances_dir', - new=mock.PropertyMock(return_value=True)) def test_get_disk_usage(self, mock_stat, mock_listdir, mock_isfile): mock_stat.side_effect = [ + # cache and instances dirs are on the same dev + mock.Mock(st_dev=0), + mock.Mock(st_dev=0), # stat calls on each file in the cache directory to get the size mock.Mock(st_blocks=10), # foo mock.Mock(st_blocks=20), # bar @@ -733,3 +730,27 @@ class ImageCacheManagerTestCase(test.NoDBTestCase): # size is calculated from as st_blocks * 512 self.assertEqual(10 * 512 + 20 * 512, manager.get_disk_usage()) + + @mock.patch('os.path.isfile') + @mock.patch('os.listdir') + @mock.patch('os.stat') + def test_get_disk_usage_io_error_while_reading( + self, mock_stat, mock_listdir, mock_isfile): + mock_stat.side_effect = [ + # cache and instances dirs are on the same dev + mock.Mock(st_dev=0), + mock.Mock(st_dev=0), + # stat calls on each file in the cache directory to get the size + mock.Mock(st_blocks=10), # foo + OSError, # error while checking bar + ] + mock_listdir.return_value = ['foo', 'bar', 'some-dir'] + mock_isfile.side_effect = [ + True, # foo + True, # bar + False, # some-dir + ] + + manager = imagecache.ImageCacheManager() + + self.assertEqual(0, manager.get_disk_usage()) diff --git a/nova/virt/libvirt/imagecache.py b/nova/virt/libvirt/imagecache.py index c05e3ff00063..38f46aac1bb5 100644 --- a/nova/virt/libvirt/imagecache.py +++ b/nova/virt/libvirt/imagecache.py @@ -355,26 +355,32 @@ class ImageCacheManager(imagecache.ImageCacheManager): self._age_and_verify_swap_images(context, base_dir) def get_disk_usage(self): - if not self.cache_dir_is_on_same_dev_as_instances_dir: - return 0 + try: + # If the cache is on a different device than the instance dir then + # it does not consume the same disk resource as instances. + # NOTE(gibi): this does not work on Windows properly as st_dev is + # always 0 + if (os.stat(CONF.instances_path).st_dev != + os.stat(self.cache_dir).st_dev): + return 0 - # NOTE(gibi): we need to use the disk size occupied from the file - # system as images in the cache will not grow to their virtual size. - # NOTE(gibi): st.blocks is always measured in 512 byte blocks see man - # fstat - return sum( - os.stat(os.path.join(self.cache_dir, f)).st_blocks * 512 - for f in os.listdir(self.cache_dir) - if os.path.isfile(os.path.join(self.cache_dir, f))) + # NOTE(gibi): we need to use the disk size occupied from the file + # system as images in the cache will not grow to their virtual + # size. + # NOTE(gibi): st.blocks is always measured in 512 byte blocks see + # man fstat + return sum( + os.stat(os.path.join(self.cache_dir, f)).st_blocks * 512 + for f in os.listdir(self.cache_dir) + if os.path.isfile(os.path.join(self.cache_dir, f))) + except OSError: + # NOTE(gibi): An error here can mean many things. E.g. the cache + # dir does not exists yet, the cache dir is deleted between the + # listdir() and the stat() calls, or a file is deleted between + # the listdir() and the stat() calls. + return 0 @property def cache_dir(self): return os.path.join( CONF.instances_path, CONF.image_cache_subdirectory_name) - - @property - def cache_dir_is_on_same_dev_as_instances_dir(self): - # NOTE(gibi): this does not work on Windows properly as st_dev is - # always 0 - return (os.stat(CONF.instances_path).st_dev == - os.stat(self.cache_dir).st_dev)