[libvirt] [PATCH v1 00/26] move qemucaps validations from qemu_command to qemu_domain
Daniel Henrique Barboza
danielhb413 at gmail.com
Tue Dec 10 10:22:37 UTC 2019
Hi,
I failed to make a reservation about what I've done in patch in the
cover letter. In the commit msg I mentioned about the XML files
considering SPICE support as default to ppc64, aarch64 and s390 archs and
fixing all the xmls.
It is worth disclaiming that what I can assert to be true is the lack of
SPICE support for ppc64. I can't put my money on the lack of SPICE for
the other 2 archs - what I did in the patch was to assume that the emulator
capabilities for aarch64 and s390, that didn't report SPICE support, was
accurate. I think it's an educated guess, but a guess nevertheless.
Thanks,
DHB
On 12/9/19 8:15 PM, 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.
> -------
>
> 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(-)
>
More information about the libvir-list
mailing list