Re: [PATCH v1 16/31] bridge_driver: Adapt to new virNetworkObjList accessors

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Feb 26, 2015 at 15:17:25 +0100, Michal Privoznik wrote:
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
>  src/network/bridge_driver.c | 333 ++++++++++++++++++++------------------------
>  1 file changed, 148 insertions(+), 185 deletions(-)
> 

Some bikeshedding below ...

> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index 268af49..1c73342 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -342,105 +342,91 @@ networkBridgeDummyNicName(const char *brname)
>      return nicname;
>  }
>  
> -/* Update the internal status of all allegedly active networks
> - * according to external conditions on the host (i.e. anything that
> - * isn't stored directly in each network's state file). */
> -static void
> -networkUpdateAllState(void)
> +static int
> +networkUpdateAllState(virNetworkObjPtr obj,

This function now updates state of one network, thus the 'All' word is
misleading.

> +                      void *opaque ATTRIBUTE_UNUSED)
>  {
> -    size_t i;
> +    int ret = -1;
>  
> -    for (i = 0; i < driver->networks->count; i++) {
> -        virNetworkObjPtr obj = driver->networks->objs[i];
> +    virNetworkObjLock(obj);
> +    if (!virNetworkObjIsActive(obj)) {
> +        virNetworkObjUnlock(obj);
> +        return 0;
> +    }
>  
> -        virNetworkObjLock(obj);
> -        if (!virNetworkObjIsActive(obj)) {
> -            virNetworkObjUnlock(obj);
> -            continue;
> -        }
> +    switch (obj->def->forward.type) {
> +    case VIR_NETWORK_FORWARD_NONE:
> +    case VIR_NETWORK_FORWARD_NAT:
> +    case VIR_NETWORK_FORWARD_ROUTE:
> +        /* If bridge doesn't exist, then mark it inactive */
> +        if (!(obj->def->bridge && virNetDevExists(obj->def->bridge) == 1))
> +            obj->active = 0;
> +        break;
>  
> -        switch (obj->def->forward.type) {
> -        case VIR_NETWORK_FORWARD_NONE:
> -        case VIR_NETWORK_FORWARD_NAT:
> -        case VIR_NETWORK_FORWARD_ROUTE:
> -            /* If bridge doesn't exist, then mark it inactive */
> -            if (!(obj->def->bridge && virNetDevExists(obj->def->bridge) == 1))
> +    case VIR_NETWORK_FORWARD_BRIDGE:
> +        if (obj->def->bridge) {
> +            if (virNetDevExists(obj->def->bridge) != 1)
>                  obj->active = 0;
>              break;
> -
> -        case VIR_NETWORK_FORWARD_BRIDGE:
> -            if (obj->def->bridge) {
> -                if (virNetDevExists(obj->def->bridge) != 1)
> -                    obj->active = 0;
> -                break;
> -            }
> -            /* intentionally drop through to common case for all
> -             * macvtap networks (forward='bridge' with no bridge
> -             * device defined is macvtap using its 'bridge' mode)
> -             */
> -        case VIR_NETWORK_FORWARD_PRIVATE:
> -        case VIR_NETWORK_FORWARD_VEPA:
> -        case VIR_NETWORK_FORWARD_PASSTHROUGH:
> -            /* so far no extra checks */
> -            break;
> -
> -        case VIR_NETWORK_FORWARD_HOSTDEV:
> -            /* so far no extra checks */
> -            break;
> -        }
> -
> -        /* Try and read dnsmasq/radvd pids of active networks */
> -        if (obj->active && obj->def->ips && (obj->def->nips > 0)) {
> -            char *radvdpidbase;
> -
> -            ignore_value(virPidFileReadIfAlive(driver->pidDir,
> -                                               obj->def->name,
> -                                               &obj->dnsmasqPid,
> -                                               dnsmasqCapsGetBinaryPath(driver->dnsmasqCaps)));
> -            radvdpidbase = networkRadvdPidfileBasename(obj->def->name);
> -            if (!radvdpidbase)
> -                break;
> -            ignore_value(virPidFileReadIfAlive(driver->pidDir,
> -                                               radvdpidbase,
> -                                               &obj->radvdPid, RADVD));
> -            VIR_FREE(radvdpidbase);
>          }
> -
> -        virNetworkObjUnlock(obj);
> +        /* intentionally drop through to common case for all
> +         * macvtap networks (forward='bridge' with no bridge
> +         * device defined is macvtap using its 'bridge' mode)
> +         */
> +    case VIR_NETWORK_FORWARD_PRIVATE:
> +    case VIR_NETWORK_FORWARD_VEPA:
> +    case VIR_NETWORK_FORWARD_PASSTHROUGH:
> +        /* so far no extra checks */
> +        break;
> +
> +    case VIR_NETWORK_FORWARD_HOSTDEV:
> +        /* so far no extra checks */
> +        break;
>      }
>  
> -    /* remove inactive transient networks */
> -    i = 0;
> -    while (i < driver->networks->count) {
> -        virNetworkObjPtr obj = driver->networks->objs[i];
> -        virNetworkObjLock(obj);
> -
> -        if (!obj->persistent && !obj->active) {
> -            networkRemoveInactive(obj);
> -            continue;
> -        }
> +    /* Try and read dnsmasq/radvd pids of active networks */
> +    if (obj->active && obj->def->ips && (obj->def->nips > 0)) {
> +        char *radvdpidbase;
> +
> +        ignore_value(virPidFileReadIfAlive(driver->pidDir,
> +                                           obj->def->name,
> +                                           &obj->dnsmasqPid,
> +                                           dnsmasqCapsGetBinaryPath(driver->dnsmasqCaps)));
> +        radvdpidbase = networkRadvdPidfileBasename(obj->def->name);
> +        if (!radvdpidbase)
> +            goto cleanup;
> +
> +        ignore_value(virPidFileReadIfAlive(driver->pidDir,
> +                                           radvdpidbase,
> +                                           &obj->radvdPid, RADVD));
> +        VIR_FREE(radvdpidbase);
> +    }
>  
> +    ret = 0;
> + cleanup:
> +    if (!obj->persistent && !obj->active)
> +        networkRemoveInactive(obj);
> +    else
>          virNetworkObjUnlock(obj);
> -        i++;
> -    }
> +    return ret;
>  }
>  
> -
> -static void
> -networkAutostartConfigs(void)
> +static int
> +networkAutostartConfigs(virNetworkObjPtr net,
> +                        void *opaque ATTRIBUTE_UNUSED)

Same here, plural may be misleading.

ACK with or without naming fixed.

Peter

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]