[libvirt] [PATCH 1/7] conf: clarify what is returned for actual bandwidth and vlan
Michal Privoznik
mprivozn at redhat.com
Mon Feb 24 16:46:20 UTC 2014
On 21.02.2014 14:58, Laine Stump wrote:
> In practice, if a virDomainNetDef has a virDomainActualNetDef
> allocated, the ActualNetDef will *always* contain the bandwidth and
> vlan data from the NetDef (unless there was also a portgroup involved
> - see networkAllocateActualDevice()).
>
> However, virDomainNetGetActual(Bandwidth|Vlan)() were coded to make it
> appear as if it might be possible to have a valid bandwidth/vlan in
> the NetDef, but a NULL in the ActualNetDef. Believing this un-truth
> could lead to writing unnecessarily defensive code when dealing with
> the virDomainGetActual*() functions, so this patch makes it more
> obvious:
>
> If there is an ActualNetDef, it will always have a copy of the
> various appropriate bits from its parent NetDef, and the
> virDomainGetActual* function will *always* return the data from the
> ActualNetDef, not from the NetDef.
>
> The reason for this effective-NOP patch is that a subsequent patch to
> change virDomainNetDefFormat will rely on the above rule.
> ---
> src/conf/domain_conf.c | 20 ++++++++++++++------
> 1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 064a40e..755066c 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -18638,8 +18638,11 @@ virDomainNetGetActualVirtPortProfile(virDomainNetDefPtr iface)
> virNetDevBandwidthPtr
> virDomainNetGetActualBandwidth(virDomainNetDefPtr iface)
> {
> + /* if there is an ActualNetDef, *always* return
> + * its bandwidth rather than the NetDef's bandwidth.
> + */
> if (iface->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
> - iface->data.network.actual && iface->data.network.actual->bandwidth) {
> + iface->data.network.actual) {
> return iface->data.network.actual->bandwidth;
> }
> return iface->bandwidth;
> @@ -18648,12 +18651,17 @@ virDomainNetGetActualBandwidth(virDomainNetDefPtr iface)
> virNetDevVlanPtr
> virDomainNetGetActualVlan(virDomainNetDefPtr iface)
> {
> + virNetDevVlanPtr vlan = &iface->vlan;
> +
> + /* if there is an ActualNetDef, *always* return
> + * its vlan rather than the NetDef's vlan.
> + */
> if (iface->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
> - iface->data.network.actual &&
> - iface->data.network.actual->vlan.nTags > 0)
> - return &iface->data.network.actual->vlan;
> - if (iface->vlan.nTags > 0)
> - return &iface->vlan;
> + iface->data.network.actual)
> + vlan = &iface->data.network.actual->vlan;
> +
> + if (vlan->nTags > 0)
> + return vlan;
> return 0;
When you're at this, can you s/0/NULL/? We are returning a pointer after
all.
> }
>
>
ACK
Michal
More information about the libvir-list
mailing list