2:10 a.m.
This repost of an unreviewed patchset from last week is purely to
rebase - the "openvswitch in <network> 9 patch series is a
prerequisite of this, and this is a prerequisite of the vlan patch
series resend coming up.
These are all fairly short and simple, so shouldn't require too much
hair pulling to review :-)
=====
The forward devices in direct mode (aka macvtap) networks already have
a counter to keep track of how many guest interfaces are using each
physical device. This series renames that counter to "connections",
then adds another connects counter to the toplevel of each network,
and outputs both of these counters (when non-0) with the live XML for
the network (i.e. it isn't output if you ask for the --inactive xml).
Someone working on a management application had requested this info,
and it already existed, so I thought I may as well expose it.
2:10 a.m.
New subject: [libvirt] [PATCHv2 1/5] conf: use a unique data type for PF array in virDomainNetDef
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
8:55 a.m.
New subject: [libvirt] [PATCHv2 1/5] conf: use a unique data type for PF array in virDomainNetDef
On 08/14/2012 01: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(-)
ACK.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
10:22 a.m.
New subject: [libvirt] [PATCHv2 1/5] conf: use a unique data type for PF array in virDomainNetDef
Looks good to me.
Acked-by: Kyle Mestery <kmestery(a)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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
2:10 a.m.
New subject: [libvirt] [PATCHv2 2/5] conf: rename interface "usageCount" to "connections"
I want to include this count in the xml output of networks, but
calling it "connections" in the XML sounds better than "usageCount",
and it
would be better if the name in the XML matched the variable name.
In a few places, usageCount was being initialized to 0, but this is
unnecessary, because VIR_ALLOC_N zero-fills everything anyway.
---
src/conf/network_conf.c | 2 --
src/conf/network_conf.h | 2 +-
src/network/bridge_driver.c | 46 ++++++++++++++++++++++-----------------------
3 files changed, 24 insertions(+), 26 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 666118c..905c644 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -1137,7 +1137,6 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
}
if (forwardDev) {
- def->forwardIfs[0].usageCount = 0;
def->forwardIfs[0].dev = forwardDev;
forwardDev = NULL;
def->nForwardIfs++;
@@ -1169,7 +1168,6 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
def->forwardIfs[ii].dev = forwardDev;
forwardDev = NULL;
- def->forwardIfs[ii].usageCount = 0;
def->nForwardIfs++;
}
}
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 040d912..23f1632 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -132,7 +132,7 @@ typedef struct _virNetworkForwardIfDef virNetworkForwardIfDef;
typedef virNetworkForwardIfDef *virNetworkForwardIfDefPtr;
struct _virNetworkForwardIfDef {
char *dev; /* name of device */
- int usageCount; /* how many guest interfaces are bound to this device? */
+ int connections; /* how many guest interfaces are connected to this device? */
};
typedef struct _virNetworkForwardPfDef virNetworkForwardPfDef;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index ec99e4d..77b38d2 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2914,10 +2914,11 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
} else {
/* pick an interface from the pool */
- /* PASSTHROUGH mode, and PRIVATE Mode + 802.1Qbh both require
- * exclusive access to a device, so current usageCount must be
- * 0. Other modes can share, so just search for the one with
- * the lowest usageCount.
+ /* PASSTHROUGH mode, and PRIVATE Mode + 802.1Qbh both
+ * require exclusive access to a device, so current
+ * connections count must be 0. Other modes can share, so
+ * just search for the one with the lowest number of
+ * connections.
*/
if (netdef->forwardType == VIR_NETWORK_FORWARD_PASSTHROUGH) {
if ((netdef->nForwardPfs > 0) && (netdef->nForwardIfs
<= 0)) {
@@ -2949,14 +2950,13 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
virReportOOMError();
goto cleanup;
}
- netdef->forwardIfs[ii].usageCount = 0;
}
}
- /* pick first dev with 0 usageCount */
+ /* pick first dev with 0 connections */
for (ii = 0; ii < netdef->nForwardIfs; ii++) {
- if (netdef->forwardIfs[ii].usageCount == 0) {
+ if (netdef->forwardIfs[ii].connections == 0) {
dev = &netdef->forwardIfs[ii];
break;
}
@@ -2966,9 +2966,9 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
(iface->data.network.actual->virtPortProfile->virtPortType
== VIR_NETDEV_VPORT_PROFILE_8021QBH)) {
- /* pick first dev with 0 usageCount */
+ /* pick first dev with 0 connections */
for (ii = 0; ii < netdef->nForwardIfs; ii++) {
- if (netdef->forwardIfs[ii].usageCount == 0) {
+ if (netdef->forwardIfs[ii].connections == 0) {
dev = &netdef->forwardIfs[ii];
break;
}
@@ -2977,7 +2977,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
/* pick least used dev */
dev = &netdef->forwardIfs[0];
for (ii = 1; ii < netdef->nForwardIfs; ii++) {
- if (netdef->forwardIfs[ii].usageCount < dev->usageCount)
+ if (netdef->forwardIfs[ii].connections < dev->connections)
dev = &netdef->forwardIfs[ii];
}
}
@@ -3002,9 +3002,9 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
if (dev) {
/* we are now assured of success, so mark the allocation */
- dev->usageCount++;
- VIR_DEBUG("Using physical device %s, usageCount %d",
- dev->dev, dev->usageCount);
+ dev->connections++;
+ VIR_DEBUG("Using physical device %s, %d connections",
+ dev->dev, dev->connections);
}
ret = 0;
cleanup:
@@ -3077,7 +3077,7 @@ networkNotifyActualDevice(virDomainNetDefPtr iface)
int ii;
virNetworkForwardIfDefPtr dev = NULL;
- /* find the matching interface in the pool and increment its usageCount */
+ /* find the matching interface and increment its connections */
for (ii = 0; ii < netdef->nForwardIfs; ii++) {
if (STREQ(actualDev, netdef->forwardIfs[ii].dev)) {
@@ -3094,10 +3094,10 @@ networkNotifyActualDevice(virDomainNetDefPtr iface)
}
/* PASSTHROUGH mode, and PRIVATE Mode + 802.1Qbh both require
- * exclusive access to a device, so current usageCount must be
- * 0 in those cases.
+ * exclusive access to a device, so current connections count
+ * must be 0 in those cases.
*/
- if ((dev->usageCount > 0) &&
+ if ((dev->connections > 0) &&
((netdef->forwardType == VIR_NETWORK_FORWARD_PASSTHROUGH) ||
((netdef->forwardType == VIR_NETWORK_FORWARD_PRIVATE) &&
iface->data.network.actual->virtPortProfile &&
@@ -3109,9 +3109,9 @@ networkNotifyActualDevice(virDomainNetDefPtr iface)
goto cleanup;
}
/* we are now assured of success, so mark the allocation */
- dev->usageCount++;
- VIR_DEBUG("Using physical device %s, usageCount %d",
- dev->dev, dev->usageCount);
+ dev->connections++;
+ VIR_DEBUG("Using physical device %s, %d connections",
+ dev->dev, dev->connections);
}
ret = 0;
@@ -3194,9 +3194,9 @@ networkReleaseActualDevice(virDomainNetDefPtr iface)
goto cleanup;
}
- dev->usageCount--;
- VIR_DEBUG("Releasing physical device %s, usageCount %d",
- dev->dev, dev->usageCount);
+ dev->connections--;
+ VIR_DEBUG("Releasing physical device %s, %d connections",
+ dev->dev, dev->connections);
}
ret = 0;
--
1.7.11.2
9:01 a.m.
New subject: [libvirt] [PATCHv2 2/5] conf: rename interface "usageCount" to "connections"
On 08/14/2012 01:10 AM, Laine Stump wrote:
I want to include this count in the xml output of networks, but
calling it "connections" in the XML sounds better than "usageCount",
and it
would be better if the name in the XML matched the variable name.
In a few places, usageCount was being initialized to 0, but this is
unnecessary, because VIR_ALLOC_N zero-fills everything anyway.
---
src/conf/network_conf.c | 2 --
src/conf/network_conf.h | 2 +-
src/network/bridge_driver.c | 46 ++++++++++++++++++++++-----------------------
3 files changed, 24 insertions(+), 26 deletions(-)
ACK.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
10:23 a.m.
New subject: [libvirt] [PATCHv2 2/5] conf: rename interface "usageCount" to "connections"
Looks good to me.
Acked-by: Kyle Mestery <kmestery(a)cisco.com>
On Aug 14, 2012, at 2:10 AM, Laine Stump wrote:
I want to include this count in the xml output of networks, but
calling it "connections" in the XML sounds better than "usageCount",
and it
would be better if the name in the XML matched the variable name.
In a few places, usageCount was being initialized to 0, but this is
unnecessary, because VIR_ALLOC_N zero-fills everything anyway.
---
src/conf/network_conf.c | 2 --
src/conf/network_conf.h | 2 +-
src/network/bridge_driver.c | 46 ++++++++++++++++++++++-----------------------
3 files changed, 24 insertions(+), 26 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 666118c..905c644 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -1137,7 +1137,6 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
}
if (forwardDev) {
- def->forwardIfs[0].usageCount = 0;
def->forwardIfs[0].dev = forwardDev;
forwardDev = NULL;
def->nForwardIfs++;
@@ -1169,7 +1168,6 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
def->forwardIfs[ii].dev = forwardDev;
forwardDev = NULL;
- def->forwardIfs[ii].usageCount = 0;
def->nForwardIfs++;
}
}
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 040d912..23f1632 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -132,7 +132,7 @@ typedef struct _virNetworkForwardIfDef virNetworkForwardIfDef;
typedef virNetworkForwardIfDef *virNetworkForwardIfDefPtr;
struct _virNetworkForwardIfDef {
char *dev; /* name of device */
- int usageCount; /* how many guest interfaces are bound to this device? */
+ int connections; /* how many guest interfaces are connected to this device? */
};
typedef struct _virNetworkForwardPfDef virNetworkForwardPfDef;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index ec99e4d..77b38d2 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2914,10 +2914,11 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
} else {
/* pick an interface from the pool */
- /* PASSTHROUGH mode, and PRIVATE Mode + 802.1Qbh both require
- * exclusive access to a device, so current usageCount must be
- * 0. Other modes can share, so just search for the one with
- * the lowest usageCount.
+ /* PASSTHROUGH mode, and PRIVATE Mode + 802.1Qbh both
+ * require exclusive access to a device, so current
+ * connections count must be 0. Other modes can share, so
+ * just search for the one with the lowest number of
+ * connections.
*/
if (netdef->forwardType == VIR_NETWORK_FORWARD_PASSTHROUGH) {
if ((netdef->nForwardPfs > 0) && (netdef->nForwardIfs
<= 0)) {
@@ -2949,14 +2950,13 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
virReportOOMError();
goto cleanup;
}
- netdef->forwardIfs[ii].usageCount = 0;
}
}
- /* pick first dev with 0 usageCount */
+ /* pick first dev with 0 connections */
for (ii = 0; ii < netdef->nForwardIfs; ii++) {
- if (netdef->forwardIfs[ii].usageCount == 0) {
+ if (netdef->forwardIfs[ii].connections == 0) {
dev = &netdef->forwardIfs[ii];
break;
}
@@ -2966,9 +2966,9 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
(iface->data.network.actual->virtPortProfile->virtPortType
== VIR_NETDEV_VPORT_PROFILE_8021QBH)) {
- /* pick first dev with 0 usageCount */
+ /* pick first dev with 0 connections */
for (ii = 0; ii < netdef->nForwardIfs; ii++) {
- if (netdef->forwardIfs[ii].usageCount == 0) {
+ if (netdef->forwardIfs[ii].connections == 0) {
dev = &netdef->forwardIfs[ii];
break;
}
@@ -2977,7 +2977,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
/* pick least used dev */
dev = &netdef->forwardIfs[0];
for (ii = 1; ii < netdef->nForwardIfs; ii++) {
- if (netdef->forwardIfs[ii].usageCount < dev->usageCount)
+ if (netdef->forwardIfs[ii].connections < dev->connections)
dev = &netdef->forwardIfs[ii];
}
}
@@ -3002,9 +3002,9 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
if (dev) {
/* we are now assured of success, so mark the allocation */
- dev->usageCount++;
- VIR_DEBUG("Using physical device %s, usageCount %d",
- dev->dev, dev->usageCount);
+ dev->connections++;
+ VIR_DEBUG("Using physical device %s, %d connections",
+ dev->dev, dev->connections);
}
ret = 0;
cleanup:
@@ -3077,7 +3077,7 @@ networkNotifyActualDevice(virDomainNetDefPtr iface)
int ii;
virNetworkForwardIfDefPtr dev = NULL;
- /* find the matching interface in the pool and increment its usageCount */
+ /* find the matching interface and increment its connections */
for (ii = 0; ii < netdef->nForwardIfs; ii++) {
if (STREQ(actualDev, netdef->forwardIfs[ii].dev)) {
@@ -3094,10 +3094,10 @@ networkNotifyActualDevice(virDomainNetDefPtr iface)
}
/* PASSTHROUGH mode, and PRIVATE Mode + 802.1Qbh both require
- * exclusive access to a device, so current usageCount must be
- * 0 in those cases.
+ * exclusive access to a device, so current connections count
+ * must be 0 in those cases.
*/
- if ((dev->usageCount > 0) &&
+ if ((dev->connections > 0) &&
((netdef->forwardType == VIR_NETWORK_FORWARD_PASSTHROUGH) ||
((netdef->forwardType == VIR_NETWORK_FORWARD_PRIVATE) &&
iface->data.network.actual->virtPortProfile &&
@@ -3109,9 +3109,9 @@ networkNotifyActualDevice(virDomainNetDefPtr iface)
goto cleanup;
}
/* we are now assured of success, so mark the allocation */
- dev->usageCount++;
- VIR_DEBUG("Using physical device %s, usageCount %d",
- dev->dev, dev->usageCount);
+ dev->connections++;
+ VIR_DEBUG("Using physical device %s, %d connections",
+ dev->dev, dev->connections);
}
ret = 0;
@@ -3194,9 +3194,9 @@ networkReleaseActualDevice(virDomainNetDefPtr iface)
goto cleanup;
}
- dev->usageCount--;
- VIR_DEBUG("Releasing physical device %s, usageCount %d",
- dev->dev, dev->usageCount);
+ dev->connections--;
+ VIR_DEBUG("Releasing physical device %s, %d connections",
+ dev->dev, dev->connections);
}
ret = 0;
--
1.7.11.2
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
2:10 a.m.
New subject: [libvirt] [PATCHv2 3/5] conf: output forward device connections count in network XML
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
9:17 a.m.
New subject: [libvirt] [PATCHv2 3/5] conf: output forward device connections count in network XML
On 08/14/2012 01: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).
s/whan/when/
ACK.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
10:25 a.m.
New subject: [libvirt] [PATCHv2 3/5] conf: output forward device connections count in network XML
Looks good to me.
Acked-by: Kyle Mestery <kmestery(a)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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
2:10 a.m.
New subject: [libvirt] [PATCHv2 4/5] network: change cleanup: to error: in network*() functions
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
9:47 a.m.
New subject: [libvirt] [PATCHv2 4/5] network: change cleanup: to error: in network*() functions
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(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
3:47 p.m.
New subject: [libvirt] [PATCHv2 4/5] network: change cleanup: to error: in network*() functions
On 08/14/2012 10:47 AM, Eric Blake wrote:
On 08/14/2012 01:10 AM, Laine Stump wrote:
> @@ -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.
Well, I can create a new "error:" label that just does this:
success:
ret = 0;
cleanup:
/* blah blah */
return ret;
error:
goto cleanup;
I do really want an explicit error: label, but also understand why you
don't want a successful exit to go past a label named "error".
respin coming up.
5:17 p.m.
New subject: [libvirt] [PATCHv3 4/5] network: change cleanup: to success/cleanup/error: in network*() functions
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
5:30 p.m.
New subject: [libvirt] [PATCHv3 4/5] network: change cleanup: to success/cleanup/error: in network*() functions
On 08/14/2012 04:17 PM, 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 a new success: label is added
s/the existing//
(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:
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;
Yep, that style is fine with me. An alternative style (but no need to
respin now) would be:
success:
/* anything done only on success */
ret = 0;
cleanup:
if (ret < 0) {
/* anything only done on error */
}
return ret;
but while it is fewer labels and no backwards goto, it loses out on the
nice 'goto error' naming, so you are just trading one style for another
with no real benefit.
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(-)
ACK as-is.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
10:26 a.m.
New subject: [libvirt] [PATCHv2 4/5] network: change cleanup: to error: in network*() functions
Looks good.
Acked-by: Kyle Mestery <kmestery(a)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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
2:10 a.m.
New subject: [libvirt] [PATCHv2 5/5] network: add connections counter to networks
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
10:27 a.m.
New subject: [libvirt] [PATCHv2 5/5] network: add connections counter to networks
Looks good to me.
Acked-by: Kyle Mestery <kmestery(a)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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
10:55 a.m.
New subject: [libvirt] [PATCHv2 5/5] network: add connections counter to networks
On 08/14/2012 01: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(-)
Still, I can't help but wonder if someone will try to feed the live
dumpxml output back into libvirt to define a new network config, and
trigger an rng failure as a result - you need to touch
docs/schemas/network.rng. I also think this is missing documentation -
it may be an output-only parameter, but is worth mentioning in
docs/formatnetwork.html.in.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
12:02 p.m.
New subject: [libvirt] [PATCHv2 5/5] network: add connections counter to networks
On 08/14/2012 11:55 AM, Eric Blake wrote:
On 08/14/2012 01: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(-)
Still, I can't help but wonder if someone will try to feed the live
dumpxml output back into libvirt to define a new network config, and
trigger an rng failure as a result - you need to touch
docs/schemas/network.rng. I also think this is missing documentation -
it may be an output-only parameter, but is worth mentioning in
docs/formatnetwork.html.in.
Correct on both counts. You've mostly trained me well wrt rng and docs,
but sometimes I forget :-)
I'll resubmit this last patch, but push them all at once.
5:21 p.m.
New subject: [libvirt] [PATCHv3 5/5] network: add connections counter to networks
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.
---
Changes from v2:
Eric pointed out that even though connections isn't configurable, it
should still be documented, and might show up in someone's XML that
they input to the API. So in this revision, I added connections to the
rng for both <network> and to the <forward> element's <interface>
subelement, and also added a short bit describing it in the
appropriate places in formatnetwork.html.
docs/formatnetwork.html.in | 32 ++++++++++++++++++++++++--------
docs/schemas/network.rng | 10 ++++++++++
src/conf/network_conf.c | 6 +++++-
src/conf/network_conf.h | 1 +
src/network/bridge_driver.c | 10 ++++++++++
5 files changed, 50 insertions(+), 9 deletions(-)
diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
index daa6524..a010cbd 100644
--- a/docs/formatnetwork.html.in
+++ b/docs/formatnetwork.html.in
@@ -15,8 +15,14 @@
<p>
The root element required for all virtual networks is
- named <code>network</code> and has no attributes.
- The network XML format is available <span class="since">since
0.3.0</span>
+ named <code>network</code> and has no configurable attributes
+ (although <span class="since">since 0.10.0</span> there is
one
+ optional read-only attribute - when examining the live
+ configuration of a network, the
+ attribute <code>connections</code>, if present, specifies the
+ number of guest interfaces currently connected via this
+ network). The network XML format is
+ available <span class="since">since 0.3.0</span>
</p>
<h3><a name="elementsMetadata">General
metadata</a></h3>
@@ -233,12 +239,22 @@
</forward>
...
</pre>
- Additionally, <span class="since">since 0.9.10</span>,
libvirt
- allows a shorthand for specifying all virtual interfaces
- associated with a single physical function, by using
- the <code><pf></code> subelement to call out the
- corresponding physical interface associated with multiple
- virtual interfaces:
+ <p>
+ <span class="since">since 0.10.0</span>,
+ <code><interface></code> also has an optional
read-only
+ attribute - when examining the live configuration of a
+ network, the attribute <code>connections</code>, if present,
+ specifies the number of guest interfaces currently connected
+ via this physical interface.
+ </p>
+ <p>
+ Additionally, <span class="since">since 0.9.10</span>,
libvirt
+ allows a shorthand for specifying all virtual interfaces
+ associated with a single physical function, by using
+ the <code><pf></code> subelement to call out the
+ corresponding physical interface associated with multiple
+ virtual interfaces:
+ </p>
<pre>
...
<forward mode='passthrough'>
diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng
index 2ae879e..25df266 100644
--- a/docs/schemas/network.rng
+++ b/docs/schemas/network.rng
@@ -12,6 +12,11 @@
<define name="network">
<element name="network">
+ <optional>
+ <attribute name="connections">
+ <data type="unsignedInt"/>
+ </attribute>
+ </optional>
<interleave>
<!-- The name of the network, used to refer to it through the API
@@ -91,6 +96,11 @@
<attribute name='dev'>
<ref name='deviceName'/>
</attribute>
+ <optional>
+ <attribute name="connections">
+ <data type="unsignedInt"/>
+ </attribute>
+ </optional>
</element>
</zeroOrMore>
<optional>
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 c86a157..efc79c1 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;
cleanup:
for (ii = 0; ii < num_virt_fns; ii++)
@@ -3117,6 +3121,9 @@ networkNotifyActualDevice(virDomainNetDefPtr iface)
}
success:
+ netdef->connections++;
+ VIR_DEBUG("Using network %s, %d connections",
+ netdef->name, netdef->connections);
ret = 0;
cleanup:
if (network)
@@ -3205,6 +3212,9 @@ networkReleaseActualDevice(virDomainNetDefPtr iface)
}
success:
+ netdef->connections--;
+ VIR_DEBUG("Releasing network %s, %d connections",
+ netdef->name, netdef->connections);
ret = 0;
cleanup:
if (network)
--
1.7.11.2
5:32 p.m.
New subject: [libvirt] [PATCHv3 5/5] network: add connections counter to networks
On 08/14/2012 04:21 PM, 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.
ACK.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
11 p.m.
After another spin of a couple of the patches, everything was ACKed, so
it its now pushed.
Thanks to both Kyle and Eric for reviewing.
4518
days inactive
4519
days old
22 comments
3 participants
participants (3)
-
Eric Blake -
Kyle Mestery (kmestery) -
Laine Stump