openstack/nova
Code Issues Proposed changes

Merge "Move execs of touch to privsep."

Browse Source
This commit is contained in:
Jenkins 2017-09-08 18:21:28 +00:00 committed by Gerrit Code Review
commit 4a8020c2e6
11 changed files with 67 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
etc/nova/rootwrap.d/compute.filters
View File

@ -233,8 +233,5 @@ ploop: RegExpFilter, ploop, root, ploop, init, -s, .*, -f, .*, -t, .*, .*
# nova/virt/libvirt/utils.py: 'xend', 'status'
xend: CommandFilter, xend, root
# nova/virt/libvirt/utils.py:
touch: CommandFilter, touch, root
# nova/virt/libvirt/volume/vzstorage.py
pstorage-mount: CommandFilter, pstorage-mount, root

12
nova/privsep/dac_admin.py
View File

@ -65,6 +65,18 @@ def chmod(path, mode):
os.chmod(path, mode)
@nova.privsep.dac_admin_pctxt.entrypoint
def utime(path):
if not os.path.exists(path):
raise exception.FileNotFound(file_path=path)
# NOTE(mikal): the old version of this used execute(touch, ...), which
# would apparently fail on shared storage when multiple instances were
# being launched at the same time. If we see failures here, we might need
# to wrap this in a try / except.
os.utime(path, None)
class path(object):
@staticmethod
@nova.privsep.dac_admin_pctxt.entrypoint

4
nova/tests/unit/virt/libvirt/fake_libvirt_utils.py
View File

@ -82,10 +82,6 @@ def write_to_file(path, contents, umask=None):
pass
def update_mtime(path):
pass
def extract_snapshot(disk_path, source_fmt, out_path, dest_fmt):
files[out_path] = b''

15
nova/tests/unit/virt/libvirt/test_driver.py
View File

