Looks good to me. Acked-by: Kyle Mestery <kmestery@cisco.com> On Aug 14, 2012, at 2:10 AM, Laine Stump wrote:

This array was originally defined using the existing virNetworkForwardIfDef, but that struct has a UsageCount field that isn't used in the case of PFs. This patch just copies that struct and removes UsageCount. It ends up being a struct with a single field, but I left it as a struct in case we need to add other fields to it in the future. --- src/conf/network_conf.c | 9 +++++++-- src/conf/network_conf.h | 8 +++++++- 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 783c388..666118c 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -97,6 +97,12 @@ virNetworkForwardIfDefClear(virNetworkForwardIfDefPtr def) VIR_FREE(def->dev); }

+static void +virNetworkForwardPfDefClear(virNetworkForwardPfDefPtr def) +{ + VIR_FREE(def->dev); +} + static void virNetworkIpDefClear(virNetworkIpDefPtr def) { int ii; @@ -157,7 +163,7 @@ void virNetworkDefFree(virNetworkDefPtr def) VIR_FREE(def->domain);

for (ii = 0 ; ii < def->nForwardPfs && def->forwardPfs ; ii++) { - virNetworkForwardIfDefClear(&def->forwardPfs[ii]); + virNetworkForwardPfDefClear(&def->forwardPfs[ii]); } VIR_FREE(def->forwardPfs);

@@ -1113,7 +1119,6 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) goto error; }

- def->forwardPfs->usageCount = 0; def->forwardPfs->dev = forwardDev; forwardDev = NULL; def->nForwardPfs++; diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index a95b382..040d912 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -135,6 +135,12 @@ struct _virNetworkForwardIfDef { int usageCount; /* how many guest interfaces are bound to this device? */ };

+typedef struct _virNetworkForwardPfDef virNetworkForwardPfDef; +typedef virNetworkForwardPfDef *virNetworkForwardPfDefPtr; +struct _virNetworkForwardPfDef { + char *dev; /* name of device */ +}; + typedef struct _virPortGroupDef virPortGroupDef; typedef virPortGroupDef *virPortGroupDefPtr; struct _virPortGroupDef { @@ -164,7 +170,7 @@ struct _virNetworkDef { * interfaces), they will be listed here. */ size_t nForwardPfs; - virNetworkForwardIfDefPtr forwardPfs; + virNetworkForwardPfDefPtr forwardPfs;

size_t nForwardIfs; virNetworkForwardIfDefPtr forwardIfs; -- 1.7.11.2

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

Looks good to me. Acked-by: Kyle Mestery <kmestery@cisco.com> On Aug 14, 2012, at 2:10 AM, Laine Stump wrote:

It may be useful for management applications to know which physical network devices are in use by guests. This information is already available in the network objects, but wasn't output in the XML. This patch outputs it whan the INACTIVE flag isn't set (and if it's non-0). --- src/conf/network_conf.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 905c644..ca5b759 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -1495,8 +1495,14 @@ char *virNetworkDefFormat(const virNetworkDefPtr def, unsigned int flags) if (def->nForwardIfs && (!def->nForwardPfs || !(flags & VIR_NETWORK_XML_INACTIVE))) { for (ii = 0; ii < def->nForwardIfs; ii++) { - virBufferEscapeString(&buf, " <interface dev='%s'/>\n", + virBufferEscapeString(&buf, " <interface dev='%s'", def->forwardIfs[ii].dev); + if (!(flags & VIR_NETWORK_XML_INACTIVE) && + (def->forwardIfs[ii].connections > 0)) { + virBufferAsprintf(&buf, " connections='%d'", + def->forwardIfs[ii].connections); + } + virBufferAddLit(&buf, "/>\n"); } } if (def->nForwardPfs || def->nForwardIfs) -- 1.7.11.2

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

On 08/14/2012 01:10 AM, Laine Stump wrote:

A later patch will be adding a counter that will be incremented/decremented each time an guest interface starts/stops using a particular network. For this to work, all types of networks need to go through a common return sequence rather than returning early. To setup for this, the existing cleanup: label is renamed to error:, a new cleanup: label is added (when necessary), and early returns are changed to goto cleanup (except the case where the interface type != NETWORK). --- src/network/bridge_driver.c | 106 ++++++++++++++++++++++---------------------- 1 file changed, 53 insertions(+), 53 deletions(-)

@@ -3007,7 +3006,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) dev->dev, dev->connections); } ret = 0; -cleanup: +error: for (ii = 0; ii < num_virt_fns; ii++)

