[libvirt] [PATCH v3 24/36] network: introduce networkNotifyPort

[Laine Stump]

On 3/19/19 8:46 AM, Daniel P. Berrangé wrote:
> Separate network port notification code from the domain driver network
> callback implementation.
>
> Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
> ---
>   src/network/bridge_driver.c | 106 +++++++++++++++++++++---------------
>   1 file changed, 63 insertions(+), 43 deletions(-)
>
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index 374416e692..d953215315 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -4727,74 +4727,51 @@ networkAllocateActualDevice(virNetworkPtr net,
>   }
>   
>   
> -/* networkNotifyActualDevice:
> - * @dom: domain definition that @iface belongs to
> - * @iface:  the domain's NetDef with an "actual" device already filled in.
> +/* networkNotifyPort:
> + * @obj: the network to notify
> + * @port: the port definition to notify
>    *
>    * Called to notify the network driver when libvirtd is restarted and
>    * finds an already running domain. If appropriate it will force an
>    * allocation of the actual->direct.linkdev to get everything back in
>    * order.
> - *
> - * Returns 0 on success, -1 on failure.
>    */
>   static int
> -networkNotifyActualDevice(virNetworkPtr net,
> -                          virDomainDefPtr dom,
> -                          virDomainNetDefPtr iface)
> +networkNotifyPort(virNetworkObjPtr obj,
> +                  virNetworkPortDefPtr port)
>   {
> -    virNetworkDriverStatePtr driver = networkGetDriver();
> -    virNetworkObjPtr obj;
>       virNetworkDefPtr netdef;
>       virNetworkForwardIfDefPtr dev = NULL;
> -    virNetworkPortDefPtr port = NULL;
>       size_t i;
>       int ret = -1;
>   
> -    obj = virNetworkObjFindByName(driver->networks, net->name);
> -    if (!obj) {
> -        virReportError(VIR_ERR_NO_NETWORK,
> -                       _("no network with matching name '%s'"),
> -                       net->name);
> -        goto error;
> -    }
> -
> -    if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) {
> -        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                       _("Expected a interface for a virtual network"));
> -        goto error;
> -    }
> -
>       netdef = virNetworkObjGetDef(obj);
>   
>       if (!virNetworkObjIsActive(obj)) {
>           virReportError(VIR_ERR_OPERATION_INVALID,
>                          _("network '%s' is not active"),
>                          netdef->name);
> -        goto error;
> -    }
> -
> -    if (!(port = virDomainNetDefActualToNetworkPort(dom, iface)))
>           goto cleanup;
> +    }
>   
>       switch (port->plugtype) {
>       case VIR_NETWORK_PORT_PLUG_TYPE_NONE:
>           virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                          _("Unexpectedly got a network port without a plug"));
> -        goto error;
> +        goto cleanup;
>   
>       case VIR_NETWORK_PORT_PLUG_TYPE_BRIDGE:
>           /* see if we're connected to the correct bridge */
>           if (!netdef->bridge) {
>               virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                              _("Unexpectedly got a network port plugged into a bridge"));


I didn't notice this in the patch where it was added. It should maybe 
say "...for a network with no bridge device" or something like that.


