From 3e06135bec88c1c308124f2f0a55d448434ae8ae Mon Sep 17 00:00:00 2001 From: Lee Yarwood Date: Fri, 6 Mar 2020 17:02:35 +0000 Subject: [PATCH] images: Make JSON the default output format of calls to qemu-img info oslo.utils is planning to make JSON the default output format parsed when creating QemuImgInfo objects. As such this change makes JSON the default output_format requested when calling qemu-img info. The majority of this change is actually test removal from nova.tests.unit.virt.libvirt.test_utils as these human readable qemu-img based tests now duplicate tests found in oslo.utils itself. Change-Id: I56676713571e79f05ee3f0bffc5da8386e02c5d4 --- nova/privsep/qemu.py | 11 +- nova/tests/unit/privsep/test_qemu.py | 24 ++ nova/tests/unit/virt/libvirt/test_driver.py | 18 +- nova/tests/unit/virt/libvirt/test_utils.py | 280 -------------------- nova/tests/unit/virt/test_images.py | 10 +- nova/virt/images.py | 20 +- nova/virt/libvirt/driver.py | 3 +- 7 files changed, 46 insertions(+), 320 deletions(-) diff --git a/nova/privsep/qemu.py b/nova/privsep/qemu.py index 2b2acfd3e8c7..529c24faf311 100644 --- a/nova/privsep/qemu.py +++ b/nova/privsep/qemu.py @@ -82,17 +82,16 @@ def unprivileged_convert_image(source, dest, in_format, out_format, @nova.privsep.sys_admin_pctxt.entrypoint -def privileged_qemu_img_info(path, format=None, output_format=None): +def privileged_qemu_img_info(path, format=None): """Return an oject containing the parsed output from qemu-img info This is a privileged call to qemu-img info using the sys_admin_pctxt entrypoint allowing host block devices etc to be accessed. """ - return unprivileged_qemu_img_info( - path, format=format, output_format=output_format) + return unprivileged_qemu_img_info(path, format=format) -def unprivileged_qemu_img_info(path, format=None, output_format=None): +def unprivileged_qemu_img_info(path, format=None): """Return an object containing the parsed output from qemu-img info.""" try: # The following check is about ploop images that reside within @@ -103,12 +102,10 @@ def unprivileged_qemu_img_info(path, format=None, output_format=None): cmd = ( 'env', 'LC_ALL=C', 'LANG=C', 'qemu-img', 'info', path, - '--force-share', + '--force-share', '--output=json', ) if format is not None: cmd = cmd + ('-f', format) - if output_format is not None: - cmd = cmd + ("--output=%s" % (output_format),) out, err = processutils.execute(*cmd, prlimit=QEMU_IMG_LIMITS) except processutils.ProcessExecutionError as exp: if exp.exit_code == -9: diff --git a/nova/tests/unit/privsep/test_qemu.py b/nova/tests/unit/privsep/test_qemu.py index 5fbc17898373..85c48aa4ae27 100644 --- a/nova/tests/unit/privsep/test_qemu.py +++ b/nova/tests/unit/privsep/test_qemu.py @@ -52,3 +52,27 @@ class QemuTestCase(test.NoDBTestCase): def test_convert_image_unprivileged(self): self._test_convert_image(nova.privsep.qemu.unprivileged_convert_image) + + @mock.patch('oslo_concurrency.processutils.execute') + @mock.patch('os.path.isdir') + def _test_qemu_img_info(self, method, mock_isdir, mock_execute): + mock_isdir.return_value = False + mock_execute.return_value = (mock.sentinel.out, None) + expected_cmd = ( + 'env', 'LC_ALL=C', 'LANG=C', 'qemu-img', 'info', + mock.sentinel.path, '--force-share', '--output=json', '-f', + mock.sentinel.format) + + # Assert that the output from processutils is returned + self.assertEqual( + mock.sentinel.out, + method(mock.sentinel.path, format=mock.sentinel.format)) + # Assert that the expected command is used + mock_execute.assert_called_once_with( + *expected_cmd, prlimit=nova.privsep.qemu.QEMU_IMG_LIMITS) + + def test_privileged_qemu_img_info(self): + self._test_qemu_img_info(nova.privsep.qemu.privileged_qemu_img_info) + + def test_unprivileged_qemu_img_info(self): + self._test_qemu_img_info(nova.privsep.qemu.unprivileged_qemu_img_info) diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 540fcd6a3076..040e44b59db1 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -9464,8 +9464,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, mock_get_encryption_metadata.assert_called_once_with( self.context, drvr._volume_api, uuids.volume_id, connection_info) - mock_qemu_img_info.assert_called_once_with( - mock.sentinel.device_path, output_format='json') + mock_qemu_img_info.assert_called_once_with(mock.sentinel.device_path) # Assert that the Libvirt call to resize the device within the instance # is called with the LUKSv1 payload offset taken into account. @@ -9522,8 +9521,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, mock_get_encryption_metadata.assert_called_once_with( self.context, drvr._volume_api, uuids.volume_id, connection_info) - mock_qemu_img_info.assert_called_once_with( - 'rbd:pool/volume', output_format='json') + mock_qemu_img_info.assert_called_once_with('rbd:pool/volume') # Assert that the Libvirt call to resize the device within the instance # is called with the LUKSv1 payload offset taken into account. @@ -19862,10 +19860,10 @@ class LibvirtConnTestCase(test.NoDBTestCase, @mock.patch('os.path.exists') @mock.patch('os.path.getsize') @mock.patch('os.path.isdir') - @mock.patch('oslo_concurrency.processutils.execute') + @mock.patch('nova.virt.images.qemu_img_info') @mock.patch.object(host.Host, '_get_domain') def test_get_instance_disk_info_parallels_ct(self, mock_get_domain, - mock_execute, + mock_qemu_img_info, mock_isdir, mock_getsize, mock_exists, @@ -19880,10 +19878,9 @@ class LibvirtConnTestCase(test.NoDBTestCase, "" "") - ret = ("image: /test/disk/root.hds\n" - "file format: parallels\n" - "virtual size: 20G (21474836480 bytes)\n" - "disk size: 789M\n") + mock_qemu_img_info.return_value = mock.Mock( + virtual_size=21474836480, image="/test/disk/root.hds", + file_format="ploop", size=827327254, backing_file=None) self.flags(virt_type='parallels', group='libvirt') instance = objects.Instance(**self.test_instance) @@ -19903,7 +19900,6 @@ class LibvirtConnTestCase(test.NoDBTestCase, mock_getsize.side_effect = getsize_sideeffect mock_exists.return_value = True mock_isdir.return_value = True - mock_execute.return_value = (ret, '') info = drvr.get_instance_disk_info(instance) info = jsonutils.loads(info) diff --git a/nova/tests/unit/virt/libvirt/test_utils.py b/nova/tests/unit/virt/libvirt/test_utils.py index 97b138614566..c53dfd239304 100644 --- a/nova/tests/unit/virt/libvirt/test_utils.py +++ b/nova/tests/unit/virt/libvirt/test_utils.py @@ -37,7 +37,6 @@ from nova import test from nova.tests import fixtures as nova_fixtures from nova.tests.unit import fake_instance from nova.tests.unit.virt.libvirt import fakelibvirt -from nova.virt.disk import api as disk from nova.virt import images from nova.virt.libvirt import guest as libvirt_guest from nova.virt.libvirt import utils as libvirt_utils @@ -93,244 +92,6 @@ class LibvirtUtilsTestCase(test.NoDBTestCase): mock_exists.assert_called_once_with("%s/DiskDescriptor.xml" % path) self.assertEqual('ploop', d_type) - @mock.patch('os.path.exists', return_value=True) - @mock.patch('oslo_concurrency.processutils.execute') - def test_disk_backing(self, mock_execute, mock_exists): - path = '/myhome/disk.config' - template_output = """image: %(path)s -file format: raw -virtual size: 2K (2048 bytes) -cluster_size: 65536 -disk size: 96K -""" - output = template_output % ({ - 'path': path, - }) - mock_execute.return_value = (output, '') - d_backing = libvirt_utils.get_disk_backing_file(path) - mock_execute.assert_called_once_with( - 'env', 'LC_ALL=C', 'LANG=C', 'qemu-img', 'info', path, - '--force-share', prlimit=nova.privsep.qemu.QEMU_IMG_LIMITS) - mock_exists.assert_called_once_with(path) - self.assertIsNone(d_backing) - - def _test_disk_size(self, mock_execute, path, expected_size): - d_size = libvirt_utils.get_disk_size(path) - self.assertEqual(expected_size, d_size) - mock_execute.assert_called_once_with( - 'env', 'LC_ALL=C', 'LANG=C', 'qemu-img', 'info', path, - '--force-share', prlimit=nova.privsep.qemu.QEMU_IMG_LIMITS) - - @mock.patch('os.path.exists', return_value=True) - def test_disk_size(self, mock_exists): - path = '/myhome/disk.config' - template_output = """image: %(path)s -file format: raw -virtual size: %(v_size)s (%(vsize_b)s bytes) -cluster_size: 65536 -disk size: 96K -""" - for i in range(0, 128): - bytes = i * 65336 - kbytes = bytes / 1024 - mbytes = kbytes / 1024 - output = template_output % ({ - 'v_size': "%sM" % (mbytes), - 'vsize_b': i, - 'path': path, - }) - with mock.patch('oslo_concurrency.processutils.execute', - return_value=(output, '')) as mock_execute: - self._test_disk_size(mock_execute, path, i) - output = template_output % ({ - 'v_size': "%sK" % (kbytes), - 'vsize_b': i, - 'path': path, - }) - with mock.patch('oslo_concurrency.processutils.execute', - return_value=(output, '')) as mock_execute: - self._test_disk_size(mock_execute, path, i) - - @mock.patch('os.path.exists', return_value=True) - @mock.patch('oslo_concurrency.processutils.execute') - def test_qemu_img_info_json(self, mock_execute, mock_exists): - path = "disk.config" - example_output = """{ - "virtual-size": 67108864, - "filename": "disk.config", - "cluster-size": 65536, - "format": "raw", - "actual-size": 98304 -} -""" - mock_execute.return_value = (example_output, '') - image_info = images.qemu_img_info(path, output_format='json') - mock_execute.assert_called_once_with( - 'env', 'LC_ALL=C', 'LANG=C', 'qemu-img', 'info', path, - '--force-share', '--output=json', - prlimit=nova.privsep.qemu.QEMU_IMG_LIMITS) - mock_exists.assert_called_once_with(path) - self.assertEqual('disk.config', image_info.image) - self.assertEqual('raw', image_info.file_format) - self.assertEqual(67108864, image_info.virtual_size) - self.assertEqual(98304, image_info.disk_size) - self.assertEqual(65536, image_info.cluster_size) - - @mock.patch('os.path.exists', return_value=True) - @mock.patch('oslo_concurrency.processutils.execute') - def test_qemu_info_canon(self, mock_execute, mock_exists): - path = "disk.config" - example_output = """image: disk.config -file format: raw -virtual size: 64M (67108864 bytes) -cluster_size: 65536 -disk size: 96K -blah BLAH: bb -""" - mock_execute.return_value = (example_output, '') - image_info = images.qemu_img_info(path) - mock_execute.assert_called_once_with( - 'env', 'LC_ALL=C', 'LANG=C', 'qemu-img', 'info', path, - '--force-share', prlimit=nova.privsep.qemu.QEMU_IMG_LIMITS) - mock_exists.assert_called_once_with(path) - self.assertEqual('disk.config', image_info.image) - self.assertEqual('raw', image_info.file_format) - self.assertEqual(67108864, image_info.virtual_size) - self.assertEqual(98304, image_info.disk_size) - self.assertEqual(65536, image_info.cluster_size) - - @mock.patch('os.path.exists', return_value=True) - @mock.patch('oslo_concurrency.processutils.execute') - def test_qemu_info_canon2(self, mock_execute, mock_exists): - path = "disk.config" - example_output = """image: disk.config -file format: QCOW2 -virtual size: 67108844 -cluster_size: 65536 -disk size: 963434 -backing file: /var/lib/nova/a328c7998805951a_2 -""" - mock_execute.return_value = (example_output, '') - image_info = images.qemu_img_info(path) - mock_execute.assert_called_once_with( - 'env', 'LC_ALL=C', 'LANG=C', 'qemu-img', 'info', path, - '--force-share', prlimit=nova.privsep.qemu.QEMU_IMG_LIMITS) - mock_exists.assert_called_once_with(path) - self.assertEqual('disk.config', image_info.image) - self.assertEqual('qcow2', image_info.file_format) - self.assertEqual(67108844, image_info.virtual_size) - self.assertEqual(963434, image_info.disk_size) - self.assertEqual(65536, image_info.cluster_size) - self.assertEqual('/var/lib/nova/a328c7998805951a_2', - image_info.backing_file) - - @mock.patch('os.path.exists', return_value=True) - @mock.patch('os.path.isdir', return_value=True) - @mock.patch('oslo_concurrency.processutils.execute') - def test_qemu_info_ploop(self, mock_execute, mock_isdir, mock_exists): - path = "/var/lib/nova" - example_output = """image: root.hds -file format: parallels -virtual size: 3.0G (3221225472 bytes) -disk size: 706M -""" - mock_execute.return_value = (example_output, '') - image_info = images.qemu_img_info(path) - mock_execute.assert_called_once_with( - 'env', 'LC_ALL=C', 'LANG=C', 'qemu-img', 'info', - os.path.join(path, 'root.hds'), '--force-share', - prlimit=nova.privsep.qemu.QEMU_IMG_LIMITS) - mock_isdir.assert_called_once_with(path) - self.assertEqual(2, mock_exists.call_count) - self.assertEqual(path, mock_exists.call_args_list[0][0][0]) - self.assertEqual(os.path.join(path, 'DiskDescriptor.xml'), - mock_exists.call_args_list[1][0][0]) - self.assertEqual('root.hds', image_info.image) - self.assertEqual('parallels', image_info.file_format) - self.assertEqual(3221225472, image_info.virtual_size) - self.assertEqual(740294656, image_info.disk_size) - - @mock.patch('os.path.exists', return_value=True) - @mock.patch('oslo_concurrency.processutils.execute') - def test_qemu_backing_file_actual(self, - mock_execute, mock_exists): - path = "disk.config" - example_output = """image: disk.config -file format: raw -virtual size: 64M (67108864 bytes) -cluster_size: 65536 -disk size: 96K -Snapshot list: -ID TAG VM SIZE DATE VM CLOCK -1 d9a9784a500742a7bb95627bb3aace38 0 2012-08-20 10:52:46 00:00:00.000 -backing file: /var/lib/nova/a328c7998805951a_2 (actual path: /b/3a988059e51a_2) -""" - mock_execute.return_value = (example_output, '') - image_info = images.qemu_img_info(path) - mock_execute.assert_called_once_with( - 'env', 'LC_ALL=C', 'LANG=C', 'qemu-img', 'info', path, - '--force-share', prlimit=nova.privsep.qemu.QEMU_IMG_LIMITS) - mock_exists.assert_called_once_with(path) - self.assertEqual('disk.config', image_info.image) - self.assertEqual('raw', image_info.file_format) - self.assertEqual(67108864, image_info.virtual_size) - self.assertEqual(98304, image_info.disk_size) - self.assertEqual(1, len(image_info.snapshots)) - self.assertEqual('/b/3a988059e51a_2', - image_info.backing_file) - - @mock.patch('os.path.exists', return_value=True) - @mock.patch('oslo_concurrency.processutils.execute') - def test_qemu_info_convert(self, mock_execute, mock_exists): - path = "disk.config" - example_output = """image: disk.config -file format: raw -virtual size: 64M -disk size: 96K -Snapshot list: -ID TAG VM SIZE DATE VM CLOCK -1 d9a9784a500742a7bb95627bb3aace38 0 2012-08-20 10:52:46 00:00:00.000 -3 d9a9784a500742a7bb95627bb3aace38 0 2012-08-20 10:52:46 00:00:00.000 -4 d9a9784a500742a7bb95627bb3aace38 0 2012-08-20 10:52:46 00:00:00.000 -junk stuff: bbb -""" - mock_execute.return_value = (example_output, '') - image_info = images.qemu_img_info(path) - mock_execute.assert_called_once_with( - 'env', 'LC_ALL=C', 'LANG=C', 'qemu-img', 'info', path, - '--force-share', prlimit=nova.privsep.qemu.QEMU_IMG_LIMITS) - mock_exists.assert_called_once_with(path) - self.assertEqual('disk.config', image_info.image) - self.assertEqual('raw', image_info.file_format) - self.assertEqual(67108864, image_info.virtual_size) - self.assertEqual(98304, image_info.disk_size) - - @mock.patch('os.path.exists', return_value=True) - @mock.patch('oslo_concurrency.processutils.execute') - def test_qemu_info_snaps(self, mock_execute, mock_exists): - path = "disk.config" - example_output = """image: disk.config -file format: raw -virtual size: 64M (67108864 bytes) -disk size: 96K -Snapshot list: -ID TAG VM SIZE DATE VM CLOCK -1 d9a9784a500742a7bb95627bb3aace38 0 2012-08-20 10:52:46 00:00:00.000 -3 d9a9784a500742a7bb95627bb3aace38 0 2012-08-20 10:52:46 00:00:00.000 -4 d9a9784a500742a7bb95627bb3aace38 0 2012-08-20 10:52:46 00:00:00.000 -""" - mock_execute.return_value = (example_output, '') - image_info = images.qemu_img_info(path) - mock_execute.assert_called_once_with( - 'env', 'LC_ALL=C', 'LANG=C', 'qemu-img', 'info', path, - '--force-share', prlimit=nova.privsep.qemu.QEMU_IMG_LIMITS) - mock_exists.assert_called_once_with(path) - self.assertEqual('disk.config', image_info.image) - self.assertEqual('raw', image_info.file_format) - self.assertEqual(67108864, image_info.virtual_size) - self.assertEqual(98304, image_info.disk_size) - self.assertEqual(3, len(image_info.snapshots)) - def test_valid_hostname_normal(self): self.assertTrue(libvirt_utils.is_valid_hostname("hello.world.com")) @@ -467,22 +228,6 @@ ID TAG VM SIZE DATE VM CLOCK libvirt_utils.pick_disk_driver_name(version)) mock_execute.reset_mock() - @mock.patch('os.path.exists', return_value=True) - @mock.patch('oslo_concurrency.processutils.execute') - def test_get_disk_size(self, mock_execute, mock_exists): - path = '/some/path' - example_output = """image: 00000001 -file format: raw -virtual size: 4.4M (4592640 bytes) -disk size: 4.4M -""" - mock_execute.return_value = (example_output, '') - self.assertEqual(4592640, disk.get_disk_size('/some/path')) - mock_execute.assert_called_once_with( - 'env', 'LC_ALL=C', 'LANG=C', 'qemu-img', 'info', path, - '--force-share', prlimit=nova.privsep.qemu.QEMU_IMG_LIMITS) - mock_exists.assert_called_once_with(path) - def test_copy_image(self): dst_fd, dst_path = tempfile.mkstemp() try: @@ -741,31 +486,6 @@ disk size: 4.4M del self.executes - def test_get_disk_backing_file(self): - with_actual_path = False - - def fake_execute(*args, **kwargs): - if with_actual_path: - return ("some: output\n" - "backing file: /foo/bar/baz (actual path: /a/b/c)\n" - "...: ...\n"), '' - else: - return ("some: output\n" - "backing file: /foo/bar/baz\n" - "...: ...\n"), '' - - def return_true(*args, **kwargs): - return True - - self.stub_out('oslo_concurrency.processutils.execute', fake_execute) - self.stub_out('os.path.exists', return_true) - - out = libvirt_utils.get_disk_backing_file('') - self.assertEqual(out, 'baz') - with_actual_path = True - out = libvirt_utils.get_disk_backing_file('') - self.assertEqual(out, 'c') - def test_get_instance_path_at_destination(self): instance = fake_instance.fake_instance_obj(None, name='fake_inst', uuid=uuids.instance) diff --git a/nova/tests/unit/virt/test_images.py b/nova/tests/unit/virt/test_images.py index 296aa24b09d7..199d4cf8e1c6 100644 --- a/nova/tests/unit/virt/test_images.py +++ b/nova/tests/unit/virt/test_images.py @@ -41,22 +41,20 @@ class QemuTestCase(test.NoDBTestCase): '/fake/path') @mock.patch.object(os.path, 'exists', return_value=True) - @mock.patch('oslo_concurrency.processutils.execute', - return_value=('stdout', None)) + @mock.patch('nova.privsep.qemu.unprivileged_qemu_img_info', + return_value={}) def test_qemu_info_with_no_errors(self, path_exists, utils_execute): image_info = images.qemu_img_info('/fake/path') self.assertTrue(image_info) - self.assertTrue(str(image_info)) - @mock.patch('oslo_concurrency.processutils.execute', - return_value=('stdout', None)) + @mock.patch('nova.privsep.qemu.unprivileged_qemu_img_info', + return_value={}) def test_qemu_info_with_rbd_path(self, utils_execute): # Assert that the use of a RBD URI as the path doesn't raise # exception.DiskNotFound image_info = images.qemu_img_info('rbd:volume/pool') self.assertTrue(image_info) - self.assertTrue(str(image_info)) @mock.patch.object(compute_utils, 'disk_ops_semaphore') @mock.patch('nova.privsep.utils.supports_direct_io', return_value=True) diff --git a/nova/virt/images.py b/nova/virt/images.py index 800ebca3c79d..5358f3766ac3 100644 --- a/nova/virt/images.py +++ b/nova/virt/images.py @@ -39,30 +39,22 @@ CONF = nova.conf.CONF IMAGE_API = glance.API() -def qemu_img_info(path, format=None, output_format=None): +def qemu_img_info(path, format=None): """Return an object containing the parsed output from qemu-img info.""" if not os.path.exists(path) and not path.startswith('rbd:'): raise exception.DiskNotFound(location=path) - info = nova.privsep.qemu.unprivileged_qemu_img_info( - path, format=format, output_format=output_format) - if output_format: - return imageutils.QemuImgInfo(info, format=output_format) - else: - return imageutils.QemuImgInfo(info) + info = nova.privsep.qemu.unprivileged_qemu_img_info(path, format=format) + return imageutils.QemuImgInfo(info, format='json') -def privileged_qemu_img_info(path, format=None, output_format=None): +def privileged_qemu_img_info(path, format=None, output_format='json'): """Return an object containing the parsed output from qemu-img info.""" if not os.path.exists(path) and not path.startswith('rbd:'): raise exception.DiskNotFound(location=path) - info = nova.privsep.qemu.privileged_qemu_img_info( - path, format=format, output_format=output_format) - if output_format: - return imageutils.QemuImgInfo(info, format=output_format) - else: - return imageutils.QemuImgInfo(info) + info = nova.privsep.qemu.privileged_qemu_img_info(path, format=format) + return imageutils.QemuImgInfo(info, format='json') def convert_image(source, dest, in_format, out_format, run_as_root=False, diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 49ca4ec56ef2..d708187cf8c2 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -2017,8 +2017,7 @@ class LibvirtDriver(driver.ComputeDriver): path = 'unknown' raise exception.DiskNotFound(location='unknown') - info = images.privileged_qemu_img_info( - path, output_format='json') + info = images.privileged_qemu_img_info(path) format_specific_data = info.format_specific['data'] payload_offset = format_specific_data['payload-offset']