It looks weird to have the 'ret = 0' success case fall through into the 'error:' label.

@@ -3114,8 +3113,9 @@ networkNotifyActualDevice(virDomainNetDefPtr iface) dev->dev, dev->connections); }

- ret = 0; cleanup: + ret = 0; +error:

and here, wouldn't it be better to name things 'success' for early exit with ret = 0, and 'cleanup' for all cleanup whether on error or on fallthrough from the ret = 0 case?

@@ -3136,7 +3136,7 @@ int networkReleaseActualDevice(virDomainNetDefPtr iface) { struct network_driver *driver = driverState; - virNetworkObjPtr network = NULL; + virNetworkObjPtr network;

I did a double-take on this one...

virNetworkDefPtr netdef; const char *actualDev; int ret = -1; @@ -3144,13 +3144,6 @@ networkReleaseActualDevice(virDomainNetDefPtr iface) if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) return 0;

- if (!iface->data.network.actual || - (virDomainNetGetActualType(iface) != VIR_DOMAIN_NET_TYPE_DIRECT)) { - VIR_DEBUG("Nothing to release to network %s", iface->data.network.name); - ret = 0; - goto cleanup; - }

...but it is correct; the code motion here means you no longer reach the end labels prior to network being assigned.

@@ -3199,8 +3198,9 @@ networkReleaseActualDevice(virDomainNetDefPtr iface) dev->dev, dev->connections); }

- ret = 0; cleanup: + ret = 0; +error:

Another case where this might make more sense: success: ret = 0; cleanup: The cleanups are mechanical, but I'm not sure I like the resulting naming. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

