When there's no new definition the helper overwrote the old one with NULL. --- src/conf/network_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 8976f2a..93d1b4c 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -359,7 +359,7 @@ virNetworkObjSetDefTransient(virNetworkObjPtr network, bool live) void virNetworkObjUnsetDefTransient(virNetworkObjPtr network) { - if (network->def) { + if (network->newDef) { virNetworkDefFree(network->def); network->def = network->newDef; network->newDef = NULL; -- 1.7.12.4

On 10/25/12 22:34, Laine Stump wrote: Thanks. In a patch 4/6 of this series, I added code that accesses the definition right before tne network object is discarded. This caused segfault of the daemon when I tried to get rid of the dnsmasq and radvd status files when removing a network completely. Peter

The argument check_active is used only as a boolean so this patch changes the type and updates callers. --- src/conf/network_conf.c | 2 +- src/conf/network_conf.h | 2 +- src/network/bridge_driver.c | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 93d1b4c..8a3cc75 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -3056,7 +3056,7 @@ cleanup: int virNetworkObjIsDuplicate(virNetworkObjListPtr doms, virNetworkDefPtr def, - unsigned int check_active) + bool check_active) { int ret = -1; int dupVM = 0; diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 4c0c8c1..3e46304 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -337,7 +337,7 @@ virNetworkObjUpdate(virNetworkObjPtr obj, int virNetworkObjIsDuplicate(virNetworkObjListPtr doms, virNetworkDefPtr def, - unsigned int check_active); + bool check_active); void virNetworkObjLock(virNetworkObjPtr obj); void virNetworkObjUnlock(virNetworkObjPtr obj); diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 8837843..43a5540 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2672,7 +2672,7 @@ static virNetworkPtr networkCreate(virConnectPtr conn, const char *xml) { if (!(def = virNetworkDefParseString(xml))) goto cleanup; - if (virNetworkObjIsDuplicate(&driver->networks, def, 1) < 0) + if (virNetworkObjIsDuplicate(&driver->networks, def, true) < 0) goto cleanup; /* Only the three L3 network types that are configured by libvirt @@ -2731,7 +2731,7 @@ static virNetworkPtr networkDefine(virConnectPtr conn, const char *xml) { if (!(def = virNetworkDefParseString(xml))) goto cleanup; - if (virNetworkObjIsDuplicate(&driver->networks, def, 0) < 0) + if (virNetworkObjIsDuplicate(&driver->networks, def, false) < 0) goto cleanup; /* Only the three L3 network types that are configured by libvirt -- 1.7.12.4

The hosts file was created in the network definition function. This patch moves the place the file is being created to the point where dnsmasq is being started. --- src/network/bridge_driver.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 43a5540..976c132 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -846,6 +846,8 @@ networkStartDhcpDaemon(virNetworkObjPtr network) char *pidfile = NULL; int ret = -1; dnsmasqContext *dctx = NULL; + virNetworkIpDefPtr ipdef; + int i; if (!virNetworkDefGetIpByIndex(network->def, AF_UNSPEC, 0)) { /* no IPv6 addresses, so we don't need to run radvd */ @@ -886,6 +888,18 @@ networkStartDhcpDaemon(virNetworkObjPtr network) if (ret < 0) goto cleanup; + /* populate dnsmasq hosts file */ + for (i = 0; (ipdef = virNetworkDefGetIpByIndex(network->def, AF_UNSPEC, i)); i++) { + if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET) && + (ipdef->nranges || ipdef->nhosts)) { + if (networkBuildDnsmasqHostsfile(dctx, ipdef, + network->def->dns) < 0) + goto cleanup; + + break; + } + } + ret = dnsmasqSave(dctx); if (ret < 0) goto cleanup; @@ -2724,7 +2738,6 @@ static virNetworkPtr networkDefine(virConnectPtr conn, const char *xml) { virNetworkObjPtr network = NULL; virNetworkPtr ret = NULL; int ii; - dnsmasqContext* dctx = NULL; networkDriverLock(driver); @@ -2781,21 +2794,12 @@ static virNetworkPtr networkDefine(virConnectPtr conn, const char *xml) { goto cleanup; } - if (ipv4def) { - dctx = dnsmasqContextNew(def->name, DNSMASQ_STATE_DIR); - if (dctx == NULL || - networkBuildDnsmasqHostsfile(dctx, ipv4def, def->dns) < 0 || - dnsmasqSave(dctx) < 0) - goto cleanup; - } - VIR_INFO("Defining network '%s'", def->name); ret = virGetNetwork(conn, def->name, def->uuid); cleanup: if (freeDef) virNetworkDefFree(def); - dnsmasqContextFree(dctx); if (network) virNetworkObjUnlock(network); networkDriverUnlock(driver); -- 1.7.12.4

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); + + } else if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET6)) { + /* clean up files left over by radvd */ + + if (!(radvdconfigfile = networkRadvdConfigFileName(def->name))) + goto no_memory; + + if (!(radvdpidbase = networkRadvdPidfileBasename(def->name))) + goto no_memory; + + unlink(radvdconfigfile); + virPidFileDelete(NETWORK_PID_DIR, radvdpidbase); + } + } + + virNetworkRemoveInactive(&driver->networks, net); + + ret = 0; + +cleanup: + VIR_FREE(leasefile); + VIR_FREE(radvdconfigfile); + VIR_FREE(radvdpidbase); + dnsmasqContextFree(dctx); + return ret; + +no_memory: + virReportOOMError(); + goto cleanup; +} + static char * networkBridgeDummyNicName(const char *brname) { @@ -2806,12 +2867,11 @@ cleanup: return ret; } -static int networkUndefine(virNetworkPtr net) { +static int +networkUndefine(virNetworkPtr net) { struct network_driver *driver = net->conn->networkPrivateData; virNetworkObjPtr network; - virNetworkIpDefPtr ipdef; - bool dhcp_present = false, v6present = false; - int ret = -1, ii; + int ret = -1; networkDriverLock(driver); @@ -2833,58 +2893,12 @@ static int networkUndefine(virNetworkPtr net) { network) < 0) goto cleanup; - /* we only support dhcp on one IPv4 address per defined network */ - for (ii = 0; - (ipdef = virNetworkDefGetIpByIndex(network->def, AF_UNSPEC, ii)); - ii++) { - if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET)) { - if (ipdef->nranges || ipdef->nhosts) - dhcp_present = true; - } else if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET6)) { - v6present = true; - } - } - - if (dhcp_present) { - char *leasefile; - dnsmasqContext *dctx = dnsmasqContextNew(network->def->name, DNSMASQ_STATE_DIR); - if (dctx == NULL) - goto cleanup; - - dnsmasqDelete(dctx); - dnsmasqContextFree(dctx); - - leasefile = networkDnsmasqLeaseFileName(network->def->name); - if (!leasefile) - goto cleanup; - unlink(leasefile); - VIR_FREE(leasefile); - } - - if (v6present) { - char *configfile = networkRadvdConfigFileName(network->def->name); - - if (!configfile) { - virReportOOMError(); - goto cleanup; - } - unlink(configfile); - VIR_FREE(configfile); - - char *radvdpidbase = networkRadvdPidfileBasename(network->def->name); - - if (!(radvdpidbase)) { - virReportOOMError(); - goto cleanup; - } - virPidFileDelete(NETWORK_PID_DIR, radvdpidbase); - VIR_FREE(radvdpidbase); - + VIR_INFO("Undefining network '%s'", network->def->name); + if (networkRemoveInactive(driver, network) < 0) { + network = NULL; + goto cleanup; } - VIR_INFO("Undefining network '%s'", network->def->name); - virNetworkRemoveInactive(&driver->networks, - network); network = NULL; ret = 0; @@ -3085,10 +3099,15 @@ static int networkDestroy(virNetworkPtr net) { goto cleanup; } - ret = networkShutdownNetwork(driver, network); + if ((ret = networkShutdownNetwork(driver, network)) < 0) + goto cleanup; + if (!network->persistent) { - virNetworkRemoveInactive(&driver->networks, - network); + if (networkRemoveInactive(driver, network) < 0) { + network = NULL; + ret = -1; + goto cleanup; + } network = NULL; } -- 1.7.12.4

On 10/25/12 22:56, Laine Stump wrote: 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. Agreed, I didn't think about this option. Peter

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. --- Diff to v1: unconditionaly delete the config files even if they might not exist --- src/network/bridge_driver.c | 123 ++++++++++++++++++++++++-------------------- 1 file changed, 66 insertions(+), 57 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 08e55d9..ca254a3 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -155,6 +155,57 @@ 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; + virNetworkDefPtr def = virNetworkObjGetPersistentDef(net); + + int ret = -1; + + /* remove the (possibly) existing dnsmasq and radvd files */ + if (!(dctx = dnsmasqContextNew(def->name, DNSMASQ_STATE_DIR))) + goto cleanup; + + if (!(leasefile = networkDnsmasqLeaseFileName(def->name))) + goto cleanup; + + if (!(radvdconfigfile = networkRadvdConfigFileName(def->name))) + goto no_memory; + + if (!(radvdpidbase = networkRadvdPidfileBasename(def->name))) + goto no_memory; + + /* dnsmasq */ + dnsmasqDelete(dctx); + unlink(leasefile); + + /* radvd */ + unlink(radvdconfigfile); + virPidFileDelete(NETWORK_PID_DIR, radvdpidbase); + + /* remove the network definition */ + virNetworkRemoveInactive(&driver->networks, net); + + ret = 0; + +cleanup: + VIR_FREE(leasefile); + VIR_FREE(radvdconfigfile); + VIR_FREE(radvdpidbase); + dnsmasqContextFree(dctx); + return ret; + +no_memory: + virReportOOMError(); + goto cleanup; +} + static char * networkBridgeDummyNicName(const char *brname) { @@ -2822,12 +2873,11 @@ cleanup: return ret; } -static int networkUndefine(virNetworkPtr net) { +static int +networkUndefine(virNetworkPtr net) { struct network_driver *driver = net->conn->networkPrivateData; virNetworkObjPtr network; - virNetworkIpDefPtr ipdef; - bool dhcp_present = false, v6present = false; - int ret = -1, ii; + int ret = -1; networkDriverLock(driver); @@ -2849,58 +2899,12 @@ static int networkUndefine(virNetworkPtr net) { network) < 0) goto cleanup; - /* we only support dhcp on one IPv4 address per defined network */ - for (ii = 0; - (ipdef = virNetworkDefGetIpByIndex(network->def, AF_UNSPEC, ii)); - ii++) { - if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET)) { - if (ipdef->nranges || ipdef->nhosts) - dhcp_present = true; - } else if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET6)) { - v6present = true; - } - } - - if (dhcp_present) { - char *leasefile; - dnsmasqContext *dctx = dnsmasqContextNew(network->def->name, DNSMASQ_STATE_DIR); - if (dctx == NULL) - goto cleanup; - - dnsmasqDelete(dctx); - dnsmasqContextFree(dctx); - - leasefile = networkDnsmasqLeaseFileName(network->def->name); - if (!leasefile) - goto cleanup; - unlink(leasefile); - VIR_FREE(leasefile); - } - - if (v6present) { - char *configfile = networkRadvdConfigFileName(network->def->name); - - if (!configfile) { - virReportOOMError(); - goto cleanup; - } - unlink(configfile); - VIR_FREE(configfile); - - char *radvdpidbase = networkRadvdPidfileBasename(network->def->name); - - if (!(radvdpidbase)) { - virReportOOMError(); - goto cleanup; - } - virPidFileDelete(NETWORK_PID_DIR, radvdpidbase); - VIR_FREE(radvdpidbase); - + VIR_INFO("Undefining network '%s'", network->def->name); + if (networkRemoveInactive(driver, network) < 0) { + network = NULL; + goto cleanup; } - VIR_INFO("Undefining network '%s'", network->def->name); - virNetworkRemoveInactive(&driver->networks, - network); network = NULL; ret = 0; @@ -3101,10 +3105,15 @@ static int networkDestroy(virNetworkPtr net) { goto cleanup; } - ret = networkShutdownNetwork(driver, network); + if ((ret = networkShutdownNetwork(driver, network)) < 0) + goto cleanup; + if (!network->persistent) { - virNetworkRemoveInactive(&driver->networks, - network); + if (networkRemoveInactive(driver, network) < 0) { + network = NULL; + ret = -1; + goto cleanup; + } network = NULL; } -- 1.7.12.4

When a transient network was created some of the checks weren't run on the definition allowing to start invalid networks. This patch splits out code to the network validation function and re-uses that code when creating transient networks. --- src/network/bridge_driver.c | 96 +++++++++++++++++++-------------------------- 1 file changed, 40 insertions(+), 56 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 45fca0e..e90444d 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2689,11 +2689,48 @@ cleanup: static int -networkValidate(virNetworkDefPtr def) +networkValidate(struct network_driver *driver, + virNetworkDefPtr def, + bool check_active) { int ii; bool vlanUsed, vlanAllowed; const char *defaultPortGroup = NULL; + virNetworkIpDefPtr ipdef; + bool ipv4def = false; + int i; + + /* check for duplicate networks */ + if (virNetworkObjIsDuplicate(&driver->networks, def, check_active) < 0) + return -1; + + /* Only the three L3 network types that are configured by libvirt + * need to have a bridge device name / mac address provided + */ + if (def->forwardType == VIR_NETWORK_FORWARD_NONE || + def->forwardType == VIR_NETWORK_FORWARD_NAT || + def->forwardType == VIR_NETWORK_FORWARD_ROUTE) { + + if (virNetworkSetBridgeName(&driver->networks, def, 1)) + return -1; + + virNetworkSetBridgeMacAddr(def); + } + + /* We only support dhcp on one IPv4 address per defined network */ + for (i = 0; (ipdef = virNetworkDefGetIpByIndex(def, AF_INET, i)); i++) { + if (ipdef->nranges || ipdef->nhosts) { + if (ipv4def) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Multiple dhcp sections found. " + "dhcp is supported only for a " + "single IPv4 address on each network")); + return -1; + } else { + ipv4def = true; + } + } + } /* The only type of networks that currently support transparent * vlan configuration are those using hostdev sr-iov devices from @@ -2747,23 +2784,7 @@ static virNetworkPtr networkCreate(virConnectPtr conn, const char *xml) { if (!(def = virNetworkDefParseString(xml))) goto cleanup; - if (virNetworkObjIsDuplicate(&driver->networks, def, true) < 0) - goto cleanup; - - /* Only the three L3 network types that are configured by libvirt - * need to have a bridge device name / mac address provided - */ - if (def->forwardType == VIR_NETWORK_FORWARD_NONE || - def->forwardType == VIR_NETWORK_FORWARD_NAT || - def->forwardType == VIR_NETWORK_FORWARD_ROUTE) { - - if (virNetworkSetBridgeName(&driver->networks, def, 1)) - goto cleanup; - - virNetworkSetBridgeMacAddr(def); - } - - if (networkValidate(def) < 0) + if (networkValidate(driver, def, true) < 0) goto cleanup; /* NB: "live" is false because this transient network hasn't yet @@ -2793,54 +2814,17 @@ cleanup: static virNetworkPtr networkDefine(virConnectPtr conn, const char *xml) { struct network_driver *driver = conn->networkPrivateData; - virNetworkIpDefPtr ipdef, ipv4def = NULL; virNetworkDefPtr def; bool freeDef = true; virNetworkObjPtr network = NULL; virNetworkPtr ret = NULL; - int ii; networkDriverLock(driver); if (!(def = virNetworkDefParseString(xml))) goto cleanup; - if (virNetworkObjIsDuplicate(&driver->networks, def, false) < 0) - goto cleanup; - - /* Only the three L3 network types that are configured by libvirt - * need to have a bridge device name / mac address provided - */ - if (def->forwardType == VIR_NETWORK_FORWARD_NONE || - def->forwardType == VIR_NETWORK_FORWARD_NAT || - def->forwardType == VIR_NETWORK_FORWARD_ROUTE) { - - if (virNetworkSetBridgeName(&driver->networks, def, 1)) - goto cleanup; - - virNetworkSetBridgeMacAddr(def); - } - - /* We only support dhcp on one IPv4 address per defined network */ - for (ii = 0; - (ipdef = virNetworkDefGetIpByIndex(def, AF_UNSPEC, ii)); - ii++) { - if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET)) { - if (ipdef->nranges || ipdef->nhosts) { - if (ipv4def) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Multiple dhcp sections found. " - "dhcp is supported only for a " - "single IPv4 address on each network")); - goto cleanup; - } else { - ipv4def = ipdef; - } - } - } - } - - if (networkValidate(def) < 0) + if (networkValidate(driver, def, false) < 0) goto cleanup; if (!(network = virNetworkAssignDef(&driver->networks, def, false))) -- 1.7.12.4

Until now, the network undefine API was able to undefine only inactive networks. The restriction doesn't make sense any more so this patch implements changing networks to transient. --- src/network/bridge_driver.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index e90444d..95aaea9 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2856,6 +2856,7 @@ networkUndefine(virNetworkPtr net) { struct network_driver *driver = net->conn->networkPrivateData; virNetworkObjPtr network; int ret = -1; + bool active = false; networkDriverLock(driver); @@ -2866,24 +2867,25 @@ networkUndefine(virNetworkPtr net) { goto cleanup; } - if (virNetworkObjIsActive(network)) { - virReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("network is still active")); - goto cleanup; - } + if (virNetworkObjIsActive(network)) + active = true; if (virNetworkDeleteConfig(driver->networkConfigDir, driver->networkAutostartDir, network) < 0) goto cleanup; + network->persistent = 0; + VIR_INFO("Undefining network '%s'", network->def->name); - if (networkRemoveInactive(driver, network) < 0) { + if (!active) { + if (networkRemoveInactive(driver, network) < 0) { + network = NULL; + goto cleanup; + } network = NULL; - goto cleanup; } - network = NULL; ret = 0; cleanup: -- 1.7.12.4

When the assignment fails, the network object is not unlocked and next call that would use it deadlocks. --- src/conf/network_conf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 93d1b4c..1955853 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -295,6 +295,7 @@ virNetworkAssignDef(virNetworkObjListPtr nets, if ((network = virNetworkFindByName(nets, def->name))) { if (virNetworkObjAssignDef(network, def, live) < 0) { + virNetworkObjUnlock(network); return NULL; } return network; -- 1.7.12.4

When assigning the new persistent definition for a transient network (thus making it persistent) the network needs to be marked persistent before actually atempting to assign the definition. --- src/network/bridge_driver.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 22bd99b..f0dbd66 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2833,6 +2833,13 @@ static virNetworkPtr networkDefine(virConnectPtr conn, const char *xml) { if (networkValidate(driver, def, false) < 0) goto cleanup; + /* make the network persistent if we're defininig it */ + if ((network = virNetworkFindByName(&driver->networks, def->name))) { + network->persistent = 1; + virNetworkObjUnlock(network); + network = NULL; + } + if (!(network = virNetworkAssignDef(&driver->networks, def, false))) goto cleanup; freeDef = false; -- 1.7.12.4

When assigning the new persistent definition for a transient network (thus making it persistent) the network needs to be marked persistent before actually atempting to assign the definition. --- src/network/bridge_driver.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 22bd99b..643b00c 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2820,7 +2820,7 @@ cleanup: static virNetworkPtr networkDefine(virConnectPtr conn, const char *xml) { struct network_driver *driver = conn->networkPrivateData; - virNetworkDefPtr def; + virNetworkDefPtr def = NULL; bool freeDef = true; virNetworkObjPtr network = NULL; virNetworkPtr ret = NULL; @@ -2833,11 +2833,17 @@ static virNetworkPtr networkDefine(virConnectPtr conn, const char *xml) { if (networkValidate(driver, def, false) < 0) goto cleanup; - if (!(network = virNetworkAssignDef(&driver->networks, def, false))) - goto cleanup; - freeDef = false; + if ((network = virNetworkFindByName(&driver->networks, def->name))) { + network->persistent = 1; + if (virNetworkObjAssignDef(network, def, false) < 0) + goto cleanup; + } else { + if (!(network = virNetworkAssignDef(&driver->networks, def, false))) + goto cleanup; + } - network->persistent = 1; + /* def was asigned */ + freeDef = false; if (virNetworkSaveConfig(driver->networkConfigDir, def) < 0) { virNetworkRemoveInactive(&driver->networks, network); -- 1.7.12.4

On 11/01/2012 11:25 AM, Eric Blake wrote: Transient networks have never worked very well (mainly because nobody has ever used them) and the other patches in this series (plus more) are needed to make them work properly, so I think it makes more sense to save them all until after 1.0 is released.

On 2012年12月28日 10:50, Osier Yang wrote: Checked LXC and QEMU driver, qemu's defineXML is a bit better as it already tries to restore the previous def, but it still misses to restore vm->persistent. LXC driver is just like network driver, no restoring. I guess same problems are existing for other drivers which supports persistent object, so we will need a bunch of patches for them.