From 35a591d33d8b1a6c30bf40ddc48a07715fd87339 Mon Sep 17 00:00:00 2001 From: Sean Mooney Date: Tue, 26 Mar 2019 13:06:25 +0000 Subject: [PATCH] extend libvirt video model support - This change extends the VideoModel field object to allow 3 new values (virtio, gop, none) - This change makes the libvirt driver use ALL tuple from the nova.fields.VideoModel object instead of declaring a second tuple inline for validation. - This change allows the virtio video model to now be used for all architectures when explicitly requested via the hw_video_model image metadata property - This change introduces unit tests and a release note for the new capablities. Change-Id: I2830ccfc81cfa9654cfeac7ad5effc294f523552 Implements: blueprint libvirt-video-device-models --- .../ImageMetaPropsPayload.json | 2 +- nova/notifications/objects/image.py | 3 +- nova/objects/fields.py | 5 +- nova/objects/image_meta.py | 12 +++- .../test_instance.py | 4 +- .../objects/test_notification.py | 2 +- nova/tests/unit/objects/test_image_meta.py | 17 ++++++ nova/tests/unit/objects/test_objects.py | 2 +- nova/tests/unit/virt/libvirt/test_driver.py | 60 +++++++++++++++++++ nova/virt/libvirt/driver.py | 22 ++++++- ...-video-model-support-d630b99ef5039f51.yaml | 13 ++++ 11 files changed, 131 insertions(+), 11 deletions(-) create mode 100644 releasenotes/notes/extend-libvirt-video-model-support-d630b99ef5039f51.yaml diff --git a/doc/notification_samples/common_payloads/ImageMetaPropsPayload.json b/doc/notification_samples/common_payloads/ImageMetaPropsPayload.json index e5c32fa61376..2dcc72ba4534 100644 --- a/doc/notification_samples/common_payloads/ImageMetaPropsPayload.json +++ b/doc/notification_samples/common_payloads/ImageMetaPropsPayload.json @@ -4,5 +4,5 @@ "hw_architecture": "x86_64" }, "nova_object.name": "ImageMetaPropsPayload", - "nova_object.version": "1.0" + "nova_object.version": "1.1" } diff --git a/nova/notifications/objects/image.py b/nova/notifications/objects/image.py index d68259731c91..b01b4eade411 100644 --- a/nova/notifications/objects/image.py +++ b/nova/notifications/objects/image.py @@ -105,7 +105,8 @@ class ImageMetaPayload(base.NotificationPayloadBase): @nova_base.NovaObjectRegistry.register_notification class ImageMetaPropsPayload(base.NotificationPayloadBase): # Version 1.0: Initial version - VERSION = '1.0' + # Version 1.1: Added 'gop', 'virtio' and 'none' to hw_video_model field + VERSION = '1.1' SCHEMA = { 'hw_architecture': ('image_meta_props', 'hw_architecture'), diff --git a/nova/objects/fields.py b/nova/objects/fields.py index 02056e21b077..b4d43df195c1 100644 --- a/nova/objects/fields.py +++ b/nova/objects/fields.py @@ -505,8 +505,11 @@ class VideoModel(BaseNovaEnum): VGA = "vga" VMVGA = "vmvga" XEN = "xen" + VIRTIO = 'virtio' + GOP = 'gop' + NONE = 'none' - ALL = (CIRRUS, QXL, VGA, VMVGA, XEN) + ALL = (CIRRUS, QXL, VGA, VMVGA, XEN, VIRTIO, GOP, NONE) class VIFModel(BaseNovaEnum): diff --git a/nova/objects/image_meta.py b/nova/objects/image_meta.py index 6bc151d7f72e..cce40e9b90a1 100644 --- a/nova/objects/image_meta.py +++ b/nova/objects/image_meta.py @@ -171,12 +171,22 @@ class ImageMetaProps(base.NovaObject): # Version 1.19: Added 'img_hide_hypervisor_id' type field # Version 1.20: Added 'traits_required' list field # Version 1.21: Added 'hw_time_hpet' field - VERSION = '1.21' + # Version 1.22: Added 'gop', 'virtio' and 'none' to hw_video_model field + VERSION = '1.22' def obj_make_compatible(self, primitive, target_version): super(ImageMetaProps, self).obj_make_compatible(primitive, target_version) target_version = versionutils.convert_version_to_tuple(target_version) + # NOTE(sean-k-mooney): unlike other nova object we version this object + # when composed object are updated. + if target_version < (1, 22): + video = primitive.get('hw_video_model', None) + if video in ('gop', 'virtio', 'none'): + raise exception.ObjectActionError( + action='obj_make_compatible', + reason='hw_video_model=%s not supported in version %s' % ( + video, target_version)) if target_version < (1, 21): primitive.pop('hw_time_hpet', None) if target_version < (1, 20): diff --git a/nova/tests/functional/notification_sample_tests/test_instance.py b/nova/tests/functional/notification_sample_tests/test_instance.py index cf9d68f92319..e713f5981ad2 100644 --- a/nova/tests/functional/notification_sample_tests/test_instance.py +++ b/nova/tests/functional/notification_sample_tests/test_instance.py @@ -1246,7 +1246,7 @@ class TestInstanceNotificationSample( 'nova_object.data': {}, 'nova_object.name': 'ImageMetaPropsPayload', 'nova_object.namespace': 'nova', - 'nova_object.version': u'1.0'}, + 'nova_object.version': u'1.1'}, 'image.size': 58145823, 'image.tags': [], 'scheduler_hints': {'_nova_check_type': ['rebuild']}, @@ -1341,7 +1341,7 @@ class TestInstanceNotificationSample( 'nova_object.data': {}, 'nova_object.name': 'ImageMetaPropsPayload', 'nova_object.namespace': 'nova', - 'nova_object.version': u'1.0'}, + 'nova_object.version': u'1.1'}, 'image.size': 58145823, 'image.tags': [], 'scheduler_hints': {'_nova_check_type': ['rebuild']}, diff --git a/nova/tests/unit/notifications/objects/test_notification.py b/nova/tests/unit/notifications/objects/test_notification.py index 44652495d394..c05287bfa193 100644 --- a/nova/tests/unit/notifications/objects/test_notification.py +++ b/nova/tests/unit/notifications/objects/test_notification.py @@ -381,7 +381,7 @@ notification_object_data = { 'FlavorNotification': '1.0-a73147b93b520ff0061865849d3dfa56', 'FlavorPayload': '1.4-2e7011b8b4e59167fe8b7a0a81f0d452', 'ImageMetaPayload': '1.0-0e65beeacb3393beed564a57bc2bc989', - 'ImageMetaPropsPayload': '1.0-0665065e198b4ab1b03aa80f442d2302', + 'ImageMetaPropsPayload': '1.1-789c420945f2cae6ac64ca8dffbcb1b8', 'InstanceActionNotification': '1.0-a73147b93b520ff0061865849d3dfa56', 'InstanceActionPayload': '1.8-4fa3da9cbf0761f1f700ae578f36dc2f', 'InstanceActionRebuildNotification': diff --git a/nova/tests/unit/objects/test_image_meta.py b/nova/tests/unit/objects/test_image_meta.py index e85641c7d0e3..0384f5b85cf7 100644 --- a/nova/tests/unit/objects/test_image_meta.py +++ b/nova/tests/unit/objects/test_image_meta.py @@ -349,6 +349,23 @@ class TestImageMetaProps(test.NoDBTestCase): self.assertRaises(exception.ObjectActionError, obj.obj_to_primitive, '1.0') + def test_obj_make_compatible_video_model(self): + # assert that older video models are not preserved. + obj = objects.ImageMetaProps( + hw_video_model=objects.fields.VideoModel.QXL) + primitive = obj.obj_to_primitive('1.0') + self.assertIn("hw_video_model", primitive['nova_object.data']) + + # Virtio, GOP and None were added in 1.22 and should raise and + # exception when backleveling. + models = [objects.fields.VideoModel.VIRTIO, + objects.fields.VideoModel.GOP, + objects.fields.VideoModel.NONE] + for model in models: + obj = objects.ImageMetaProps(hw_video_model=model) + self.assertRaises(exception.ObjectActionError, + obj.obj_to_primitive, '1.0') + def test_obj_make_compatible_watchdog_action_not_disabled(self): """Tests that we don't pop the hw_watchdog_action if the value is not 'disabled'. diff --git a/nova/tests/unit/objects/test_objects.py b/nova/tests/unit/objects/test_objects.py index 44bafbab4ad0..03c52aa6608c 100644 --- a/nova/tests/unit/objects/test_objects.py +++ b/nova/tests/unit/objects/test_objects.py @@ -1071,7 +1071,7 @@ object_data = { 'HyperVLiveMigrateData': '1.4-e265780e6acfa631476c8170e8d6fce0', 'IDEDeviceBus': '1.0-29d4c9f27ac44197f01b6ac1b7e16502', 'ImageMeta': '1.8-642d1b2eb3e880a367f37d72dd76162d', - 'ImageMetaProps': '1.21-f3721d8f744a9507a1966c81c386b532', + 'ImageMetaProps': '1.22-b1c9ea78c43e8d7989a7d1f0f34d149a', 'Instance': '2.5-94e3881f0b9a06c2c3640e44816b51de', 'InstanceAction': '1.1-f9f293e526b66fca0d05c3b3a2d13914', 'InstanceActionEvent': '1.2-b2f368b8a29d8d872b1f6ea841e820a0', diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 513669588e00..e5ad46402379 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -5410,6 +5410,66 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertEqual(cfg.devices[5].type, "qxl") self.assertEqual(cfg.devices[5].vram, 64 * units.Mi / units.Ki) + def _test_add_video_driver(self, model): + self.flags(virt_type='kvm', group='libvirt') + # we could have used VNC here also we just need to enable + # one of the graphic consoles libvirt supports or else + # the call to _guest_add_video_device will not work. + self.flags(enabled=True, agent_enabled=True, group='spice') + + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) + guest = vconfig.LibvirtConfigGuest() + instance_ref = objects.Instance(**self.test_instance) + flavor = instance_ref.get_flavor() + image_meta = objects.ImageMeta.from_dict({ + 'properties': {'hw_video_model': model}}) + + self.assertTrue(drvr._guest_add_video_device(guest)) + video = drvr._add_video_driver(guest, image_meta, + flavor) + self.assertEqual(model, video.type) + + def test__add_video_driver(self): + self._test_add_video_driver('qxl') + + def test__video_model_supported(self): + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) + with mock.patch.object(drvr._host, "has_min_version", + return_value=True) as min_version_mock: + model_versions = libvirt_driver.MIN_LIBVIRT_VIDEO_MODEL_VERSIONS + # assert that all known vif models pass + for model in nova.objects.fields.VideoModel.ALL: + min_version_mock.reset_mock() + self.assertTrue(drvr._video_model_supported(model)) + # and that vif models with minium versions are checked + if model in model_versions: + ver = model_versions[model] + min_version_mock.assert_called_with(lv_ver=ver) + else: + min_version_mock.assert_not_called() + # then assert that fake models fail + self.assertFalse(drvr._video_model_supported("fake")) + # finally if the min version is not met assert that + # the video model is not supported. + min_version_mock.return_value = False + self.assertFalse(drvr._video_model_supported("gop")) + + @mock.patch.object(libvirt_driver.LibvirtDriver, '_video_model_supported') + def test__add_video_driver_gop(self, _supports_gop_video): + _supports_gop_video.return_value = True + self._test_add_video_driver('gop') + _supports_gop_video.return_value = False + self.assertRaises(exception.InvalidVideoMode, + self._test_add_video_driver, 'gop') + + @mock.patch.object(libvirt_driver.LibvirtDriver, '_video_model_supported') + def test__add_video_driver_none(self, _supports_none_video): + _supports_none_video.return_value = True + self._test_add_video_driver('none') + _supports_none_video.return_value = False + self.assertRaises(exception.InvalidVideoMode, + self._test_add_video_driver, 'none') + @mock.patch('nova.virt.disk.api.teardown_container') @mock.patch('nova.virt.libvirt.driver.LibvirtDriver.get_info') @mock.patch('nova.virt.disk.api.setup_container') diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index d6fffcef55f1..d794d94c7322 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -284,6 +284,12 @@ MIN_QEMU_NATIVE_TLS_VERSION = (2, 11, 0) VGPU_RESOURCE_SEMAPHORE = "vgpu_resources" +# see https://libvirt.org/formatdomain.html#elementsVideo +MIN_LIBVIRT_VIDEO_MODEL_VERSIONS = { + fields.VideoModel.GOP: (3, 2, 0), + fields.VideoModel.NONE: (4, 6, 0), +} + class LibvirtDriver(driver.ComputeDriver): def __init__(self, virtapi, read_only=False): @@ -4775,9 +4781,15 @@ class LibvirtDriver(driver.ComputeDriver): raise exception.SerialPortNumberLimitExceeded( allowed=ALLOWED_QEMU_SERIAL_PORTS, virt_type=virt_type) + def _video_model_supported(self, model): + if model not in fields.VideoModel.ALL: + return False + min_ver = MIN_LIBVIRT_VIDEO_MODEL_VERSIONS.get(model) + if min_ver and not self._host.has_min_version(lv_ver=min_ver): + return False + return True + def _add_video_driver(self, guest, image_meta, flavor): - VALID_VIDEO_DEVICES = ("vga", "cirrus", "vmvga", - "xen", "qxl", "virtio") video = vconfig.LibvirtConfigGuestVideo() # NOTE(ldbragst): The following logic sets the video.type # depending on supported defaults given the architecture, @@ -4803,7 +4815,7 @@ class LibvirtDriver(driver.ComputeDriver): video.type = 'qxl' if image_meta.properties.get('hw_video_model'): video.type = image_meta.properties.hw_video_model - if (video.type not in VALID_VIDEO_DEVICES): + if not self._video_model_supported(video.type): raise exception.InvalidVideoMode(model=video.type) # Set video memory, only if the flavor's limit is set @@ -4816,6 +4828,10 @@ class LibvirtDriver(driver.ComputeDriver): video.vram = video_ram * units.Mi / units.Ki guest.add_device(video) + # NOTE(sean-k-mooney): return the video device we added + # for simpler testing. + return video + def _add_qga_device(self, guest, instance): qga = vconfig.LibvirtConfigGuestChannel() qga.type = "unix" diff --git a/releasenotes/notes/extend-libvirt-video-model-support-d630b99ef5039f51.yaml b/releasenotes/notes/extend-libvirt-video-model-support-d630b99ef5039f51.yaml new file mode 100644 index 000000000000..3cc5b93a571c --- /dev/null +++ b/releasenotes/notes/extend-libvirt-video-model-support-d630b99ef5039f51.yaml @@ -0,0 +1,13 @@ +--- +features: + - | + In this release support was added for two additional libvirt video models: + ``gop``, the UEFI graphic output protocol device model; and the ``none`` + device model. Existing support for ``virtio`` has been extended to all + architectures and may now be requested via the ``hw_video_model`` image + metadata property. Prior to this release the ``virtio`` video model was + unconditionally enabled for ``AARCH64``. This is unchanged but it can now + be explicitly enabled on all supported architectures. The ``none`` video + model can be used to disable emulated video devices when using pGPU or + vGPU passthrough. +