A later patch will be adding a counter that will be incremented/decremented each time an guest interface starts/stops using a particular network. For this to work, all types of networks need to go through a common return sequence rather than returning early. To setup for this, the existing a new success: label is added (when necessary), a new error: label is added which does any cleanup necessary only for error returns and then does goto cleanup, and early returns are changed to goto error if it's a failure, or goto success if it's successful. This way the intent of all the gotos is unambiguous, and a successful return path never encounters the "error:" label. --- Change from v1: at Eric's suggestion, instead of having a cleanup label jumped to on success, and an inline "error" label jumped to on failure: cleanup: ret = 0; error: /* common exit code */ return ret; I modified these function so that they have three labels: success: /* anything only done on success */ ret = 0; cleanup: /* common cleanup */ return ret; error: /* anything only done on error */ goto cleanup; This has the dual advantage of making the intent of all gotos painfully clear, as well as not confusing anyone by having the control flow of a successful exit go past the error: label. src/network/bridge_driver.c | 124 ++++++++++++++++++++++++-------------------- 1 file changed, 69 insertions(+), 55 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 77b38d2..c86a157 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2767,9 +2767,8 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) virReportError(VIR_ERR_NO_NETWORK, _("no network with matching name '%s'"), iface->data.network.name); - goto cleanup; + goto error; } - netdef = network->def; /* portgroup can be present for any type of network, in particular @@ -2786,12 +2785,12 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) if (!iface->data.network.actual && (VIR_ALLOC(iface->data.network.actual) < 0)) { virReportOOMError(); - goto cleanup; + goto error; } if (virNetDevBandwidthCopy(&iface->data.network.actual->bandwidth, portgroup->bandwidth) < 0) - goto cleanup; + goto error; } if ((netdef->forwardType == VIR_NETWORK_FORWARD_NONE) || @@ -2813,14 +2812,14 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) if (!iface->data.network.actual && (VIR_ALLOC(iface->data.network.actual) < 0)) { virReportOOMError(); - goto cleanup; + goto error; } iface->data.network.actual->type = VIR_DOMAIN_NET_TYPE_BRIDGE; iface->data.network.actual->data.bridge.brname = strdup(netdef->bridge); if (!iface->data.network.actual->data.bridge.brname) { virReportOOMError(); - goto cleanup; + goto error; } /* merge virtualports from interface, network, and portgroup to @@ -2831,7 +2830,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) netdef->virtPortProfile, portgroup ? portgroup->virtPortProfile : NULL) < 0) { - goto cleanup; + goto error; } virtport = iface->data.network.actual->virtPortProfile; if (virtport) { @@ -2842,7 +2841,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) "'%s' which uses a bridge device"), virNetDevVPortTypeToString(virtport->virtPortType), netdef->name); - goto cleanup; + goto error; } } @@ -2858,7 +2857,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) if (!iface->data.network.actual && (VIR_ALLOC(iface->data.network.actual) < 0)) { virReportOOMError(); - goto cleanup; + goto error; } /* Set type=direct and appropriate <source mode='xxx'/> */ @@ -2886,7 +2885,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) netdef->virtPortProfile, portgroup ? portgroup->virtPortProfile : NULL) < 0) { - goto cleanup; + goto error; } virtport = iface->data.network.actual->virtPortProfile; if (virtport) { @@ -2898,7 +2897,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) "'%s' which uses a macvtap device"), virNetDevVPortTypeToString(virtport->virtPortType), netdef->name); - goto cleanup; + goto error; } } @@ -2910,7 +2909,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) _("network '%s' uses a direct mode, but " "has no forward dev and no interface pool"), netdef->name); - goto cleanup; + goto error; } else { /* pick an interface from the pool */ @@ -2927,19 +2926,19 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not get Virtual functions on %s"), netdef->forwardPfs->dev); - goto cleanup; + goto error; } if (num_virt_fns == 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("No Vf's present on SRIOV PF %s"), netdef->forwardPfs->dev); - goto cleanup; + goto error; } if ((VIR_ALLOC_N(netdef->forwardIfs, num_virt_fns)) < 0) { virReportOOMError(); - goto cleanup; + goto error; } netdef->nForwardIfs = num_virt_fns; @@ -2948,7 +2947,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) netdef->forwardIfs[ii].dev = strdup(vfname[ii]); if (!netdef->forwardIfs[ii].dev) { virReportOOMError(); - goto cleanup; + goto error; } } } @@ -2987,18 +2986,18 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) _("network '%s' requires exclusive access " "to interfaces, but none are available"), netdef->name); - goto cleanup; + goto error; } iface->data.network.actual->data.direct.linkdev = strdup(dev->dev); if (!iface->data.network.actual->data.direct.linkdev) { virReportOOMError(); - goto cleanup; + goto error; } } } if (virNetDevVPortProfileCheckComplete(virtport, true) < 0) - goto cleanup; + goto error; if (dev) { /* we are now assured of success, so mark the allocation */ @@ -3013,11 +3012,14 @@ cleanup: VIR_FREE(vfname); if (network) virNetworkObjUnlock(network); - if (ret < 0) { + return ret; + +error: + if (iface->type == VIR_DOMAIN_NET_TYPE_NETWORK) { virDomainActualNetDefFree(iface->data.network.actual); iface->data.network.actual = NULL; } - return ret; + goto cleanup; } /* networkNotifyActualDevice: @@ -3042,12 +3044,6 @@ networkNotifyActualDevice(virDomainNetDefPtr iface) if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) return 0; - if (!iface->data.network.actual || - (virDomainNetGetActualType(iface) != VIR_DOMAIN_NET_TYPE_DIRECT)) { - VIR_DEBUG("Nothing to claim from network %s", iface->data.network.name); - return 0; - } - networkDriverLock(driver); network = virNetworkFindByName(&driver->networks, iface->data.network.name); networkDriverUnlock(driver); @@ -3055,7 +3051,14 @@ networkNotifyActualDevice(virDomainNetDefPtr iface) virReportError(VIR_ERR_NO_NETWORK, _("no network with matching name '%s'"), iface->data.network.name); - goto cleanup; + goto error; + } + netdef = network->def; + + if (!iface->data.network.actual || + (virDomainNetGetActualType(iface) != VIR_DOMAIN_NET_TYPE_DIRECT)) { + VIR_DEBUG("Nothing to claim from network %s", iface->data.network.name); + goto success; } actualDev = virDomainNetGetActualDirectDev(iface); @@ -3063,16 +3066,15 @@ networkNotifyActualDevice(virDomainNetDefPtr iface) virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("the interface uses a direct " "mode, but has no source dev")); - goto cleanup; + goto error; } - netdef = network->def; if (netdef->nForwardIfs == 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; + goto error; } else { int ii; virNetworkForwardIfDefPtr dev = NULL; @@ -3090,7 +3092,7 @@ networkNotifyActualDevice(virDomainNetDefPtr iface) virReportError(VIR_ERR_INTERNAL_ERROR, _("network '%s' doesn't have dev='%s' in use by domain"), netdef->name, actualDev); - goto cleanup; + goto error; } /* PASSTHROUGH mode, and PRIVATE Mode + 802.1Qbh both require @@ -3106,7 +3108,7 @@ networkNotifyActualDevice(virDomainNetDefPtr iface) virReportError(VIR_ERR_INTERNAL_ERROR, _("network '%s' claims dev='%s' is already in use by a different domain"), netdef->name, actualDev); - goto cleanup; + goto error; } /* we are now assured of success, so mark the allocation */ dev->connections++; @@ -3114,11 +3116,15 @@ networkNotifyActualDevice(virDomainNetDefPtr iface) dev->dev, dev->connections); } +success: ret = 0; cleanup: if (network) virNetworkObjUnlock(network); return ret; + +error: + goto cleanup; } @@ -3136,7 +3142,7 @@ int networkReleaseActualDevice(virDomainNetDefPtr iface) { struct network_driver *driver = driverState; - virNetworkObjPtr network = NULL; + virNetworkObjPtr network; virNetworkDefPtr netdef; const char *actualDev; int ret = -1; @@ -3144,13 +3150,6 @@ networkReleaseActualDevice(virDomainNetDefPtr iface) if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) return 0; - if (!iface->data.network.actual || - (virDomainNetGetActualType(iface) != VIR_DOMAIN_NET_TYPE_DIRECT)) { - VIR_DEBUG("Nothing to release to network %s", iface->data.network.name); - ret = 0; - goto cleanup; - } - networkDriverLock(driver); network = virNetworkFindByName(&driver->networks, iface->data.network.name); networkDriverUnlock(driver); @@ -3158,7 +3157,14 @@ networkReleaseActualDevice(virDomainNetDefPtr iface) virReportError(VIR_ERR_NO_NETWORK, _("no network with matching name '%s'"), iface->data.network.name); - goto cleanup; + goto error; + } + netdef = network->def; + + if (!iface->data.network.actual || + (virDomainNetGetActualType(iface) != VIR_DOMAIN_NET_TYPE_DIRECT)) { + VIR_DEBUG("Nothing to release to network %s", iface->data.network.name); + goto success; } actualDev = virDomainNetGetActualDirectDev(iface); @@ -3166,16 +3172,15 @@ networkReleaseActualDevice(virDomainNetDefPtr iface) virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("the interface uses a direct " "mode, but has no source dev")); - goto cleanup; + goto error; } - netdef = network->def; if (netdef->nForwardIfs == 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; + goto error; } else { int ii; virNetworkForwardIfDefPtr dev = NULL; @@ -3191,7 +3196,7 @@ networkReleaseActualDevice(virDomainNetDefPtr iface) virReportError(VIR_ERR_INTERNAL_ERROR, _("network '%s' doesn't have dev='%s' in use by domain"), netdef->name, actualDev); - goto cleanup; + goto error; } dev->connections--; @@ -3199,13 +3204,19 @@ networkReleaseActualDevice(virDomainNetDefPtr iface) dev->dev, dev->connections); } +success: ret = 0; cleanup: if (network) virNetworkObjUnlock(network); - virDomainActualNetDefFree(iface->data.network.actual); - iface->data.network.actual = NULL; + if (iface->type == VIR_DOMAIN_NET_TYPE_NETWORK) { + virDomainActualNetDefFree(iface->data.network.actual); + iface->data.network.actual = NULL; + } return ret; + +error: + goto cleanup; } /* @@ -3232,7 +3243,7 @@ networkGetNetworkAddress(const char *netname, char **netaddr) { int ret = -1; struct network_driver *driver = driverState; - virNetworkObjPtr network = NULL; + virNetworkObjPtr network; virNetworkDefPtr netdef; virNetworkIpDefPtr ipdef; virSocketAddr addr; @@ -3247,7 +3258,7 @@ networkGetNetworkAddress(const char *netname, char **netaddr) virReportError(VIR_ERR_NO_NETWORK, _("no network with matching name '%s'"), netname); - goto cleanup; + goto error; } netdef = network->def; @@ -3289,18 +3300,21 @@ networkGetNetworkAddress(const char *netname, char **netaddr) if (dev_name) { if (virNetDevGetIPv4Address(dev_name, &addr) < 0) - goto cleanup; - + goto error; addrptr = &addr; } - if (addrptr && - (*netaddr = virSocketAddrFormat(addrptr))) { - ret = 0; + if (!(addrptr && + (*netaddr = virSocketAddrFormat(addrptr)))) { + goto error; } + ret = 0; cleanup: if (network) virNetworkObjUnlock(network); return ret; + +error: + goto cleanup; } -- 1.7.11.2

