On 10/25/2012 05:06 PM, Peter Krempa wrote:
> 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.
Good point. I retract that suggestion.
>
>>
>> 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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list