On 10/15/20 8:46 AM, Michal Privoznik wrote:
> 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@redhat.com>
Thanks for the review! Applied all your suggestions in patches 03, 05, 08 and
15 and pushed.
DHB
>
> Michal
>