Looks good. Acked-by: Kyle Mestery <kmestery@cisco.com> On Aug 14, 2012, at 2:10 AM, Laine Stump wrote:

A later patch will be adding a counter that will be incremented/decremented each time an guest interface starts/stops using a particular network. For this to work, all types of networks need to go through a common return sequence rather than returning early. To setup for this, the existing cleanup: label is renamed to error:, a new cleanup: label is added (when necessary), and early returns are changed to goto cleanup (except the case where the interface type != NETWORK). --- src/network/bridge_driver.c | 106 ++++++++++++++++++++++---------------------- 1 file changed, 53 insertions(+), 53 deletions(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 77b38d2..680b3f3 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2767,9 +2767,8 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) virReportError(VIR_ERR_NO_NETWORK, _("no network with matching name '%s'"), iface->data.network.name); - goto cleanup; + goto error; } - netdef = network->def;

/* portgroup can be present for any type of network, in particular @@ -2786,12 +2785,12 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) if (!iface->data.network.actual && (VIR_ALLOC(iface->data.network.actual) < 0)) { virReportOOMError(); - goto cleanup; + goto error; }

if (virNetDevBandwidthCopy(&iface->data.network.actual->bandwidth, portgroup->bandwidth) < 0) - goto cleanup; + goto error; }

if ((netdef->forwardType == VIR_NETWORK_FORWARD_NONE) || @@ -2813,14 +2812,14 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) if (!iface->data.network.actual && (VIR_ALLOC(iface->data.network.actual) < 0)) { virReportOOMError(); - goto cleanup; + goto error; }

iface->data.network.actual->type = VIR_DOMAIN_NET_TYPE_BRIDGE; iface->data.network.actual->data.bridge.brname = strdup(netdef->bridge); if (!iface->data.network.actual->data.bridge.brname) { virReportOOMError(); - goto cleanup; + goto error; }

/* merge virtualports from interface, network, and portgroup to @@ -2831,7 +2830,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) netdef->virtPortProfile, portgroup ? portgroup->virtPortProfile : NULL) < 0) { - goto cleanup; + goto error; } virtport = iface->data.network.actual->virtPortProfile; if (virtport) { @@ -2842,7 +2841,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) "'%s' which uses a bridge device"), virNetDevVPortTypeToString(virtport->virtPortType), netdef->name); - goto cleanup; + goto error; } }

@@ -2858,7 +2857,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) if (!iface->data.network.actual && (VIR_ALLOC(iface->data.network.actual) < 0)) { virReportOOMError(); - goto cleanup; + goto error; }

/* Set type=direct and appropriate <source mode='xxx'/> */ @@ -2886,7 +2885,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) netdef->virtPortProfile, portgroup ? portgroup->virtPortProfile : NULL) < 0) { - goto cleanup; + goto error; } virtport = iface->data.network.actual->virtPortProfile; if (virtport) { @@ -2898,7 +2897,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) "'%s' which uses a macvtap device"), virNetDevVPortTypeToString(virtport->virtPortType), netdef->name); - goto cleanup; + goto error; } }