@ -9690,7 +9690,9 @@ class LibvirtConnTestCase(test.NoDBTestCase,
self.context, instance,
"/fake/instance/dir", disk_info)
def test_create_images_and_backing_images_not_exist_fallback(self):
@mock.patch('nova.privsep.dac_admin.utime')
def test_create_images_and_backing_images_not_exist_fallback(self,
mock_utime):
conn = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
base_dir = os.path.join(CONF.instances_path,
@ -9742,6 +9744,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
mock.call(self.context, ramdisk_path, instance.ramdisk_id)
])
mock_utime.assert_called()
@mock.patch.object(libvirt_driver.libvirt_utils, 'fetch_image')
def test_create_images_and_backing_images_exist(self, mock_fetch_image):
conn = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
@ -9768,7 +9772,9 @@ class LibvirtConnTestCase(test.NoDBTestCase,
'/fake/instance/dir', disk_info)
self.assertFalse(mock_fetch_image.called)
def test_create_images_and_backing_ephemeral_gets_created(self):
@mock.patch('nova.privsep.dac_admin.utime')
def test_create_images_and_backing_ephemeral_gets_created(self,
mock_utime):
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
base_dir = os.path.join(CONF.instances_path,
@ -9812,6 +9818,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
mock.call(ephemeral_backing, 1 * units.Gi)
])
mock_utime.assert_called()
def test_create_images_and_backing_disk_info_none(self):
instance = objects.Instance(**self.test_instance)
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
@ -11160,7 +11168,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
'myVol', '/dev/something',
run_as_root=True)
def test_create_ephemeral_specified_fs_not_valid(self):
@mock.patch('nova.privsep.dac_admin.utime')
def test_create_ephemeral_specified_fs_not_valid(self, mock_utime):
CONF.set_override('default_ephemeral_format', 'ext4')
ephemerals = [{'device_type': 'disk',
'disk_bus': 'virtio',

40
nova/tests/unit/virt/libvirt/test_imagebackend.py
View File

@ -341,7 +341,8 @@ class FlatTestCase(_ImageTestCase, test.NoDBTestCase):
@mock.patch.object(imagebackend.disk, 'extend')
@mock.patch.object(fake_libvirt_utils, 'copy_image')
@mock.patch.object(imagebackend.utils, 'synchronized')
def test_create_image(self, mock_sync, mock_copy, mock_extend):
@mock.patch('nova.privsep.dac_admin.utime')
def test_create_image(self, mock_utime, mock_sync, mock_copy, mock_extend):
mock_sync.side_effect = lambda *a, **kw: self._fake_deco
fn = mock.MagicMock()
image = self.image_class(self.INSTANCE, self.NAME)
@ -351,6 +352,7 @@ class FlatTestCase(_ImageTestCase, test.NoDBTestCase):
fn.assert_called_once_with(target=self.TEMPLATE_PATH, image_id=None)
self.assertTrue(mock_sync.called)
self.assertFalse(mock_extend.called)
mock_utime.assert_called()
@mock.patch.object(imagebackend.disk, 'extend')
@mock.patch.object(fake_libvirt_utils, 'copy_image')
@ -372,8 +374,9 @@ class FlatTestCase(_ImageTestCase, test.NoDBTestCase):
@mock.patch.object(imagebackend.utils, 'synchronized')
@mock.patch.object(images, 'qemu_img_info',
return_value=imageutils.QemuImgInfo())
def test_create_image_extend(self, mock_qemu, mock_sync, mock_copy,
mock_extend):
@mock.patch('nova.privsep.dac_admin.utime')
def test_create_image_extend(self, mock_utime, mock_qemu, mock_sync,
mock_copy, mock_extend):
mock_sync.side_effect = lambda *a, **kw: self._fake_deco
fn = mock.MagicMock()
mock_qemu.return_value.virtual_size = 1024
@ -389,6 +392,7 @@ class FlatTestCase(_ImageTestCase, test.NoDBTestCase):
imgmodel.LocalFileImage(self.PATH, imgmodel.FORMAT_RAW),
self.SIZE)
mock_qemu.assert_called_once_with(self.TEMPLATE_PATH)
mock_utime.assert_called()
@mock.patch.object(os.path, 'exists')
@mock.patch.object(imagebackend.images, 'qemu_img_info')
@ -498,7 +502,9 @@ class Qcow2TestCase(_ImageTestCase, test.NoDBTestCase):
@mock.patch.object(imagebackend.utils, 'synchronized')
@mock.patch.object(fake_libvirt_utils, 'create_cow_image')
@mock.patch.object(imagebackend.disk, 'extend')
def test_create_image(self, mock_extend, mock_create, mock_sync):
@mock.patch('nova.privsep.dac_admin.utime')
def test_create_image(self, mock_utime, mock_extend, mock_create,
mock_sync):
mock_sync.side_effect = lambda *a, **kw: self._fake_deco
fn = mock.MagicMock()
image = self.image_class(self.INSTANCE, self.NAME)
@ -509,13 +515,15 @@ class Qcow2TestCase(_ImageTestCase, test.NoDBTestCase):
fn.assert_called_once_with(target=self.TEMPLATE_PATH)
self.assertTrue(mock_sync.called)
self.assertFalse(mock_extend.called)
mock_utime.assert_called()
@mock.patch.object(imagebackend.utils, 'synchronized')
@mock.patch.object(fake_libvirt_utils, 'create_cow_image')
@mock.patch.object(imagebackend.disk, 'extend')
@mock.patch.object(os.path, 'exists', side_effect=[])
@mock.patch.object(imagebackend.Image, 'verify_base_size')
def test_create_image_with_size(self, mock_verify, mock_exist,
@mock.patch('nova.privsep.dac_admin.utime')
def test_create_image_with_size(self, mock_utime, mock_verify, mock_exist,
mock_extend, mock_create, mock_sync):
mock_sync.side_effect = lambda *a, **kw: self._fake_deco
fn = mock.MagicMock()
@ -537,14 +545,16 @@ class Qcow2TestCase(_ImageTestCase, test.NoDBTestCase):
fn.assert_called_once_with(target=self.TEMPLATE_PATH)
mock_exist.assert_has_calls(exist_calls)
self.assertTrue(mock_sync.called)
mock_utime.assert_called()
@mock.patch.object(imagebackend.utils, 'synchronized')
@mock.patch.object(fake_libvirt_utils, 'create_cow_image')
@mock.patch.object(imagebackend.disk, 'extend')
@mock.patch.object(os.path, 'exists', side_effect=[])
@mock.patch.object(imagebackend.Qcow2, 'get_disk_size')
def test_create_image_too_small(self, mock_get, mock_exist, mock_extend,
mock_create, mock_sync):
@mock.patch('nova.privsep.dac_admin.utime')
def test_create_image_too_small(self, mock_utime, mock_get, mock_exist,
mock_extend, mock_create, mock_sync):
mock_sync.side_effect = lambda *a, **kw: self._fake_deco
mock_get.return_value = self.SIZE
fn = mock.MagicMock()
@ -569,8 +579,10 @@ class Qcow2TestCase(_ImageTestCase, test.NoDBTestCase):
@mock.patch.object(os.path, 'exists', side_effect=[])
@mock.patch.object(imagebackend.Image, 'verify_base_size')
@mock.patch.object(fake_libvirt_utils, 'copy_image')
def test_generate_resized_backing_files(self, mock_copy, mock_verify,
mock_exist, mock_extend, mock_get,
@mock.patch('nova.privsep.dac_admin.utime')
def test_generate_resized_backing_files(self, mock_utime, mock_copy,
mock_verify, mock_exist,
mock_extend, mock_get,
mock_create, mock_sync):
mock_sync.side_effect = lambda *a, **kw: self._fake_deco
mock_get.return_value = self.QCOW2_BASE
@ -597,6 +609,7 @@ class Qcow2TestCase(_ImageTestCase, test.NoDBTestCase):
fn.assert_called_once_with(target=self.TEMPLATE_PATH)
self.assertTrue(mock_sync.called)
self.assertFalse(mock_create.called)
mock_utime.assert_called()
@mock.patch.object(imagebackend.utils, 'synchronized')
@mock.patch.object(fake_libvirt_utils, 'create_cow_image')
@ -604,10 +617,11 @@ class Qcow2TestCase(_ImageTestCase, test.NoDBTestCase):
@mock.patch.object(imagebackend.disk, 'extend')
@mock.patch.object(os.path, 'exists', side_effect=[])
@mock.patch.object(imagebackend.Image, 'verify_base_size')
def test_qcow2_exists_and_has_no_backing_file(self, mock_verify,
mock_exist, mock_extend,
mock_get, mock_create,
mock_sync):
@mock.patch('nova.privsep.dac_admin.utime')
def test_qcow2_exists_and_has_no_backing_file(self, mock_utime,
mock_verify, mock_exist,
mock_extend, mock_get,
mock_create, mock_sync):
mock_sync.side_effect = lambda *a, **kw: self._fake_deco
mock_get.return_value = None
fn = mock.MagicMock()

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

@ -34,7 +34,6 @@ from nova.tests.unit import fake_instance
from nova.tests import uuidsentinel as uuids
from nova import utils
from nova.virt.libvirt import imagecache
from nova.virt.libvirt import utils as libvirt_utils
CONF = nova.conf.CONF
@ -430,8 +429,8 @@ class ImageCacheManagerTestCase(test.NoDBTestCase):
self.assertNotEqual(stream.getvalue().find('Failed to remove'),
-1)
@mock.patch.object(libvirt_utils, 'update_mtime')
def test_mark_in_use(self, mock_mtime):
@mock.patch('nova.privsep.dac_admin.utime')
def test_mark_in_use(self, mock_utime):
img = '123'
with self._make_base_file() as fname:
@ -440,13 +439,13 @@ class ImageCacheManagerTestCase(test.NoDBTestCase):
image_cache_manager.used_images = {'123': (1, 0, ['banana-42'])}
image_cache_manager._mark_in_use(img, fname)
mock_mtime.assert_called_once_with(fname)
mock_utime.assert_called_once_with(fname)
self.assertEqual(image_cache_manager.unexplained_images, [])
self.assertEqual(image_cache_manager.removable_base_files, [])
@mock.patch.object(libvirt_utils, 'update_mtime')
@mock.patch('nova.privsep.dac_admin.utime')
@mock.patch.object(lockutils, 'external_lock')
def test_verify_base_images(self, mock_lock, mock_mtime):
def test_verify_base_images(self, mock_lock, mock_utime):
hashed_1 = '356a192b7913b04c54574d18c28d46e6395428ab'
hashed_21 = '472b07b9fcf2c2451e8781e944bf5f77cd8457c8'
hashed_22 = '12c6fc06c99a462375eeb3f43dfd832b08ca9e17'
@ -679,12 +678,12 @@ class ImageCacheManagerTestCase(test.NoDBTestCase):
self.assertEqual(image_cache_manager.back_swap_images, expect_set)
@mock.patch.object(lockutils, 'external_lock')
@mock.patch.object(libvirt_utils, 'update_mtime')
@mock.patch('os.path.exists')
@mock.patch('os.path.getmtime')
@mock.patch('os.remove')
def test_age_and_verify_swap_images(self, mock_remove, mock_getmtime,
mock_exist, mock_mtime, mock_lock):
@mock.patch('nova.privsep.dac_admin.utime')
def test_age_and_verify_swap_images(self, mock_utime, mock_remove,
mock_getmtime, mock_exist, mock_lock):
base_dir = '/tmp_age_test'
self.flags(image_info_filename_pattern=base_dir + '/%(image)s.info',
group='libvirt')

7
nova/tests/unit/virt/libvirt/test_utils.py
View File

@ -758,13 +758,6 @@ disk size: 4.4M
image_arch = libvirt_utils.get_arch(image_meta)
self.assertEqual(obj_fields.Architecture.X86_64, image_arch)
def test_update_mtime_error(self):
with mock.patch('nova.utils.execute',
side_effect=processutils.ProcessExecutionError):
with mock.patch.object(libvirt_utils.LOG, 'warning') as mock_log:
libvirt_utils.update_mtime(mock.sentinel.path)
self.assertTrue(mock_log.called)
def test_is_mounted(self):
mount_path = "/var/lib/nova/mnt"
source = "192.168.0.1:/nova"

7
nova/virt/libvirt/imagebackend.py
View File

@ -33,6 +33,7 @@ from nova import exception
from nova.i18n import _
from nova import image
from nova import keymgr
from nova.privsep import dac_admin
from nova import utils
from nova.virt.disk import api as disk
from nova.virt.image import model as imgmodel
@ -540,7 +541,7 @@ class Flat(Image):
# NOTE(mikal): Update the mtime of the base file so the image
# cache manager knows it is in use.
libvirt_utils.update_mtime(base)
dac_admin.utime(base)
self.verify_base_size(base, size)
if not os.path.exists(self.path):
with fileutils.remove_path_on_error(self.path):
@ -596,7 +597,7 @@ class Qcow2(Image):
# NOTE(ankit): Update the mtime of the base file so the image
# cache manager knows it is in use.
libvirt_utils.update_mtime(base)
dac_admin.utime(base)
self.verify_base_size(base, size)
legacy_backing_size = None
@ -1090,7 +1091,7 @@ class Ploop(Image):
prepare_template(target=base, *args, **kwargs)
else:
# Disk already exists in cache, just update time
libvirt_utils.update_mtime(base)
dac_admin.utime(base)
self.verify_base_size(base, size)
if os.path.exists(self.path):

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

@ -32,6 +32,7 @@ from oslo_utils import encodeutils
import six
import nova.conf
from nova.privsep import dac_admin
from nova import utils
from nova.virt import imagecache
from nova.virt.libvirt import utils as libvirt_utils
@ -326,7 +327,7 @@ class ImageCacheManager(imagecache.ImageCacheManager):
LOG.debug('image %(id)s at (%(base_file)s): image is in use',
{'id': img_id, 'base_file': base_file})
libvirt_utils.update_mtime(base_file)
dac_admin.utime(base_file)
def _age_and_verify_swap_images(self, context, base_dir):
LOG.debug('Verify swap images')
@ -334,7 +335,7 @@ class ImageCacheManager(imagecache.ImageCacheManager):
for ent in self.back_swap_images:
base_file = os.path.join(base_dir, ent)
if ent in self.used_swap_images and os.path.exists(base_file):
libvirt_utils.update_mtime(base_file)
dac_admin.utime(base_file)
elif self.remove_unused_base_images:
self._remove_swap_file(base_file)

17
nova/virt/libvirt/utils.py
View File

@ -253,23 +253,6 @@ def write_to_file(path, contents, umask=None):
os.umask(saved_umask)
def update_mtime(path):
"""Touch a file without being the owner.
:param path: File bump the mtime on
"""
try:
utils.execute('touch', '-c', path, run_as_root=True)
except processutils.ProcessExecutionError as exc:
# touch can intermittently fail when launching several instances with
# the same base image and using shared storage, so log the exception
# but don't fail. Ideally we'd know if we were on shared storage and
# would re-raise the error if we are not on shared storage.
LOG.warning("Failed to update mtime on path %(path)s. "
"Error: %(error)s",
{'path': path, "error": exc})
def _id_map_to_config(id_map):
return "%s:%s:%s" % (id_map.start, id_map.target, id_map.count)

2
releasenotes/notes/privsep-queens-rootwrap-adds-907aa1bc8e3eb2ca.yaml
View File

@ -5,4 +5,4 @@ upgrade:
rootwrap configuration.
- |
The following commands are no longer required to be listed in your rootwrap
configuration: cat; chown; readlink.
configuration: cat; chown; readlink; touch.