Re: [PATCH] conf: add support for 'edid' attribute to video model
On 31/07/2025 13:04, Daniel P. Berrangé wrote:
On Thu, Jul 31, 2025 at 10:50:40AM +0100, Mark Cave-Ayland wrote:
> Add the ability to enable/disable exposing the EDID information to the guest.
> This allows migration from legacy machine types that have EDID disabled to a
> newer machine type without any change becoming visible to the guest.
Conceptually migrating between two different machine types should be
impossible regardless of the EDID setting, because libvirt's ABI
stability check should have rejected the changed machine type name
upfront.
That's true: the migration example is from an internal use case here.
> The edid attribute can specified in the domain XML as below:
>
> <video>
> <model type='virtio' edid='off'/>
> </video>
Supporting EDID as a setting of course makes sense regardless though
so that's fine. Just don't justify it in the commit message based on
the migration point. It is justiable simply because this is a QEMU
feature that makes sense to be able to control
Fair enough. I'll drop the 2nd sentence from the top paragraph in v2 to
avoid further confusion.
> If the edid attribute is unspecified, it is not generated so that
the
> virtualisation platform will continue to use its default.
>
> The edid attribute is only valid for the vga, boch and virtio display models
> and is currently only implemented for the QEMU driver.
>
> Signed-off-by: Mark Cave-Ayland <mark.caveayland(a)nutanix.com>
> ---
> docs/formatdomain.rst | 8 ++++++++
> src/conf/domain_conf.c | 5 +++++
> src/conf/domain_conf.h | 1 +
> src/conf/domain_validate.c | 11 +++++++++++
> src/conf/schemas/domaincommon.rng | 5 +++++
> src/qemu/qemu_command.c | 7 +++++++
> 6 files changed, 37 insertions(+)
Lacking test XML file coverage
Peter also pointed this out, so I'll have a look at this before
submitting v2.
With regards,
Daniel
ATB,
Mark.