@@ -2910,7 +2909,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) _("network '%s' uses a direct mode, but " "has no forward dev and no interface pool"), netdef->name); - goto cleanup; + goto error; } else { /* pick an interface from the pool */

@@ -2927,19 +2926,19 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not get Virtual functions on %s"), netdef->forwardPfs->dev); - goto cleanup; + goto error; }

if (num_virt_fns == 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("No Vf's present on SRIOV PF %s"), netdef->forwardPfs->dev); - goto cleanup; + goto error; }

if ((VIR_ALLOC_N(netdef->forwardIfs, num_virt_fns)) < 0) { virReportOOMError(); - goto cleanup; + goto error; }

netdef->nForwardIfs = num_virt_fns; @@ -2948,7 +2947,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) netdef->forwardIfs[ii].dev = strdup(vfname[ii]); if (!netdef->forwardIfs[ii].dev) { virReportOOMError(); - goto cleanup; + goto error; } } } @@ -2987,18 +2986,18 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) _("network '%s' requires exclusive access " "to interfaces, but none are available"), netdef->name); - goto cleanup; + goto error; } iface->data.network.actual->data.direct.linkdev = strdup(dev->dev); if (!iface->data.network.actual->data.direct.linkdev) { virReportOOMError(); - goto cleanup; + goto error; } } }

