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

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.

> > > 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(a)redhat.com&gt; but please see my comment to 2/3 before pushing.