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@xxxxxxxxxx> but please see my
comment to 2/3 before pushing.
Thanks!
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