if (virNetDevVPortProfileCheckComplete(virtport, true) < 0) - goto cleanup; + goto error;

if (dev) { /* we are now assured of success, so mark the allocation */ @@ -3007,7 +3006,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) dev->dev, dev->connections); } ret = 0; -cleanup: +error: for (ii = 0; ii < num_virt_fns; ii++) VIR_FREE(vfname[ii]); VIR_FREE(vfname); @@ -3042,12 +3041,6 @@ networkNotifyActualDevice(virDomainNetDefPtr iface) if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) return 0;

- if (!iface->data.network.actual || - (virDomainNetGetActualType(iface) != VIR_DOMAIN_NET_TYPE_DIRECT)) { - VIR_DEBUG("Nothing to claim from network %s", iface->data.network.name); - return 0; - } - networkDriverLock(driver); network = virNetworkFindByName(&driver->networks, iface->data.network.name); networkDriverUnlock(driver); @@ -3055,6 +3048,13 @@ networkNotifyActualDevice(virDomainNetDefPtr iface) virReportError(VIR_ERR_NO_NETWORK, _("no network with matching name '%s'"), iface->data.network.name); + goto error; + } + netdef = network->def; + + if (!iface->data.network.actual || + (virDomainNetGetActualType(iface) != VIR_DOMAIN_NET_TYPE_DIRECT)) { + VIR_DEBUG("Nothing to claim from network %s", iface->data.network.name); goto cleanup; }

