[libvirt] [PATCH 0/3] qemu: make runtime validation of NetDefs more consistent

[Laine Stump]

On 9/13/19 4:02 PM, Michal Prívozník wrote:
> On 9/13/19 4:52 PM, Laine Stump wrote:
>> There is some validation of NetDefs (<interface>) that can't be done
>> until runtime, due to not knowing the exact configuration until that
>> time. This needs to happen in 3 places: 1) in the qemu commandline
>> when a new guest is started, 2) when an <interface> is hotplugged, and
>> 3) when an <interface> is updated. Until now, there some (but not all)
>> validation copy-pasted between (1) and (2), and none of that was
>> present for (3). These patches move all the validation from (1) (which
>> is the most complete) into a separate function, and then call that
>> function from all three places, so the exact same validation is done
>> in all cases.
>>
>> (These patches are a followup to a patch I sent *long* ago in a naive
>> attempt to fix https://bugzilla.redhat.com/1502754 - my original patch
>> did fix the problem when starting a guest with an invalid <interface>
>> config, but not when hotplugging or updating such an interface.)
>>
>> NB: I noticed that if someone tries to specify <bandwidth> for an
>> interface type that doesn't support it, we only give a warning, not an
>> error, due to a fear that there are existing management apps that add
>> <bandwidth> to all types of interfaces, and we don't want them to
>> suddenly fail to run (I got this info from the BZ associated with the
>> commit that added the warning). This seems to me to be going a bit too
>> far - I think that (at least upstream) we should turn this into an
>> error, and let the regression testing of said management apps catch
>> the behavior change so they can fix their code. (insert
>> Kermit-drinking-coffee meme here)
> 
> Yes, that was exactly the reasoning we had when introducing the warning.
> Initially, when I implemented QoS it did not error out on unsupported
> interface type, but I guess we can do so now.


Yep, I completely understand your reluctance at the time to make it a 
full fledged error - especially if the patch was going to be backported 
to a downstream build during a minor update :-)

I'm going to add some more things to this new validation function; I'll 
see if I can slip in a patch for this too.


> 
>>
>>
>> Laine Stump (3):
>>    conf: make arg to virDomainNetGetActualVirtPortProfile() a const
>>    qemu: move runtime netdev validation into a separate function
>>    qemu: call common NetDef validation for hotplug and device update
>>
>>   src/conf/domain_conf.c  |  2 +-
>>   src/conf/domain_conf.h  |  2 +-
>>   src/qemu/qemu_command.c | 52 +--------------------------
>>   src/qemu/qemu_domain.c  | 80 +++++++++++++++++++++++++++++++++++++++++
>>   src/qemu/qemu_domain.h  |  4 +++
>>   src/qemu/qemu_hotplug.c | 32 +++++------------
>>   6 files changed, 95 insertions(+), 77 deletions(-)
>>
> 
> Reviewed-by: Michal Privoznik <mprivozn at redhat.com>  but please see my
> comment to 2/3 before pushing.
> 


Thanks!