On 2012年12月28日 10:50, Osier Yang wrote:
On 2012年11月02日 01:30, Laine Stump wrote:
On 11/01/2012 11:25 AM, Eric Blake wrote:
On 10/29/2012 03:35 AM, Peter Krempa wrote:
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(-) You might want to get Laine's opinion as well, but I think this is a candidate for 1.0.0, and looks right to me.
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.
ACK.
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;
Except the introduced regression [1]. This could cause incorrect result once virNetworkObjAssignDef fails, though I believe it's unlikely to fail (Assuming there is a same network, but transient, it will be changed to persistent).
+ } 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);
And virNetworkRemoveInactive could be bindly removed a previous inactive network obj. Also we have to restore the previous network's def if virNetworkSaveConfig fails.
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.
[4] https://www.redhat.com/archives/libvir-list/2012-December/msg01355.html
Osier
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list