@@ -3063,16 +3063,15 @@ networkNotifyActualDevice(virDomainNetDefPtr iface) virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("the interface uses a direct " "mode, but has no source dev")); - goto cleanup; + goto error; }

- netdef = network->def; if (netdef->nForwardIfs == 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; + goto error; } else { int ii; virNetworkForwardIfDefPtr dev = NULL; @@ -3090,7 +3089,7 @@ networkNotifyActualDevice(virDomainNetDefPtr iface) virReportError(VIR_ERR_INTERNAL_ERROR, _("network '%s' doesn't have dev='%s' in use by domain"), netdef->name, actualDev); - goto cleanup; + goto error; }

/* PASSTHROUGH mode, and PRIVATE Mode + 802.1Qbh both require @@ -3106,7 +3105,7 @@ networkNotifyActualDevice(virDomainNetDefPtr iface) virReportError(VIR_ERR_INTERNAL_ERROR, _("network '%s' claims dev='%s' is already in use by a different domain"), netdef->name, actualDev); - goto cleanup; + goto error; } /* we are now assured of success, so mark the allocation */ dev->connections++; @@ -3114,8 +3113,9 @@ networkNotifyActualDevice(virDomainNetDefPtr iface) dev->dev, dev->connections); }

- ret = 0; cleanup: + ret = 0; +error: if (network) virNetworkObjUnlock(network); return ret; @@ -3136,7 +3136,7 @@ int networkReleaseActualDevice(virDomainNetDefPtr iface) { struct network_driver *driver = driverState; - virNetworkObjPtr network = NULL; + virNetworkObjPtr network; virNetworkDefPtr netdef; const char *actualDev; int ret = -1; @@ -3144,13 +3144,6 @@ networkReleaseActualDevice(virDomainNetDefPtr iface) if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) return 0;

- if (!iface->data.network.actual || - (virDomainNetGetActualType(iface) != VIR_DOMAIN_NET_TYPE_DIRECT)) { - VIR_DEBUG("Nothing to release to network %s", iface->data.network.name); - ret = 0; - goto cleanup; - } - networkDriverLock(driver); network = virNetworkFindByName(&driver->networks, iface->data.network.name); networkDriverUnlock(driver); @@ -3158,6 +3151,13 @@ networkReleaseActualDevice(virDomainNetDefPtr iface) virReportError(VIR_ERR_NO_NETWORK, _("no network with matching name '%s'"), iface->data.network.name); + goto error; + } + netdef = network->def; + + if (!iface->data.network.actual || + (virDomainNetGetActualType(iface) != VIR_DOMAIN_NET_TYPE_DIRECT)) { + VIR_DEBUG("Nothing to release to network %s", iface->data.network.name); goto cleanup; }

@@ -3166,16 +3166,15 @@ networkReleaseActualDevice(virDomainNetDefPtr iface) virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("the interface uses a direct " "mode, but has no source dev")); - goto cleanup; + goto error; }

- netdef = network->def; if (netdef->nForwardIfs == 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; + goto error; } else { int ii; virNetworkForwardIfDefPtr dev = NULL; @@ -3191,7 +3190,7 @@ networkReleaseActualDevice(virDomainNetDefPtr iface) virReportError(VIR_ERR_INTERNAL_ERROR, _("network '%s' doesn't have dev='%s' in use by domain"), netdef->name, actualDev); - goto cleanup; + goto error; }

dev->connections--; @@ -3199,8 +3198,9 @@ networkReleaseActualDevice(virDomainNetDefPtr iface) dev->dev, dev->connections); }

