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