Re: [PATCH v1 00/24] move more validations to qemu_validate.c

On 10/14/20 10:42 PM, Daniel Henrique Barboza wrote: > Hi, > > This is another step following up the work done in [1]. > > Recapping, the idea is to move any validation that can > be done in parse/define time to qemu_validate.c instead > of let it sit inside a command line build function in > qemu_command.c. I'll again copy/paste the reasoning Cole > Robinson gave for this work back then (see [2] for more > info): > > ------- > 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. > ------- > > Not all validations were moved with these series, but I covered most > of them. > > > One thing worth noticing is the existence of 'post start' validations, > most of them related to PCI addressing, and other notable cases such as > NUMA nodes and CPU models, that can't be moved to parse time. The reason > is that the QEMU driver will change them during start time. > I contemplated move them to the existing qemuBuildCommandLineValidate(), > but that would remove them from the hotplug path that uses the > qemuBuild*DevStr() functions. I don't have an easy answer on how to handle > these 'post start' validations ATM, so for now they remain in > qemu_command.c. > > > [1] https://www.redhat.com/archives/libvir-list/2019-December/msg00570.html > [2] https://www.redhat.com/archives/libvir-list/2019-October/msg00568.html > >   src/qemu/qemu_command.c                       | 379 +-------- >   src/qemu/qemu_command.h                       |   4 +- >   src/qemu/qemu_hotplug.c                       |   4 +- >   src/qemu/qemu_validate.c                      | 740 ++++++++++++++---- >   tests/qemuhotplugtest.c                       |   1 + >   .../disk-sata-incompatible-address.err        |   2 +- >   .../net-virtio-rxqueuesize-invalid-size.err   |   2 +- >   .../pseries-panic-address.err                 |   2 +- >   tests/qemuxml2argvtest.c                      |  28 +- >   tests/qemuxml2xmltest.c                       |  62 +- >   10 files changed, 706 insertions(+), 518 deletions(-) > Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com&gt;

Michal