openstack
/
nova
Code Issues Proposed changes

Merge "Guard against missing image cache directory" into stable/train

This commit is contained in:
Zuul 2020-07-09 21:39:07 +00:00 committed by Gerrit Code Review
parent 5e36e91bb2 f26823c780
commit 64ec4198c0
2 changed files with 66 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

65
nova/tests/unit/virt/libvirt/test_imagecache.py
View File

@ -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())

40
nova/virt/libvirt/imagecache.py
View File

@ -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)