On 3/19/19 8:46 AM, Daniel P. Berrangé wrote:
> Convert the virDomainNetDef object into a virNetworkPortDef object
> at the start of networkReleaseActualDevice. This largely decouples
> the method impl from the domain object type.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> src/network/bridge_driver.c | 137 +++++++++++++++---------------------
> 1 file changed, 56 insertions(+), 81 deletions(-)
>
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index 158b9ce447..d2bcb81912 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -4921,10 +4921,10 @@ networkReleaseActualDevice(virNetworkPtr net,
> virDomainNetDefPtr iface)
> {
> virNetworkDriverStatePtr driver = networkGetDriver();
> - virDomainNetType actualType = virDomainNetGetActualType(iface);
> virNetworkObjPtr obj;
> virNetworkDefPtr netdef;
> virNetworkForwardIfDefPtr dev = NULL;
> + virNetworkPortDefPtr port = NULL;
> size_t i;
> int ret = -1;
>
> @@ -4933,77 +4933,49 @@ networkReleaseActualDevice(virNetworkPtr net,
> virReportError(VIR_ERR_NO_NETWORK,
> _("no network with matching name '%s'"),
> net->name);
> - goto error;
> + goto cleanup;
> }
>
> if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("Expected a interface for a virtual network"));
> - goto error;
> + goto cleanup;
> }
>
> + if (iface->data.network.actual == NULL) {
> + ret = 0;
> + goto cleanup;
> + }
> +
> + if (!(port = virDomainNetDefActualToNetworkPort(dom, iface)))
> + goto cleanup;
> +
> netdef = virNetworkObjGetDef(obj);
>
> - switch ((virNetworkForwardType) netdef->forward.type) {
> - case VIR_NETWORK_FORWARD_NONE:
> - case VIR_NETWORK_FORWARD_NAT:
> - case VIR_NETWORK_FORWARD_ROUTE:
> - case VIR_NETWORK_FORWARD_OPEN:
> - if (iface->data.network.actual &&
> - networkUnplugBandwidth(obj, iface->bandwidth,
> - &iface->data.network.actual->class_id) < 0)
> - goto error;
> + switch ((virNetworkPortPlugType)port->plugtype) {
> + case VIR_NETWORK_PORT_PLUG_TYPE_NONE:
> + VIR_DEBUG("Releasing network device with no plug type");
> break;
>
> - case VIR_NETWORK_FORWARD_BRIDGE:
> - if (iface->data.network.actual &&
> - actualType == VIR_DOMAIN_NET_TYPE_BRIDGE &&
> - networkUnplugBandwidth(obj, iface->bandwidth,
> - &iface->data.network.actual->class_id) < 0)
> - goto error;
> - break;
> - case VIR_NETWORK_FORWARD_PRIVATE:
> - case VIR_NETWORK_FORWARD_VEPA:
> - case VIR_NETWORK_FORWARD_PASSTHROUGH:
> - case VIR_NETWORK_FORWARD_HOSTDEV:
> + case VIR_NETWORK_PORT_PLUG_TYPE_BRIDGE:
> + if (networkUnplugBandwidth(obj, port->bandwidth,
> + &port->class_id) < 0)
> + goto cleanup;
> break;
>
> - case VIR_NETWORK_FORWARD_LAST:
> - default:
> - virReportEnumRangeError(virNetworkForwardType, netdef->forward.type);
> - goto error;
> - }
> -
> - if ((!iface->data.network.actual) ||
> - ((actualType != VIR_DOMAIN_NET_TYPE_DIRECT) &&
> - (actualType != VIR_DOMAIN_NET_TYPE_HOSTDEV))) {
> - VIR_DEBUG("Nothing to release to network %s", iface->data.network.name);
> - goto success;
> - }
> -
> - if (netdef->forward.nifs == 0) {
> - virReportError(VIR_ERR_INTERNAL_ERROR,
> - _("network '%s' uses a direct/hostdev mode, but "
> - "has no forward dev and no interface pool"),
> - netdef->name);
> - goto error;
> - }
> -
> - if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) {
> - const char *actualDev;
> -
> - actualDev = virDomainNetGetActualDirectDev(iface);
> - if (!actualDev) {
> - virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> - _("the interface uses a direct mode, "
> - "but has no source dev"));
> - goto error;
> + case VIR_NETWORK_PORT_PLUG_TYPE_DIRECT:
> + if (netdef->forward.nifs == 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("network '%s' uses a direct mode, but "
> + "has no forward dev and no interface pool"),
> + netdef->name);
> + goto cleanup;
> }
>
> for (i = 0; i < netdef->forward.nifs; i++) {
> if (netdef->forward.ifs[i].type
> == VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV &&
> - STREQ(actualDev, netdef->forward.ifs[i].device.dev)) {
> + STREQ(port->plug.direct.linkdev, netdef->forward.ifs[i].device.dev)) {
> dev = &netdef->forward.ifs[i];
> break;
> }
> @@ -5013,23 +4985,24 @@ networkReleaseActualDevice(virNetworkPtr net,
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("network '%s' doesn't have dev='%s' "
> "in use by domain"),
> - netdef->name, actualDev);
> - goto error;
> + netdef->name, port->plug.direct.linkdev);
> + goto cleanup;
> }
> - } else /* if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) */ {
> - virDomainHostdevDefPtr hostdev;
> + break;
>
> - hostdev = virDomainNetGetActualHostdev(iface);
> - if (!hostdev) {
> + case VIR_NETWORK_PORT_PLUG_TYPE_HOSTDEV_PCI:
> + if (netdef->forward.nifs == 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> - "%s", _("the interface uses a hostdev mode, but has no hostdev"));
> - goto error;
> + _("network '%s' uses a hostdev mode, but "
> + "has no forward dev and no interface pool"),
> + netdef->name);
> + goto cleanup;
> }
>
> for (i = 0; i < netdef->forward.nifs; i++) {
> if (netdef->forward.ifs[i].type
> == VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_PCI &&
> - virPCIDeviceAddressEqual(&hostdev->source.subsys.u.pci.addr,
> + virPCIDeviceAddressEqual(&port->plug.hostdevpci.addr,
> &netdef->forward.ifs[i].device.pci)) {
> dev = &netdef->forward.ifs[i];
> break;
> @@ -5041,26 +5014,30 @@ networkReleaseActualDevice(virNetworkPtr net,
> _("network '%s' doesn't have "
> "PCI device %04x:%02x:%02x.%x in use by domain"),
> netdef->name,
> - hostdev->source.subsys.u.pci.addr.domain,
> - hostdev->source.subsys.u.pci.addr.bus,
> - hostdev->source.subsys.u.pci.addr.slot,
> - hostdev->source.subsys.u.pci.addr.function);
> - goto error;
> + port->plug.hostdevpci.addr.domain,
> + port->plug.hostdevpci.addr.bus,
> + port->plug.hostdevpci.addr.slot,
> + port->plug.hostdevpci.addr.function);
> + goto cleanup;
> }
> + break;
> +
> + case VIR_NETWORK_PORT_PLUG_TYPE_LAST:
> + default:
> + virReportEnumRangeError(virNetworkPortPlugType, port->plugtype);
> + goto cleanup;
> }
>
> - success:
> virNetworkObjMacMgrDel(obj, driver->dnsmasqStateDir, dom->name, &iface->mac);
Don't you want to change this ^^ to "&port->mac"?
>
> - if (iface->data.network.actual) {
> - netdef->connections--;
> - if (dev)
> - dev->connections--;
> - /* finally we can call the 'unplugged' hook script if any */
> - networkRunHook(obj, dom, iface, VIR_HOOK_NETWORK_OP_IFACE_UNPLUGGED,
> - VIR_HOOK_SUBOP_BEGIN);
> - networkLogAllocation(netdef, dev, &iface->mac, false);
> - }
> + netdef->connections--;
> + if (dev)
> + dev->connections--;
> + /* finally we can call the 'unplugged' hook script if any */
> + networkRunHook(obj, dom, iface, VIR_HOOK_NETWORK_OP_IFACE_UNPLUGGED,
> + VIR_HOOK_SUBOP_BEGIN);
> + networkLogAllocation(netdef, dev, &iface->mac, false);
Same with this ^^
Reviewed-by: Laine Stump <laine@laine.org>
anyway, because whether or not you intended to completely eliminate all
references to iface during this patch, I know it ends up that way in the
end anyway :-)
> +
> ret = 0;
> cleanup:
> virNetworkObjEndAPI(&obj);
> @@ -5068,10 +5045,8 @@ networkReleaseActualDevice(virNetworkPtr net,
> virDomainActualNetDefFree(iface->data.network.actual);
> iface->data.network.actual = NULL;
> }
> + virNetworkPortDefFree(port);
> return ret;
> -
> - error:
> - goto cleanup;
> }
>
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list