Re: [libvirt] [PATCH 08/10] network: separate Start/Shutdown functions for new network types
On 07/08/2011 09:32 AM, Daniel P. Berrange wrote: On Tue, Jul 05, 2011 at 03:45:56AM -0400, Laine Stump wrote: Previously all networks were composed of bridge devices created and managed by libvirt, and the same operations needed to be done for all of them when they were started and stopped (create and start the bridge device, configure its MAC address and IP address, add iptables rules). The new network types are (for now at least) managed outside of libvirt, and the network object is used only to contain information about the network, which is then used as each individual guest connects itself. This means that when starting/stopping one of these new networks, we really want to do nothing, aside from marking the network as active/inactive. This has been setup as toplevel Start/Shutdown functions that do the small bit of common stuff, then have a switch statement to execute network type-specific start/shutdown code, then do a bit more common code. The type-specific functions called for the new host bridge and macvtap based types are currently empty. In the future these functions may actually do something, and we will surely add more functions that are similarly patterned. Once everything has settled, we can make a table of sub-driver function pointers for each network type, and store a pointer to that table in the network object, then we can replace the switch statements with calls to functions in the table. The final step in this will be to add a new table (and corresponding new functions) for new network types as they are added. --- src/network/bridge_driver.c | 188 +++ 1 files changed, 138 insertions(+), 50 deletions(-) ACK, there is pretty much no functional change for existing network types. + +VIR_INFO(Starting up network '%s', network-def-name); +network-active = 1; ... network-active = 0; We could take this opportunity to change to using a bool and true/false I think I'd rather do that in a separate patch - there are several other ints used as booleans in network_conf, domain_conf, and nwfilter_conf (at least) and it would probably be good to make them all consistent. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 08/10] network: separate Start/Shutdown functions for new network types
On Tue, Jul 05, 2011 at 03:45:56AM -0400, Laine Stump wrote: Previously all networks were composed of bridge devices created and managed by libvirt, and the same operations needed to be done for all of them when they were started and stopped (create and start the bridge device, configure its MAC address and IP address, add iptables rules). The new network types are (for now at least) managed outside of libvirt, and the network object is used only to contain information about the network, which is then used as each individual guest connects itself. This means that when starting/stopping one of these new networks, we really want to do nothing, aside from marking the network as active/inactive. This has been setup as toplevel Start/Shutdown functions that do the small bit of common stuff, then have a switch statement to execute network type-specific start/shutdown code, then do a bit more common code. The type-specific functions called for the new host bridge and macvtap based types are currently empty. In the future these functions may actually do something, and we will surely add more functions that are similarly patterned. Once everything has settled, we can make a table of sub-driver function pointers for each network type, and store a pointer to that table in the network object, then we can replace the switch statements with calls to functions in the table. The final step in this will be to add a new table (and corresponding new functions) for new network types as they are added. --- src/network/bridge_driver.c | 188 +++ 1 files changed, 138 insertions(+), 50 deletions(-) ACK, there is pretty much no functional change for existing network types. + +VIR_INFO(Starting up network '%s', network-def-name); +network-active = 1; ... network-active = 0; We could take this opportunity to change to using a bool and true/false 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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 08/10] network: separate Start/Shutdown functions for new network types
Previously all networks were composed of bridge devices created and
managed by libvirt, and the same operations needed to be done for all
of them when they were started and stopped (create and start the
bridge device, configure its MAC address and IP address, add iptables
rules). The new network types are (for now at least) managed outside
of libvirt, and the network object is used only to contain information
about the network, which is then used as each individual guest
connects itself.
This means that when starting/stopping one of these new networks, we
really want to do nothing, aside from marking the network as
active/inactive.
This has been setup as toplevel Start/Shutdown functions that do the
small bit of common stuff, then have a switch statement to execute
network type-specific start/shutdown code, then do a bit more common
code. The type-specific functions called for the new host bridge and
macvtap based types are currently empty.
In the future these functions may actually do something, and we will
surely add more functions that are similarly patterned. Once
everything has settled, we can make a table of sub-driver function
pointers for each network type, and store a pointer to that table in
the network object, then we can replace the switch statements with
calls to functions in the table.
The final step in this will be to add a new table (and corresponding
new functions) for new network types as they are added.
---
src/network/bridge_driver.c | 188 +++
1 files changed, 138 insertions(+), 50 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index cb49356..69d4c35 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -97,10 +97,22 @@ static void networkDriverUnlock(struct network_driver
*driver)
static int networkShutdown(void);
-static int networkStartNetworkDaemon(struct network_driver *driver,
+static int networkStartNetwork(struct network_driver *driver,
+ virNetworkObjPtr network);
+
+static int networkShutdownNetwork(struct network_driver *driver,
+ virNetworkObjPtr network);
+
+static int networkStartNetworkVirtual(struct network_driver *driver,
virNetworkObjPtr network);
-static int networkShutdownNetworkDaemon(struct network_driver *driver,
+static int networkShutdownNetworkVirtual(struct network_driver *driver,
+virNetworkObjPtr network);
+
+static int networkStartNetworkExternal(struct network_driver *driver,
+ virNetworkObjPtr network);
+
+static int networkShutdownNetworkExternal(struct network_driver *driver,
virNetworkObjPtr network);
static void networkReloadIptablesRules(struct network_driver *driver);
@@ -252,9 +264,10 @@ networkAutostartConfigs(struct network_driver *driver) {
for (i = 0 ; i driver-networks.count ; i++) {
virNetworkObjLock(driver-networks.objs[i]);
if (driver-networks.objs[i]-autostart
-!virNetworkObjIsActive(driver-networks.objs[i])
-networkStartNetworkDaemon(driver, driver-networks.objs[i]) 0) {
+!virNetworkObjIsActive(driver-networks.objs[i])) {
+if (networkStartNetwork(driver, driver-networks.objs[i]) 0) {
/* failed to start but already logged */
+}
}
virNetworkObjUnlock(driver-networks.objs[i]);
}
@@ -1689,7 +1702,7 @@ networkAddAddrToBridge(struct network_driver *driver,
}
static int
-networkStartNetworkDaemon(struct network_driver *driver,
+networkStartNetworkVirtual(struct network_driver *driver,
virNetworkObjPtr network)
{
int ii, err;
@@ -1698,12 +1711,6 @@ networkStartNetworkDaemon(struct network_driver *driver,
virNetworkIpDefPtr ipdef;
char *macTapIfName = NULL;
-if (virNetworkObjIsActive(network)) {
-networkReportError(VIR_ERR_OPERATION_INVALID,
- %s, _(network is already active));
-return -1;
-}
-
/* Check to see if any network IP collides with an existing route */
if (networkCheckRouteCollision(network) 0)
return -1;
@@ -1805,26 +1812,10 @@ networkStartNetworkDaemon(struct network_driver *driver,
if (v6present networkStartRadvd(network) 0)
goto err4;
-/* Persist the live configuration now we have bridge info */
-if (virNetworkSaveConfig(NETWORK_STATE_DIR, network-def) 0) {
-goto err5;
-}
-
VIR_FREE(macTapIfName);
-VIR_INFO(Starting up network '%s', network-def-name);
-network-active = 1;
return 0;
- err5:
-if (!save_err)
-save_err = virSaveLastError();
-
-if (network-radvdPid 0) {
-kill(network-radvdPid, SIGTERM);
-network-radvdPid = -1;
-}
-
err4:
if (!save_err)
