[libvirt] [PATCH v1 00/26] move qemucaps validations from qemu_command to qemu_domain

[Cole Robinson]

On 12/10/19 5:56 AM, Peter Krempa wrote:
> On Tue, Dec 10, 2019 at 09:58:38 +0000, Daniel Berrange wrote:
>> On Mon, Dec 09, 2019 at 08:15:05PM -0300, Daniel Henrique Barboza wrote:
>>> (series based on master commit 97cafa610ecf5)
>>>
>>> This work was proposed by Cole in [1]. This is Cole's reasoning for
>>> it, copy/pasted from [1]:
>>>
>>> -------
>>> The benefits of moving to validate time is that XML is rejected by
>>> 'virsh define' rather than at 'virsh start' time. It also makes it easier
>>> to follow the cli building code, and makes it easier to verify qemu_command.c
>>> test suite code coverage for the important stuff like covering every CLI
>>> option. It's also a good intermediate step for sharing validation with
>>> domain capabilities building, like is done presently for rng models.
>>> -------
>>
>> I've not looked at the patches, but surely moving this validate from
>> start time, to be define time errors is going to cause functional
>> regressions in our ABI behaviour.
>>
>> My libvirtd daemons installs have many XML files defined which will
>> fail validation at various points in time, depending on what QEMU
>> builds I happen to have deployed. I only need them to pass the
>> validation when actually starting the VM.
> 
> Validation is not done on the persistent or status XML files exactly for
> this reason.
> 
> It's re-done when starting the VM so this should not be an issue.
> 

Yup, that's what the VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE flag is all
about. It's used to skip validation at daemon startup, and various cases
like DomainDefCopy

Conceptually this moves closer to IMO the ideal model:

1) Serialize XML into C struct
2) Validate contents against hypervisor capabilities
3) Serialize C struct into command line string

Which has benefits: simplifies code in #1 and #3 which is the
interesting stuff, easier to test full code coverage of #1 and #3, gives
us the option to move validation into domaincapabilities code so we can
report to the user whether a feature will be rejected or not.

Also I suspect if we moved to using virtblocks for cli stuff, untangling
validation from the cli builder would be a step in that direction

- Cole