[libvirt] [PATCH v2 5/7] conf: Introduce new <hostdev> attribute 'display'

[Michal Privoznik]

On 07/11/2018 09:58 AM, Erik Skultety wrote:
> On Tue, Jul 10, 2018 at 01:39:03PM +0200, Michal Privoznik wrote:
>> On 07/09/2018 06:24 PM, Erik Skultety wrote:
>>> QEMU 2.12 introduced a new type of display for mediated devices using
>>> vfio-pci backend which allows a mediated device to be used as a VGA
>>> compatible device as an alternative to an emulated video device. QEMU
>>> exposes this feature via a vfio device property 'display' with supported
>>> values 'on/off/auto' (default is 'off').
>>>
>>> This patch adds the necessary bits to domain config handling in order to
>>> expose this feature. Since there's no convenient way for libvirt to come
>>> up with usable defaults for the display setting, simply because libvirt
>>> is not able to figure out which of the display implementations - dma-buf
>>> which requires OpenGL support vs vfio regions which doesn't need OpenGL
>>> (works with OpenGL enabled too) - the underlying mdev uses.
>>>
>>> Signed-off-by: Erik Skultety <eskultet at redhat.com>
>>> ---
> ...
> 
>>> +static int
>>> +qemuDomainHostdevDefPostParse(const virDomainHostdevDef *hostdev,
>>> +                              const virDomainDef *def)
>>> +{
>>> +    const virDomainHostdevSubsysMediatedDev *mdevsrc;
>>> +
>>> +    if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) {
>>> +        switch ((virDomainHostdevSubsysType) hostdev->source.subsys.type) {
>>> +        case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
>>> +        case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
>>> +        case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI:
>>> +        case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST:
>>> +            break;
>>> +        case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV:
>>> +            mdevsrc = &hostdev->source.subsys.u.mdev;
>>> +            return qemuDomainHostdevMdevDefPostParse(mdevsrc, def);
>>> +        case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
>>> +        default:
>>> +            virReportEnumRangeError(virDomainHostdevSubsysType,
>>> +                                    hostdev->source.subsys.type);
>>> +            return -1;
>>> +        }
>>> +    }
>>> +
>>> +    return 0;
>>> +}
>>> +
>>
>> Again, these two ^^ are validate callbacks not PostParse. You are not
>> filling in missing information, you are checking (=validating) whether
>> user provided XML is valid.
> 
> Actually, both of them were, back in the RFC series I linked (I completely
> agree with you) but I was asked to convert them to post parse so that xml2xml
> test could fail for invalid XMLs. Let me know whether I should convert them
> back.

Asked by whom?

I realize that we still have some validation done while parsing XML. You
can call it historical reasons, our laziness to fully convert the code,
whatever. But, if we are introducing new code I see no excuse to not
place it properly.

And if we are worried about xml2xml test - why is it not running
validation then?

Michal