5:15 p.m.
(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.
-------
Cole also mentioned that the machine features validation was a good
place to start. I took it a step further and did it across the
board on all qemu_command.c.
This series didn't create any new validation, just moved them
to domain define time. Any other outcome is unintended.
Not all cases where covered in this work. For a first version
I decided to do only the most trivial cases. These are the
other cases I left out for another day:
- all verifications contained in functions that are also called
by qemu_hotplug.c. This means that the hotplug process will also
use the validation code, and we can't just move it to qemu_domain.c.
Besides, not all validation done in domain define time applies
to hotplug, so it's not simply a matter of calling the same function
from qemu_domain in qemu_hotplug. I have patches that moved the
verification of all virtio options to qemu_domain.c, while also
considering qemu_hotplug validation. In the end I decided to leave
it away from this work for now because (1) it took 8 patches just
for virtio case alone and (2) there are a lot of these cases in
qemu_command.c and it would be too much to do it all at in this
same series.
- moving CPU Model validation is trickier than the rest because
there are code in DefPostParse() that makes CPU Model changes that
are then validated in qemu_command.c. Moving the validation to define
time doesn't cut in this case - the validation is considering
PostParse changes in the CPU Model and some of the will fail if
done by qemuDomainDefValidate time. I didn't think too much about
how to handle this case, but given that the approach would be
different from the other cases handled here, I left it out too.
- SHMem: part of SHMem validation is being used by hotplug code.
I could've moved the non-hotplug validation to define time,
instead I decided it's better to to leave it to do it all at
once in the future.
- DBus-VMState: validation of this fella must be done by first
querying if the hash vm->priv->dbusVMState has elements.
qemuDomainDefValidate does not have 'vm->priv' in it's API,
meaning that I would need to either change the API to include
it (which means changing domainValidateCallback), or do the
validation by another branch where I can get access to vm->priv.
- vmcoreinfo: there are no challenges in moving vmcoreinfo
validation to qemuDomainDefValidate. The problem were the
unit tests. Moving this validation to domain define time
breaks 925 tests on qemuxml2xmltest.c, including tests labelled
as 'minimal' which should pass under any circurstances, as
far as I understand. It is possible to just shoehorn
QEMU_CAPS_DEVICE_VMCOREINFO in the qemuCaps of all tests, but
this would tamper the idea of having to deal with NONE or
LATEST or a specific set of capabilities for certain tests.
Another case I would rather discuss separately.
[1] https://www.redhat.com/archives/libvir-list/2019-October/msg00568.html
Daniel Henrique Barboza (26):
qemu_command.c: move PSeries features validation to qemu_domain.c
qemu_command.c: move mem.nosharepages validation to qemu_domain.c
qemu_command.c: move validation of vmport to qemu_domain.c
qemu_command.c: move nvdimm validation to qemu_domain.c
qemu_command.c: move I/O APIC validation to qemu_domain.c
numa_conf: add virDomainNumaNodesDistancesAreBeingSet() helper
qemu_command.c move NUMA validation to qemu_domain.c
qemu_command.c: move NVRAM validation to qemu_domain.c
qemu_command: move qemuBuildSoundDevStr caps validation to qemu_domain
qemu_command.c: move sound codec validation to qemu_domain.c
qemu_command: move qemuBuildHubDevStr caps validation to qemu_domain
qemu_command: move qemuBuildChrChardevStr caps validation to
qemu_domain
qemu: move qemuBuildHostdevCommandLine caps validation to qemu_domain
qemu_command.c: move vmGenID validation to qemu_domain.c
qemu: move qemuBuildSgaCommandLine validation to qemu_domain.c
qemu: move virDomainClockDef validation to qemu_domain.c
qemu: move qemuBuildPMCommandLine validation to qemu_domain.c
qemu: move qemuBuildBootCommandLine validation to qemu_domain.c
qemu_command.c: move pcihole64 validation to qemu_domain.c
qemu: move qemuBuildGraphicsSDLCommandLine validation to qemu_domain.c
qemu: move qemuBuildGraphicsVNCCommandLine validation to qemu_domain.c
qemu: move qemuBuildGraphicsSPICECommandLine validation to
qemu_domain.c
qemu: move qemuBuildGraphicsEGLHeadlessCommandLine validation to
qemu_domain.c
qemu: move qemuBuildSmartcardCommandLine validation to qemu_domain.c
qemu: move qemuBuildConsoleCommandLine validation to qemu_domain.c
qemu: move qemuBuildTPMDevStr TPM validation to qemu_domain.c
src/conf/numa_conf.c | 19 +
src/conf/numa_conf.h | 2 +
src/libvirt_private.syms | 1 +
src/qemu/qemu_command.c | 575 +---------
src/qemu/qemu_command.h | 1 +
src/qemu/qemu_domain.c | 983 ++++++++++++++++--
tests/qemuhotplugtest.c | 10 +
tests/qemumemlocktest.c | 16 +-
.../default-video-type-aarch64.xml | 2 +-
.../default-video-type-ppc64.xml | 2 +-
.../default-video-type-s390x.xml | 2 +-
.../graphics-egl-headless.args | 2 +-
.../graphics-spice-egl-headless.args | 2 +-
.../graphics-vnc-egl-headless.args | 2 +-
tests/qemuxml2argvtest.c | 80 +-
...ault-video-type-aarch64.aarch64-latest.xml | 4 +-
.../default-video-type-ppc64.ppc64-latest.xml | 4 +-
.../default-video-type-s390x.s390x-latest.xml | 4 +-
tests/qemuxml2xmltest.c | 244 +++--
19 files changed, 1246 insertions(+), 709 deletions(-)
--
2.23.0