> -            goto error;
> +            goto cleanup;
>           }
>           break;
>   
>       case VIR_NETWORK_PORT_PLUG_TYPE_DIRECT:
>           if (networkCreateInterfacePool(netdef) < 0)
> -            goto error;
> +            goto cleanup;
>   
>           /* find the matching interface and increment its connections */
>           for (i = 0; i < netdef->forward.nifs; i++) {
> @@ -4813,7 +4790,7 @@ networkNotifyActualDevice(virNetworkPtr net,
>                                "in use by network port '%s'"),
>                              netdef->name, port->plug.direct.linkdev,
>                              port->uuid);
> -            goto error;
> +            goto cleanup;
>           }
>   
>           /* PASSTHROUGH mode and PRIVATE Mode + 802.1Qbh both require
> @@ -4829,14 +4806,14 @@ networkNotifyActualDevice(virNetworkPtr net,
>                              _("network '%s' claims dev='%s' is already in "
>                                "use by a different port"),
>                              netdef->name, port->plug.direct.linkdev);
> -            goto error;
> +            goto cleanup;
>           }
>           break;
>   
>       case VIR_NETWORK_PORT_PLUG_TYPE_HOSTDEV_PCI:
>   
>           if (networkCreateInterfacePool(netdef) < 0)
> -            goto error;
> +            goto cleanup;
>   
>           /* find the matching interface and increment its connections */
>           for (i = 0; i < netdef->forward.nifs; i++) {
> @@ -4858,7 +4835,7 @@ networkNotifyActualDevice(virNetworkPtr net,
>                              port->plug.hostdevpci.addr.bus,
>                              port->plug.hostdevpci.addr.slot,
>                              port->plug.hostdevpci.addr.function);
> -            goto error;
> +            goto cleanup;
>           }
>   
>           /* PASSTHROUGH mode, PRIVATE Mode + 802.1Qbh, and hostdev (PCI
> @@ -4874,7 +4851,7 @@ networkNotifyActualDevice(virNetworkPtr net,
>                              netdef->name,
>                              dev->device.pci.domain, dev->device.pci.bus,
>                              dev->device.pci.slot, dev->device.pci.function);
> -            goto error;
> +            goto cleanup;
>           }
>   
>           break;
> @@ -4882,7 +4859,7 @@ networkNotifyActualDevice(virNetworkPtr net,
>       case VIR_NETWORK_PORT_PLUG_TYPE_LAST:
>       default:
>           virReportEnumRangeError(virNetworkPortPlugType, port->plugtype);
> -        goto error;
> +        goto cleanup;
>       }
>   
>       netdef->connections++;
> @@ -4895,18 +4872,61 @@ networkNotifyActualDevice(virNetworkPtr net,
>           if (dev)
>               dev->connections--;
>           netdef->connections--;
> -        goto error;
> +        goto cleanup;
>       }
> -    networkLogAllocation(netdef, dev, &iface->mac, true);
> +    networkLogAllocation(netdef, dev, &port->mac, true);
> +
>       ret = 0;
> + cleanup:
> +    return ret;
> +}
> +
> +
> +static int
> +networkNotifyActualDevice(virNetworkPtr net,
> +                          virDomainDefPtr dom,
> +                          virDomainNetDefPtr iface)
> +{
> +    virNetworkDriverStatePtr driver = networkGetDriver();
> +    virNetworkObjPtr obj;
> +    virNetworkDefPtr netdef;
> +    virNetworkPortDefPtr port = NULL;
> +    int ret = -1;
> +
> +    obj = virNetworkObjFindByName(driver->networks, net->name);
> +    if (!obj) {
> +        virReportError(VIR_ERR_NO_NETWORK,
> +                       _("no network with matching name '%s'"),
> +                       net->name);
> +        goto cleanup;
> +    }
>   
> +    if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("Expected a interface for a virtual network"));
> +        goto cleanup;
> +    }
> +
> +    netdef = virNetworkObjGetDef(obj);
> +
> +    if (!virNetworkObjIsActive(obj)) {
> +        virReportError(VIR_ERR_OPERATION_INVALID,
> +                       _("network '%s' is not active"),
> +                       netdef->name);
> +        goto cleanup;
> +    }
> +
> +    if (!(port = virDomainNetDefActualToNetworkPort(dom, iface)))
> +        goto cleanup;
> +
> +    if (networkNotifyPort(obj, port) < 0)
> +        goto cleanup;
> +
> +    ret = 0;
>    cleanup:
>       virNetworkObjEndAPI(&obj);
>       virNetworkPortDefFree(port);
>       return ret;
> -
> - error:
> -    goto cleanup;
>   }
>   
>   


Like the last one, this is a fairly mechanical change...


Reviewed-by: Laine Stump <laine at laine.org>