diff --git a/etc/nova/rootwrap.d/compute.filters b/etc/nova/rootwrap.d/compute.filters index d4124462862a..8786df809ac5 100644 --- a/etc/nova/rootwrap.d/compute.filters +++ b/etc/nova/rootwrap.d/compute.filters @@ -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 diff --git a/nova/privsep/dac_admin.py b/nova/privsep/dac_admin.py index d43c26dfe911..e84070ccab27 100644 --- a/nova/privsep/dac_admin.py +++ b/nova/privsep/dac_admin.py @@ -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 diff --git a/nova/tests/unit/virt/libvirt/fake_libvirt_utils.py b/nova/tests/unit/virt/libvirt/fake_libvirt_utils.py index be27277b5e48..1dcbc2df304d 100644 --- a/nova/tests/unit/virt/libvirt/fake_libvirt_utils.py +++ b/nova/tests/unit/virt/libvirt/fake_libvirt_utils.py @@ -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'' diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 65e98c8c8cc0..7b3216f3d500 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -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', diff --git a/nova/tests/unit/virt/libvirt/test_imagebackend.py b/nova/tests/unit/virt/libvirt/test_imagebackend.py index a083a2cf3038..e48c133dced3 100644 --- a/nova/tests/unit/virt/libvirt/test_imagebackend.py +++ b/nova/tests/unit/virt/libvirt/test_imagebackend.py @@ -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() diff --git a/nova/tests/unit/virt/libvirt/test_imagecache.py b/nova/tests/unit/virt/libvirt/test_imagecache.py index 289a6d49c4a3..0c7a86d347a7 100644 --- a/nova/tests/unit/virt/libvirt/test_imagecache.py +++ b/nova/tests/unit/virt/libvirt/test_imagecache.py @@ -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') diff --git a/nova/tests/unit/virt/libvirt/test_utils.py b/nova/tests/unit/virt/libvirt/test_utils.py index a7bb935d6452..646a72c85991 100644 --- a/nova/tests/unit/virt/libvirt/test_utils.py +++ b/nova/tests/unit/virt/libvirt/test_utils.py @@ -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" diff --git a/nova/virt/libvirt/imagebackend.py b/nova/virt/libvirt/imagebackend.py index 4b75b0023820..a6cd05f7548d 100644 --- a/nova/virt/libvirt/imagebackend.py +++ b/nova/virt/libvirt/imagebackend.py @@ -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): diff --git a/nova/virt/libvirt/imagecache.py b/nova/virt/libvirt/imagecache.py index e87317df299e..dcb0cb66e8af 100644 --- a/nova/virt/libvirt/imagecache.py +++ b/nova/virt/libvirt/imagecache.py @@ -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) diff --git a/nova/virt/libvirt/utils.py b/nova/virt/libvirt/utils.py index 395dcae1ce8d..b334be3439de 100644 --- a/nova/virt/libvirt/utils.py +++ b/nova/virt/libvirt/utils.py @@ -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) diff --git a/releasenotes/notes/privsep-queens-rootwrap-adds-907aa1bc8e3eb2ca.yaml b/releasenotes/notes/privsep-queens-rootwrap-adds-907aa1bc8e3eb2ca.yaml index 2a2c4d3579c9..874ca7522526 100644 --- a/releasenotes/notes/privsep-queens-rootwrap-adds-907aa1bc8e3eb2ca.yaml +++ b/releasenotes/notes/privsep-queens-rootwrap-adds-907aa1bc8e3eb2ca.yaml @@ -5,4 +5,4 @@ upgrade: rootwrap configuration. - | The following commands are no longer required to be listed in your rootwrap - configuration: cat; chown; readlink. \ No newline at end of file + configuration: cat; chown; readlink; touch. \ No newline at end of file