On 10/25/12 22:56, Laine Stump wrote:
On 10/25/2012 11:18 AM, Peter Krempa wrote:
The network driver didn't care about config files when a network was
destroyed, just when it was undefined leaving behind files for transient
networks.
This patch splits out the cleanup code to a helper function that handles
the cleanup if the inactive network object is being removed and re-uses
this code when getting rid of inactive networks.
---
src/network/bridge_driver.c | 133 +++++++++++++++++++++++++-------------------
1 file changed, 76 insertions(+), 57 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 976c132..45fca0e 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -155,6 +155,67 @@ networkRadvdConfigFileName(const char *netname)
return configfile;
}
+/* do needed cleanup steps and remove the network from the list */
+static int
+networkRemoveInactive(struct network_driver *driver,
+ virNetworkObjPtr net)
+{
+ char *leasefile = NULL;
+ char *radvdconfigfile = NULL;
+ char *radvdpidbase = NULL;
+ dnsmasqContext *dctx = NULL;
+ virNetworkIpDefPtr ipdef;
+ virNetworkDefPtr def = virNetworkObjGetPersistentDef(net);
+
+ int ret = -1;
+ int i;
+
+ /* we only support dhcp on one IPv4 address per defined network */
+ for (i = 0;
+ (ipdef = virNetworkDefGetIpByIndex(def, AF_UNSPEC, i));
+ i++) {
+ if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET) &&
+ (ipdef->nranges || ipdef->nhosts)) {
+ /* clean up files left over by dnsmasq */
+ if (!(dctx = dnsmasqContextNew(def->name, DNSMASQ_STATE_DIR)))
+ goto cleanup;
+
+ if (!(leasefile = networkDnsmasqLeaseFileName(def->name)))
+ goto cleanup;
+
+ dnsmasqDelete(dctx);
+ unlink(leasefile);
A couple of things about this:
1) I think it would be fine to do all of these deletes anytime a network
is destroyed, not just when it's undefined (although I guess it's
possible someone might rely on this behavior, it's not documented and
not part of the API (and I don't know why they would rely on it anyway).
That was the way I implemented it at first. I know it's not documented
in any way but deleting the lease file has one implication: Every time
you destroy the network you are risking that your guests are being
re-addressed. It can be fully expected while using DHCP without static
leases, but I think of it as a pretty sweet feature and I remember
addresses of some of my guests and I'm glad they get the same addresses
every time. On the other hand, the static hosts file can be deleted when
destroying the net every time without any problems.
2) Since we're not checking for errors anyway, I think we should just
always try the deletes/unlinks - it's possible that the configuration of
the network changed during its lifetime and the IP
addresses/ranges/hosts/whatever were deleted, so that now the network
isn't doing dhcp, but it was in the past and has stale files left around.
Agreed, I didn't think about this option.
Peter
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
