[libvirt] [PATCH 0/3] Prevent removing a in-used static bridge and destroying a in-used virtual network
8:08 a.m.
* Get the live state info of a virtual network through netcf in networkGetXMLDesc.
* Add --system flag for net-dumpxml to show the live state info.
* Check the live state info in net-destroy.
* Add --force flag for net-destroy to forcibly destroy the virtual network.
* Check the transient interfaces info in iface-unbridge.
---
Lin Ma (3):
bridge_driver: Return the live state info of a given virtual network
virsh: prevent destroying a in-used network for net-destroy
virsh: prevent removing a in-used bridge for iface-unbridge
include/libvirt/libvirt-network.h | 1 +
src/Makefile.am | 3 +
src/network/bridge_driver.c | 141 ++++++++++++++++++++++++++++++++++-
src/network/bridge_driver_platform.h | 7 ++
tests/Makefile.am | 4 +
tools/virsh-interface.c | 25 ++++++-
tools/virsh-network.c | 62 ++++++++++++++-
tools/virsh.pod | 8 +-
8 files changed, 241 insertions(+), 10 deletions(-)
--
1.8.4
8:08 a.m.
New subject: [libvirt] [PATCH 1/3] bridge_driver: Return the live state info of a given virtual network
It constructs a temporary static config of the network, Obtains all of
attached interfaces information through netcf, Then removes the config.
Signed-off-by: Lin Ma <lma(a)suse.com>
---
include/libvirt/libvirt-network.h | 1 +
src/Makefile.am | 3 +
src/network/bridge_driver.c | 141 ++++++++++++++++++++++++++++++++++-
src/network/bridge_driver_platform.h | 7 ++
tests/Makefile.am | 4 +
5 files changed, 155 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt-network.h b/include/libvirt/libvirt-network.h
index 308f27f..9b09546 100644
--- a/include/libvirt/libvirt-network.h
+++ b/include/libvirt/libvirt-network.h
@@ -30,6 +30,7 @@
typedef enum {
VIR_NETWORK_XML_INACTIVE = (1 << 0), /* dump inactive network information */
+ VIR_NETWORK_XML_IFACE_ATTACHED = (1 << 1), /* dump current live state */
} virNetworkXMLFlags;
/**
diff --git a/src/Makefile.am b/src/Makefile.am
index b41c6d4..d22ae7e 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1412,6 +1412,9 @@ if WITH_NETWORK
noinst_LTLIBRARIES += libvirt_driver_network_impl.la
libvirt_driver_network_la_SOURCES =
libvirt_driver_network_la_LIBADD = libvirt_driver_network_impl.la
+if WITH_NETCF
+libvirt_driver_network_la_LIBADD += $(NETCF_LIBS)
+endif WITH_NETCF
if WITH_DRIVER_MODULES
mod_LTLIBRARIES += libvirt_driver_network.la
libvirt_driver_network_la_LIBADD += ../gnulib/lib/libgnu.la \
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index c56e8f2..1e49e2e 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -3333,8 +3333,17 @@ static char *networkGetXMLDesc(virNetworkPtr net,
virNetworkObjPtr network;
virNetworkDefPtr def;
char *ret = NULL;
+#ifdef WITH_NETCF
+ struct netcf_if *iface = NULL;
+ char *bridge = NULL;
+ char *if_xml_tmp = NULL;
+ xmlDocPtr xml = NULL;
+ xmlXPathContextPtr ctxt = NULL;
+ xmlXPathObjectPtr obj = NULL;
+#endif
- virCheckFlags(VIR_NETWORK_XML_INACTIVE, NULL);
+ virCheckFlags(VIR_NETWORK_XML_INACTIVE |
+ VIR_NETWORK_XML_IFACE_ATTACHED, NULL);
if (!(network = networkObjFromNetwork(net)))
return ret;
@@ -3342,6 +3351,135 @@ static char *networkGetXMLDesc(virNetworkPtr net,
if (virNetworkGetXMLDescEnsureACL(net->conn, network->def) < 0)
goto cleanup;
+#ifdef WITH_NETCF
+ if ((flags & VIR_NETWORK_XML_INACTIVE) && network->newDef) {
+ def = network->newDef;
+ ret = virNetworkDefFormat(def, flags);
+ }
+ else if (flags & VIR_NETWORK_XML_IFACE_ATTACHED) {
+ if (!(network->def->bridge)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("network '%s' does not have a bridge
name."),
+ network->def->name);
+ goto cleanup;
+ }
+ ignore_value(VIR_STRDUP(bridge, network->def->bridge));
+
+ if (virAsprintf(&if_xml_tmp,
+ "<interface type='bridge'
name='%s'>"
+ "<start
mode='none'/><bridge/></interface>",
+ bridge) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("failed to generate temp xml for network"));
+ goto cleanup;
+ }
+
+ if (ncf_init(&driver->netcf, NULL) != 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("failed to init netcf"));
+ goto cleanup;
+ }
+
+ // create a temp bridge configuration file
+ iface = ncf_define(driver->netcf, if_xml_tmp);
+ if (!iface) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("failed to define the temp bridge %s"), bridge);
+ ncf_close(driver->netcf);
+ goto cleanup;
+ }
+
+ ret = ncf_if_xml_state(iface);
+ if (!ret) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("could not get bridge XML description"));
+ ncf_if_free(iface);
+ ncf_close(driver->netcf);
+ goto cleanup;
+ }
+
+ // remove the temp bridge configuration file
+ if (ncf_if_undefine(iface) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("failed to undefine the temp bridge %s"),
bridge);
+ ncf_if_free(iface);
+ ncf_close(driver->netcf);
+ ret = NULL;
+ goto cleanup;
+ }
+ ncf_if_free(iface);
+ ncf_close(driver->netcf);
+
+ // remove the dummp tap interface section from the result
+ if (network->def->mac_specified) {
+ char *macTapIfName = networkBridgeDummyNicName(network->def->bridge);
+ if (macTapIfName) {
+ char mac[VIR_MAC_STRING_BUFLEN];
+ xmlNodePtr cur = NULL, matchNode = NULL;
+ xmlChar *br_xml = NULL;
+ int br_xml_size;
+ char buf[64];
+ size_t i;
+ int diff_mac;
+ virMacAddrFormat(&network->def->mac, mac);
+ snprintf(buf, sizeof(buf),
"./bridge/interface[@name='%s']",
+ macTapIfName);
+ if (!(xml = virXMLParseStringCtxt(ret,
+ _("(bridge interface "
+ "definition)"),
&ctxt))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ ("Failed to parse network configuration"));
+ VIR_FREE(macTapIfName);
+ ret = NULL;
+ goto cleanup;
+ }
+ obj = xmlXPathEval(BAD_CAST buf, ctxt);
+ if (obj == NULL || obj->type != XPATH_NODESET ||
+ obj->nodesetval == NULL || obj->nodesetval->nodeNr == 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("No interface found whose name is %s"),
+ macTapIfName);
+ VIR_FREE(macTapIfName);
+ ret = NULL;
+ goto cleanup;
+ }
+ VIR_FREE(macTapIfName);
+ for (i = 0; i < obj->nodesetval->nodeNr; i++) {
+ cur = obj->nodesetval->nodeTab[i]->children;
+ while (cur != NULL) {
+ if (cur->type == XML_ELEMENT_NODE &&
+ xmlStrEqual(cur->name, BAD_CAST "mac")) {
+ char *tmp_mac = virXMLPropString(cur, "address");
+ diff_mac = virMacAddrCompare(tmp_mac, mac);
+ VIR_FREE(tmp_mac);
+ if (!diff_mac) {
+ matchNode = obj->nodesetval->nodeTab[i];
+ xmlUnlinkNode(matchNode);
+ break;
+ }
+ }
+ cur = cur->next;
+ }
+ }
+ xmlDocDumpMemory(xml, &br_xml, &br_xml_size);
+ ret = (char *)br_xml;
+ }
+ }
+ }
+ else {
+ def = network->def;
+ ret = virNetworkDefFormat(def, flags);
+ }
+
+ cleanup:
+ xmlXPathFreeObject(obj);
+ xmlXPathFreeContext(ctxt);
+ xmlFreeDoc(xml);
+ VIR_FREE(if_xml_tmp);
+ VIR_FREE(bridge);
+ if (network)
+ virNetworkObjUnlock(network);
+#else
if ((flags & VIR_NETWORK_XML_INACTIVE) && network->newDef)
def = network->newDef;
else
@@ -3352,6 +3490,7 @@ static char *networkGetXMLDesc(virNetworkPtr net,
cleanup:
if (network)
virNetworkObjUnlock(network);
+#endif
return ret;
}
diff --git a/src/network/bridge_driver_platform.h b/src/network/bridge_driver_platform.h
index 1e8264a..43ea1c3 100644
--- a/src/network/bridge_driver_platform.h
+++ b/src/network/bridge_driver_platform.h
@@ -24,6 +24,9 @@
#ifndef __VIR_BRIDGE_DRIVER_PLATFORM_H__
# define __VIR_BRIDGE_DRIVER_PLATFORM_H__
+#ifdef WITH_NETCF
+# include <netcf.h>
+#endif
# include "internal.h"
# include "virthread.h"
# include "virdnsmasq.h"
@@ -34,6 +37,10 @@
struct _virNetworkDriverState {
virMutex lock;
+#ifdef WITH_NETCF
+ struct netcf *netcf;
+#endif
+
virNetworkObjList networks;
char *networkConfigDir;
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 938270c..0662337 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -67,6 +67,10 @@ LDADDS = \
$(GNULIB_LIBS) \
../src/libvirt.la
+if WITH_NETCF
+LDADDS += $(NETCF_LIBS)
+endif WITH_NETCF
+
EXTRA_DIST = \
bhyvexml2argvdata \
bhyvexml2xmloutdata \
--
1.8.4
10:39 a.m.
New subject: [libvirt] [PATCH 1/3] bridge_driver: Return the live state info of a given virtual network
On 02.02.2015 15:08, Lin Ma wrote:
It constructs a temporary static config of the network, Obtains all
of
attached interfaces information through netcf, Then removes the config.
Signed-off-by: Lin Ma <lma(a)suse.com>
---
include/libvirt/libvirt-network.h | 1 +
src/Makefile.am | 3 +
src/network/bridge_driver.c | 141 ++++++++++++++++++++++++++++++++++-
src/network/bridge_driver_platform.h | 7 ++
tests/Makefile.am | 4 +
5 files changed, 155 insertions(+), 1 deletion(-)
We don't use TABs. If you ran 'make syntax-check' it would catch the errors.
diff --git a/include/libvirt/libvirt-network.h b/include/libvirt/libvirt-network.h
index 308f27f..9b09546 100644
--- a/include/libvirt/libvirt-network.h
+++ b/include/libvirt/libvirt-network.h
@@ -30,6 +30,7 @@
typedef enum {
VIR_NETWORK_XML_INACTIVE = (1 << 0), /* dump inactive network information */
+ VIR_NETWORK_XML_IFACE_ATTACHED = (1 << 1), /* dump current live state */
} virNetworkXMLFlags;
/**
diff --git a/src/Makefile.am b/src/Makefile.am
index b41c6d4..d22ae7e 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1412,6 +1412,9 @@ if WITH_NETWORK
noinst_LTLIBRARIES += libvirt_driver_network_impl.la
libvirt_driver_network_la_SOURCES =
libvirt_driver_network_la_LIBADD = libvirt_driver_network_impl.la
+if WITH_NETCF
+libvirt_driver_network_la_LIBADD += $(NETCF_LIBS)
+endif WITH_NETCF
if WITH_DRIVER_MODULES
mod_LTLIBRARIES += libvirt_driver_network.la
libvirt_driver_network_la_LIBADD += ../gnulib/lib/libgnu.la \
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index c56e8f2..1e49e2e 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -3333,8 +3333,17 @@ static char *networkGetXMLDesc(virNetworkPtr net,
virNetworkObjPtr network;
virNetworkDefPtr def;
char *ret = NULL;
+#ifdef WITH_NETCF
+ struct netcf_if *iface = NULL;
+ char *bridge = NULL;
+ char *if_xml_tmp = NULL;
+ xmlDocPtr xml = NULL;
+ xmlXPathContextPtr ctxt = NULL;
+ xmlXPathObjectPtr obj = NULL;
+#endif
- virCheckFlags(VIR_NETWORK_XML_INACTIVE, NULL);
+ virCheckFlags(VIR_NETWORK_XML_INACTIVE |
+ VIR_NETWORK_XML_IFACE_ATTACHED, NULL);
So even if libvirt is build without netcf the flag is still supported? I
don't think so. I mean, it's okay that it's here. But I'm missing the
check in '#else' branch.
if (!(network = networkObjFromNetwork(net)))
return ret;
@@ -3342,6 +3351,135 @@ static char *networkGetXMLDesc(virNetworkPtr net,
if (virNetworkGetXMLDescEnsureACL(net->conn, network->def) < 0)
goto cleanup;
+#ifdef WITH_NETCF
+ if ((flags & VIR_NETWORK_XML_INACTIVE) && network->newDef) {
+ def = network->newDef;
+ ret = virNetworkDefFormat(def, flags);
+ }
+ else if (flags & VIR_NETWORK_XML_IFACE_ATTACHED) {
+ if (!(network->def->bridge)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("network '%s' does not have a bridge
name."),
+ network->def->name);
+ goto cleanup;
+ }
+ ignore_value(VIR_STRDUP(bridge, network->def->bridge));
No. If strdup()-ing fails ...
+
+ if (virAsprintf(&if_xml_tmp,
+ "<interface type='bridge'
name='%s'>"
+ "<start
mode='none'/><bridge/></interface>",
+ bridge) < 0) {
.. this won't produce any meaningful XML.
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("failed to generate temp xml for network"));
Indentation's off.
+ goto cleanup;
+ }
+
+ if (ncf_init(&driver->netcf, NULL) != 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("failed to init netcf"));
+ goto cleanup;
+ }
+
+ // create a temp bridge configuration file
We tend to put comments into /* */
+ iface = ncf_define(driver->netcf, if_xml_tmp);
+ if (!iface) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("failed to define the temp bridge %s"), bridge);
+ ncf_close(driver->netcf);
+ goto cleanup;
+ }
+
+ ret = ncf_if_xml_state(iface);
+ if (!ret) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("could not get bridge XML description"));
+ ncf_if_free(iface);
+ ncf_close(driver->netcf);
+ goto cleanup;
+ }
+
+ // remove the temp bridge configuration file
+ if (ncf_if_undefine(iface) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("failed to undefine the temp bridge %s"),
bridge);
+ ncf_if_free(iface);
+ ncf_close(driver->netcf);
+ ret = NULL;
This is not the correct way to free @ret. You need to use VIR_FREE()
otherwise @ret is leaked.
+ goto cleanup;
+ }
+ ncf_if_free(iface);
+ ncf_close(driver->netcf);
+
+ // remove the dummp tap interface section from the result
+ if (network->def->mac_specified) {
+ char *macTapIfName = networkBridgeDummyNicName(network->def->bridge);
+ if (macTapIfName) {
+ char mac[VIR_MAC_STRING_BUFLEN];
+ xmlNodePtr cur = NULL, matchNode = NULL;
+ xmlChar *br_xml = NULL;
+ int br_xml_size;
+ char buf[64];
+ size_t i;
+ int diff_mac;
+ virMacAddrFormat(&network->def->mac, mac);
+ snprintf(buf, sizeof(buf),
"./bridge/interface[@name='%s']",
+ macTapIfName);
+ if (!(xml = virXMLParseStringCtxt(ret,
+ _("(bridge interface "
+ "definition)"),
&ctxt))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ ("Failed to parse network
configuration"));
+ VIR_FREE(macTapIfName);
+ ret = NULL;
+ goto cleanup;
+ }
+ obj = xmlXPathEval(BAD_CAST buf, ctxt);
+ if (obj == NULL || obj->type != XPATH_NODESET ||
+ obj->nodesetval == NULL || obj->nodesetval->nodeNr == 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("No interface found whose name is %s"),
+ macTapIfName);
+ VIR_FREE(macTapIfName);
+ ret = NULL;
+ goto cleanup;
+ }
You've meant virXPathNodeSet(), right?
+ VIR_FREE(macTapIfName);
+ for (i = 0; i < obj->nodesetval->nodeNr; i++) {
+ cur = obj->nodesetval->nodeTab[i]->children;
+ while (cur != NULL) {
+ if (cur->type == XML_ELEMENT_NODE &&
+ xmlStrEqual(cur->name, BAD_CAST "mac")) {
+ char *tmp_mac = virXMLPropString(cur, "address");
+ diff_mac = virMacAddrCompare(tmp_mac, mac);
+ VIR_FREE(tmp_mac);
+ if (!diff_mac) {
+ matchNode = obj->nodesetval->nodeTab[i];
+ xmlUnlinkNode(matchNode);
+ break;
+ }
+ }
+ cur = cur->next;
+ }
+ }
+ xmlDocDumpMemory(xml, &br_xml, &br_xml_size);
+ ret = (char *)br_xml;
Are you aware, that this will produce XML that is not acceptable by
libvirt back, right? E.g. for my 'default' network I get this:
# net-dumpxml --system default
<?xml version="1.0"?>
<interface name="virbr0" type="bridge">
<bridge>
</bridge>
<protocol family="ipv4">
<ip address="192.168.122.1" prefix="24"/>
</protocol>
</interface>
This is not a libvirt network XML.
+ }
+ }
+ }
+ else {
+ def = network->def;
+ ret = virNetworkDefFormat(def, flags);
+ }
+
+ cleanup:
This introduces yet another label of the very same name we already have.
I think these labels can be merged into one.
+ xmlXPathFreeObject(obj);
+ xmlXPathFreeContext(ctxt);
+ xmlFreeDoc(xml);
+ VIR_FREE(if_xml_tmp);
+ VIR_FREE(bridge);
+ if (network)
+ virNetworkObjUnlock(network);
+#else
if ((flags & VIR_NETWORK_XML_INACTIVE) && network->newDef)
def = network->newDef;
else
@@ -3352,6 +3490,7 @@ static char *networkGetXMLDesc(virNetworkPtr net,
cleanup:
if (network)
virNetworkObjUnlock(network);
+#endif
return ret;
}
diff --git a/src/network/bridge_driver_platform.h b/src/network/bridge_driver_platform.h
index 1e8264a..43ea1c3 100644
--- a/src/network/bridge_driver_platform.h
+++ b/src/network/bridge_driver_platform.h
@@ -24,6 +24,9 @@
#ifndef __VIR_BRIDGE_DRIVER_PLATFORM_H__
# define __VIR_BRIDGE_DRIVER_PLATFORM_H__
+#ifdef WITH_NETCF
+# include <netcf.h>
+#endif
# include "internal.h"
# include "virthread.h"
# include "virdnsmasq.h"
@@ -34,6 +37,10 @@
struct _virNetworkDriverState {
virMutex lock;
+#ifdef WITH_NETCF
+ struct netcf *netcf;
+#endif
+
So what's the good having netcf here, in the driver, if it's used
nowhere but dumpXML()? Moreover, if we init and close the netcf handle
on each function call?
virNetworkObjList networks;
char *networkConfigDir;
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 938270c..0662337 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -67,6 +67,10 @@ LDADDS = \
$(GNULIB_LIBS) \
../src/libvirt.la
+if WITH_NETCF
+LDADDS += $(NETCF_LIBS)
+endif WITH_NETCF
+
EXTRA_DIST = \
bhyvexml2argvdata \
bhyvexml2xmloutdata \
Michal
7:07 p.m.
New subject: [libvirt] [PATCH 1/3] bridge_driver: Return the live state info of a given virtual network
On 02/02/2015 09:08 AM, Lin Ma wrote:
It constructs a temporary static config of the network, Obtains all
of
attached interfaces information through netcf, Then removes the config.
Signed-off-by: Lin Ma <lma(a)suse.com>
---
include/libvirt/libvirt-network.h | 1 +
src/Makefile.am | 3 +
src/network/bridge_driver.c | 141 ++++++++++++++++++++++++++++++++++-
src/network/bridge_driver_platform.h | 7 ++
tests/Makefile.am | 4 +
5 files changed, 155 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt-network.h b/include/libvirt/libvirt-network.h
index 308f27f..9b09546 100644
--- a/include/libvirt/libvirt-network.h
+++ b/include/libvirt/libvirt-network.h
@@ -30,6 +30,7 @@
typedef enum {
VIR_NETWORK_XML_INACTIVE = (1 << 0), /* dump inactive network information */
+ VIR_NETWORK_XML_IFACE_ATTACHED = (1 << 1), /* dump current live state */
} virNetworkXMLFlags;
/**
diff --git a/src/Makefile.am b/src/Makefile.am
index b41c6d4..d22ae7e 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1412,6 +1412,9 @@ if WITH_NETWORK
noinst_LTLIBRARIES += libvirt_driver_network_impl.la
libvirt_driver_network_la_SOURCES =
libvirt_driver_network_la_LIBADD = libvirt_driver_network_impl.la
+if WITH_NETCF
+libvirt_driver_network_la_LIBADD += $(NETCF_LIBS)
+endif WITH_NETCF
if WITH_DRIVER_MODULES
mod_LTLIBRARIES += libvirt_driver_network.la
libvirt_driver_network_la_LIBADD += ../gnulib/lib/libgnu.la \
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index c56e8f2..1e49e2e 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -3333,8 +3333,17 @@ static char *networkGetXMLDesc(virNetworkPtr net,
virNetworkObjPtr network;
virNetworkDefPtr def;
char *ret = NULL;
+#ifdef WITH_NETCF
+ struct netcf_if *iface = NULL;
+ char *bridge = NULL;
+ char *if_xml_tmp = NULL;
+ xmlDocPtr xml = NULL;
+ xmlXPathContextPtr ctxt = NULL;
+ xmlXPathObjectPtr obj = NULL;
+#endif
- virCheckFlags(VIR_NETWORK_XML_INACTIVE, NULL);
+ virCheckFlags(VIR_NETWORK_XML_INACTIVE |
+ VIR_NETWORK_XML_IFACE_ATTACHED, NULL);
if (!(network = networkObjFromNetwork(net)))
return ret;
@@ -3342,6 +3351,135 @@ static char *networkGetXMLDesc(virNetworkPtr net,
if (virNetworkGetXMLDescEnsureACL(net->conn, network->def) < 0)
goto cleanup;
+#ifdef WITH_NETCF
+ if ((flags & VIR_NETWORK_XML_INACTIVE) && network->newDef) {
+ def = network->newDef;
+ ret = virNetworkDefFormat(def, flags);
+ }
+ else if (flags & VIR_NETWORK_XML_IFACE_ATTACHED) {
+ if (!(network->def->bridge)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("network '%s' does not have a bridge
name."),
+ network->def->name);
+ goto cleanup;
+ }
+ ignore_value(VIR_STRDUP(bridge, network->def->bridge));
+
+ if (virAsprintf(&if_xml_tmp,
+ "<interface type='bridge'
name='%s'>"
+ "<start
mode='none'/><bridge/></interface>",
+ bridge) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("failed to generate temp xml for network"));
+ goto cleanup;
+ }
+
+ if (ncf_init(&driver->netcf, NULL) != 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("failed to init netcf"));
+ goto cleanup;
+ }
+
+ // create a temp bridge configuration file
+ iface = ncf_define(driver->netcf, if_xml_tmp);
NACK. This is very much against what netcf (and the virInterface API)
was intended to do, and doing this is nearly *guaranteed* to completely
mess up NetworkManager, which monitors the ifcfg files that are created
by ncf_define.
This is one of the things that we regularly have to tell people that
they *shouldn't* do - the bridge devices created by libvirt must never
be referenced in a system config file; those are by definition
persistent (even though you're deleting it after a short time), and
immediately come in the radar of whatever host system daemon is managing
the persistent network device configs.
Also, I'm not sure I like having netcf called by the bridge driver.
Anyway, if all you want to do is check whether or not a network is in
use, an even more reasonable thing would be to simply look at the
connections attribute of the net (i.e. def->connections) and check that
it is 0 - every connection from a libvirt-managed guest is counted there
(and it's re-computed every time libvirtd is restarted), and anybody who
connects to a libvirt-created bridge without going through libvirt to do
it deserves what they get.
+ if (!iface) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("failed to define the temp bridge %s"), bridge);
+ ncf_close(driver->netcf);
+ goto cleanup;
+ }
+
+ ret = ncf_if_xml_state(iface);
+ if (!ret) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("could not get bridge XML description"));
+ ncf_if_free(iface);
+ ncf_close(driver->netcf);
+ goto cleanup;
+ }
+
+ // remove the temp bridge configuration file
+ if (ncf_if_undefine(iface) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("failed to undefine the temp bridge %s"),
bridge);
+ ncf_if_free(iface);
+ ncf_close(driver->netcf);
+ ret = NULL;
+ goto cleanup;
+ }
+ ncf_if_free(iface);
+ ncf_close(driver->netcf);
+
+ // remove the dummp tap interface section from the result
+ if (network->def->mac_specified) {
+ char *macTapIfName = networkBridgeDummyNicName(network->def->bridge);
+ if (macTapIfName) {
+ char mac[VIR_MAC_STRING_BUFLEN];
+ xmlNodePtr cur = NULL, matchNode = NULL;
+ xmlChar *br_xml = NULL;
+ int br_xml_size;
+ char buf[64];
+ size_t i;
+ int diff_mac;
+ virMacAddrFormat(&network->def->mac, mac);
+ snprintf(buf, sizeof(buf),
"./bridge/interface[@name='%s']",
+ macTapIfName);
+ if (!(xml = virXMLParseStringCtxt(ret,
+ _("(bridge interface "
+ "definition)"),
&ctxt))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ ("Failed to parse network
configuration"));
+ VIR_FREE(macTapIfName);
+ ret = NULL;
+ goto cleanup;
+ }
+ obj = xmlXPathEval(BAD_CAST buf, ctxt);
+ if (obj == NULL || obj->type != XPATH_NODESET ||
+ obj->nodesetval == NULL || obj->nodesetval->nodeNr == 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("No interface found whose name is %s"),
+ macTapIfName);
+ VIR_FREE(macTapIfName);
+ ret = NULL;
+ goto cleanup;
+ }
+ VIR_FREE(macTapIfName);
+ for (i = 0; i < obj->nodesetval->nodeNr; i++) {
+ cur = obj->nodesetval->nodeTab[i]->children;
+ while (cur != NULL) {
+ if (cur->type == XML_ELEMENT_NODE &&
+ xmlStrEqual(cur->name, BAD_CAST "mac")) {
+ char *tmp_mac = virXMLPropString(cur, "address");
+ diff_mac = virMacAddrCompare(tmp_mac, mac);
+ VIR_FREE(tmp_mac);
+ if (!diff_mac) {
+ matchNode = obj->nodesetval->nodeTab[i];
+ xmlUnlinkNode(matchNode);
+ break;
+ }
+ }
+ cur = cur->next;
+ }
+ }
+ xmlDocDumpMemory(xml, &br_xml, &br_xml_size);
+ ret = (char *)br_xml;
+ }
+ }
+ }
+ else {
+ def = network->def;
+ ret = virNetworkDefFormat(def, flags);
+ }
+
+ cleanup:
+ xmlXPathFreeObject(obj);
+ xmlXPathFreeContext(ctxt);
+ xmlFreeDoc(xml);
+ VIR_FREE(if_xml_tmp);
+ VIR_FREE(bridge);
+ if (network)
+ virNetworkObjUnlock(network);
+#else
if ((flags & VIR_NETWORK_XML_INACTIVE) && network->newDef)
def = network->newDef;
else
@@ -3352,6 +3490,7 @@ static char *networkGetXMLDesc(virNetworkPtr net,
cleanup:
if (network)
virNetworkObjUnlock(network);
+#endif
return ret;
}
diff --git a/src/network/bridge_driver_platform.h b/src/network/bridge_driver_platform.h
index 1e8264a..43ea1c3 100644
--- a/src/network/bridge_driver_platform.h
+++ b/src/network/bridge_driver_platform.h
@@ -24,6 +24,9 @@
#ifndef __VIR_BRIDGE_DRIVER_PLATFORM_H__
# define __VIR_BRIDGE_DRIVER_PLATFORM_H__
+#ifdef WITH_NETCF
+# include <netcf.h>
+#endif
# include "internal.h"
# include "virthread.h"
# include "virdnsmasq.h"
@@ -34,6 +37,10 @@
struct _virNetworkDriverState {
virMutex lock;
+#ifdef WITH_NETCF
+ struct netcf *netcf;
+#endif
+
virNetworkObjList networks;
char *networkConfigDir;
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 938270c..0662337 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -67,6 +67,10 @@ LDADDS = \
$(GNULIB_LIBS) \
../src/libvirt.la
+if WITH_NETCF
+LDADDS += $(NETCF_LIBS)
+endif WITH_NETCF
+
EXTRA_DIST = \
bhyvexml2argvdata \
bhyvexml2xmloutdata \
3:13 a.m.
New subject: [libvirt] [PATCH 1/3] bridge_driver: Return the live state info of a given virtual network
On Mon, Feb 02, 2015 at 10:08:21PM +0800, Lin Ma wrote:
It constructs a temporary static config of the network, Obtains all
of
attached interfaces information through netcf, Then removes the config.
Signed-off-by: Lin Ma <lma(a)suse.com>
---
include/libvirt/libvirt-network.h | 1 +
src/Makefile.am | 3 +
src/network/bridge_driver.c | 141 ++++++++++++++++++++++++++++++++++-
src/network/bridge_driver_platform.h | 7 ++
tests/Makefile.am | 4 +
5 files changed, 155 insertions(+), 1 deletion(-)
So, IIUC, the point of this change is to display a list of all the TAP
devices attached to a network. I'm not entirely convinced this is the
right approach to take. Also do we want to limit this to reporting of
devices inside bridges. It could be equally useful to see a list of
macvtap devices associated with a physical NIC.
Modifying the XML to include the list of VIFs is certainly one options,
but I'm not sure it is necceessarily the best. As Laine points out this
is a output only attribute - not something you can feed in with the XML
when defining the network.
Perhaps we want to have an actual API for this - a virNetworkListPorts
which returns a list of associated devices (of any type) from guests.
Or perhaps we should be reporting a list of virDomainPtr's that are
attached to the network, or perhaps even both.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
9:36 a.m.
New subject: [libvirt] 答复： Re: [PATCH 1/3] bridge_driver: Return the live state info of a given virtual network
>>> "Daniel P. Berrange"
<berrange(a)redhat.com> 2015-2-4 下午 17:17 >>>
On Mon, Feb 02, 2015 at 10:08:21PM +0800, Lin Ma wrote:
> It constructs a temporary static config of the network, Obtains all of
> attached interfaces information through netcf, Then removes the config.
>
> Signed-off-by: Lin Ma
> ---
> include/libvirt/libvirt-network.h | 1 +
> src/Makefile.am | 3 +
> src/network/bridge_driver.c | 141 ++++++++++++++++++++++++++++++++++-
> src/network/bridge_driver_platform.h | 7 ++
> tests/Makefile.am | 4 +
> 5 files changed, 155 insertions(+), 1 deletion(-)
So, IIUC, the point of this change is to display a list of all the TAP
devices attached to a network. I'm not entirely convinced this is the
right approach to take. Also do we want to limit this to reporting of
devices inside bridges. It could be equally useful to see a list of
macvtap devices associated with a physical NIC.
Obtaining a list of macvtap devices
associated with a physical NIC,
It's a good idea. I intend to implement this api, also implement a new
virsh subcommand to show these information to user.
If you agreed with my thought, Then what are the proper api name and
virsh subcommand name you suggest? and what about the output format?
Modifying the XML to include the list of VIFs is certainly one
options,
but I'm not sure it is necceessarily the best. As Laine points out this
is a output only attribute - not something you can feed in with the XML
when defining the network.
Perhaps we want to have an actual API for this - a virNetworkListPorts
which returns a list of associated devices (of any type) from guests.
Or perhaps we should be reporting a list of virDomainPtr's that are
attached to the network, or perhaps even both.
3:39 a.m.
New subject: [libvirt] 答复： Re: [PATCH 1/3] bridge_driver: Return the live state info of a given virtual network
On 05.02.2015 16:36, Lin Ma wrote:
>>>> "Daniel P. Berrange" <berrange(a)redhat.com> 2015-2-4 下午
17:17 >>>
>On Mon, Feb 02, 2015 at 10:08:21PM +0800, Lin Ma wrote:
>> It constructs a temporary static config of the network, Obtains all of
>> attached interfaces information through netcf, Then removes the config.
>>
>> Signed-off-by: Lin Ma
>> ---
>> include/libvirt/libvirt-network.h | 1 +
>> src/Makefile.am | 3 +
>> src/network/bridge_driver.c | 141 ++++++++++++++++++++++++++++++++++-
>> src/network/bridge_driver_platform.h | 7 ++
>> tests/Makefile.am | 4 +
>> 5 files changed, 155 insertions(+), 1 deletion(-)
>
>So, IIUC, the point of this change is to display a list of all the TAP
>devices attached to a network. I'm not entirely convinced this is the
>right approach to take. Also do we want to limit this to reporting of
>devices inside bridges. It could be equally useful to see a list of
>macvtap devices associated with a physical NIC.
Obtaining a list of macvtap devices associated with a physical NIC,
It's a good idea. I intend to implement this api, also implement a new
virsh subcommand to show these information to user.
If you agreed with my thought, Then what are the proper api name and
virsh subcommand name you suggest? and what about the output format?
Well, let see. We already have a way of expressing node devices (even
virtual net interfaces): virNode* set of API. For instance:
virsh # nodedev-dumpxml net_macvtap0_52_54_00_62_35_0f
<device>
<name>net_macvtap0_52_54_00_62_35_0f</name>
<path>/sys/devices/virtual/net/macvtap0</path>
<parent>computer</parent>
<capability type='net'>
<interface>macvtap0</interface>
<address>52:54:00:62:35:0f</address>
<link state='unknown'/>
<capability type='80203'/>
</capability>
</device>
I think it would be possible to extend macvtap XMLs with <parent/>
element to reflect NIC over which macvtap is created. Although we are
aiming at the opposite mapping. But then again - we don't have any
<child/> element in nodedev XML yet. When trying to build a nodedev
tree, users are required to enumerate nodes and look for <parent/> (or
use virNodeDeviceGetParent()).
Michal
3:44 a.m.
New subject: [libvirt] 答复： Re: [PATCH 1/3] bridge_driver: Return the live state info of a given virtual network
On Mon, Feb 09, 2015 at 10:39:35AM +0100, Michal Privoznik wrote:
On 05.02.2015 16:36, Lin Ma wrote:
>
>>>>> "Daniel P. Berrange" <berrange(a)redhat.com> 2015-2-4
下午 17:17 >>>
>>On Mon, Feb 02, 2015 at 10:08:21PM +0800, Lin Ma wrote:
>>> It constructs a temporary static config of the network, Obtains all of
>>> attached interfaces information through netcf, Then removes the config.
>>>
>>> Signed-off-by: Lin Ma
>>> ---
>>> include/libvirt/libvirt-network.h | 1 +
>>> src/Makefile.am | 3 +
>>> src/network/bridge_driver.c | 141 ++++++++++++++++++++++++++++++++++-
>>> src/network/bridge_driver_platform.h | 7 ++
>>> tests/Makefile.am | 4 +
>>> 5 files changed, 155 insertions(+), 1 deletion(-)
>>
>>So, IIUC, the point of this change is to display a list of all the TAP
>>devices attached to a network. I'm not entirely convinced this is the
>>right approach to take. Also do we want to limit this to reporting of
>>devices inside bridges. It could be equally useful to see a list of
>>macvtap devices associated with a physical NIC.
> Obtaining a list of macvtap devices associated with a physical NIC,
> It's a good idea. I intend to implement this api, also implement a new
> virsh subcommand to show these information to user.
> If you agreed with my thought, Then what are the proper api name and
> virsh subcommand name you suggest? and what about the output format?
Well, let see. We already have a way of expressing node devices (even
virtual net interfaces): virNode* set of API. For instance:
virsh # nodedev-dumpxml net_macvtap0_52_54_00_62_35_0f
<device>
<name>net_macvtap0_52_54_00_62_35_0f</name>
<path>/sys/devices/virtual/net/macvtap0</path>
<parent>computer</parent>
<capability type='net'>
<interface>macvtap0</interface>
<address>52:54:00:62:35:0f</address>
<link state='unknown'/>
<capability type='80203'/>
</capability>
</device>
I think it would be possible to extend macvtap XMLs with <parent/>
element to reflect NIC over which macvtap is created. Although we are
aiming at the opposite mapping. But then again - we don't have any
<child/> element in nodedev XML yet. When trying to build a nodedev
tree, users are required to enumerate nodes and look for <parent/> (or
use virNodeDeviceGetParent()).
Based on discussions elsewhere in the thread, I'm not sure there is a
compelling reason for any API additions in this respect anymore. We
are going to put all the logic directly in the virNetworkDestroyFlags
method impl now. This addition about enumerating macvtap devices was
only needed when virsh was trying todo the logic
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
8:08 a.m.
New subject: [libvirt] [PATCH 2/3] virsh: prevent destroying a in-used network for net-destroy
* add --system flag for net-dumpxml to show information about the
attached interfaces of the virtual network.
* call virNetworkGetXMLDesc in cmdNetworkDestroy to get the live state
info to check whether the virtual network is in use.
* add --force flag for net-destroy to forcibly destroy the virtual
network even if it's in use.
Signed-off-by: Lin Ma <lma(a)suse.com>
---
tools/virsh-network.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++----
tools/virsh.pod | 8 +++++--
2 files changed, 64 insertions(+), 6 deletions(-)
diff --git a/tools/virsh-network.c b/tools/virsh-network.c
index 5f8743c..17a5710 100644
--- a/tools/virsh-network.c
+++ b/tools/virsh-network.c
@@ -253,6 +253,10 @@ static const vshCmdOptDef opts_network_destroy[] = {
.flags = VSH_OFLAG_REQ,
.help = N_("network name or uuid")
},
+ {.name = "force",
+ .type = VSH_OT_BOOL,
+ .help = N_("Forcefully stop a given network")
+ },
{.name = NULL}
};
@@ -260,20 +264,55 @@ static bool
cmdNetworkDestroy(vshControl *ctl, const vshCmd *cmd)
{
virNetworkPtr network;
- bool ret = true;
+ bool ret = false;
const char *name;
+ char *br_xml = NULL;
+ xmlDocPtr xml_doc = NULL;
+ xmlXPathContextPtr ctxt = NULL;
if (!(network = vshCommandOptNetwork(ctl, cmd, &name)))
- return false;
+ goto cleanup;
+
+#ifdef WITH_NETCF
+ if (!vshCommandOptBool(cmd, "force")) {
+ if (virNetworkIsActive(network) <= 0) {
+ vshError(ctl, "%s", _("network is not active"));
+ goto cleanup;
+ }
+ if (!(br_xml = virNetworkGetXMLDesc(network,
+ VIR_NETWORK_XML_IFACE_ATTACHED)))
+ goto cleanup;
+
+ if (!(xml_doc = virXMLParseStringCtxt(br_xml,
+ _("(bridge interface "
+ "definition)"), &ctxt))) {
+ vshError(ctl, "%s", _("Failed to parse network
configuration"));
+ goto cleanup;
+ }
+
+ if (virXPathNode("./bridge/interface[1]", ctxt) != NULL) {
+ vshError(ctl, "%s", _("The network is in use by
guests"));
+ vshPrint(ctl, "%s", _("Use --force flag if you do want to do
so"));
+ goto cleanup;
+ }
+ }
+#endif
if (virNetworkDestroy(network) == 0) {
vshPrint(ctl, _("Network %s destroyed\n"), name);
} else {
vshError(ctl, _("Failed to destroy network %s"), name);
- ret = false;
+ goto cleanup;
}
- virNetworkFree(network);
+ ret = true;
+ cleanup:
+ if(network)
+ virNetworkFree(network);
+ VIR_FREE(br_xml);
+ xmlXPathFreeContext(ctxt);
+ xmlFreeDoc(xml_doc);
+
return ret;
}
@@ -300,6 +339,10 @@ static const vshCmdOptDef opts_network_dumpxml[] = {
.type = VSH_OT_BOOL,
.help = N_("network information of an inactive domain")
},
+ {.name = "system",
+ .type = VSH_OT_BOOL,
+ .help = N_("current live state information including attached
interfaces")
+ },
{.name = NULL}
};
@@ -311,6 +354,7 @@ cmdNetworkDumpXML(vshControl *ctl, const vshCmd *cmd)
char *dump;
unsigned int flags = 0;
int inactive;
+ bool system = false;
if (!(network = vshCommandOptNetwork(ctl, cmd, NULL)))
return false;
@@ -319,6 +363,16 @@ cmdNetworkDumpXML(vshControl *ctl, const vshCmd *cmd)
if (inactive)
flags |= VIR_NETWORK_XML_INACTIVE;
+ system = vshCommandOptBool(cmd, "system");
+ if (system) {
+ flags = VIR_NETWORK_XML_IFACE_ATTACHED;
+ if (virNetworkIsActive(network) <= 0) {
+ vshError(ctl, "%s", _("--system only works on active
network"));
+ virNetworkFree(network);
+ return false;
+ }
+ }
+
dump = virNetworkGetXMLDesc(network, flags);
if (dump != NULL) {
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 804458e..3373ada 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -2672,16 +2672,20 @@ to get a description of the XML network format used by libvirt.
Define a persistent virtual network from an XML I<file>, the network is just
defined but not instantiated (started).
-=item B<net-destroy> I<network>
+=item B<net-destroy> I<network> [I<--force>]
Destroy (stop) a given transient or persistent virtual network
specified by its name or UUID. This takes effect immediately.
+If I<--force> is specified, then forcefully destroy the virtual
+network even if it's in use.
-=item B<net-dumpxml> I<network> [I<--inactive>]
+=item B<net-dumpxml> I<network> [I<--inactive>] [I<--system>]
Output the virtual network information as an XML dump to stdout.
If I<--inactive> is specified, then physical functions are not
expanded into their associated virtual functions.
+If I<--system> is specified, then directly output the current
+live state corresponding to this network from system.
=item B<net-edit> I<network>
--
1.8.4
10:39 a.m.
New subject: [libvirt] [PATCH 2/3] virsh: prevent destroying a in-used network for net-destroy
On 02.02.2015 15:08, Lin Ma wrote:
* add --system flag for net-dumpxml to show information about the
attached interfaces of the virtual network.
* call virNetworkGetXMLDesc in cmdNetworkDestroy to get the live state
info to check whether the virtual network is in use.
* add --force flag for net-destroy to forcibly destroy the virtual
network even if it's in use.
Signed-off-by: Lin Ma <lma(a)suse.com>
---
tools/virsh-network.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++----
tools/virsh.pod | 8 +++++--
2 files changed, 64 insertions(+), 6 deletions(-)
diff --git a/tools/virsh-network.c b/tools/virsh-network.c
index 5f8743c..17a5710 100644
--- a/tools/virsh-network.c
+++ b/tools/virsh-network.c
@@ -253,6 +253,10 @@ static const vshCmdOptDef opts_network_destroy[] = {
.flags = VSH_OFLAG_REQ,
.help = N_("network name or uuid")
},
+ {.name = "force",
+ .type = VSH_OT_BOOL,
+ .help = N_("Forcefully stop a given network")
+ },
{.name = NULL}
};
@@ -260,20 +264,55 @@ static bool
cmdNetworkDestroy(vshControl *ctl, const vshCmd *cmd)
{
virNetworkPtr network;
- bool ret = true;
+ bool ret = false;
const char *name;
+ char *br_xml = NULL;
+ xmlDocPtr xml_doc = NULL;
+ xmlXPathContextPtr ctxt = NULL;
if (!(network = vshCommandOptNetwork(ctl, cmd, &name)))
- return false;
+ goto cleanup;
+
+#ifdef WITH_NETCF
No. Even if we (the client) were built without netcf support, we may be
talking to a daemon built with the support and therefore we may want
--force.
+ if (!vshCommandOptBool(cmd, "force")) {
+ if (virNetworkIsActive(network) <= 0) {
+ vshError(ctl, "%s", _("network is not active"));
+ goto cleanup;
+ }
+ if (!(br_xml = virNetworkGetXMLDesc(network,
+ VIR_NETWORK_XML_IFACE_ATTACHED)))
+ goto cleanup;
+
+ if (!(xml_doc = virXMLParseStringCtxt(br_xml,
+ _("(bridge interface "
+ "definition)"), &ctxt)))
{
+ vshError(ctl, "%s", _("Failed to parse network
configuration"));
+ goto cleanup;
+ }
+
+ if (virXPathNode("./bridge/interface[1]", ctxt) != NULL) {
+ vshError(ctl, "%s", _("The network is in use by
guests"));
+ vshPrint(ctl, "%s", _("Use --force flag if you do want to do
so"));
+ goto cleanup;
I must say I'm not a big fan of this. It's a change in behaviour, while
previously users were able to destroy a running network (and cut off
domains), it won't be possible using the same command anymore.
On the other hand, preventing them from shooting in their own foot is
nice thing to do.
+ }
+ }
+#endif
if (virNetworkDestroy(network) == 0) {
vshPrint(ctl, _("Network %s destroyed\n"), name);
} else {
vshError(ctl, _("Failed to destroy network %s"), name);
- ret = false;
+ goto cleanup;
}
- virNetworkFree(network);
+ ret = true;
+ cleanup:
+ if(network)
+ virNetworkFree(network);
+ VIR_FREE(br_xml);
+ xmlXPathFreeContext(ctxt);
+ xmlFreeDoc(xml_doc);
+
return ret;
}
@@ -300,6 +339,10 @@ static const vshCmdOptDef opts_network_dumpxml[] = {
.type = VSH_OT_BOOL,
.help = N_("network information of an inactive domain")
},
+ {.name = "system",
+ .type = VSH_OT_BOOL,
+ .help = N_("current live state information including attached
interfaces")
+ },
{.name = NULL}
};
@@ -311,6 +354,7 @@ cmdNetworkDumpXML(vshControl *ctl, const vshCmd *cmd)
char *dump;
unsigned int flags = 0;
int inactive;
+ bool system = false;
if (!(network = vshCommandOptNetwork(ctl, cmd, NULL)))
return false;
@@ -319,6 +363,16 @@ cmdNetworkDumpXML(vshControl *ctl, const vshCmd *cmd)
if (inactive)
flags |= VIR_NETWORK_XML_INACTIVE;
+ system = vshCommandOptBool(cmd, "system");
+ if (system) {
+ flags = VIR_NETWORK_XML_IFACE_ATTACHED;
+ if (virNetworkIsActive(network) <= 0) {
+ vshError(ctl, "%s", _("--system only works on active
network"));
+ virNetworkFree(network);
+ return false;
+ }
+ }
+
dump = virNetworkGetXMLDesc(network, flags);
if (dump != NULL) {
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 804458e..3373ada 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -2672,16 +2672,20 @@ to get a description of the XML network format used by libvirt.
Define a persistent virtual network from an XML I<file>, the network is just
defined but not instantiated (started).
-=item B<net-destroy> I<network>
+=item B<net-destroy> I<network> [I<--force>]
Destroy (stop) a given transient or persistent virtual network
specified by its name or UUID. This takes effect immediately.
+If I<--force> is specified, then forcefully destroy the virtual
+network even if it's in use.
-=item B<net-dumpxml> I<network> [I<--inactive>]
+=item B<net-dumpxml> I<network> [I<--inactive>] [I<--system>]
Output the virtual network information as an XML dump to stdout.
If I<--inactive> is specified, then physical functions are not
expanded into their associated virtual functions.
+If I<--system> is specified, then directly output the current
+live state corresponding to this network from system.
=item B<net-edit> I<network>
Documenting new command arguments, that's nice.
Michal
7:21 p.m.
New subject: [libvirt] [PATCH 2/3] virsh: prevent destroying a in-used network for net-destroy
On 02/02/2015 09:08 AM, Lin Ma wrote:
* add --system flag for net-dumpxml to show information about the
attached interfaces of the virtual network.
I don't like this extra flag - I think it is unnecessary. If we're going
to more info to the status, then we can just always add it to the status.
As to how it can be provided in the status XML (getting back to patch
1/3 which was using the netcf API in a way that I didn't like), better
data can be gathered into a list in the network object as guests are
created - this would be done in networkAllocateActualDevice() (and
networkNotifyActualDevice()) at the points where connections is
incremented for the network, and removed from the list in
networkReleaseActualDevice() where connections is decremented. You'll
notice that those functions get both the virDomainDefPtr and the
virDomainNetDefPtr, so they have enough information to store the name of
the domain as well as the namne of the interface.
I recall a long time ago considering adding this information to the
network XML, but I was dissuaded for some reason; unfortunately I don't
remember why...
* call virNetworkGetXMLDesc in cmdNetworkDestroy to get the live state
info to check whether the virtual network is in use.
* add --force flag for net-destroy to forcibly destroy the virtual
network even if it's in use.
As Michal points out (and I pointed out when you sent your previous
patchset), this does change behavior that some scripts may be dependent
on. I too like the increased safety, but am concerned about breaking
functionality. In cases like this, I always like to hear the learned
advice of Dan Berrange - Daniel, what do you say about changing the
default behavior of virsh net-destroy? Safe enough?
Signed-off-by: Lin Ma <lma(a)suse.com>
---
tools/virsh-network.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++----
tools/virsh.pod | 8 +++++--
2 files changed, 64 insertions(+), 6 deletions(-)
diff --git a/tools/virsh-network.c b/tools/virsh-network.c
index 5f8743c..17a5710 100644
--- a/tools/virsh-network.c
+++ b/tools/virsh-network.c
@@ -253,6 +253,10 @@ static const vshCmdOptDef opts_network_destroy[] = {
.flags = VSH_OFLAG_REQ,
.help = N_("network name or uuid")
},
+ {.name = "force",
+ .type = VSH_OT_BOOL,
+ .help = N_("Forcefully stop a given network")
+ },
{.name = NULL}
};
@@ -260,20 +264,55 @@ static bool
cmdNetworkDestroy(vshControl *ctl, const vshCmd *cmd)
{
virNetworkPtr network;
- bool ret = true;
+ bool ret = false;
const char *name;
+ char *br_xml = NULL;
+ xmlDocPtr xml_doc = NULL;
+ xmlXPathContextPtr ctxt = NULL;
if (!(network = vshCommandOptNetwork(ctl, cmd, &name)))
- return false;
+ goto cleanup;
+
+#ifdef WITH_NETCF
+ if (!vshCommandOptBool(cmd, "force")) {
+ if (virNetworkIsActive(network) <= 0) {
+ vshError(ctl, "%s", _("network is not active"));
+ goto cleanup;
+ }
+ if (!(br_xml = virNetworkGetXMLDesc(network,
+ VIR_NETWORK_XML_IFACE_ATTACHED)))
+ goto cleanup;
+
+ if (!(xml_doc = virXMLParseStringCtxt(br_xml,
+ _("(bridge interface "
+ "definition)"), &ctxt)))
{
+ vshError(ctl, "%s", _("Failed to parse network
configuration"));
+ goto cleanup;
+ }
+
+ if (virXPathNode("./bridge/interface[1]", ctxt) != NULL) {
+ vshError(ctl, "%s", _("The network is in use by
guests"));
+ vshPrint(ctl, "%s", _("Use --force flag if you do want to do
so"));
+ goto cleanup;
+ }
+ }
+#endif
if (virNetworkDestroy(network) == 0) {
vshPrint(ctl, _("Network %s destroyed\n"), name);
} else {
vshError(ctl, _("Failed to destroy network %s"), name);
- ret = false;
+ goto cleanup;
}
- virNetworkFree(network);
+ ret = true;
+ cleanup:
+ if(network)
+ virNetworkFree(network);
+ VIR_FREE(br_xml);
+ xmlXPathFreeContext(ctxt);
+ xmlFreeDoc(xml_doc);
+
return ret;
}
@@ -300,6 +339,10 @@ static const vshCmdOptDef opts_network_dumpxml[] = {
.type = VSH_OT_BOOL,
.help = N_("network information of an inactive domain")
},
+ {.name = "system",
+ .type = VSH_OT_BOOL,
+ .help = N_("current live state information including attached
interfaces")
+ },
{.name = NULL}
};
@@ -311,6 +354,7 @@ cmdNetworkDumpXML(vshControl *ctl, const vshCmd *cmd)
char *dump;
unsigned int flags = 0;
int inactive;
+ bool system = false;
if (!(network = vshCommandOptNetwork(ctl, cmd, NULL)))
return false;
@@ -319,6 +363,16 @@ cmdNetworkDumpXML(vshControl *ctl, const vshCmd *cmd)
if (inactive)
flags |= VIR_NETWORK_XML_INACTIVE;
+ system = vshCommandOptBool(cmd, "system");
+ if (system) {
+ flags = VIR_NETWORK_XML_IFACE_ATTACHED;
+ if (virNetworkIsActive(network) <= 0) {
+ vshError(ctl, "%s", _("--system only works on active
network"));
+ virNetworkFree(network);
+ return false;
+ }
+ }
+
dump = virNetworkGetXMLDesc(network, flags);
if (dump != NULL) {
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 804458e..3373ada 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -2672,16 +2672,20 @@ to get a description of the XML network format used by libvirt.
Define a persistent virtual network from an XML I<file>, the network is just
defined but not instantiated (started).
-=item B<net-destroy> I<network>
+=item B<net-destroy> I<network> [I<--force>]
Destroy (stop) a given transient or persistent virtual network
specified by its name or UUID. This takes effect immediately.
+If I<--force> is specified, then forcefully destroy the virtual
+network even if it's in use.
-=item B<net-dumpxml> I<network> [I<--inactive>]
+=item B<net-dumpxml> I<network> [I<--inactive>] [I<--system>]
Output the virtual network information as an XML dump to stdout.
If I<--inactive> is specified, then physical functions are not
expanded into their associated virtual functions.
+If I<--system> is specified, then directly output the current
+live state corresponding to this network from system.
=item B<net-edit> I<network>
3:07 a.m.
New subject: [libvirt] [PATCH 2/3] virsh: prevent destroying a in-used network for net-destroy
On Tue, Feb 03, 2015 at 08:21:42PM -0500, Laine Stump wrote:
On 02/02/2015 09:08 AM, Lin Ma wrote:
> * add --system flag for net-dumpxml to show information about the
> attached interfaces of the virtual network.
I don't like this extra flag - I think it is unnecessary. If we're going
to more info to the status, then we can just always add it to the status.
As to how it can be provided in the status XML (getting back to patch
1/3 which was using the netcf API in a way that I didn't like), better
data can be gathered into a list in the network object as guests are
created - this would be done in networkAllocateActualDevice() (and
networkNotifyActualDevice()) at the points where connections is
incremented for the network, and removed from the list in
networkReleaseActualDevice() where connections is decremented. You'll
notice that those functions get both the virDomainDefPtr and the
virDomainNetDefPtr, so they have enough information to store the name of
the domain as well as the namne of the interface.
I recall a long time ago considering adding this information to the
network XML, but I was dissuaded for some reason; unfortunately I don't
remember why...
>
> * call virNetworkGetXMLDesc in cmdNetworkDestroy to get the live state
> info to check whether the virtual network is in use.
>
> * add --force flag for net-destroy to forcibly destroy the virtual
> network even if it's in use.
As Michal points out (and I pointed out when you sent your previous
patchset), this does change behavior that some scripts may be dependent
on. I too like the increased safety, but am concerned about breaking
functionality. In cases like this, I always like to hear the learned
advice of Dan Berrange - Daniel, what do you say about changing the
default behavior of virsh net-destroy? Safe enough?
I don't think it is acceptable to change the behaviour. The 'destroy'
commands are unambiguous about the fact that they are a forceful
operation that will always succeed. Note that for the 'virsh destroy'
command we added a 'graceful' option, thus preserving default behaviour
> -=item B<net-dumpxml> I<network>
[I<--inactive>]
> +=item B<net-dumpxml> I<network> [I<--inactive>]
[I<--system>]
>
> Output the virtual network information as an XML dump to stdout.
> If I<--inactive> is specified, then physical functions are not
> expanded into their associated virtual functions.
> +If I<--system> is specified, then directly output the current
> +live state corresponding to this network from system.
This description of the 'system' flag just makes it sound like it is
the same as not specifying the --inactive flag. ie If the network is
running, the XML we emit is always the live XML state
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
8:08 a.m.
New subject: [libvirt] [PATCH 3/3] virsh: prevent removing a in-used bridge for iface-unbridge
By checking transient interfaces, It obtains the live information of
attached interfaces to see if the bridge is in use.
Signed-off-by: Lin Ma <lma(a)suse.com>
---
tools/virsh-interface.c | 25 ++++++++++++++++++++++---
1 file changed, 22 insertions(+), 3 deletions(-)
diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c
index 5f848b6..ff40be0 100644
--- a/tools/virsh-interface.c
+++ b/tools/virsh-interface.c
@@ -1040,11 +1040,11 @@ cmdInterfaceUnbridge(vshControl *ctl, const vshCmd *cmd)
const char *br_name;
char *if_type = NULL, *if_name = NULL;
bool nostart = false;
- char *br_xml = NULL;
+ char *br_xml = NULL, *br_xml_transient_if = NULL;
xmlChar *if_xml = NULL;
int if_xml_size;
- xmlDocPtr xml_doc = NULL;
- xmlXPathContextPtr ctxt = NULL;
+ xmlDocPtr xml_doc = NULL, xml_doc_transient_if = NULL;
+ xmlXPathContextPtr ctxt = NULL, ctxt_transient_if = NULL;
xmlNodePtr top_node, if_node, cur;
/* Get a handle to the original device */
@@ -1103,6 +1103,22 @@ cmdInterfaceUnbridge(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
}
+ /* verify whether there is any transient interface attached to bridge. */
+ if (!(br_xml_transient_if = virInterfaceGetXMLDesc(br_handle, 0)))
+ goto cleanup;
+
+ if (!(xml_doc_transient_if = virXMLParseStringCtxt(br_xml_transient_if,
+ _("(bridge interface
definition)"),
+ &ctxt_transient_if))) {
+ vshError(ctl, _("Failed to parse configuration of %s"), br_name);
+ goto cleanup;
+ }
+
+ if (virXPathNode("./bridge/interface[2]", ctxt_transient_if) != NULL) {
+ vshError(ctl, "%s", _("The bridge is in use by transient
interfaces"));
+ goto cleanup;
+ }
+
/* Change the type and name of the outer/master interface to
* the type/name of the attached slave interface.
*/
@@ -1198,10 +1214,13 @@ cmdInterfaceUnbridge(vshControl *ctl, const vshCmd *cmd)
virInterfaceFree(br_handle);
VIR_FREE(if_xml);
VIR_FREE(br_xml);
+ VIR_FREE(br_xml_transient_if);
VIR_FREE(if_type);
VIR_FREE(if_name);
xmlXPathFreeContext(ctxt);
+ xmlXPathFreeContext(ctxt_transient_if);
xmlFreeDoc(xml_doc);
+ xmlFreeDoc(xml_doc_transient_if);
return ret;
}
--
1.8.4
10:39 a.m.
New subject: [libvirt] [PATCH 3/3] virsh: prevent removing a in-used bridge for iface-unbridge
On 02.02.2015 15:08, Lin Ma wrote:
By checking transient interfaces, It obtains the live information of
attached interfaces to see if the bridge is in use.
Signed-off-by: Lin Ma <lma(a)suse.com>
---
tools/virsh-interface.c | 25 ++++++++++++++++++++++---
1 file changed, 22 insertions(+), 3 deletions(-)
Technically, this is a v2 to a previous patch (I mildly recall seeing
something like this in the past).
diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c
index 5f848b6..ff40be0 100644
--- a/tools/virsh-interface.c
+++ b/tools/virsh-interface.c
@@ -1040,11 +1040,11 @@ cmdInterfaceUnbridge(vshControl *ctl, const vshCmd *cmd)
const char *br_name;
char *if_type = NULL, *if_name = NULL;
bool nostart = false;
- char *br_xml = NULL;
+ char *br_xml = NULL, *br_xml_transient_if = NULL;
xmlChar *if_xml = NULL;
int if_xml_size;
- xmlDocPtr xml_doc = NULL;
- xmlXPathContextPtr ctxt = NULL;
+ xmlDocPtr xml_doc = NULL, xml_doc_transient_if = NULL;
+ xmlXPathContextPtr ctxt = NULL, ctxt_transient_if = NULL;
xmlNodePtr top_node, if_node, cur;
/* Get a handle to the original device */
@@ -1103,6 +1103,22 @@ cmdInterfaceUnbridge(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
}
+ /* verify whether there is any transient interface attached to bridge. */
+ if (!(br_xml_transient_if = virInterfaceGetXMLDesc(br_handle, 0)))
+ goto cleanup;
+
+ if (!(xml_doc_transient_if = virXMLParseStringCtxt(br_xml_transient_if,
+ _("(bridge interface
definition)"),
+ &ctxt_transient_if))) {
+ vshError(ctl, _("Failed to parse configuration of %s"), br_name);
+ goto cleanup;
+ }
+
+ if (virXPathNode("./bridge/interface[2]", ctxt_transient_if) != NULL) {
+ vshError(ctl, "%s", _("The bridge is in use by transient
interfaces"));
+ goto cleanup;
+ }
+
/* Change the type and name of the outer/master interface to
* the type/name of the attached slave interface.
*/
@@ -1198,10 +1214,13 @@ cmdInterfaceUnbridge(vshControl *ctl, const vshCmd *cmd)
virInterfaceFree(br_handle);
VIR_FREE(if_xml);
VIR_FREE(br_xml);
+ VIR_FREE(br_xml_transient_if);
VIR_FREE(if_type);
VIR_FREE(if_name);
xmlXPathFreeContext(ctxt);
+ xmlXPathFreeContext(ctxt_transient_if);
xmlFreeDoc(xml_doc);
+ xmlFreeDoc(xml_doc_transient_if);
return ret;
}
ACK. I'll merge this tomorrow (unless somebody beats me).
Michal
1:53 a.m.
New subject: [libvirt] [PATCH 3/3] virsh: prevent removing a in-used bridge for iface-unbridge
On 02/03/2015 11:39 AM, Michal Privoznik wrote:
On 02.02.2015 15:08, Lin Ma wrote:
> By checking transient interfaces, It obtains the live information of
> attached interfaces to see if the bridge is in use.
>
> Signed-off-by: Lin Ma <lma(a)suse.com>
> ---
> tools/virsh-interface.c | 25 ++++++++++++++++++++++---
> 1 file changed, 22 insertions(+), 3 deletions(-)
Technically, this is a v2 to a previous patch (I mildly recall seeing
something like this in the past).
It looks to be the same patch, just with reference to a private bug
report removed, and preceded with the check for net-destroy (since I had
said in my response to the original patch that the behavior of
iface-unbridge was made to be similar to net-destroy, and that my
opinion was that either neither should be changed, or both).
> diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c
> index 5f848b6..ff40be0 100644
> --- a/tools/virsh-interface.c
> +++ b/tools/virsh-interface.c
> @@ -1040,11 +1040,11 @@ cmdInterfaceUnbridge(vshControl *ctl, const vshCmd *cmd)
> const char *br_name;
> char *if_type = NULL, *if_name = NULL;
> bool nostart = false;
> - char *br_xml = NULL;
> + char *br_xml = NULL, *br_xml_transient_if = NULL;
> xmlChar *if_xml = NULL;
> int if_xml_size;
> - xmlDocPtr xml_doc = NULL;
> - xmlXPathContextPtr ctxt = NULL;
> + xmlDocPtr xml_doc = NULL, xml_doc_transient_if = NULL;
> + xmlXPathContextPtr ctxt = NULL, ctxt_transient_if = NULL;
> xmlNodePtr top_node, if_node, cur;
>
> /* Get a handle to the original device */
> @@ -1103,6 +1103,22 @@ cmdInterfaceUnbridge(vshControl *ctl, const vshCmd *cmd)
> goto cleanup;
> }
>
> + /* verify whether there is any transient interface attached to bridge. */
> + if (!(br_xml_transient_if = virInterfaceGetXMLDesc(br_handle, 0)))
> + goto cleanup;
> +
> + if (!(xml_doc_transient_if = virXMLParseStringCtxt(br_xml_transient_if,
> + _("(bridge interface
definition)"),
> + &ctxt_transient_if))) {
> + vshError(ctl, _("Failed to parse configuration of %s"), br_name);
> + goto cleanup;
> + }
> +
> + if (virXPathNode("./bridge/interface[2]", ctxt_transient_if) != NULL)
{
> + vshError(ctl, "%s", _("The bridge is in use by transient
interfaces"));
> + goto cleanup;
> + }
> +
> /* Change the type and name of the outer/master interface to
> * the type/name of the attached slave interface.
> */
> @@ -1198,10 +1214,13 @@ cmdInterfaceUnbridge(vshControl *ctl, const vshCmd *cmd)
> virInterfaceFree(br_handle);
> VIR_FREE(if_xml);
> VIR_FREE(br_xml);
> + VIR_FREE(br_xml_transient_if);
> VIR_FREE(if_type);
> VIR_FREE(if_name);
> xmlXPathFreeContext(ctxt);
> + xmlXPathFreeContext(ctxt_transient_if);
> xmlFreeDoc(xml_doc);
> + xmlFreeDoc(xml_doc_transient_if);
> return ret;
> }
>
>
ACK. I'll merge this tomorrow (unless somebody beats me).
Please don't push it as is. I think the behavior of iface-unbridge
should match whatever is done to net-destroy (if anything). If the
change is made, it should be made to both at the same time, and this one
should also have a --force option to allow overriding the extra check,
as patch 2/3 does for net-destroy.
(BTW, I'm still curious about Suse's use of netcf, as there have been no
updates to the Suse driver in netcf since the initial port was imported
several years ago. Are there downstream changes that can be sent upstream?)
9:57 a.m.
New subject: [libvirt] [PATCH 3/3] virsh: prevent removing a in-used bridge for iface-unbridge
Laine Stump wrote:
(BTW, I'm still curious about Suse's use of netcf, as there
have been no
updates to the Suse driver in netcf since the initial port was imported
several years ago. Are there downstream changes that can be sent upstream?)
SUSE doesn't use netcf. We were unable to find a volunteer from the
networking team to maintain the initial port. Instead they provided a
netcf interface on top of their technologies, which at least allows us
to use the libvirt netcf backend.
Regards,
Jim
12:59 p.m.
New subject: [libvirt] [PATCH 3/3] virsh: prevent removing a in-used bridge for iface-unbridge
On 02/04/2015 10:57 AM, Jim Fehlig wrote:
Laine Stump wrote:
> (BTW, I'm still curious about Suse's use of netcf, as there have been no
> updates to the Suse driver in netcf since the initial port was imported
> several years ago. Are there downstream changes that can be sent upstream?)
>
SUSE doesn't use netcf. We were unable to find a volunteer from the
networking team to maintain the initial port. Instead they provided a
netcf interface on top of their technologies, which at least allows us
to use the libvirt netcf backend.
Ah yes, I vaguely remember this. They basically made a copy of the APIs
in netcf.h, then provided their own library under it. This worries me,
because it means that the two are in danger of getting out of sync in
some subtle (or not so subtle) way. For example, netcf recently added
support for reporting the link state (operstate from sysfs) and speed in
its status output. If this other library for SUSE had been implemented
as a backend in netcf, Suse would have gotten that addition more or less
for free (since it is linux-specific, but not distro-specific), but
instead people will complain that this feature (already used/expected in
the output of libvirt virsh iface-dumpxml) doesn't work on SUSE until
someone adds it to the SUSE work-alike library. (Even worse would be if
something was implemented in both libraries, but behaved slightly
differently in one).
You know, there is no hard requirement that the current "SUSE" backend
in netcf be the one that stays there - that backend was written by a
person for (I think) some embedded system that was using a SUSE-based
self-built distro, and they have long since abandoned it. If you have
any interest in putting in a backend based on the code your network
people have already written for their own library (either replacing the
current SUSE backend, or in addition to it), I'd be happy to help with
it. It may turn out to be more trouble than it's worth, but it's at
least worth considering.
Can you point me at the package that is implementing the netcf API, so I
can poke around a bit?
9:31 p.m.
New subject: [libvirt] [PATCH 3/3] virsh: prevent removing a in-used bridge for iface-unbridge
Laine Stump wrote:
On 02/04/2015 10:57 AM, Jim Fehlig wrote:
> Laine Stump wrote:
>
>> (BTW, I'm still curious about Suse's use of netcf, as there have been no
>> updates to the Suse driver in netcf since the initial port was imported
>> several years ago. Are there downstream changes that can be sent upstream?)
>>
>>
> SUSE doesn't use netcf. We were unable to find a volunteer from the
> networking team to maintain the initial port. Instead they provided a
> netcf interface on top of their technologies, which at least allows us
> to use the libvirt netcf backend.
>
Ah yes, I vaguely remember this. They basically made a copy of the APIs
in netcf.h, then provided their own library under it. This worries me,
because it means that the two are in danger of getting out of sync in
some subtle (or not so subtle) way.
I suspect they already have -:(. If I've found the correct package
devel area, there haven't been any updates in six months.
For example, netcf recently added
support for reporting the link state (operstate from sysfs) and speed in
its status output. If this other library for SUSE had been implemented
as a backend in netcf, Suse would have gotten that addition more or less
for free (since it is linux-specific, but not distro-specific), but
instead people will complain that this feature (already used/expected in
the output of libvirt virsh iface-dumpxml) doesn't work on SUSE until
someone adds it to the SUSE work-alike library. (Even worse would be if
something was implemented in both libraries, but behaved slightly
differently in one).
You know, there is no hard requirement that the current "SUSE" backend
in netcf be the one that stays there - that backend was written by a
person for (I think) some embedded system that was using a SUSE-based
self-built distro, and they have long since abandoned it. If you have
any interest in putting in a backend based on the code your network
people have already written for their own library (either replacing the
current SUSE backend, or in addition to it), I'd be happy to help with
it. It may turn out to be more trouble than it's worth, but it's at
least worth considering.
Can you point me at the package that is implementing the netcf API, so I
can poke around a bit?
I'm pretty sure netcontrol is maintained here
https://build.opensuse.org/package/show/network:utilities/netcontrol
I'll let the networking folks know about this thread too.
Regards,
Jim
9:15 a.m.
New subject: [libvirt] 答复： Re: [PATCH 3/3] virsh: prevent removing a in-used bridge for iface-unbridge
>>> Laine Stump <laine(a)laine.org> 2015-2-4 下午 17:12
>>>
On 02/03/2015 11:39 AM, Michal Privoznik wrote:
> On 02.02.2015 15:08, Lin Ma wrote:
>> By checking transient interfaces, It obtains the live information of
>> attached interfaces to see if the bridge is in use.
>>
>> Signed-off-by: Lin Ma
>> ---
>> tools/virsh-interface.c | 25 ++++++++++++++++++++++---
>> 1 file changed, 22 insertions(+), 3 deletions(-)
> Technically, this is a v2 to a previous patch (I mildly recall seeing
> something like this in the past).
It looks to be the same patch, just with reference to a private bug
report removed, and preceded with the check for net-destroy (since I had
said in my response to the original patch that the behavior of
iface-unbridge was made to be similar to net-destroy, and that my
opinion was that either neither should be changed, or both).
It seems like that we
decided to keep the original net-destroy behaviour, Then
let's keep iface-unbridge's too.
>
>> diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c
>> index 5f848b6..ff40be0 100644
>> --- a/tools/virsh-interface.c
>> +++ b/tools/virsh-interface.c
>> @@ -1040,11 +1040,11 @@ cmdInterfaceUnbridge(vshControl *ctl, const vshCmd *cmd)
>> const char *br_name;
>> char *if_type = NULL, *if_name = NULL;
>> bool nostart = false;
>> - char *br_xml = NULL;
>> + char *br_xml = NULL, *br_xml_transient_if = NULL;
>> xmlChar *if_xml = NULL;
>> int if_xml_size;
>> - xmlDocPtr xml_doc = NULL;
>> - xmlXPathContextPtr ctxt = NULL;
>> + xmlDocPtr xml_doc = NULL, xml_doc_transient_if = NULL;
>> + xmlXPathContextPtr ctxt = NULL, ctxt_transient_if = NULL;
>> xmlNodePtr top_node, if_node, cur;
>>
>> /* Get a handle to the original device */
>> @@ -1103,6 +1103,22 @@ cmdInterfaceUnbridge(vshControl *ctl, const vshCmd *cmd)
>> goto cleanup;
>> }
>>
>> + /* verify whether there is any transient interface attached to bridge. */
>> + if (!(br_xml_transient_if = virInterfaceGetXMLDesc(br_handle, 0)))
>> + goto cleanup;
>> +
>> + if (!(xml_doc_transient_if = virXMLParseStringCtxt(br_xml_transient_if,
>> + _("(bridge interface
definition)"),
>> + &ctxt_transient_if)))
{
>> + vshError(ctl, _("Failed to parse configuration of %s"),
br_name);
>> + goto cleanup;
>> + }
>> +
>> + if (virXPathNode("./bridge/interface[2]", ctxt_transient_if) !=
NULL) {
>> + vshError(ctl, "%s", _("The bridge is in use by transient
interfaces"));
>> + goto cleanup;
>> + }
>> +
>> /* Change the type and name of the outer/master interface to
>> * the type/name of the attached slave interface.
>> */
>> @@ -1198,10 +1214,13 @@ cmdInterfaceUnbridge(vshControl *ctl, const vshCmd *cmd)
>> virInterfaceFree(br_handle);
>> VIR_FREE(if_xml);
>> VIR_FREE(br_xml);
>> + VIR_FREE(br_xml_transient_if);
>> VIR_FREE(if_type);
>> VIR_FREE(if_name);
>> xmlXPathFreeContext(ctxt);
>> + xmlXPathFreeContext(ctxt_transient_if);
>> xmlFreeDoc(xml_doc);
>> + xmlFreeDoc(xml_doc_transient_if);
>> return ret;
>> }
>>
>>
> ACK. I'll merge this tomorrow (unless somebody beats me).
Please don't push it as is. I think the behavior of iface-unbridge
should match whatever is done to net-destroy (if anything). If the
change is made, it should be made to both at the same time, and this one
should also have a --force option to allow overriding the extra check,
as patch 2/3 does for net-destroy.
As Daniel points out, destroy is The net-destroy
already shows to user that
it's a forceful operation, So we don't need a --force option, then iface-unbridge
either.
10:47 a.m.
On 02.02.2015 15:08, Lin Ma wrote:
* Get the live state info of a virtual network through netcf in
networkGetXMLDesc.
* Add --system flag for net-dumpxml to show the live state info.
* Check the live state info in net-destroy.
* Add --force flag for net-destroy to forcibly destroy the virtual network.
* Check the transient interfaces info in iface-unbridge.
---
Lin Ma (3):
bridge_driver: Return the live state info of a given virtual network
virsh: prevent destroying a in-used network for net-destroy
virsh: prevent removing a in-used bridge for iface-unbridge
include/libvirt/libvirt-network.h | 1 +
src/Makefile.am | 3 +
src/network/bridge_driver.c | 141 ++++++++++++++++++++++++++++++++++-
src/network/bridge_driver_platform.h | 7 ++
tests/Makefile.am | 4 +
tools/virsh-interface.c | 25 ++++++-
tools/virsh-network.c | 62 ++++++++++++++-
tools/virsh.pod | 8 +-
8 files changed, 241 insertions(+), 10 deletions(-)
So I've spent some time thinking about this. I don't really like the
idea of producing completely different XML (it doesn't even have
<network/> as its root element!). But I see what you're trying to
achieve. How about putting the bridged interfaces into the network
definition (on request signalized by a flag, of course).
I think we've come to a point where we need to introduce live and config
XML (like we have for domains). We already have that to some extent, but
not truly.
On the other hand, looking into virNetworkDefFormat(), there's this
'@connections' attribute to the <network/> element which is a nonzero
integer if the network is in use. Won't that suffice for what you're
trying to achieve?
Michal
1:21 a.m.
On 02/03/2015 11:47 AM, Michal Privoznik wrote:
On 02.02.2015 15:08, Lin Ma wrote:
> * Get the live state info of a virtual network through netcf in networkGetXMLDesc.
> * Add --system flag for net-dumpxml to show the live state info.
> * Check the live state info in net-destroy.
> * Add --force flag for net-destroy to forcibly destroy the virtual network.
> * Check the transient interfaces info in iface-unbridge.
>
> ---
> Lin Ma (3):
> bridge_driver: Return the live state info of a given virtual network
> virsh: prevent destroying a in-used network for net-destroy
> virsh: prevent removing a in-used bridge for iface-unbridge
>
> include/libvirt/libvirt-network.h | 1 +
> src/Makefile.am | 3 +
> src/network/bridge_driver.c | 141 ++++++++++++++++++++++++++++++++++-
> src/network/bridge_driver_platform.h | 7 ++
> tests/Makefile.am | 4 +
> tools/virsh-interface.c | 25 ++++++-
> tools/virsh-network.c | 62 ++++++++++++++-
> tools/virsh.pod | 8 +-
> 8 files changed, 241 insertions(+), 10 deletions(-)
>
So I've spent some time thinking about this. I don't really like the
idea of producing completely different XML (it doesn't even have
<network/> as its root element!). But I see what you're trying to
achieve. How about putting the bridged interfaces into the network
definition (on request signalized by a flag, of course).
I think that if we're going to add the list of connected guest devices
to a network's status, that it should just always be there - adding a
new flag for every little bit of different status sets a bad precedent
and sets us up to have an infinitely increasing number of flags.
Like I mentioned in my response to one of the patches, I think this
information is better gathered and maintained by the bridge driver as
guests connect to / disconnect from a network; this avoids the necessity
of dragging netcf into the picture and allows much better information -
the name of the domain using each interface can be included.
I think we've come to a point where we need to introduce live and config
XML (like we have for domains). We already have that to some extent, but
not truly.
I think the situation is more that we have live and config XML, but
there may be a small problem here and there. As a matter of fact, a long
time ago virsh wasn't using the live XML to edit a network, and this was
causing problems (e.g. if you tried to edit a network twice without
restarting it, the second time you would lose all the changes from the
first edit).
On the other hand, looking into virNetworkDefFormat(), there's this
'@connections' attribute to the <network/> element which is a nonzero
integer if the network is in use. Won't that suffice for what you're
trying to achieve?
In my opinion, yes - that's exactly the type of thing it was intended
for :-)
3:16 a.m.
On Wed, Feb 04, 2015 at 02:21:18AM -0500, Laine Stump wrote:
On 02/03/2015 11:47 AM, Michal Privoznik wrote:
> On 02.02.2015 15:08, Lin Ma wrote:
>> * Get the live state info of a virtual network through netcf in
networkGetXMLDesc.
>> * Add --system flag for net-dumpxml to show the live state info.
>> * Check the live state info in net-destroy.
>> * Add --force flag for net-destroy to forcibly destroy the virtual network.
>> * Check the transient interfaces info in iface-unbridge.
>>
>> ---
>> Lin Ma (3):
>> bridge_driver: Return the live state info of a given virtual network
>> virsh: prevent destroying a in-used network for net-destroy
>> virsh: prevent removing a in-used bridge for iface-unbridge
>>
>> include/libvirt/libvirt-network.h | 1 +
>> src/Makefile.am | 3 +
>> src/network/bridge_driver.c | 141 ++++++++++++++++++++++++++++++++++-
>> src/network/bridge_driver_platform.h | 7 ++
>> tests/Makefile.am | 4 +
>> tools/virsh-interface.c | 25 ++++++-
>> tools/virsh-network.c | 62 ++++++++++++++-
>> tools/virsh.pod | 8 +-
>> 8 files changed, 241 insertions(+), 10 deletions(-)
>>
> So I've spent some time thinking about this. I don't really like the
> idea of producing completely different XML (it doesn't even have
> <network/> as its root element!). But I see what you're trying to
> achieve. How about putting the bridged interfaces into the network
> definition (on request signalized by a flag, of course).
I think that if we're going to add the list of connected guest devices
to a network's status, that it should just always be there - adding a
new flag for every little bit of different status sets a bad precedent
and sets us up to have an infinitely increasing number of flags.
Like I mentioned in my response to one of the patches, I think this
information is better gathered and maintained by the bridge driver as
guests connect to / disconnect from a network; this avoids the necessity
of dragging netcf into the picture and allows much better information -
the name of the domain using each interface can be included.
I think we should consider whether we should expose it as an explicit
API too, rather than just stuffing it into the XML as a way to avoid
the work of defining an API.
The XML is generally describing the configuration of the virtual
network. This isn't really configuration information, but rather
a reporting about usage of the network, so it isn't a clearly
compelling thing to put in the XML.
> I think we've come to a point where we need to introduce
live and config
> XML (like we have for domains). We already have that to some extent, but
> not truly.
I think the situation is more that we have live and config XML, but
there may be a small problem here and there. As a matter of fact, a long
time ago virsh wasn't using the live XML to edit a network, and this was
causing problems (e.g. if you tried to edit a network twice without
restarting it, the second time you would lose all the changes from the
first edit).
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
8:58 a.m.
On Wed, Feb 04, 2015 at 02:21:18AM -0500, Laine Stump wrote:
On 02/03/2015 11:47 AM, Michal Privoznik wrote:
> On 02.02.2015 15:08, Lin Ma wrote:
>> * Get the live state info of a virtual network through netcf in
networkGetXMLDesc.
>> * Add --system flag for net-dumpxml to show the live state info.
>> * Check the live state info in net-destroy.
>> * Add --force flag for net-destroy to forcibly destroy the virtual network.
>> * Check the transient interfaces info in iface-unbridge.
>>
>> ---
>> Lin Ma (3):
>> bridge_driver: Return the live state info of a given virtual network
>> virsh: prevent destroying a in-used network for net-destroy
>> virsh: prevent removing a in-used bridge for iface-unbridge
>>
>> include/libvirt/libvirt-network.h | 1 +
>> src/Makefile.am | 3 +
>> src/network/bridge_driver.c | 141 ++++++++++++++++++++++++++++++++++-
>> src/network/bridge_driver_platform.h | 7 ++
>> tests/Makefile.am | 4 +
>> tools/virsh-interface.c | 25 ++++++-
>> tools/virsh-network.c | 62 ++++++++++++++-
>> tools/virsh.pod | 8 +-
>> 8 files changed, 241 insertions(+), 10 deletions(-)
>>
> So I've spent some time thinking about this. I don't really like the
> idea of producing completely different XML (it doesn't even have
> <network/> as its root element!). But I see what you're trying to
> achieve. How about putting the bridged interfaces into the network
> definition (on request signalized by a flag, of course).
I think that if we're going to add the list of connected guest devices
to a network's status, that it should just always be there - adding a
new flag for every little bit of different status sets a bad precedent
and sets us up to have an infinitely increasing number of flags.
Like I mentioned in my response to one of the patches, I think this
information is better gathered and maintained by the bridge driver as
guests connect to / disconnect from a network; this avoids the necessity
of dragging netcf into the picture and allows much better information -
the name of the domain using each interface can be included.
You know if this checking for guests were done in the virNetworkDestroy
API implementation in src/network/bridge_driver.c, instead of in
virsh, then we would not need any of this code for extending the XML
nor parsing it in virsh. We could simply check the number of connections
which is something we have directly available internally. So 95% of this
patch series would go away
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
11:39 a.m.
On 02/04/2015 09:58 AM, Daniel P. Berrange wrote:
On Wed, Feb 04, 2015 at 02:21:18AM -0500, Laine Stump wrote:
> On 02/03/2015 11:47 AM, Michal Privoznik wrote:
>> On 02.02.2015 15:08, Lin Ma wrote:
>>> * Get the live state info of a virtual network through netcf in
networkGetXMLDesc.
>>> * Add --system flag for net-dumpxml to show the live state info.
>>> * Check the live state info in net-destroy.
>>> * Add --force flag for net-destroy to forcibly destroy the virtual network.
>>> * Check the transient interfaces info in iface-unbridge.
>>>
>>> ---
>>> Lin Ma (3):
>>> bridge_driver: Return the live state info of a given virtual network
>>> virsh: prevent destroying a in-used network for net-destroy
>>> virsh: prevent removing a in-used bridge for iface-unbridge
>>>
>>> include/libvirt/libvirt-network.h | 1 +
>>> src/Makefile.am | 3 +
>>> src/network/bridge_driver.c | 141
++++++++++++++++++++++++++++++++++-
>>> src/network/bridge_driver_platform.h | 7 ++
>>> tests/Makefile.am | 4 +
>>> tools/virsh-interface.c | 25 ++++++-
>>> tools/virsh-network.c | 62 ++++++++++++++-
>>> tools/virsh.pod | 8 +-
>>> 8 files changed, 241 insertions(+), 10 deletions(-)
>>>
>> So I've spent some time thinking about this. I don't really like the
>> idea of producing completely different XML (it doesn't even have
>> <network/> as its root element!). But I see what you're trying to
>> achieve. How about putting the bridged interfaces into the network
>> definition (on request signalized by a flag, of course).
> I think that if we're going to add the list of connected guest devices
> to a network's status, that it should just always be there - adding a
> new flag for every little bit of different status sets a bad precedent
> and sets us up to have an infinitely increasing number of flags.
>
> Like I mentioned in my response to one of the patches, I think this
> information is better gathered and maintained by the bridge driver as
> guests connect to / disconnect from a network; this avoids the necessity
> of dragging netcf into the picture and allows much better information -
> the name of the domain using each interface can be included.
You know if this checking for guests were done in the virNetworkDestroy
API implementation in src/network/bridge_driver.c, instead of in
virsh, then we would not need any of this code for extending the XML
nor parsing it in virsh. We could simply check the number of connections
which is something we have directly available internally. So 95% of this
patch series would go away
Except that the virNetworkDestroy() API was created before we realized
it was a good idea to have a flags argument to every new API. So that
would require creating a new virNetworkDestroyFlags() API (and the first
flag to be defined would be VIR_NETWORK_DESTROY_GRACEFUL).
Note that a network's connections count *is* returned in the status XML,
so virsh could check that and it would work even when connecting to
older servers. I think I prefer your idea, though, because it is more
consistent with what is done for the virDomain API (it won't work when
talking to older servers though, and virsh will have to be written to
fall back to virNetworkDestroy() when the server is too old to have
virNetworkDestroyFlags().
11:41 a.m.
On Wed, Feb 04, 2015 at 12:39:56PM -0500, Laine Stump wrote:
On 02/04/2015 09:58 AM, Daniel P. Berrange wrote:
> On Wed, Feb 04, 2015 at 02:21:18AM -0500, Laine Stump wrote:
>> On 02/03/2015 11:47 AM, Michal Privoznik wrote:
>>> On 02.02.2015 15:08, Lin Ma wrote:
>>>> * Get the live state info of a virtual network through netcf in
networkGetXMLDesc.
>>>> * Add --system flag for net-dumpxml to show the live state info.
>>>> * Check the live state info in net-destroy.
>>>> * Add --force flag for net-destroy to forcibly destroy the virtual
network.
>>>> * Check the transient interfaces info in iface-unbridge.
>>>>
>>>> ---
>>>> Lin Ma (3):
>>>> bridge_driver: Return the live state info of a given virtual network
>>>> virsh: prevent destroying a in-used network for net-destroy
>>>> virsh: prevent removing a in-used bridge for iface-unbridge
>>>>
>>>> include/libvirt/libvirt-network.h | 1 +
>>>> src/Makefile.am | 3 +
>>>> src/network/bridge_driver.c | 141
++++++++++++++++++++++++++++++++++-
>>>> src/network/bridge_driver_platform.h | 7 ++
>>>> tests/Makefile.am | 4 +
>>>> tools/virsh-interface.c | 25 ++++++-
>>>> tools/virsh-network.c | 62 ++++++++++++++-
>>>> tools/virsh.pod | 8 +-
>>>> 8 files changed, 241 insertions(+), 10 deletions(-)
>>>>
>>> So I've spent some time thinking about this. I don't really like
the
>>> idea of producing completely different XML (it doesn't even have
>>> <network/> as its root element!). But I see what you're trying to
>>> achieve. How about putting the bridged interfaces into the network
>>> definition (on request signalized by a flag, of course).
>> I think that if we're going to add the list of connected guest devices
>> to a network's status, that it should just always be there - adding a
>> new flag for every little bit of different status sets a bad precedent
>> and sets us up to have an infinitely increasing number of flags.
>>
>> Like I mentioned in my response to one of the patches, I think this
>> information is better gathered and maintained by the bridge driver as
>> guests connect to / disconnect from a network; this avoids the necessity
>> of dragging netcf into the picture and allows much better information -
>> the name of the domain using each interface can be included.
> You know if this checking for guests were done in the virNetworkDestroy
> API implementation in src/network/bridge_driver.c, instead of in
> virsh, then we would not need any of this code for extending the XML
> nor parsing it in virsh. We could simply check the number of connections
> which is something we have directly available internally. So 95% of this
> patch series would go away
Except that the virNetworkDestroy() API was created before we realized
it was a good idea to have a flags argument to every new API. So that
would require creating a new virNetworkDestroyFlags() API (and the first
flag to be defined would be VIR_NETWORK_DESTROY_GRACEFUL).
Note that a network's connections count *is* returned in the status XML,
so virsh could check that and it would work even when connecting to
older servers. I think I prefer your idea, though, because it is more
consistent with what is done for the virDomain API (it won't work when
talking to older servers though, and virsh will have to be written to
fall back to virNetworkDestroy() when the server is too old to have
virNetworkDestroyFlags().
Yep, adding a virNetworkDestroyFlags() is no big deal - we'll inevitably
need it at some point. There's probably an argument for identifying all
our remaining APIs which lack a flags parameter and fixing it once and
for all :-)
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
9:24 a.m.
New subject: [libvirt] 答复： Re: [PATCH 0/3] Prevent removing a in-used static bridge and destroying a in-used virtual network
>>> "Daniel P. Berrange"
<berrange(a)redhat.com> 2015-2-4 下午 23:10 >>>
On Wed, Feb 04, 2015 at 02:21:18AM -0500, Laine Stump wrote:
> On 02/03/2015 11:47 AM, Michal Privoznik wrote:
> > On 02.02.2015 15:08, Lin Ma wrote:
> >> * Get the live state info of a virtual network through netcf in
networkGetXMLDesc.
> >> * Add --system flag for net-dumpxml to show the live state info.
> >> * Check the live state info in net-destroy.
> >> * Add --force flag for net-destroy to forcibly destroy the virtual network.
> >> * Check the transient interfaces info in iface-unbridge.
> >>
> >> ---
> >> Lin Ma (3):
> >> bridge_driver: Return the live state info of a given virtual network
> >> virsh: prevent destroying a in-used network for net-destroy
> >> virsh: prevent removing a in-used bridge for iface-unbridge
> >>
> >> include/libvirt/libvirt-network.h | 1 +
> >> src/Makefile.am | 3 +
> >> src/network/bridge_driver.c | 141
++++++++++++++++++++++++++++++++++-
> >> src/network/bridge_driver_platform.h | 7 ++
> >> tests/Makefile.am | 4 +
> >> tools/virsh-interface.c | 25 ++++++-
> >> tools/virsh-network.c | 62 ++++++++++++++-
> >> tools/virsh.pod | 8 +-
> >> 8 files changed, 241 insertions(+), 10 deletions(-)
> >>
> > So I've spent some time thinking about this. I don't really like the
> > idea of producing completely different XML (it doesn't even have
> > as its root element!). But I see what you're trying to
> > achieve. How about putting the bridged interfaces into the network
> > definition (on request signalized by a flag, of course).
>
> I think that if we're going to add the list of connected guest devices
> to a network's status, that it should just always be there - adding a
> new flag for every little bit of different status sets a bad precedent
> and sets us up to have an infinitely increasing number of flags.
>
> Like I mentioned in my response to one of the patches, I think this
> information is better gathered and maintained by the bridge driver as
> guests connect to / disconnect from a network; this avoids the necessity
> of dragging netcf into the picture and allows much better information -
> the name of the domain using each interface can be included.
You know if this checking for guests were done in the virNetworkDestroy
API implementation in src/network/bridge_driver.c, instead of in
virsh, then we would not need any of this code for extending the XML
nor parsing it in virsh. We could simply check the number of connections
which is something we have directly available internally. So 95% of this
patch series would go away
:-C I'm sad...... :-), Anyway, Because we keep the
original behaviour of
net-destroy, So we keep iface-unbridge no changes as well, Then droping
this patch series makes sense.
3543
days inactive
3559
days old
25 comments
5 participants
participants (5)
-
Daniel P. Berrange -
Jim Fehlig -
Laine Stump -
Lin Ma -
Michal Privoznik