[libvirt PATCH 0/8] Include networkportdef.h in domain_conf.h
The netdev_bandwidth_conf module contains XML parsing and formatting functions operating on types from util/virnetdevbandwidth.h as well as helper functions using types from domain_conf.h and network_conf.h It does not, however, introduce any new types, so there's no need to include its header in other header files. Move its inclusion in networkportdef.h to the corresponding networkportdef.c file, where it's used, which clears the path for networkportdef.h inclusion in domain_conf.h. Patch 1 is unrelated; Patch 5 was intended to help remove the dependency of the header file on network_conf.h (by passing int instead of enum) and patch 6 would lessen the dependency from domain_conf.h to virconftypes.h, but later I realized this might not be necessary. (Thanks, Pavel!) Ján Tomko (8): conf: virnwfilterbindingdef: include virxml.h bridge: include netdev_bandwidth_conf.h conf: virnetworkportdef: include virnetdevmacvlan conf: rename virNetDevSupportBandwidth to virNetDevSupportsBandwidth conf: virNetDevSupportsBandwidth: move into the C file conf: do not pass vm object to virDomainClearNetBandwidth conf: reduce includes in virnetworkportdef.h conf: include virnetworkportdef.h in domain_conf.h src/conf/domain_conf.h | 6 +----- src/conf/netdev_bandwidth_conf.c | 35 ++++++++++++++++++++++++++------ src/conf/netdev_bandwidth_conf.h | 26 ++---------------------- src/conf/virnetworkportdef.c | 5 +++++ src/conf/virnetworkportdef.h | 3 --- src/conf/virnwfilterbindingdef.h | 1 + src/libvirt_private.syms | 1 + src/lxc/lxc_driver.c | 4 ++-- src/lxc/lxc_process.c | 2 +- src/network/bridge_driver.c | 2 ++ src/qemu/qemu_command.c | 2 +- src/qemu/qemu_driver.c | 4 ++-- src/qemu/qemu_hotplug.c | 4 ++-- src/qemu/qemu_process.c | 2 +- 14 files changed, 50 insertions(+), 47 deletions(-) -- 2.24.1
The ParseNode function takes arguments with types from libxml. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/virnwfilterbindingdef.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/conf/virnwfilterbindingdef.h b/src/conf/virnwfilterbindingdef.h index 3d291fd6bf..2b83d5bd21 100644 --- a/src/conf/virnwfilterbindingdef.h +++ b/src/conf/virnwfilterbindingdef.h @@ -25,6 +25,7 @@ #include "virmacaddr.h" #include "virhash.h" #include "virbuffer.h" +#include "virxml.h" typedef struct _virNWFilterBindingDef virNWFilterBindingDef; typedef virNWFilterBindingDef *virNWFilterBindingDefPtr; -- 2.24.1
This file uses the virNetDevBandwidth*Floor helpers without including the correct include, relying on virnetworkportdef.h to include it. Signed-off-by: Ján Tomko <jtomko@redhat.com> Fixes: 17f430eb5cfaaa3388077f2b0856f011f0b2a4c3 --- src/network/bridge_driver.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index fbc0bea238..00896fc87d 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -68,6 +68,8 @@ #include "virjson.h" #include "virnetworkportdef.h" +#include "netdev_bandwidth_conf.h" + #define VIR_FROM_THIS VIR_FROM_NETWORK #define MAX_BRIDGE_ID 256 -- 2.24.1
On 2/22/20 5:31 PM, Ján Tomko wrote:
This file uses the virNetDevBandwidth*Floor helpers without including the correct include, relying on virnetworkportdef.h to include it.
Signed-off-by: Ján Tomko <jtomko@redhat.com> Fixes: 17f430eb5cfaaa3388077f2b0856f011f0b2a4c3
I have a question about this "Fixes" field. I've noticed you put it into some commit messages, but I can't figure out the pattern. I thought that you are blaming commits from previous releases (to help distro maintainers), but the commit you are referencing here is in the same release as this one. And some commits in this series don't have the field at all. Michal
On Tue, Feb 25, 2020 at 04:37:28PM +0100, Michal Privoznik wrote:
On 2/22/20 5:31 PM, Ján Tomko wrote:
This file uses the virNetDevBandwidth*Floor helpers without including the correct include, relying on virnetworkportdef.h to include it.
Signed-off-by: Ján Tomko <jtomko@redhat.com> Fixes: 17f430eb5cfaaa3388077f2b0856f011f0b2a4c3
I have a question about this "Fixes" field.
Feel free to ask it ;)
I've noticed you put it into some commit messages, but I can't figure out the pattern. I thought that you are blaming commits from previous releases (to help distro maintainers), but the commit you are referencing here is in the same release as this one. And some commits in this series don't have the field at all.
It is to leave a trace for future readers. The pattern is - I put it in in cases where I was bothered to look for it - here it was because rebasing on master actually broke the build for me. For other bugs, I'm usually curious how long that has been broken or in which releases we need to fix this. Of course, a) the wording "Fixes" in this case is similar to using "Resolves" for a related bugzilla link - Fixes somehow implies it was "broken". We cannot really consider a patch that complies correctly broken. The more accurate wording would be 'I-would-have-squashed-this-into:' b) I assume nobody is going to backport an include cleanup, so it is not useful in that regard either. Hope this answers your untold question. Jano
Michal
This is pulled in via domain_conf.h somehow, but it is directly used. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/virnetworkportdef.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/conf/virnetworkportdef.c b/src/conf/virnetworkportdef.c index a0705a8322..a972b698cc 100644 --- a/src/conf/virnetworkportdef.c +++ b/src/conf/virnetworkportdef.c @@ -24,6 +24,7 @@ #include "virerror.h" #include "virstring.h" #include "virfile.h" +#include "virnetdevmacvlan.h" #include "virnetworkportdef.h" #include "network_conf.h" -- 2.24.1
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/netdev_bandwidth_conf.c | 2 +- src/conf/netdev_bandwidth_conf.h | 2 +- src/lxc/lxc_driver.c | 4 ++-- src/lxc/lxc_process.c | 2 +- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_driver.c | 4 ++-- src/qemu/qemu_hotplug.c | 4 ++-- 7 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/conf/netdev_bandwidth_conf.c b/src/conf/netdev_bandwidth_conf.c index 5cbb9f46e4..349b451e41 100644 --- a/src/conf/netdev_bandwidth_conf.c +++ b/src/conf/netdev_bandwidth_conf.c @@ -293,7 +293,7 @@ virDomainClearNetBandwidth(virDomainObjPtr vm) for (i = 0; i < vm->def->nnets; i++) { type = virDomainNetGetActualType(vm->def->nets[i]); if (virDomainNetGetActualBandwidth(vm->def->nets[i]) && - virNetDevSupportBandwidth(type)) + virNetDevSupportsBandwidth(type)) virNetDevBandwidthClear(vm->def->nets[i]->ifname); } } diff --git a/src/conf/netdev_bandwidth_conf.h b/src/conf/netdev_bandwidth_conf.h index b9e57168be..172f9a50f9 100644 --- a/src/conf/netdev_bandwidth_conf.h +++ b/src/conf/netdev_bandwidth_conf.h @@ -37,7 +37,7 @@ int virNetDevBandwidthFormat(const virNetDevBandwidth *def, void virDomainClearNetBandwidth(virDomainObjPtr vm) ATTRIBUTE_NONNULL(1); -static inline bool virNetDevSupportBandwidth(virDomainNetType type) +static inline bool virNetDevSupportsBandwidth(virDomainNetType type) { switch (type) { case VIR_DOMAIN_NET_TYPE_BRIDGE: diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index f7376188f0..f01c71f9e2 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -3920,7 +3920,7 @@ lxcDomainAttachDeviceNetLive(virLXCDriverPtr driver, /* Set bandwidth or warn if requested and not supported. */ actualBandwidth = virDomainNetGetActualBandwidth(net); if (actualBandwidth) { - if (virNetDevSupportBandwidth(actualType)) { + if (virNetDevSupportsBandwidth(actualType)) { if (virNetDevBandwidthSet(net->ifname, actualBandwidth, false, !virDomainNetTypeSharesHostView(net)) < 0) goto cleanup; @@ -4377,7 +4377,7 @@ lxcDomainDetachDeviceNetLive(virDomainObjPtr vm, /* clear network bandwidth */ if (virDomainNetGetActualBandwidth(detach) && - virNetDevSupportBandwidth(actualType) && + virNetDevSupportsBandwidth(actualType) && virNetDevBandwidthClear(detach->ifname)) goto cleanup; diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 6851b3e3e2..d05304dd8f 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -621,7 +621,7 @@ virLXCProcessSetupInterfaces(virLXCDriverPtr driver, /* Set bandwidth or warn if requested and not supported. */ actualBandwidth = virDomainNetGetActualBandwidth(net); if (actualBandwidth) { - if (virNetDevSupportBandwidth(type)) { + if (virNetDevSupportsBandwidth(type)) { if (virNetDevBandwidthSet(net->ifname, actualBandwidth, false, !virDomainNetTypeSharesHostView(net)) < 0) goto cleanup; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f69a9e651c..c44f50b2a8 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8080,7 +8080,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, /* Set bandwidth or warn if requested and not supported. */ actualBandwidth = virDomainNetGetActualBandwidth(net); if (actualBandwidth) { - if (virNetDevSupportBandwidth(actualType)) { + if (virNetDevSupportsBandwidth(actualType)) { if (virNetDevBandwidthSet(net->ifname, actualBandwidth, false, !virDomainNetTypeSharesHostView(net)) < 0) goto cleanup; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 39e1f044e0..0faf79e0d1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11594,12 +11594,12 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, if (net) { actualType = virDomainNetGetActualType(net); - qosSupported = virNetDevSupportBandwidth(actualType); + qosSupported = virNetDevSupportsBandwidth(actualType); } if (qosSupported && persistentNet) { actualType = virDomainNetGetActualType(persistentNet); - qosSupported = virNetDevSupportBandwidth(actualType); + qosSupported = virNetDevSupportsBandwidth(actualType); } if (!qosSupported) { diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 9800491755..ca18bb9e5f 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1342,7 +1342,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, /* Set bandwidth or warn if requested and not supported. */ actualBandwidth = virDomainNetGetActualBandwidth(net); if (actualBandwidth) { - if (virNetDevSupportBandwidth(actualType)) { + if (virNetDevSupportsBandwidth(actualType)) { if (virNetDevBandwidthSet(net->ifname, actualBandwidth, false, !virDomainNetTypeSharesHostView(net)) < 0) goto cleanup; @@ -4582,7 +4582,7 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver, return -1; if (virDomainNetGetActualBandwidth(net) && - virNetDevSupportBandwidth(virDomainNetGetActualType(net)) && + virNetDevSupportsBandwidth(virDomainNetGetActualType(net)) && virNetDevBandwidthClear(net->ifname) < 0) VIR_WARN("cannot clear bandwidth setting for device : %s", net->ifname); -- 2.24.1
On Sat, Feb 22, 2020 at 05:31:55PM +0100, Ján Tomko wrote:
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/netdev_bandwidth_conf.c | 2 +- src/conf/netdev_bandwidth_conf.h | 2 +- src/lxc/lxc_driver.c | 4 ++-- src/lxc/lxc_process.c | 2 +- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_driver.c | 4 ++-- src/qemu/qemu_hotplug.c | 4 ++-- 7 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/src/conf/netdev_bandwidth_conf.c b/src/conf/netdev_bandwidth_conf.c index 5cbb9f46e4..349b451e41 100644 --- a/src/conf/netdev_bandwidth_conf.c +++ b/src/conf/netdev_bandwidth_conf.c @@ -293,7 +293,7 @@ virDomainClearNetBandwidth(virDomainObjPtr vm) for (i = 0; i < vm->def->nnets; i++) { type = virDomainNetGetActualType(vm->def->nets[i]); if (virDomainNetGetActualBandwidth(vm->def->nets[i]) && - virNetDevSupportBandwidth(type)) + virNetDevSupportsBandwidth(type)) virNetDevBandwidthClear(vm->def->nets[i]->ifname); } } diff --git a/src/conf/netdev_bandwidth_conf.h b/src/conf/netdev_bandwidth_conf.h index b9e57168be..172f9a50f9 100644 --- a/src/conf/netdev_bandwidth_conf.h +++ b/src/conf/netdev_bandwidth_conf.h @@ -37,7 +37,7 @@ int virNetDevBandwidthFormat(const virNetDevBandwidth *def, void virDomainClearNetBandwidth(virDomainObjPtr vm) ATTRIBUTE_NONNULL(1);
-static inline bool virNetDevSupportBandwidth(virDomainNetType type) +static inline bool virNetDevSupportsBandwidth(virDomainNetType type) { switch (type) { case VIR_DOMAIN_NET_TYPE_BRIDGE: diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index f7376188f0..f01c71f9e2 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -3920,7 +3920,7 @@ lxcDomainAttachDeviceNetLive(virLXCDriverPtr driver, /* Set bandwidth or warn if requested and not supported. */ actualBandwidth = virDomainNetGetActualBandwidth(net); if (actualBandwidth) { - if (virNetDevSupportBandwidth(actualType)) { + if (virNetDevSupportsBandwidth(actualType)) { if (virNetDevBandwidthSet(net->ifname, actualBandwidth, false, !virDomainNetTypeSharesHostView(net)) < 0) goto cleanup; @@ -4377,7 +4377,7 @@ lxcDomainDetachDeviceNetLive(virDomainObjPtr vm,
/* clear network bandwidth */ if (virDomainNetGetActualBandwidth(detach) && - virNetDevSupportBandwidth(actualType) && + virNetDevSupportsBandwidth(actualType) && virNetDevBandwidthClear(detach->ifname)) goto cleanup;
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 6851b3e3e2..d05304dd8f 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -621,7 +621,7 @@ virLXCProcessSetupInterfaces(virLXCDriverPtr driver, /* Set bandwidth or warn if requested and not supported. */ actualBandwidth = virDomainNetGetActualBandwidth(net); if (actualBandwidth) { - if (virNetDevSupportBandwidth(type)) { + if (virNetDevSupportsBandwidth(type)) { if (virNetDevBandwidthSet(net->ifname, actualBandwidth, false, !virDomainNetTypeSharesHostView(net)) < 0) goto cleanup; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f69a9e651c..c44f50b2a8 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8080,7 +8080,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, /* Set bandwidth or warn if requested and not supported. */ actualBandwidth = virDomainNetGetActualBandwidth(net); if (actualBandwidth) { - if (virNetDevSupportBandwidth(actualType)) { + if (virNetDevSupportsBandwidth(actualType)) { if (virNetDevBandwidthSet(net->ifname, actualBandwidth, false, !virDomainNetTypeSharesHostView(net)) < 0) goto cleanup; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 39e1f044e0..0faf79e0d1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11594,12 +11594,12 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
if (net) { actualType = virDomainNetGetActualType(net); - qosSupported = virNetDevSupportBandwidth(actualType); + qosSupported = virNetDevSupportsBandwidth(actualType); }
if (qosSupported && persistentNet) { actualType = virDomainNetGetActualType(persistentNet); - qosSupported = virNetDevSupportBandwidth(actualType); + qosSupported = virNetDevSupportsBandwidth(actualType); }
if (!qosSupported) { diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 9800491755..ca18bb9e5f 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1342,7 +1342,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, /* Set bandwidth or warn if requested and not supported. */ actualBandwidth = virDomainNetGetActualBandwidth(net); if (actualBandwidth) { - if (virNetDevSupportBandwidth(actualType)) { + if (virNetDevSupportsBandwidth(actualType)) { if (virNetDevBandwidthSet(net->ifname, actualBandwidth, false, !virDomainNetTypeSharesHostView(net)) < 0) goto cleanup; @@ -4582,7 +4582,7 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver, return -1;
if (virDomainNetGetActualBandwidth(net) && - virNetDevSupportBandwidth(virDomainNetGetActualType(net)) && + virNetDevSupportsBandwidth(virDomainNetGetActualType(net)) && virNetDevBandwidthClear(net->ifname) < 0) VIR_WARN("cannot clear bandwidth setting for device : %s", net->ifname); -- 2.24.1
Good change, I for one found the original, seemingly imperative name rather confusing. Somewhat related, there's a function called virNetDevBandwidthSupportsFloor() in the same header file whose name seems a bit dubious as well. Now admittedly I was the one to add it :-) but on second thought, it doesn't really check a bandwidth property, actually it checks a network property (it takes virNetworkForwardType as its argument). Would it make sense to rename it to something like virNetworkSupportsFloor() (and then also probably move it a better header file)? Reviewed-by: Pavel Mores <pmores@redhat.com>
Make the header easier to read and let the compiler inline what it wants. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/netdev_bandwidth_conf.c | 23 +++++++++++++++++++++++ src/conf/netdev_bandwidth_conf.h | 24 +----------------------- src/libvirt_private.syms | 1 + 3 files changed, 25 insertions(+), 23 deletions(-) diff --git a/src/conf/netdev_bandwidth_conf.c b/src/conf/netdev_bandwidth_conf.c index 349b451e41..2fe0499638 100644 --- a/src/conf/netdev_bandwidth_conf.c +++ b/src/conf/netdev_bandwidth_conf.c @@ -299,6 +299,29 @@ virDomainClearNetBandwidth(virDomainObjPtr vm) } +bool virNetDevSupportsBandwidth(virDomainNetType type) +{ + switch ((virDomainNetType) type) { + case VIR_DOMAIN_NET_TYPE_BRIDGE: + case VIR_DOMAIN_NET_TYPE_NETWORK: + case VIR_DOMAIN_NET_TYPE_DIRECT: + case VIR_DOMAIN_NET_TYPE_ETHERNET: + return true; + case VIR_DOMAIN_NET_TYPE_USER: + case VIR_DOMAIN_NET_TYPE_VHOSTUSER: + case VIR_DOMAIN_NET_TYPE_SERVER: + case VIR_DOMAIN_NET_TYPE_CLIENT: + case VIR_DOMAIN_NET_TYPE_MCAST: + case VIR_DOMAIN_NET_TYPE_UDP: + case VIR_DOMAIN_NET_TYPE_INTERNAL: + case VIR_DOMAIN_NET_TYPE_HOSTDEV: + case VIR_DOMAIN_NET_TYPE_LAST: + break; + } + return false; +} + + bool virNetDevBandwidthHasFloor(const virNetDevBandwidth *b) { diff --git a/src/conf/netdev_bandwidth_conf.h b/src/conf/netdev_bandwidth_conf.h index 172f9a50f9..5a70cb6bba 100644 --- a/src/conf/netdev_bandwidth_conf.h +++ b/src/conf/netdev_bandwidth_conf.h @@ -37,28 +37,6 @@ int virNetDevBandwidthFormat(const virNetDevBandwidth *def, void virDomainClearNetBandwidth(virDomainObjPtr vm) ATTRIBUTE_NONNULL(1); -static inline bool virNetDevSupportsBandwidth(virDomainNetType type) -{ - switch (type) { - case VIR_DOMAIN_NET_TYPE_BRIDGE: - case VIR_DOMAIN_NET_TYPE_NETWORK: - case VIR_DOMAIN_NET_TYPE_DIRECT: - case VIR_DOMAIN_NET_TYPE_ETHERNET: - return true; - case VIR_DOMAIN_NET_TYPE_USER: - case VIR_DOMAIN_NET_TYPE_VHOSTUSER: - case VIR_DOMAIN_NET_TYPE_SERVER: - case VIR_DOMAIN_NET_TYPE_CLIENT: - case VIR_DOMAIN_NET_TYPE_MCAST: - case VIR_DOMAIN_NET_TYPE_UDP: - case VIR_DOMAIN_NET_TYPE_INTERNAL: - case VIR_DOMAIN_NET_TYPE_HOSTDEV: - case VIR_DOMAIN_NET_TYPE_LAST: - break; - } - return false; -} - - +bool virNetDevSupportsBandwidth(virDomainNetType type); bool virNetDevBandwidthHasFloor(const virNetDevBandwidth *b); bool virNetDevBandwidthSupportsFloor(virNetworkForwardType type); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8883aa89cc..fbcc26ad98 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -737,6 +737,7 @@ virNetDevBandwidthFormat; virNetDevBandwidthHasFloor; virNetDevBandwidthParse; virNetDevBandwidthSupportsFloor; +virNetDevSupportsBandwidth; # conf/netdev_vlan_conf.h -- 2.24.1
This function only uses the domain definition. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/netdev_bandwidth_conf.c | 10 +++++----- src/conf/netdev_bandwidth_conf.h | 2 +- src/qemu/qemu_process.c | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/conf/netdev_bandwidth_conf.c b/src/conf/netdev_bandwidth_conf.c index 2fe0499638..396ac62019 100644 --- a/src/conf/netdev_bandwidth_conf.c +++ b/src/conf/netdev_bandwidth_conf.c @@ -285,16 +285,16 @@ virNetDevBandwidthFormat(const virNetDevBandwidth *def, } void -virDomainClearNetBandwidth(virDomainObjPtr vm) +virDomainClearNetBandwidth(virDomainDefPtr def) { size_t i; virDomainNetType type; - for (i = 0; i < vm->def->nnets; i++) { - type = virDomainNetGetActualType(vm->def->nets[i]); - if (virDomainNetGetActualBandwidth(vm->def->nets[i]) && + for (i = 0; i < def->nnets; i++) { + type = virDomainNetGetActualType(def->nets[i]); + if (virDomainNetGetActualBandwidth(def->nets[i]) && virNetDevSupportsBandwidth(type)) - virNetDevBandwidthClear(vm->def->nets[i]->ifname); + virNetDevBandwidthClear(def->nets[i]->ifname); } } diff --git a/src/conf/netdev_bandwidth_conf.h b/src/conf/netdev_bandwidth_conf.h index 5a70cb6bba..34046eacd7 100644 --- a/src/conf/netdev_bandwidth_conf.h +++ b/src/conf/netdev_bandwidth_conf.h @@ -34,7 +34,7 @@ int virNetDevBandwidthFormat(const virNetDevBandwidth *def, unsigned int class_id, virBufferPtr buf); -void virDomainClearNetBandwidth(virDomainObjPtr vm) +void virDomainClearNetBandwidth(virDomainDefPtr def) ATTRIBUTE_NONNULL(1); bool virNetDevSupportsBandwidth(virDomainNetType type); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 8c1ed76677..3099099819 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -7348,7 +7348,7 @@ void qemuProcessStop(virQEMUDriverPtr driver, } /* Clear network bandwidth */ - virDomainClearNetBandwidth(vm); + virDomainClearNetBandwidth(vm->def); virDomainConfVMNWFilterTeardown(vm); -- 2.24.1
All the _conf includes are only needed in the C file. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/virnetworkportdef.c | 4 ++++ src/conf/virnetworkportdef.h | 3 --- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/conf/virnetworkportdef.c b/src/conf/virnetworkportdef.c index a972b698cc..66d900088a 100644 --- a/src/conf/virnetworkportdef.c +++ b/src/conf/virnetworkportdef.c @@ -28,6 +28,10 @@ #include "virnetworkportdef.h" #include "network_conf.h" +#include "netdev_bandwidth_conf.h" +#include "netdev_vlan_conf.h" +#include "netdev_vport_profile_conf.h" + #define VIR_FROM_THIS VIR_FROM_NETWORK VIR_ENUM_IMPL(virNetworkPortPlug, diff --git a/src/conf/virnetworkportdef.h b/src/conf/virnetworkportdef.h index 78cf2c1ba4..72da8b6915 100644 --- a/src/conf/virnetworkportdef.h +++ b/src/conf/virnetworkportdef.h @@ -28,9 +28,6 @@ #include "virnetdevbandwidth.h" #include "virpci.h" #include "virxml.h" -#include "netdev_vport_profile_conf.h" -#include "netdev_bandwidth_conf.h" -#include "netdev_vlan_conf.h" typedef struct _virNetworkPortDef virNetworkPortDef; typedef virNetworkPortDef *virNetworkPortDefPtr; -- 2.24.1
Now that this file no longer transitively includes domain_conf.h, it can be included here. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/domain_conf.h | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index cdc4d25700..d85c2d7706 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -44,6 +44,7 @@ #include "virnetdevvportprofile.h" #include "virnetdevbandwidth.h" #include "virnetdevvlan.h" +#include "virnetworkportdef.h" #include "virobject.h" #include "device_conf.h" #include "virbitmap.h" @@ -3643,11 +3644,6 @@ bool virDomainDefLifecycleActionAllowed(virDomainLifecycle type, virDomainLifecycleAction action); -// Forward decl to avoid pulling in virnetworkportdef.h because -// that pulls in virhostdev.h which pulls in domain_conf.h (evil) -typedef struct _virNetworkPortDef virNetworkPortDef; -typedef virNetworkPortDef *virNetworkPortDefPtr; - virNetworkPortDefPtr virDomainNetDefToNetworkPort(virDomainDefPtr dom, virDomainNetDefPtr iface); -- 2.24.1
On 2/22/20 5:31 PM, Ján Tomko wrote:
The netdev_bandwidth_conf module contains XML parsing and formatting functions operating on types from util/virnetdevbandwidth.h as well as helper functions using types from domain_conf.h and network_conf.h
It does not, however, introduce any new types, so there's no need to include its header in other header files.
Move its inclusion in networkportdef.h to the corresponding networkportdef.c file, where it's used, which clears the path for networkportdef.h inclusion in domain_conf.h.
Patch 1 is unrelated;
Patch 5 was intended to help remove the dependency of the header file on network_conf.h (by passing int instead of enum) and patch 6 would lessen the dependency from domain_conf.h to virconftypes.h, but later I realized this might not be necessary. (Thanks, Pavel!)
Ján Tomko (8): conf: virnwfilterbindingdef: include virxml.h bridge: include netdev_bandwidth_conf.h conf: virnetworkportdef: include virnetdevmacvlan conf: rename virNetDevSupportBandwidth to virNetDevSupportsBandwidth conf: virNetDevSupportsBandwidth: move into the C file conf: do not pass vm object to virDomainClearNetBandwidth conf: reduce includes in virnetworkportdef.h conf: include virnetworkportdef.h in domain_conf.h
src/conf/domain_conf.h | 6 +----- src/conf/netdev_bandwidth_conf.c | 35 ++++++++++++++++++++++++++------ src/conf/netdev_bandwidth_conf.h | 26 ++---------------------- src/conf/virnetworkportdef.c | 5 +++++ src/conf/virnetworkportdef.h | 3 --- src/conf/virnwfilterbindingdef.h | 1 + src/libvirt_private.syms | 1 + src/lxc/lxc_driver.c | 4 ++-- src/lxc/lxc_process.c | 2 +- src/network/bridge_driver.c | 2 ++ src/qemu/qemu_command.c | 2 +- src/qemu/qemu_driver.c | 4 ++-- src/qemu/qemu_hotplug.c | 4 ++-- src/qemu/qemu_process.c | 2 +- 14 files changed, 50 insertions(+), 47 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (3)
-
Ján Tomko -
Michal Privoznik -
Pavel Mores