Re: [libvirt] [PATCH v3 14/36] network: stop passing virDomainNetDefPtr into bandwidth functions

The networkPlugBandwidth & networkUnplugBandwidth methods currently take a virDomainNetDefPtr. To remove the dependency on the domain config struct, pass individual parameters instead. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt;

--- src/network/bridge_driver.c | 94 ++++++++++++++++++++----------------- 1 file changed, 50 insertions(+), 44 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 45a45b95d7..d4ca2930cc 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -169,11 +169,14 @@ networkRefreshDaemons(virNetworkDriverStatePtr driver); static int networkPlugBandwidth(virNetworkObjPtr obj, - virDomainNetDefPtr iface); + virMacAddrPtr mac, + virNetDevBandwidthPtr ifaceBand, + unsigned int *class_id); static int networkUnplugBandwidth(virNetworkObjPtr obj, - virDomainNetDefPtr iface); + virNetDevBandwidthPtr ifaceBand, + unsigned int *class_id); static void networkNetworkObjTaint(virNetworkObjPtr obj, @@ -4488,7 +4491,9 @@ networkAllocateActualDevice(virNetworkPtr net, goto error; } - if (networkPlugBandwidth(obj, iface) < 0) + if (networkPlugBandwidth(obj, &iface->mac, iface->bandwidth, + iface->data.network.actual ? + &iface->data.network.actual->class_id : NULL) < 0) goto error; break; @@ -4581,7 +4586,9 @@ networkAllocateActualDevice(virNetworkPtr net, } } - if (networkPlugBandwidth(obj, iface) < 0) + if (networkPlugBandwidth(obj, &iface->mac, iface->bandwidth, + iface->data.network.actual ? + &iface->data.network.actual->class_id : NULL) < 0) goto error; break; } @@ -4988,14 +4995,17 @@ networkReleaseActualDevice(virNetworkPtr net, case VIR_NETWORK_FORWARD_NAT: case VIR_NETWORK_FORWARD_ROUTE: case VIR_NETWORK_FORWARD_OPEN: - if (iface->data.network.actual && networkUnplugBandwidth(obj, iface) < 0) + if (iface->data.network.actual && + networkUnplugBandwidth(obj, iface->bandwidth, + &iface->data.network.actual->class_id) < 0) goto error; break; case VIR_NETWORK_FORWARD_BRIDGE: if (iface->data.network.actual && actualType == VIR_DOMAIN_NET_TYPE_BRIDGE && - networkUnplugBandwidth(obj, iface) < 0) + networkUnplugBandwidth(obj, iface->bandwidth, + &iface->data.network.actual->class_id) < 0) goto error; break; case VIR_NETWORK_FORWARD_PRIVATE: @@ -5135,7 +5145,7 @@ static int networkCheckBandwidth(virNetworkObjPtr obj, virNetDevBandwidthPtr ifaceBand, virNetDevBandwidthPtr oldBandwidth, - virMacAddr ifaceMac, + virMacAddrPtr ifaceMac, unsigned long long *new_rate) { int ret = -1; @@ -5145,7 +5155,7 @@ networkCheckBandwidth(virNetworkObjPtr obj, unsigned long long tmp_new_rate = 0; char ifmac[VIR_MAC_STRING_BUFLEN]; - virMacAddrFormat(&ifaceMac, ifmac); + virMacAddrFormat(ifaceMac, ifmac); if (ifaceBand && ifaceBand->in && ifaceBand->in->floor && !(netBand && netBand->in)) { @@ -5230,44 +5240,45 @@ networkNextClassID(virNetworkObjPtr obj) static int networkPlugBandwidthImpl(virNetworkObjPtr obj, - virDomainNetDefPtr iface, + virMacAddrPtr mac, virNetDevBandwidthPtr ifaceBand, + unsigned int *class_id, unsigned long long new_rate) { virNetworkDriverStatePtr driver = networkGetDriver(); virNetworkDefPtr def = virNetworkObjGetDef(obj); virBitmapPtr classIdMap = virNetworkObjGetClassIdMap(obj); unsigned long long tmp_floor_sum = virNetworkObjGetFloorSum(obj); - ssize_t class_id = 0; + ssize_t next_id = 0; int plug_ret; int ret = -1; /* generate new class_id */ - if ((class_id = networkNextClassID(obj)) < 0) { + if ((next_id = networkNextClassID(obj)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not generate next class ID")); goto cleanup; } plug_ret = virNetDevBandwidthPlug(def->bridge, def->bandwidth, - &iface->mac, ifaceBand, class_id); + mac, ifaceBand, next_id); if (plug_ret < 0) { - ignore_value(virNetDevBandwidthUnplug(def->bridge, class_id)); + ignore_value(virNetDevBandwidthUnplug(def->bridge, next_id)); goto cleanup; } /* QoS was set, generate new class ID */ - iface->data.network.actual->class_id = class_id; + *class_id = next_id; /* update sum of 'floor'-s of attached NICs */ tmp_floor_sum += ifaceBand->in->floor; virNetworkObjSetFloorSum(obj, tmp_floor_sum); /* update status file */ if (virNetworkObjSaveStatus(driver->stateDir, obj) < 0) { - ignore_value(virBitmapClearBit(classIdMap, class_id)); + ignore_value(virBitmapClearBit(classIdMap, next_id)); tmp_floor_sum -= ifaceBand->in->floor; virNetworkObjSetFloorSum(obj, tmp_floor_sum); - iface->data.network.actual->class_id = 0; - ignore_value(virNetDevBandwidthUnplug(def->bridge, class_id)); + *class_id = 0; + ignore_value(virNetDevBandwidthUnplug(def->bridge, next_id)); goto cleanup; } /* update rate for non guaranteed NICs */ @@ -5285,16 +5296,17 @@ networkPlugBandwidthImpl(virNetworkObjPtr obj, static int networkPlugBandwidth(virNetworkObjPtr obj, - virDomainNetDefPtr iface) + virMacAddrPtr mac, + virNetDevBandwidthPtr ifaceBand, + unsigned int *class_id) { int ret = -1; int plug_ret; unsigned long long new_rate = 0; char ifmac[VIR_MAC_STRING_BUFLEN]; - virNetDevBandwidthPtr ifaceBand = virDomainNetGetActualBandwidth(iface); if ((plug_ret = networkCheckBandwidth(obj, ifaceBand, NULL, - iface->mac, &new_rate)) < 0) { + mac, &new_rate)) < 0) { /* helper reported error */ goto cleanup; } @@ -5305,16 +5317,9 @@ networkPlugBandwidth(virNetworkObjPtr obj, goto cleanup; } - virMacAddrFormat(&iface->mac, ifmac); - if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK || - !iface->data.network.actual) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Cannot set bandwidth on interface '%s' of type %d"), - ifmac, iface->type); - goto cleanup; - } + virMacAddrFormat(mac, ifmac); - if (networkPlugBandwidthImpl(obj, iface, ifaceBand, new_rate) < 0) + if (networkPlugBandwidthImpl(obj, mac, ifaceBand, class_id, new_rate) < 0) goto cleanup; ret = 0; @@ -5326,7 +5331,8 @@ networkPlugBandwidth(virNetworkObjPtr obj, static int networkUnplugBandwidth(virNetworkObjPtr obj, - virDomainNetDefPtr iface) + virNetDevBandwidthPtr ifaceBand, + unsigned int *class_id) { virNetworkDefPtr def = virNetworkObjGetDef(obj); virBitmapPtr classIdMap = virNetworkObjGetClassIdMap(obj); @@ -5334,10 +5340,8 @@ networkUnplugBandwidth(virNetworkObjPtr obj, virNetworkDriverStatePtr driver = networkGetDriver(); int ret = 0; unsigned long long new_rate; - virNetDevBandwidthPtr ifaceBand = virDomainNetGetActualBandwidth(iface); - if (iface->data.network.actual && - iface->data.network.actual->class_id) { + if (class_id && *class_id) { if (!def->bandwidth || !def->bandwidth->in) { VIR_WARN("Network %s has no bandwidth but unplug requested", def->name); @@ -5349,8 +5353,7 @@ networkUnplugBandwidth(virNetworkObjPtr obj, if (def->bandwidth->in->peak > 0) new_rate = def->bandwidth->in->peak; - ret = virNetDevBandwidthUnplug(def->bridge, - iface->data.network.actual->class_id); + ret = virNetDevBandwidthUnplug(def->bridge, *class_id); if (ret < 0) goto cleanup; /* update sum of 'floor'-s of attached NICs */ @@ -5358,14 +5361,12 @@ networkUnplugBandwidth(virNetworkObjPtr obj, virNetworkObjSetFloorSum(obj, tmp_floor_sum); /* return class ID */ - ignore_value(virBitmapClearBit(classIdMap, - iface->data.network.actual->class_id)); + ignore_value(virBitmapClearBit(classIdMap, *class_id)); /* update status file */ if (virNetworkObjSaveStatus(driver->stateDir, obj) < 0) { tmp_floor_sum += ifaceBand->in->floor; virNetworkObjSetFloorSum(obj, tmp_floor_sum); - ignore_value(virBitmapSetBit(classIdMap, - iface->data.network.actual->class_id)); + ignore_value(virBitmapSetBit(classIdMap, *class_id)); goto cleanup; } /* update rate for non guaranteed NICs */ @@ -5375,7 +5376,7 @@ networkUnplugBandwidth(virNetworkObjPtr obj, VIR_WARN("Unable to update rate for 1:2 class on %s bridge", def->bridge); /* no class is associated any longer */ - iface->data.network.actual->class_id = 0; + *class_id = 0; } cleanup: @@ -5445,7 +5446,7 @@ networkBandwidthChangeAllowed(virDomainNetDefPtr iface, return false; } - if (networkCheckBandwidth(obj, newBandwidth, ifaceBand, iface->mac, NULL) < 0) + if (networkCheckBandwidth(obj, newBandwidth, ifaceBand, &iface->mac, NULL) < 0) goto cleanup; ret = true; @@ -5488,7 +5489,7 @@ networkBandwidthUpdate(virDomainNetDefPtr iface, def = virNetworkObjGetDef(obj); if ((plug_ret = networkCheckBandwidth(obj, newBandwidth, ifaceBand, - iface->mac, &new_rate)) < 0) { + &iface->mac, &new_rate)) < 0) { /* helper reported error */ goto cleanup; } @@ -5534,12 +5535,17 @@ networkBandwidthUpdate(virDomainNetDefPtr iface, } else if (newBandwidth->in && newBandwidth->in->floor) { /* .. or we need to plug in new .. */ - if (networkPlugBandwidthImpl(obj, iface, newBandwidth, new_rate) < 0) + if (networkPlugBandwidthImpl(obj, &iface->mac, newBandwidth, + iface->data.network.actual ? + &iface->data.network.actual->class_id : NULL, + new_rate) < 0) goto cleanup; } else { /* .. or unplug old. */ - if (networkUnplugBandwidth(obj, iface) < 0) + if (networkUnplugBandwidth(obj, iface->bandwidth, + iface->data.network.actual ? + &iface->data.network.actual->class_id : NULL) < 0) goto cleanup; }