Browse Source

Guard against missing image cache directory

The original fix of bug 1878024 missed an edge case where on a fresh
hypervisor the image cache directory hasn't been created yet. That
directory is only created when the first image is downloaded.

This patch makes sure that if the cache dir hasn't been created yet then
0 disk is reserved for the cache usage instead of raising and logging an
exception.

Change-Id: Id1bbc955a9099de1abc11b9063fe177896646d03
Related-Bug: #1878024
Closes-Bug: #1884214
(cherry picked from commit a85753778f)
(cherry picked from commit a6a48e876c)
(cherry picked from commit f26823c780)
tags/19.3.0
Balazs Gibizer 1 month ago
parent
commit
ce1287aa7f
2 changed files with 61 additions and 34 deletions
  1. +38
    -17
      nova/tests/unit/virt/libvirt/test_imagecache.py
  2. +23
    -17
      nova/virt/libvirt/imagecache.py

+ 38
- 17
nova/tests/unit/virt/libvirt/test_imagecache.py View File

@@ -732,12 +732,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(
@@ -745,37 +745,34 @@ class ImageCacheManagerTestCase(test.NoDBTestCase):
CONF.instances_path,
CONF.image_cache_subdirectory_name))])

mock_stat.reset_mock()
@mock.patch('os.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)]
self.assertTrue(manager.cache_dir_is_on_same_dev_as_instances_dir)
mock_listdir.return_value = []

@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.stat')
@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_listdir.return_value = []
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
@@ -791,3 +788,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())

+ 23
- 17
nova/virt/libvirt/imagecache.py View File

@@ -421,26 +421,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:
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)))
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

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

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

Loading…
Cancel
Save