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