- ret = 0; cleanup: + ret = 0; +error: if (network) virNetworkObjUnlock(network); virDomainActualNetDefFree(iface->data.network.actual); @@ -3232,7 +3232,7 @@ networkGetNetworkAddress(const char *netname, char **netaddr) { int ret = -1; struct network_driver *driver = driverState; - virNetworkObjPtr network = NULL; + virNetworkObjPtr network; virNetworkDefPtr netdef; virNetworkIpDefPtr ipdef; virSocketAddr addr; @@ -3247,7 +3247,7 @@ networkGetNetworkAddress(const char *netname, char **netaddr) virReportError(VIR_ERR_NO_NETWORK, _("no network with matching name '%s'"), netname); - goto cleanup; + goto error; } netdef = network->def;

@@ -3289,17 +3289,17 @@ networkGetNetworkAddress(const char *netname, char **netaddr)

if (dev_name) { if (virNetDevGetIPv4Address(dev_name, &addr) < 0) - goto cleanup; - + goto error; addrptr = &addr; }

- if (addrptr && - (*netaddr = virSocketAddrFormat(addrptr))) { - ret = 0; + if (!(addrptr && + (*netaddr = virSocketAddrFormat(addrptr)))) { + goto error; }

-cleanup: + ret = 0; +error: if (network) virNetworkObjUnlock(network); return ret; -- 1.7.11.2

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

Looks good to me. Acked-by: Kyle Mestery <kmestery@cisco.com> On Aug 14, 2012, at 2:10 AM, Laine Stump wrote:

Just as each physical device used by a network has a connections counter, now each network has a connections counter which is incremented once for each guest interface that connects using this network.

The count is output in the live network XML, like this:

<network connections='20'> ... </network>

It is read-only, and for informational purposes only - it isn't used internally anywhere by libvirt. --- src/conf/network_conf.c | 6 +++++- src/conf/network_conf.h | 1 + src/network/bridge_driver.c | 10 ++++++++++ 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index ca5b759..45f8e69 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -1463,7 +1463,11 @@ char *virNetworkDefFormat(const virNetworkDefPtr def, unsigned int flags) char uuidstr[VIR_UUID_STRING_BUFLEN]; int ii;

- virBufferAddLit(&buf, "<network>\n"); + virBufferAddLit(&buf, "<network"); + if (!(flags & VIR_NETWORK_XML_INACTIVE) && (def->connections > 0)) { + virBufferAsprintf(&buf, " connections='%d'", def->connections); + } + virBufferAddLit(&buf, ">\n"); virBufferEscapeString(&buf, " <name>%s</name>\n", def->name);

uuid = def->uuid; diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 23f1632..32fc765 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -156,6 +156,7 @@ struct _virNetworkDef { unsigned char uuid[VIR_UUID_BUFLEN]; bool uuid_specified; char *name; + int connections; /* # of guest interfaces connected to this network */

char *bridge; /* Name of bridge device */ char *domain; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 680b3f3..b784eb4 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -3005,6 +3005,10 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) VIR_DEBUG("Using physical device %s, %d connections", dev->dev, dev->connections); } + + netdef->connections++; + VIR_DEBUG("Using network %s, %d connections", + netdef->name, netdef->connections); ret = 0; error: for (ii = 0; ii < num_virt_fns; ii++) @@ -3114,6 +3118,9 @@ networkNotifyActualDevice(virDomainNetDefPtr iface) }

cleanup: + netdef->connections++; + VIR_DEBUG("Using network %s, %d connections", + netdef->name, netdef->connections); ret = 0; error: if (network) @@ -3199,6 +3206,9 @@ networkReleaseActualDevice(virDomainNetDefPtr iface) }

cleanup: + netdef->connections--; + VIR_DEBUG("Releasing network %s, %d connections", + netdef->name, netdef->connections); ret = 0; error: if (network) -- 1.7.11.2

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