On Thu, Feb 26, 2015 at 15:17:10 +0100, Michal Privoznik wrote: UUID is the binary representation. You need to convert it to a string first. Peter

On Thu, Feb 26, 2015 at 15:17:15 +0100, Michal Privoznik wrote: Since the function does not free @nets at the end it should really be called virNetworkObjListClear. But that was present previously too. ACK as is. Peter

On Thu, Feb 26, 2015 at 15:17:17 +0100, Michal Privoznik wrote: Since you are going to convert the network structure to an objec wouldn't it be better to add a constructor for the object right away instead of having to do it later once you hide the struct? Otherwise looks good. Peter

On 26.02.2015 16:46, Peter Krempa wrote: That's just cosmetics IMO. The only benefit would be that the commit turning the struct into virObject would be a few lines shorter. /me goes and check how many VIR_ALLOCs are added in these patches. Er, two for virNetworkObj, and 4 for virNetworkObjList. So in the end we would save up 6 lines. I can do that if you think it's worth it though. Michal

On Thu, Feb 26, 2015 at 15:17:18 +0100, Michal Privoznik wrote: This is a bit confusing. The network object is allocated in the VM driver open function ... And deleted in the VM driver ... But is cleared in the network subdriver clenup function. I think it should be put into parallelsNetworkOpen. Peter

In order to hide the object internals (and use just accessors everywhere), lets store a pointer to the object, instead of object itself. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/network/bridge_driver.c | 98 +++++++++++++++++++----------------- src/network/bridge_driver_platform.h | 2 +- 2 files changed, 52 insertions(+), 48 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index edc4ed4..1011984 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -130,7 +130,7 @@ networkObjFromNetwork(virNetworkPtr net) char uuidstr[VIR_UUID_STRING_BUFLEN]; networkDriverLock(); - network = virNetworkFindByUUID(&driver->networks, net->uuid); + network = virNetworkFindByUUID(driver->networks, net->uuid); networkDriverUnlock(); if (!network) { @@ -303,7 +303,7 @@ networkRemoveInactive(virNetworkObjPtr net) unlink(statusfile); /* remove the network definition */ - virNetworkRemoveInactive(&driver->networks, net); + virNetworkRemoveInactive(driver->networks, net); ret = 0; @@ -350,8 +350,8 @@ networkUpdateAllState(void) { size_t i; - for (i = 0; i < driver->networks.count; i++) { - virNetworkObjPtr obj = driver->networks.objs[i]; + for (i = 0; i < driver->networks->count; i++) { + virNetworkObjPtr obj = driver->networks->objs[i]; virNetworkObjLock(obj); if (!virNetworkObjIsActive(obj)) { @@ -411,8 +411,8 @@ networkUpdateAllState(void) /* remove inactive transient networks */ i = 0; - while (i < driver->networks.count) { - virNetworkObjPtr obj = driver->networks.objs[i]; + while (i < driver->networks->count) { + virNetworkObjPtr obj = driver->networks->objs[i]; virNetworkObjLock(obj); if (!obj->persistent && !obj->active) { @@ -431,15 +431,15 @@ networkAutostartConfigs(void) { size_t i; - for (i = 0; i < driver->networks.count; i++) { - virNetworkObjLock(driver->networks.objs[i]); - if (driver->networks.objs[i]->autostart && - !virNetworkObjIsActive(driver->networks.objs[i])) { - if (networkStartNetwork(driver->networks.objs[i]) < 0) { + for (i = 0; i < driver->networks->count; i++) { + virNetworkObjLock(driver->networks->objs[i]); + if (driver->networks->objs[i]->autostart && + !virNetworkObjIsActive(driver->networks->objs[i])) { + if (networkStartNetwork(driver->networks->objs[i]) < 0) { /* failed to start but already logged */ } } - virNetworkObjUnlock(driver->networks.objs[i]); + virNetworkObjUnlock(driver->networks->objs[i]); } } @@ -638,11 +638,14 @@ networkStateInitialize(bool privileged, /* if this fails now, it will be retried later with dnsmasqCapsRefresh() */ driver->dnsmasqCaps = dnsmasqCapsNewFromBinary(DNSMASQ); - if (virNetworkLoadAllState(&driver->networks, + if (VIR_ALLOC(driver->networks) < 0) + goto error; + + if (virNetworkLoadAllState(driver->networks, driver->stateDir) < 0) goto error; - if (virNetworkLoadAllConfigs(&driver->networks, + if (virNetworkLoadAllConfigs(driver->networks, driver->networkConfigDir, driver->networkAutostartDir) < 0) goto error; @@ -723,9 +726,9 @@ networkStateReload(void) return 0; networkDriverLock(); - virNetworkLoadAllState(&driver->networks, + virNetworkLoadAllState(driver->networks, driver->stateDir); - virNetworkLoadAllConfigs(&driver->networks, + virNetworkLoadAllConfigs(driver->networks, driver->networkConfigDir, driver->networkAutostartDir); networkReloadFirewallRules(); @@ -752,7 +755,8 @@ networkStateCleanup(void) virObjectEventStateFree(driver->networkEventState); /* free inactive networks */ - virNetworkObjListFree(&driver->networks); + virNetworkObjListFree(driver->networks); + VIR_FREE(driver->networks); VIR_FREE(driver->networkConfigDir); VIR_FREE(driver->networkAutostartDir); @@ -1751,8 +1755,8 @@ networkRefreshDaemons(void) VIR_INFO("Refreshing network daemons"); - for (i = 0; i < driver->networks.count; i++) { - virNetworkObjPtr network = driver->networks.objs[i]; + for (i = 0; i < driver->networks->count; i++) { + virNetworkObjPtr network = driver->networks->objs[i]; virNetworkObjLock(network); if (virNetworkObjIsActive(network) && @@ -1779,8 +1783,8 @@ networkReloadFirewallRules(void) VIR_INFO("Reloading iptables rules"); - for (i = 0; i < driver->networks.count; i++) { - virNetworkObjPtr network = driver->networks.objs[i]; + for (i = 0; i < driver->networks->count; i++) { + virNetworkObjPtr network = driver->networks->objs[i]; virNetworkObjLock(network); if (virNetworkObjIsActive(network) && @@ -2472,7 +2476,7 @@ static virNetworkPtr networkLookupByUUID(virConnectPtr conn, virNetworkPtr ret = NULL; networkDriverLock(); - network = virNetworkFindByUUID(&driver->networks, uuid); + network = virNetworkFindByUUID(driver->networks, uuid); networkDriverUnlock(); if (!network) { virReportError(VIR_ERR_NO_NETWORK, @@ -2499,7 +2503,7 @@ static virNetworkPtr networkLookupByName(virConnectPtr conn, virNetworkPtr ret = NULL; networkDriverLock(); - network = virNetworkFindByName(&driver->networks, name); + network = virNetworkFindByName(driver->networks, name); networkDriverUnlock(); if (!network) { virReportError(VIR_ERR_NO_NETWORK, @@ -2527,8 +2531,8 @@ static int networkConnectNumOfNetworks(virConnectPtr conn) return -1; networkDriverLock(); - for (i = 0; i < driver->networks.count; i++) { - virNetworkObjPtr obj = driver->networks.objs[i]; + for (i = 0; i < driver->networks->count; i++) { + virNetworkObjPtr obj = driver->networks->objs[i]; virNetworkObjLock(obj); if (virConnectNumOfNetworksCheckACL(conn, obj->def) && virNetworkObjIsActive(obj)) @@ -2548,8 +2552,8 @@ static int networkConnectListNetworks(virConnectPtr conn, char **const names, in return -1; networkDriverLock(); - for (i = 0; i < driver->networks.count && got < nnames; i++) { - virNetworkObjPtr obj = driver->networks.objs[i]; + for (i = 0; i < driver->networks->count && got < nnames; i++) { + virNetworkObjPtr obj = driver->networks->objs[i]; virNetworkObjLock(obj); if (virConnectListNetworksCheckACL(conn, obj->def) && virNetworkObjIsActive(obj)) { @@ -2581,8 +2585,8 @@ static int networkConnectNumOfDefinedNetworks(virConnectPtr conn) return -1; networkDriverLock(); - for (i = 0; i < driver->networks.count; i++) { - virNetworkObjPtr obj = driver->networks.objs[i]; + for (i = 0; i < driver->networks->count; i++) { + virNetworkObjPtr obj = driver->networks->objs[i]; virNetworkObjLock(obj); if (virConnectNumOfDefinedNetworksCheckACL(conn, obj->def) && !virNetworkObjIsActive(obj)) @@ -2602,8 +2606,8 @@ static int networkConnectListDefinedNetworks(virConnectPtr conn, char **const na return -1; networkDriverLock(); - for (i = 0; i < driver->networks.count && got < nnames; i++) { - virNetworkObjPtr obj = driver->networks.objs[i]; + for (i = 0; i < driver->networks->count && got < nnames; i++) { + virNetworkObjPtr obj = driver->networks->objs[i]; virNetworkObjLock(obj); if (virConnectListDefinedNetworksCheckACL(conn, obj->def) && !virNetworkObjIsActive(obj)) { @@ -2638,7 +2642,7 @@ networkConnectListAllNetworks(virConnectPtr conn, goto cleanup; networkDriverLock(); - ret = virNetworkObjListExport(conn, &driver->networks, nets, + ret = virNetworkObjListExport(conn, driver->networks, nets, virConnectListAllNetworksCheckACL, flags); networkDriverUnlock(); @@ -2741,7 +2745,7 @@ networkValidate(virNetworkDefPtr def, bool usesInterface = false, usesAddress = false; /* check for duplicate networks */ - if (virNetworkObjIsDuplicate(&driver->networks, def, check_active) < 0) + if (virNetworkObjIsDuplicate(driver->networks, def, check_active) < 0) return -1; /* Only the three L3 network types that are configured by libvirt @@ -2751,7 +2755,7 @@ networkValidate(virNetworkDefPtr def, def->forward.type == VIR_NETWORK_FORWARD_NAT || def->forward.type == VIR_NETWORK_FORWARD_ROUTE) { - if (virNetworkSetBridgeName(&driver->networks, def, 1)) + if (virNetworkSetBridgeName(driver->networks, def, 1)) return -1; virNetworkSetBridgeMacAddr(def); @@ -2969,12 +2973,12 @@ static virNetworkPtr networkCreateXML(virConnectPtr conn, const char *xml) * we assign the def with live = true in anticipation that it will * be started momentarily. */ - if (!(network = virNetworkAssignDef(&driver->networks, def, true))) + if (!(network = virNetworkAssignDef(driver->networks, def, true))) goto cleanup; def = NULL; if (networkStartNetwork(network) < 0) { - virNetworkRemoveInactive(&driver->networks, + virNetworkRemoveInactive(driver->networks, network); network = NULL; goto cleanup; @@ -3017,7 +3021,7 @@ static virNetworkPtr networkDefineXML(virConnectPtr conn, const char *xml) if (networkValidate(def, false) < 0) goto cleanup; - if (!(network = virNetworkAssignDef(&driver->networks, def, false))) + if (!(network = virNetworkAssignDef(driver->networks, def, false))) goto cleanup; /* def was assigned to network object */ @@ -3025,7 +3029,7 @@ static virNetworkPtr networkDefineXML(virConnectPtr conn, const char *xml) if (virNetworkSaveConfig(driver->networkConfigDir, def) < 0) { if (!virNetworkObjIsActive(network)) { - virNetworkRemoveInactive(&driver->networks, network); + virNetworkRemoveInactive(driver->networks, network); network = NULL; goto cleanup; } @@ -3065,7 +3069,7 @@ networkUndefine(virNetworkPtr net) networkDriverLock(); - network = virNetworkFindByUUID(&driver->networks, net->uuid); + network = virNetworkFindByUUID(driver->networks, net->uuid); if (!network) { virReportError(VIR_ERR_NO_NETWORK, "%s", _("no network with matching uuid")); @@ -3138,7 +3142,7 @@ networkUpdate(virNetworkPtr net, networkDriverLock(); - network = virNetworkFindByUUID(&driver->networks, net->uuid); + network = virNetworkFindByUUID(driver->networks, net->uuid); if (!network) { virReportError(VIR_ERR_NO_NETWORK, "%s", _("no network with matching uuid")); @@ -3295,7 +3299,7 @@ static int networkCreate(virNetworkPtr net) virObjectEventPtr event = NULL; networkDriverLock(); - network = virNetworkFindByUUID(&driver->networks, net->uuid); + network = virNetworkFindByUUID(driver->networks, net->uuid); if (!network) { virReportError(VIR_ERR_NO_NETWORK, @@ -3330,7 +3334,7 @@ static int networkDestroy(virNetworkPtr net) virObjectEventPtr event = NULL; networkDriverLock(); - network = virNetworkFindByUUID(&driver->networks, net->uuid); + network = virNetworkFindByUUID(driver->networks, net->uuid); if (!network) { virReportError(VIR_ERR_NO_NETWORK, @@ -3456,7 +3460,7 @@ static int networkSetAutostart(virNetworkPtr net, int ret = -1; networkDriverLock(); - network = virNetworkFindByUUID(&driver->networks, net->uuid); + network = virNetworkFindByUUID(driver->networks, net->uuid); if (!network) { virReportError(VIR_ERR_NO_NETWORK, @@ -3786,7 +3790,7 @@ networkAllocateActualDevice(virDomainDefPtr dom, iface->data.network.actual = NULL; networkDriverLock(); - network = virNetworkFindByName(&driver->networks, iface->data.network.name); + network = virNetworkFindByName(driver->networks, iface->data.network.name); networkDriverUnlock(); if (!network) { virReportError(VIR_ERR_NO_NETWORK, @@ -4195,7 +4199,7 @@ networkNotifyActualDevice(virDomainDefPtr dom, return 0; networkDriverLock(); - network = virNetworkFindByName(&driver->networks, iface->data.network.name); + network = virNetworkFindByName(driver->networks, iface->data.network.name); networkDriverUnlock(); if (!network) { virReportError(VIR_ERR_NO_NETWORK, @@ -4395,7 +4399,7 @@ networkReleaseActualDevice(virDomainDefPtr dom, return 0; networkDriverLock(); - network = virNetworkFindByName(&driver->networks, iface->data.network.name); + network = virNetworkFindByName(driver->networks, iface->data.network.name); networkDriverUnlock(); if (!network) { virReportError(VIR_ERR_NO_NETWORK, @@ -4554,7 +4558,7 @@ networkGetNetworkAddress(const char *netname, char **netaddr) *netaddr = NULL; networkDriverLock(); - network = virNetworkFindByName(&driver->networks, netname); + network = virNetworkFindByName(driver->networks, netname); networkDriverUnlock(); if (!network) { virReportError(VIR_ERR_NO_NETWORK, diff --git a/src/network/bridge_driver_platform.h b/src/network/bridge_driver_platform.h index 1e8264a..b7492e6 100644 --- a/src/network/bridge_driver_platform.h +++ b/src/network/bridge_driver_platform.h @@ -34,7 +34,7 @@ struct _virNetworkDriverState { virMutex lock; - virNetworkObjList networks; + virNetworkObjListPtr networks; char *networkConfigDir; char *networkAutostartDir; -- 2.0.5

It's returning virNetworkObjPtr after all. And it matches the pattern laid out by domain_conf.h. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/conf/network_conf.c | 6 +++--- src/conf/network_conf.h | 4 ++-- src/libvirt_private.syms | 2 +- src/network/bridge_driver.c | 14 +++++++------- src/parallels/parallels_network.c | 10 +++++----- src/test/test_driver.c | 8 ++++---- 6 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 00be8b6..30fb309 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -86,8 +86,8 @@ virNetworkObjTaint(virNetworkObjPtr obj, return true; } -virNetworkObjPtr virNetworkFindByUUID(virNetworkObjListPtr nets, - const unsigned char *uuid) +virNetworkObjPtr virNetworkObjFindByUUID(virNetworkObjListPtr nets, + const unsigned char *uuid) { size_t i; @@ -4150,7 +4150,7 @@ virNetworkObjIsDuplicate(virNetworkObjListPtr nets, virNetworkObjPtr net = NULL; /* See if a network with matching UUID already exists */ - net = virNetworkFindByUUID(nets, def->uuid); + net = virNetworkObjFindByUUID(nets, def->uuid); if (net) { /* UUID matches, but if names don't match, refuse it */ if (STRNEQ(net->def->name, def->name)) { diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 74665e0..3298376 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -301,8 +301,8 @@ virNetworkObjIsActive(const virNetworkObj *net) bool virNetworkObjTaint(virNetworkObjPtr obj, virNetworkTaintFlags taint); -virNetworkObjPtr virNetworkFindByUUID(virNetworkObjListPtr nets, - const unsigned char *uuid); +virNetworkObjPtr virNetworkObjFindByUUID(virNetworkObjListPtr nets, + const unsigned char *uuid); virNetworkObjPtr virNetworkFindByName(virNetworkObjListPtr nets, const char *name); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ba05cc6..140f3f4 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -559,13 +559,13 @@ virNetworkDefParseString; virNetworkDefUpdateSection; virNetworkDeleteConfig; virNetworkFindByName; -virNetworkFindByUUID; virNetworkForwardTypeToString; virNetworkIpDefNetmask; virNetworkIpDefPrefix; virNetworkLoadAllConfigs; virNetworkLoadAllState; virNetworkObjAssignDef; +virNetworkObjFindByUUID; virNetworkObjFree; virNetworkObjGetPersistentDef; virNetworkObjIsDuplicate; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 1011984..f67822a 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -130,7 +130,7 @@ networkObjFromNetwork(virNetworkPtr net) char uuidstr[VIR_UUID_STRING_BUFLEN]; networkDriverLock(); - network = virNetworkFindByUUID(driver->networks, net->uuid); + network = virNetworkObjFindByUUID(driver->networks, net->uuid); networkDriverUnlock(); if (!network) { @@ -2476,7 +2476,7 @@ static virNetworkPtr networkLookupByUUID(virConnectPtr conn, virNetworkPtr ret = NULL; networkDriverLock(); - network = virNetworkFindByUUID(driver->networks, uuid); + network = virNetworkObjFindByUUID(driver->networks, uuid); networkDriverUnlock(); if (!network) { virReportError(VIR_ERR_NO_NETWORK, @@ -3069,7 +3069,7 @@ networkUndefine(virNetworkPtr net) networkDriverLock(); - network = virNetworkFindByUUID(driver->networks, net->uuid); + network = virNetworkObjFindByUUID(driver->networks, net->uuid); if (!network) { virReportError(VIR_ERR_NO_NETWORK, "%s", _("no network with matching uuid")); @@ -3142,7 +3142,7 @@ networkUpdate(virNetworkPtr net, networkDriverLock(); - network = virNetworkFindByUUID(driver->networks, net->uuid); + network = virNetworkObjFindByUUID(driver->networks, net->uuid); if (!network) { virReportError(VIR_ERR_NO_NETWORK, "%s", _("no network with matching uuid")); @@ -3299,7 +3299,7 @@ static int networkCreate(virNetworkPtr net) virObjectEventPtr event = NULL; networkDriverLock(); - network = virNetworkFindByUUID(driver->networks, net->uuid); + network = virNetworkObjFindByUUID(driver->networks, net->uuid); if (!network) { virReportError(VIR_ERR_NO_NETWORK, @@ -3334,7 +3334,7 @@ static int networkDestroy(virNetworkPtr net) virObjectEventPtr event = NULL; networkDriverLock(); - network = virNetworkFindByUUID(driver->networks, net->uuid); + network = virNetworkObjFindByUUID(driver->networks, net->uuid); if (!network) { virReportError(VIR_ERR_NO_NETWORK, @@ -3460,7 +3460,7 @@ static int networkSetAutostart(virNetworkPtr net, int ret = -1; networkDriverLock(); - network = virNetworkFindByUUID(driver->networks, net->uuid); + network = virNetworkObjFindByUUID(driver->networks, net->uuid); if (!network) { virReportError(VIR_ERR_NO_NETWORK, diff --git a/src/parallels/parallels_network.c b/src/parallels/parallels_network.c index bfa7432..23eee89 100644 --- a/src/parallels/parallels_network.c +++ b/src/parallels/parallels_network.c @@ -463,7 +463,7 @@ static virNetworkPtr parallelsNetworkLookupByUUID(virConnectPtr conn, virNetworkPtr ret = NULL; parallelsDriverLock(privconn); - network = virNetworkFindByUUID(privconn->networks, uuid); + network = virNetworkObjFindByUUID(privconn->networks, uuid); parallelsDriverUnlock(privconn); if (!network) { virReportError(VIR_ERR_NO_NETWORK, @@ -513,7 +513,7 @@ static char *parallelsNetworkGetXMLDesc(virNetworkPtr net, virCheckFlags(VIR_NETWORK_XML_INACTIVE, NULL); parallelsDriverLock(privconn); - network = virNetworkFindByUUID(privconn->networks, net->uuid); + network = virNetworkObjFindByUUID(privconn->networks, net->uuid); parallelsDriverUnlock(privconn); if (!network) { @@ -537,7 +537,7 @@ static int parallelsNetworkIsActive(virNetworkPtr net) int ret = -1; parallelsDriverLock(privconn); - obj = virNetworkFindByUUID(privconn->networks, net->uuid); + obj = virNetworkObjFindByUUID(privconn->networks, net->uuid); parallelsDriverUnlock(privconn); if (!obj) { virReportError(VIR_ERR_NO_NETWORK, NULL); @@ -558,7 +558,7 @@ static int parallelsNetworkIsPersistent(virNetworkPtr net) int ret = -1; parallelsDriverLock(privconn); - obj = virNetworkFindByUUID(privconn->networks, net->uuid); + obj = virNetworkObjFindByUUID(privconn->networks, net->uuid); parallelsDriverUnlock(privconn); if (!obj) { virReportError(VIR_ERR_NO_NETWORK, NULL); @@ -580,7 +580,7 @@ static int parallelsNetworkGetAutostart(virNetworkPtr net, int ret = -1; parallelsDriverLock(privconn); - network = virNetworkFindByUUID(privconn->networks, net->uuid); + network = virNetworkObjFindByUUID(privconn->networks, net->uuid); parallelsDriverUnlock(privconn); if (!network) { virReportError(VIR_ERR_NO_NETWORK, diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 90df0e7..a747704 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -3499,7 +3499,7 @@ static virNetworkPtr testNetworkLookupByUUID(virConnectPtr conn, virNetworkPtr ret = NULL; testDriverLock(privconn); - net = virNetworkFindByUUID(privconn->networks, uuid); + net = virNetworkObjFindByUUID(privconn->networks, uuid); testDriverUnlock(privconn); if (net == NULL) { @@ -3654,7 +3654,7 @@ static int testNetworkIsActive(virNetworkPtr net) int ret = -1; testDriverLock(privconn); - obj = virNetworkFindByUUID(privconn->networks, net->uuid); + obj = virNetworkObjFindByUUID(privconn->networks, net->uuid); testDriverUnlock(privconn); if (!obj) { virReportError(VIR_ERR_NO_NETWORK, NULL); @@ -3675,7 +3675,7 @@ static int testNetworkIsPersistent(virNetworkPtr net) int ret = -1; testDriverLock(privconn); - obj = virNetworkFindByUUID(privconn->networks, net->uuid); + obj = virNetworkObjFindByUUID(privconn->networks, net->uuid); testDriverUnlock(privconn); if (!obj) { virReportError(VIR_ERR_NO_NETWORK, NULL); @@ -3812,7 +3812,7 @@ testNetworkUpdate(virNetworkPtr net, testDriverLock(privconn); - network = virNetworkFindByUUID(privconn->networks, net->uuid); + network = virNetworkObjFindByUUID(privconn->networks, net->uuid); if (!network) { virReportError(VIR_ERR_NO_NETWORK, "%s", _("no network with matching uuid")); -- 2.0.5

It's returning virNetworkObjPtr after all. And it matches the pattern laid out by domain_conf.h. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/conf/network_conf.c | 8 ++++---- src/conf/network_conf.h | 4 ++-- src/libvirt_private.syms | 2 +- src/network/bridge_driver.c | 10 +++++----- src/parallels/parallels_network.c | 2 +- src/test/test_driver.c | 16 ++++++++-------- 6 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 30fb309..d0e7e1b 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -101,8 +101,8 @@ virNetworkObjPtr virNetworkObjFindByUUID(virNetworkObjListPtr nets, return NULL; } -virNetworkObjPtr virNetworkFindByName(virNetworkObjListPtr nets, - const char *name) +virNetworkObjPtr virNetworkObjFindByName(virNetworkObjListPtr nets, + const char *name) { size_t i; @@ -370,7 +370,7 @@ virNetworkAssignDef(virNetworkObjListPtr nets, { virNetworkObjPtr network; - if ((network = virNetworkFindByName(nets, def->name))) { + if ((network = virNetworkObjFindByName(nets, def->name))) { virNetworkObjAssignDef(network, def, live); return network; } @@ -4175,7 +4175,7 @@ virNetworkObjIsDuplicate(virNetworkObjListPtr nets, ret = 1; } else { /* UUID does not match, but if a name matches, refuse it */ - net = virNetworkFindByName(nets, def->name); + net = virNetworkObjFindByName(nets, def->name); if (net) { char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(net->def->uuid, uuidstr); diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 3298376..16aea99 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -303,8 +303,8 @@ bool virNetworkObjTaint(virNetworkObjPtr obj, virNetworkObjPtr virNetworkObjFindByUUID(virNetworkObjListPtr nets, const unsigned char *uuid); -virNetworkObjPtr virNetworkFindByName(virNetworkObjListPtr nets, - const char *name); +virNetworkObjPtr virNetworkObjFindByName(virNetworkObjListPtr nets, + const char *name); void virNetworkDefFree(virNetworkDefPtr def); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 140f3f4..cbba8a9 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -558,13 +558,13 @@ virNetworkDefParseNode; virNetworkDefParseString; virNetworkDefUpdateSection; virNetworkDeleteConfig; -virNetworkFindByName; virNetworkForwardTypeToString; virNetworkIpDefNetmask; virNetworkIpDefPrefix; virNetworkLoadAllConfigs; virNetworkLoadAllState; virNetworkObjAssignDef; +virNetworkObjFindByName; virNetworkObjFindByUUID; virNetworkObjFree; virNetworkObjGetPersistentDef; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index f67822a..268af49 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2503,7 +2503,7 @@ static virNetworkPtr networkLookupByName(virConnectPtr conn, virNetworkPtr ret = NULL; networkDriverLock(); - network = virNetworkFindByName(driver->networks, name); + network = virNetworkObjFindByName(driver->networks, name); networkDriverUnlock(); if (!network) { virReportError(VIR_ERR_NO_NETWORK, @@ -3790,7 +3790,7 @@ networkAllocateActualDevice(virDomainDefPtr dom, iface->data.network.actual = NULL; networkDriverLock(); - network = virNetworkFindByName(driver->networks, iface->data.network.name); + network = virNetworkObjFindByName(driver->networks, iface->data.network.name); networkDriverUnlock(); if (!network) { virReportError(VIR_ERR_NO_NETWORK, @@ -4199,7 +4199,7 @@ networkNotifyActualDevice(virDomainDefPtr dom, return 0; networkDriverLock(); - network = virNetworkFindByName(driver->networks, iface->data.network.name); + network = virNetworkObjFindByName(driver->networks, iface->data.network.name); networkDriverUnlock(); if (!network) { virReportError(VIR_ERR_NO_NETWORK, @@ -4399,7 +4399,7 @@ networkReleaseActualDevice(virDomainDefPtr dom, return 0; networkDriverLock(); - network = virNetworkFindByName(driver->networks, iface->data.network.name); + network = virNetworkObjFindByName(driver->networks, iface->data.network.name); networkDriverUnlock(); if (!network) { virReportError(VIR_ERR_NO_NETWORK, @@ -4558,7 +4558,7 @@ networkGetNetworkAddress(const char *netname, char **netaddr) *netaddr = NULL; networkDriverLock(); - network = virNetworkFindByName(driver->networks, netname); + network = virNetworkObjFindByName(driver->networks, netname); networkDriverUnlock(); if (!network) { virReportError(VIR_ERR_NO_NETWORK, diff --git a/src/parallels/parallels_network.c b/src/parallels/parallels_network.c index 23eee89..94e77cc 100644 --- a/src/parallels/parallels_network.c +++ b/src/parallels/parallels_network.c @@ -487,7 +487,7 @@ static virNetworkPtr parallelsNetworkLookupByName(virConnectPtr conn, virNetworkPtr ret = NULL; parallelsDriverLock(privconn); - network = virNetworkFindByName(privconn->networks, name); + network = virNetworkObjFindByName(privconn->networks, name); parallelsDriverUnlock(privconn); if (!network) { virReportError(VIR_ERR_NO_NETWORK, diff --git a/src/test/test_driver.c b/src/test/test_driver.c index a747704..ecb7bcd 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -3523,7 +3523,7 @@ static virNetworkPtr testNetworkLookupByName(virConnectPtr conn, virNetworkPtr ret = NULL; testDriverLock(privconn); - net = virNetworkFindByName(privconn->networks, name); + net = virNetworkObjFindByName(privconn->networks, name); testDriverUnlock(privconn); if (net == NULL) { @@ -3764,7 +3764,7 @@ static int testNetworkUndefine(virNetworkPtr network) virObjectEventPtr event = NULL; testDriverLock(privconn); - privnet = virNetworkFindByName(privconn->networks, network->name); + privnet = virNetworkObjFindByName(privconn->networks, network->name); if (privnet == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); @@ -3852,7 +3852,7 @@ static int testNetworkCreate(virNetworkPtr network) virObjectEventPtr event = NULL; testDriverLock(privconn); - privnet = virNetworkFindByName(privconn->networks, network->name); + privnet = virNetworkObjFindByName(privconn->networks, network->name); testDriverUnlock(privconn); if (privnet == NULL) { @@ -3888,7 +3888,7 @@ static int testNetworkDestroy(virNetworkPtr network) virObjectEventPtr event = NULL; testDriverLock(privconn); - privnet = virNetworkFindByName(privconn->networks, network->name); + privnet = virNetworkObjFindByName(privconn->networks, network->name); if (privnet == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); @@ -3924,7 +3924,7 @@ static char *testNetworkGetXMLDesc(virNetworkPtr network, virCheckFlags(0, NULL); testDriverLock(privconn); - privnet = virNetworkFindByName(privconn->networks, network->name); + privnet = virNetworkObjFindByName(privconn->networks, network->name); testDriverUnlock(privconn); if (privnet == NULL) { @@ -3946,7 +3946,7 @@ static char *testNetworkGetBridgeName(virNetworkPtr network) { virNetworkObjPtr privnet; testDriverLock(privconn); - privnet = virNetworkFindByName(privconn->networks, network->name); + privnet = virNetworkObjFindByName(privconn->networks, network->name); testDriverUnlock(privconn); if (privnet == NULL) { @@ -3977,7 +3977,7 @@ static int testNetworkGetAutostart(virNetworkPtr network, int ret = -1; testDriverLock(privconn); - privnet = virNetworkFindByName(privconn->networks, network->name); + privnet = virNetworkObjFindByName(privconn->networks, network->name); testDriverUnlock(privconn); if (privnet == NULL) { @@ -4002,7 +4002,7 @@ static int testNetworkSetAutostart(virNetworkPtr network, int ret = -1; testDriverLock(privconn); - privnet = virNetworkFindByName(privconn->networks, network->name); + privnet = virNetworkObjFindByName(privconn->networks, network->name); testDriverUnlock(privconn); if (privnet == NULL) { -- 2.0.5

This API will be used in the future to call passed callback over each network object in the list. It's slightly different to it virDomainObjListForEach counterpart, because virDomainObjList uses a hash table to store domain object, while virNetworkObjList uses an array. Therefore, iterating over the entries is slightly more complicated, as we want to allow the callback to remove the network object while still iterating. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/conf/network_conf.c | 22 ++++++++++++++++++++++ src/conf/network_conf.h | 6 ++++++ src/libvirt_private.syms | 1 + 3 files changed, 29 insertions(+) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index d0e7e1b..bcfcccd 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -4290,3 +4290,25 @@ virNetworkObjListExport(virConnectPtr conn, VIR_FREE(tmp_nets); return ret; } + +int +virNetworkObjListForEach(virNetworkObjListPtr nets, + virNetworkObjListIterator callback, + void *opaque) +{ + int ret = 0; + size_t i = 0, oldcount; + + /* Deliberately don't use for-loop here. We want to allow + * @callback to remove a network object, in which case + * nets->count is decremented. */ + while (i < nets->count) { + oldcount = nets->count; + if (callback(nets->objs[i], opaque) < 0) + ret = -1; + if (oldcount == nets->count) + i++; + } + + return ret; +} diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 16aea99..b9acc0e 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -445,6 +445,12 @@ int virNetworkObjListExport(virConnectPtr conn, virNetworkObjListFilter filter, unsigned int flags); +typedef int (*virNetworkObjListIterator)(virNetworkObjPtr dom, + void *opaque); + +int virNetworkObjListForEach(virNetworkObjListPtr nets, + virNetworkObjListIterator callback, + void *opaque); /* for testing */ int virNetworkDefUpdateSection(virNetworkDefPtr def, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index cbba8a9..8577006 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -570,6 +570,7 @@ virNetworkObjFree; virNetworkObjGetPersistentDef; virNetworkObjIsDuplicate; virNetworkObjListExport; +virNetworkObjListForEach; virNetworkObjListFree; virNetworkObjLock; virNetworkObjReplacePersistentDef; -- 2.0.5

An accessor following pattern laid out by virDomainObjList* APIs. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/conf/network_conf.c | 38 ++++++++++++++++++++++++++++++++++++++ src/conf/network_conf.h | 8 ++++++++ src/libvirt_private.syms | 1 + 3 files changed, 47 insertions(+) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index bcfcccd..84becb5 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -4312,3 +4312,41 @@ virNetworkObjListForEach(virNetworkObjListPtr nets, return ret; } + +int +virNetworkObjListGetNames(virNetworkObjListPtr nets, + bool active, + char **names, + int nnames, + virNetworkObjListFilter filter, + virConnectPtr conn) +{ + int got = 0; + size_t i; + + for (i = 0; i < nets->count && got < nnames; i++) { + virNetworkObjPtr obj = nets->objs[i]; + virNetworkObjLock(obj); + if (filter && !filter(conn, obj->def)) { + virNetworkObjUnlock(obj); + continue; + } + + if ((active && virNetworkObjIsActive(obj)) || + (!active && !virNetworkObjIsActive(obj))) { + if (VIR_STRDUP(names[got], obj->def->name) < 0) { + virNetworkObjUnlock(obj); + goto error; + } + got++; + } + virNetworkObjUnlock(obj); + } + + return got; + + error: + for (i = 0; i < got; i++) + VIR_FREE(names[i]); + return -1; +} diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index b9acc0e..d357892 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -451,6 +451,14 @@ typedef int (*virNetworkObjListIterator)(virNetworkObjPtr dom, int virNetworkObjListForEach(virNetworkObjListPtr nets, virNetworkObjListIterator callback, void *opaque); + +int virNetworkObjListGetNames(virNetworkObjListPtr nets, + bool active, + char **names, + int nnames, + virNetworkObjListFilter filter, + virConnectPtr conn); + /* for testing */ int virNetworkDefUpdateSection(virNetworkDefPtr def, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8577006..3e7e389 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -572,6 +572,7 @@ virNetworkObjIsDuplicate; virNetworkObjListExport; virNetworkObjListForEach; virNetworkObjListFree; +virNetworkObjListGetNames; virNetworkObjLock; virNetworkObjReplacePersistentDef; virNetworkObjSetDefTransient; -- 2.0.5

An accessor following pattern laid out by virDomainObjList* APIs. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/conf/network_conf.c | 26 ++++++++++++++++++++++++++ src/conf/network_conf.h | 5 +++++ src/libvirt_private.syms | 1 + 3 files changed, 32 insertions(+) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 84becb5..24a5f7c 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -4350,3 +4350,29 @@ virNetworkObjListGetNames(virNetworkObjListPtr nets, VIR_FREE(names[i]); return -1; } + +int +virNetworkObjListNumOfNetworks(virNetworkObjListPtr nets, + bool active, + virNetworkObjListFilter filter, + virConnectPtr conn) +{ + int count = 0; + size_t i; + + for (i = 0; i < nets->count; i++) { + virNetworkObjPtr obj = nets->objs[i]; + virNetworkObjLock(obj); + if (filter && !filter(conn, obj->def)) { + virNetworkObjUnlock(obj); + continue; + } + + if ((active && virNetworkObjIsActive(obj)) || + (!active && !virNetworkObjIsActive(obj))) + count++; + virNetworkObjUnlock(obj); + } + + return count; +} diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index d357892..164fb1a 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -459,6 +459,11 @@ int virNetworkObjListGetNames(virNetworkObjListPtr nets, virNetworkObjListFilter filter, virConnectPtr conn); +int virNetworkObjListNumOfNetworks(virNetworkObjListPtr nets, + bool active, + virNetworkObjListFilter filter, + virConnectPtr conn); + /* for testing */ int virNetworkDefUpdateSection(virNetworkDefPtr def, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3e7e389..8aacc30 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -573,6 +573,7 @@ virNetworkObjListExport; virNetworkObjListForEach; virNetworkObjListFree; virNetworkObjListGetNames; +virNetworkObjListNumOfNetworks; virNetworkObjLock; virNetworkObjReplacePersistentDef; virNetworkObjSetDefTransient; -- 2.0.5

Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/network/bridge_driver.c | 333 ++++++++++++++++++++------------------------ 1 file changed, 148 insertions(+), 185 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 268af49..1c73342 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -342,105 +342,91 @@ networkBridgeDummyNicName(const char *brname) return nicname; } -/* Update the internal status of all allegedly active networks - * according to external conditions on the host (i.e. anything that - * isn't stored directly in each network's state file). */ -static void -networkUpdateAllState(void) +static int +networkUpdateAllState(virNetworkObjPtr obj, + void *opaque ATTRIBUTE_UNUSED) { - size_t i; + int ret = -1; - for (i = 0; i < driver->networks->count; i++) { - virNetworkObjPtr obj = driver->networks->objs[i]; + virNetworkObjLock(obj); + if (!virNetworkObjIsActive(obj)) { + virNetworkObjUnlock(obj); + return 0; + } - virNetworkObjLock(obj); - if (!virNetworkObjIsActive(obj)) { - virNetworkObjUnlock(obj); - continue; - } + switch (obj->def->forward.type) { + case VIR_NETWORK_FORWARD_NONE: + case VIR_NETWORK_FORWARD_NAT: + case VIR_NETWORK_FORWARD_ROUTE: + /* If bridge doesn't exist, then mark it inactive */ + if (!(obj->def->bridge && virNetDevExists(obj->def->bridge) == 1)) + obj->active = 0; + break; - switch (obj->def->forward.type) { - case VIR_NETWORK_FORWARD_NONE: - case VIR_NETWORK_FORWARD_NAT: - case VIR_NETWORK_FORWARD_ROUTE: - /* If bridge doesn't exist, then mark it inactive */ - if (!(obj->def->bridge && virNetDevExists(obj->def->bridge) == 1)) + case VIR_NETWORK_FORWARD_BRIDGE: + if (obj->def->bridge) { + if (virNetDevExists(obj->def->bridge) != 1) obj->active = 0; break; - - case VIR_NETWORK_FORWARD_BRIDGE: - if (obj->def->bridge) { - if (virNetDevExists(obj->def->bridge) != 1) - obj->active = 0; - break; - } - /* intentionally drop through to common case for all - * macvtap networks (forward='bridge' with no bridge - * device defined is macvtap using its 'bridge' mode) - */ - case VIR_NETWORK_FORWARD_PRIVATE: - case VIR_NETWORK_FORWARD_VEPA: - case VIR_NETWORK_FORWARD_PASSTHROUGH: - /* so far no extra checks */ - break; - - case VIR_NETWORK_FORWARD_HOSTDEV: - /* so far no extra checks */ - break; - } - - /* Try and read dnsmasq/radvd pids of active networks */ - if (obj->active && obj->def->ips && (obj->def->nips > 0)) { - char *radvdpidbase; - - ignore_value(virPidFileReadIfAlive(driver->pidDir, - obj->def->name, - &obj->dnsmasqPid, - dnsmasqCapsGetBinaryPath(driver->dnsmasqCaps))); - radvdpidbase = networkRadvdPidfileBasename(obj->def->name); - if (!radvdpidbase) - break; - ignore_value(virPidFileReadIfAlive(driver->pidDir, - radvdpidbase, - &obj->radvdPid, RADVD)); - VIR_FREE(radvdpidbase); } - - virNetworkObjUnlock(obj); + /* intentionally drop through to common case for all + * macvtap networks (forward='bridge' with no bridge + * device defined is macvtap using its 'bridge' mode) + */ + case VIR_NETWORK_FORWARD_PRIVATE: + case VIR_NETWORK_FORWARD_VEPA: + case VIR_NETWORK_FORWARD_PASSTHROUGH: + /* so far no extra checks */ + break; + + case VIR_NETWORK_FORWARD_HOSTDEV: + /* so far no extra checks */ + break; } - /* remove inactive transient networks */ - i = 0; - while (i < driver->networks->count) { - virNetworkObjPtr obj = driver->networks->objs[i]; - virNetworkObjLock(obj); - - if (!obj->persistent && !obj->active) { - networkRemoveInactive(obj); - continue; - } + /* Try and read dnsmasq/radvd pids of active networks */ + if (obj->active && obj->def->ips && (obj->def->nips > 0)) { + char *radvdpidbase; + + ignore_value(virPidFileReadIfAlive(driver->pidDir, + obj->def->name, + &obj->dnsmasqPid, + dnsmasqCapsGetBinaryPath(driver->dnsmasqCaps))); + radvdpidbase = networkRadvdPidfileBasename(obj->def->name); + if (!radvdpidbase) + goto cleanup; + + ignore_value(virPidFileReadIfAlive(driver->pidDir, + radvdpidbase, + &obj->radvdPid, RADVD)); + VIR_FREE(radvdpidbase); + } + ret = 0; + cleanup: + if (!obj->persistent && !obj->active) + networkRemoveInactive(obj); + else virNetworkObjUnlock(obj); - i++; - } + return ret; } - -static void -networkAutostartConfigs(void) +static int +networkAutostartConfigs(virNetworkObjPtr net, + void *opaque ATTRIBUTE_UNUSED) { - size_t i; + int ret = -1; - for (i = 0; i < driver->networks->count; i++) { - virNetworkObjLock(driver->networks->objs[i]); - if (driver->networks->objs[i]->autostart && - !virNetworkObjIsActive(driver->networks->objs[i])) { - if (networkStartNetwork(driver->networks->objs[i]) < 0) { - /* failed to start but already logged */ - } - } - virNetworkObjUnlock(driver->networks->objs[i]); - } + virNetworkObjLock(net); + if (net->autostart && + !virNetworkObjIsActive(net) && + networkStartNetwork(net) < 0) + goto cleanup; + + ret = 0; + cleanup: + virNetworkObjUnlock(net); + return ret; } #if HAVE_FIREWALLD @@ -650,7 +636,14 @@ networkStateInitialize(bool privileged, driver->networkAutostartDir) < 0) goto error; - networkUpdateAllState(); + + /* Update the internal status of all allegedly active + * networks according to external conditions on the host + * (i.e. anything that isn't stored directly in each + * network's state file). */ + virNetworkObjListForEach(driver->networks, + networkUpdateAllState, + NULL); networkReloadFirewallRules(); networkRefreshDaemons(); @@ -709,7 +702,9 @@ networkStateAutoStart(void) return; networkDriverLock(); - networkAutostartConfigs(); + virNetworkObjListForEach(driver->networks, + networkAutostartConfigs, + NULL); networkDriverUnlock(); } @@ -733,7 +728,9 @@ networkStateReload(void) driver->networkAutostartDir); networkReloadFirewallRules(); networkRefreshDaemons(); - networkAutostartConfigs(); + virNetworkObjListForEach(driver->networks, + networkAutostartConfigs, + NULL); networkDriverUnlock(); return 0; } @@ -1745,62 +1742,70 @@ networkRestartRadvd(virNetworkObjPtr network) } #endif /* #if 0 */ +static int +networkRefreshDaemonsHelper(virNetworkObjPtr net, + void *opaque ATTRIBUTE_UNUSED) +{ + + virNetworkObjLock(net); + if (virNetworkObjIsActive(net) && + ((net->def->forward.type == VIR_NETWORK_FORWARD_NONE) || + (net->def->forward.type == VIR_NETWORK_FORWARD_NAT) || + (net->def->forward.type == VIR_NETWORK_FORWARD_ROUTE))) { + /* Only the three L3 network types that are configured by + * libvirt will have a dnsmasq or radvd daemon associated + * with them. Here we send a SIGHUP to an existing + * dnsmasq and/or radvd, or restart them if they've + * disappeared. + */ + networkRefreshDhcpDaemon(net); + networkRefreshRadvd(net); + } + virNetworkObjUnlock(net); + return 0; +} + /* SIGHUP/restart any dnsmasq or radvd daemons. * This should be called when libvirtd is restarted. */ static void networkRefreshDaemons(void) { - size_t i; - VIR_INFO("Refreshing network daemons"); + virNetworkObjListForEach(driver->networks, + networkRefreshDaemonsHelper, + NULL); +} - for (i = 0; i < driver->networks->count; i++) { - virNetworkObjPtr network = driver->networks->objs[i]; +static int +networkReloadFirewallRulesHelper(virNetworkObjPtr net, + void *opaque ATTRIBUTE_UNUSED) +{ - virNetworkObjLock(network); - if (virNetworkObjIsActive(network) && - ((network->def->forward.type == VIR_NETWORK_FORWARD_NONE) || - (network->def->forward.type == VIR_NETWORK_FORWARD_NAT) || - (network->def->forward.type == VIR_NETWORK_FORWARD_ROUTE))) { - /* Only the three L3 network types that are configured by - * libvirt will have a dnsmasq or radvd daemon associated - * with them. Here we send a SIGHUP to an existing - * dnsmasq and/or radvd, or restart them if they've - * disappeared. - */ - networkRefreshDhcpDaemon(network); - networkRefreshRadvd(network); + virNetworkObjLock(net); + if (virNetworkObjIsActive(net) && + ((net->def->forward.type == VIR_NETWORK_FORWARD_NONE) || + (net->def->forward.type == VIR_NETWORK_FORWARD_NAT) || + (net->def->forward.type == VIR_NETWORK_FORWARD_ROUTE))) { + /* Only the three L3 network types that are configured by libvirt + * need to have iptables rules reloaded. + */ + networkRemoveFirewallRules(net->def); + if (networkAddFirewallRules(net->def) < 0) { + /* failed to add but already logged */ } - virNetworkObjUnlock(network); } + virNetworkObjUnlock(net); + return 0; } static void networkReloadFirewallRules(void) { - size_t i; - VIR_INFO("Reloading iptables rules"); - - for (i = 0; i < driver->networks->count; i++) { - virNetworkObjPtr network = driver->networks->objs[i]; - - virNetworkObjLock(network); - if (virNetworkObjIsActive(network) && - ((network->def->forward.type == VIR_NETWORK_FORWARD_NONE) || - (network->def->forward.type == VIR_NETWORK_FORWARD_NAT) || - (network->def->forward.type == VIR_NETWORK_FORWARD_ROUTE))) { - /* Only the three L3 network types that are configured by libvirt - * need to have iptables rules reloaded. - */ - networkRemoveFirewallRules(network->def); - if (networkAddFirewallRules(network->def) < 0) { - /* failed to add but already logged */ - } - } - virNetworkObjUnlock(network); - } + virNetworkObjListForEach(driver->networks, + networkReloadFirewallRulesHelper, + NULL); } /* Enable IP Forwarding. Return 0 for success, -1 for failure. */ @@ -2524,21 +2529,16 @@ static virNetworkPtr networkLookupByName(virConnectPtr conn, static int networkConnectNumOfNetworks(virConnectPtr conn) { - int nactive = 0; - size_t i; + int nactive; if (virConnectNumOfNetworksEnsureACL(conn) < 0) return -1; networkDriverLock(); - for (i = 0; i < driver->networks->count; i++) { - virNetworkObjPtr obj = driver->networks->objs[i]; - virNetworkObjLock(obj); - if (virConnectNumOfNetworksCheckACL(conn, obj->def) && - virNetworkObjIsActive(obj)) - nactive++; - virNetworkObjUnlock(obj); - } + nactive = virNetworkObjListNumOfNetworks(driver->networks, + true, + virConnectNumOfNetworksCheckACL, + conn); networkDriverUnlock(); return nactive; @@ -2546,53 +2546,32 @@ static int networkConnectNumOfNetworks(virConnectPtr conn) static int networkConnectListNetworks(virConnectPtr conn, char **const names, int nnames) { int got = 0; - size_t i; if (virConnectListNetworksEnsureACL(conn) < 0) return -1; networkDriverLock(); - for (i = 0; i < driver->networks->count && got < nnames; i++) { - virNetworkObjPtr obj = driver->networks->objs[i]; - virNetworkObjLock(obj); - if (virConnectListNetworksCheckACL(conn, obj->def) && - virNetworkObjIsActive(obj)) { - if (VIR_STRDUP(names[got], obj->def->name) < 0) { - virNetworkObjUnlock(obj); - goto cleanup; - } - got++; - } - virNetworkObjUnlock(obj); - } + got = virNetworkObjListGetNames(driver->networks, + true, names, nnames, + virConnectListNetworksCheckACL, + conn); networkDriverUnlock(); return got; - - cleanup: - networkDriverUnlock(); - for (i = 0; i < got; i++) - VIR_FREE(names[i]); - return -1; } static int networkConnectNumOfDefinedNetworks(virConnectPtr conn) { int ninactive = 0; - size_t i; if (virConnectNumOfDefinedNetworksEnsureACL(conn) < 0) return -1; networkDriverLock(); - for (i = 0; i < driver->networks->count; i++) { - virNetworkObjPtr obj = driver->networks->objs[i]; - virNetworkObjLock(obj); - if (virConnectNumOfDefinedNetworksCheckACL(conn, obj->def) && - !virNetworkObjIsActive(obj)) - ninactive++; - virNetworkObjUnlock(obj); - } + ninactive = virNetworkObjListNumOfNetworks(driver->networks, + false, + virConnectNumOfDefinedNetworksCheckACL, + conn); networkDriverUnlock(); return ninactive; @@ -2600,33 +2579,17 @@ static int networkConnectNumOfDefinedNetworks(virConnectPtr conn) static int networkConnectListDefinedNetworks(virConnectPtr conn, char **const names, int nnames) { int got = 0; - size_t i; if (virConnectListDefinedNetworksEnsureACL(conn) < 0) return -1; networkDriverLock(); - for (i = 0; i < driver->networks->count && got < nnames; i++) { - virNetworkObjPtr obj = driver->networks->objs[i]; - virNetworkObjLock(obj); - if (virConnectListDefinedNetworksCheckACL(conn, obj->def) && - !virNetworkObjIsActive(obj)) { - if (VIR_STRDUP(names[got], obj->def->name) < 0) { - virNetworkObjUnlock(obj); - goto cleanup; - } - got++; - } - virNetworkObjUnlock(obj); - } + got = virNetworkObjListGetNames(driver->networks, + false, names, nnames, + virConnectListDefinedNetworksCheckACL, + conn); networkDriverUnlock(); return got; - - cleanup: - networkDriverUnlock(); - for (i = 0; i < got; i++) - VIR_FREE(names[i]); - return -1; } static int -- 2.0.5

Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/test/test_driver.c | 64 ++++++++++---------------------------------------- 1 file changed, 12 insertions(+), 52 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index ecb7bcd..0a68283 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -3543,16 +3543,11 @@ static virNetworkPtr testNetworkLookupByName(virConnectPtr conn, static int testConnectNumOfNetworks(virConnectPtr conn) { testConnPtr privconn = conn->privateData; - int numActive = 0; - size_t i; + int numActive; testDriverLock(privconn); - for (i = 0; i < privconn->networks->count; i++) { - virNetworkObjLock(privconn->networks->objs[i]); - if (virNetworkObjIsActive(privconn->networks->objs[i])) - numActive++; - virNetworkObjUnlock(privconn->networks->objs[i]); - } + numActive = virNetworkObjListNumOfNetworks(privconn->networks, + true, NULL, conn); testDriverUnlock(privconn); return numActive; @@ -3560,44 +3555,24 @@ static int testConnectNumOfNetworks(virConnectPtr conn) static int testConnectListNetworks(virConnectPtr conn, char **const names, int nnames) { testConnPtr privconn = conn->privateData; - int n = 0; - size_t i; + int n; testDriverLock(privconn); - memset(names, 0, sizeof(*names)*nnames); - for (i = 0; i < privconn->networks->count && n < nnames; i++) { - virNetworkObjLock(privconn->networks->objs[i]); - if (virNetworkObjIsActive(privconn->networks->objs[i]) && - VIR_STRDUP(names[n++], privconn->networks->objs[i]->def->name) < 0) { - virNetworkObjUnlock(privconn->networks->objs[i]); - goto error; - } - virNetworkObjUnlock(privconn->networks->objs[i]); - } + n = virNetworkObjListGetNames(privconn->networks, + true, names, nnames, NULL, conn); testDriverUnlock(privconn); return n; - - error: - for (n = 0; n < nnames; n++) - VIR_FREE(names[n]); - testDriverUnlock(privconn); - return -1; } static int testConnectNumOfDefinedNetworks(virConnectPtr conn) { testConnPtr privconn = conn->privateData; - int numInactive = 0; - size_t i; + int numInactive; testDriverLock(privconn); - for (i = 0; i < privconn->networks->count; i++) { - virNetworkObjLock(privconn->networks->objs[i]); - if (!virNetworkObjIsActive(privconn->networks->objs[i])) - numInactive++; - virNetworkObjUnlock(privconn->networks->objs[i]); - } + numInactive = virNetworkObjListNumOfNetworks(privconn->networks, + false, NULL, conn); testDriverUnlock(privconn); return numInactive; @@ -3605,29 +3580,14 @@ static int testConnectNumOfDefinedNetworks(virConnectPtr conn) static int testConnectListDefinedNetworks(virConnectPtr conn, char **const names, int nnames) { testConnPtr privconn = conn->privateData; - int n = 0; - size_t i; + int n; testDriverLock(privconn); - memset(names, 0, sizeof(*names)*nnames); - for (i = 0; i < privconn->networks->count && n < nnames; i++) { - virNetworkObjLock(privconn->networks->objs[i]); - if (!virNetworkObjIsActive(privconn->networks->objs[i]) && - VIR_STRDUP(names[n++], privconn->networks->objs[i]->def->name) < 0) { - virNetworkObjUnlock(privconn->networks->objs[i]); - goto error; - } - virNetworkObjUnlock(privconn->networks->objs[i]); - } + n = virNetworkObjListGetNames(privconn->networks, + false, names, nnames, NULL, conn); testDriverUnlock(privconn); return n; - - error: - for (n = 0; n < nnames; n++) - VIR_FREE(names[n]); - testDriverUnlock(privconn); - return -1; } static int -- 2.0.5

Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/parallels/parallels_network.c | 66 +++++++-------------------------------- 1 file changed, 12 insertions(+), 54 deletions(-) diff --git a/src/parallels/parallels_network.c b/src/parallels/parallels_network.c index 94e77cc..868e3f5 100644 --- a/src/parallels/parallels_network.c +++ b/src/parallels/parallels_network.c @@ -344,17 +344,12 @@ int parallelsNetworkClose(virConnectPtr conn) static int parallelsConnectNumOfNetworks(virConnectPtr conn) { - int nactive = 0; - size_t i; + int nactive; parallelsConnPtr privconn = conn->privateData; parallelsDriverLock(privconn); - for (i = 0; i < privconn->networks->count; i++) { - virNetworkObjLock(privconn->networks->objs[i]); - if (virNetworkObjIsActive(privconn->networks->objs[i])) - nactive++; - virNetworkObjUnlock(privconn->networks->objs[i]); - } + nactive = virNetworkObjListNumOfNetworks(privconn->networks, + true, NULL, conn); parallelsDriverUnlock(privconn); return nactive; @@ -365,45 +360,24 @@ static int parallelsConnectListNetworks(virConnectPtr conn, int nnames) { parallelsConnPtr privconn = conn->privateData; - int got = 0; - size_t i; + int got; parallelsDriverLock(privconn); - for (i = 0; i < privconn->networks->count && got < nnames; i++) { - virNetworkObjLock(privconn->networks->objs[i]); - if (virNetworkObjIsActive(privconn->networks->objs[i])) { - if (VIR_STRDUP(names[got], privconn->networks->objs[i]->def->name) < 0) { - virNetworkObjUnlock(privconn->networks->objs[i]); - goto cleanup; - } - got++; - } - virNetworkObjUnlock(privconn->networks->objs[i]); - } + got = virNetworkObjListGetNames(privconn->networks, + true, names, nnames, NULL, conn); parallelsDriverUnlock(privconn); return got; - - cleanup: - parallelsDriverUnlock(privconn); - for (i = 0; i < got; i++) - VIR_FREE(names[i]); - return -1; } static int parallelsConnectNumOfDefinedNetworks(virConnectPtr conn) { - int ninactive = 0; - size_t i; + int ninactive; parallelsConnPtr privconn = conn->privateData; parallelsDriverLock(privconn); - for (i = 0; i < privconn->networks->count; i++) { - virNetworkObjLock(privconn->networks->objs[i]); - if (!virNetworkObjIsActive(privconn->networks->objs[i])) - ninactive++; - virNetworkObjUnlock(privconn->networks->objs[i]); - } + ninactive = virNetworkObjListNumOfNetworks(privconn->networks, + false, NULL, conn); parallelsDriverUnlock(privconn); return ninactive; @@ -414,29 +388,13 @@ static int parallelsConnectListDefinedNetworks(virConnectPtr conn, int nnames) { parallelsConnPtr privconn = conn->privateData; - int got = 0; - size_t i; + int got; parallelsDriverLock(privconn); - for (i = 0; i < privconn->networks->count && got < nnames; i++) { - virNetworkObjLock(privconn->networks->objs[i]); - if (!virNetworkObjIsActive(privconn->networks->objs[i])) { - if (VIR_STRDUP(names[got], privconn->networks->objs[i]->def->name) < 0) { - virNetworkObjUnlock(privconn->networks->objs[i]); - goto cleanup; - } - got++; - } - virNetworkObjUnlock(privconn->networks->objs[i]); - } + got = virNetworkObjListGetNames(privconn->networks, + false, names, nnames, NULL, conn); parallelsDriverUnlock(privconn); return got; - - cleanup: - parallelsDriverUnlock(privconn); - for (i = 0; i < got; i++) - VIR_FREE(names[i]); - return -1; } static int parallelsConnectListAllNetworks(virConnectPtr conn, -- 2.0.5

Well, one day this will be self-locking object, but not today. But lets prepare the code for that! Moreover, virNetworkObjListFree() is no longer needed, so turn it into virNetworkObjListDispose(). Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- cfg.mk | 1 - src/conf/network_conf.c | 53 +++++++++++++++++++++++++++++---------- src/conf/network_conf.h | 11 ++++---- src/libvirt_private.syms | 2 +- src/network/bridge_driver.c | 5 ++-- src/parallels/parallels_driver.c | 2 +- src/parallels/parallels_network.c | 2 +- src/test/test_driver.c | 13 ++++------ 8 files changed, 56 insertions(+), 33 deletions(-) diff --git a/cfg.mk b/cfg.mk index d72b039..07a794a 100644 --- a/cfg.mk +++ b/cfg.mk @@ -250,7 +250,6 @@ useless_free_options = \ # n virNetworkFree (returns int) # n virNetworkFreeName (returns int) # y virNetworkObjFree -# n virNetworkObjListFree FIXME # n virNodeDevCapsDefFree FIXME # y virNodeDeviceDefFree # n virNodeDeviceFree (returns int) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 24a5f7c..4efad43 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -73,17 +73,33 @@ VIR_ENUM_IMPL(virNetworkForwardDriverName, VIR_ENUM_IMPL(virNetworkTaint, VIR_NETWORK_TAINT_LAST, "hook-script"); -bool -virNetworkObjTaint(virNetworkObjPtr obj, - virNetworkTaintFlags taint) +static virClassPtr virNetworkObjListClass; +static void virNetworkObjListDispose(void *obj); + +static int virNetworkObjOnceInit(void) +{ + if (!(virNetworkObjListClass = virClassNew(virClassForObject(), + "virNetworkObjList", + sizeof(virNetworkObjList), + virNetworkObjListDispose))) + return -1; + return 0; +} + + +VIR_ONCE_GLOBAL_INIT(virNetworkObj) + +virNetworkObjListPtr virNetworkObjListNew(void) { - unsigned int flag = (1 << taint); + virNetworkObjListPtr nets; + + if (virNetworkObjInitialize() < 0) + return NULL; - if (obj->taint & flag) - return false; + if (!(nets = virObjectNew(virNetworkObjListClass))) + return NULL; - obj->taint |= flag; - return true; + return nets; } virNetworkObjPtr virNetworkObjFindByUUID(virNetworkObjListPtr nets, @@ -116,6 +132,19 @@ virNetworkObjPtr virNetworkObjFindByName(virNetworkObjListPtr nets, return NULL; } +bool +virNetworkObjTaint(virNetworkObjPtr obj, + virNetworkTaintFlags taint) +{ + unsigned int flag = (1 << taint); + + if (obj->taint & flag) + return false; + + obj->taint |= flag; + return true; +} + static void virPortGroupDefClear(virPortGroupDefPtr def) @@ -275,18 +304,16 @@ void virNetworkObjFree(virNetworkObjPtr net) VIR_FREE(net); } -void virNetworkObjListFree(virNetworkObjListPtr nets) +static void +virNetworkObjListDispose(void *obj) { + virNetworkObjListPtr nets = obj; size_t i; - if (!nets) - return; - for (i = 0; i < nets->count; i++) virNetworkObjFree(nets->objs[i]); VIR_FREE(nets->objs); - nets->count = 0; } /* diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 164fb1a..5725258 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -40,6 +40,7 @@ # include "device_conf.h" # include "virbitmap.h" # include "networkcommon_conf.h" +# include "virobject.h" typedef enum { VIR_NETWORK_FORWARD_NONE = 0, @@ -277,6 +278,8 @@ struct _virNetworkObj { typedef struct _virNetworkObjList virNetworkObjList; typedef virNetworkObjList *virNetworkObjListPtr; struct _virNetworkObjList { + virObject parent; + size_t count; virNetworkObjPtr *objs; }; @@ -298,19 +301,17 @@ virNetworkObjIsActive(const virNetworkObj *net) return net->active; } -bool virNetworkObjTaint(virNetworkObjPtr obj, - virNetworkTaintFlags taint); +virNetworkObjListPtr virNetworkObjListNew(void); virNetworkObjPtr virNetworkObjFindByUUID(virNetworkObjListPtr nets, const unsigned char *uuid); virNetworkObjPtr virNetworkObjFindByName(virNetworkObjListPtr nets, const char *name); - +bool virNetworkObjTaint(virNetworkObjPtr obj, + virNetworkTaintFlags taint); void virNetworkDefFree(virNetworkDefPtr def); void virNetworkObjFree(virNetworkObjPtr net); -void virNetworkObjListFree(virNetworkObjListPtr nets); - typedef bool (*virNetworkObjListFilter)(virConnectPtr conn, virNetworkDefPtr def); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8aacc30..a9ce2cd 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -571,8 +571,8 @@ virNetworkObjGetPersistentDef; virNetworkObjIsDuplicate; virNetworkObjListExport; virNetworkObjListForEach; -virNetworkObjListFree; virNetworkObjListGetNames; +virNetworkObjListNew; virNetworkObjListNumOfNetworks; virNetworkObjLock; virNetworkObjReplacePersistentDef; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 1c73342..0bde89d 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -624,7 +624,7 @@ networkStateInitialize(bool privileged, /* if this fails now, it will be retried later with dnsmasqCapsRefresh() */ driver->dnsmasqCaps = dnsmasqCapsNewFromBinary(DNSMASQ); - if (VIR_ALLOC(driver->networks) < 0) + if (!(driver->networks = virNetworkObjListNew())) goto error; if (virNetworkLoadAllState(driver->networks, @@ -752,8 +752,7 @@ networkStateCleanup(void) virObjectEventStateFree(driver->networkEventState); /* free inactive networks */ - virNetworkObjListFree(driver->networks); - VIR_FREE(driver->networks); + virObjectUnref(driver->networks); VIR_FREE(driver->networkConfigDir); VIR_FREE(driver->networkAutostartDir); diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index 32f2ede..d8bcb4b 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -208,7 +208,7 @@ parallelsOpenDefault(virConnectPtr conn) goto error; if (!(privconn->domains = virDomainObjListNew()) || - VIR_ALLOC(privconn->networks) < 0) + !(privconn->networks = virNetworkObjListNew())) goto error; if (!(privconn->domainEventState = virObjectEventStateNew())) diff --git a/src/parallels/parallels_network.c b/src/parallels/parallels_network.c index 868e3f5..e626ff6 100644 --- a/src/parallels/parallels_network.c +++ b/src/parallels/parallels_network.c @@ -337,7 +337,7 @@ int parallelsNetworkClose(virConnectPtr conn) { parallelsConnPtr privconn = conn->privateData; parallelsDriverLock(privconn); - virNetworkObjListFree(privconn->networks); + virObjectUnref(privconn->networks); parallelsDriverUnlock(privconn); return 0; } diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 0a68283..4b3aa24 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -725,7 +725,7 @@ testOpenDefault(virConnectPtr conn) goto error; if (!(privconn->domains = virDomainObjListNew()) || - VIR_ALLOC(privconn->networks) < 0) + !(privconn->networks = virNetworkObjListNew())) goto error; memmove(&privconn->nodeInfo, &defaultNodeInfo, sizeof(defaultNodeInfo)); @@ -830,8 +830,7 @@ testOpenDefault(virConnectPtr conn) error: virObjectUnref(privconn->domains); - virNetworkObjListFree(privconn->networks); - VIR_FREE(privconn->networks); + virObjectUnref(privconn->networks); virInterfaceObjListFree(&privconn->ifaces); virStoragePoolObjListFree(&privconn->pools); virNodeDeviceObjListFree(&privconn->devs); @@ -1414,7 +1413,7 @@ testOpenFromFile(virConnectPtr conn, const char *file) conn->privateData = privconn; if (!(privconn->domains = virDomainObjListNew()) || - VIR_ALLOC(privconn->networks) < 0) + !(privconn->networks = virNetworkObjListNew())) goto error; if (!(privconn->caps = testBuildCapabilities(conn))) @@ -1466,8 +1465,7 @@ testOpenFromFile(virConnectPtr conn, const char *file) xmlXPathFreeContext(ctxt); xmlFreeDoc(doc); virObjectUnref(privconn->domains); - virNetworkObjListFree(privconn->networks); - VIR_FREE(privconn->networks); + virObjectUnref(privconn->networks); virInterfaceObjListFree(&privconn->ifaces); virStoragePoolObjListFree(&privconn->pools); VIR_FREE(privconn->path); @@ -1593,8 +1591,7 @@ static int testConnectClose(virConnectPtr conn) virObjectUnref(privconn->xmlopt); virObjectUnref(privconn->domains); virNodeDeviceObjListFree(&privconn->devs); - virNetworkObjListFree(privconn->networks); - VIR_FREE(privconn->networks); + virObjectUnref(privconn->networks); virInterfaceObjListFree(&privconn->ifaces); virStoragePoolObjListFree(&privconn->pools); virObjectEventStateFree(privconn->eventState); -- 2.0.5

On 03.03.2015 11:47, Peter Krempa wrote: I just wanted the class initiatiors to be together. I prefer having it that way. Michal

Now that all the code uses accessors, don't expose the structure anyway. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/conf/network_conf.c | 7 +++++++ src/conf/network_conf.h | 6 ------ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 4efad43..b3c0ac7 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -51,6 +51,13 @@ /* currently, /sbin/tc implementation allows up to 16 bits for minor class size */ #define CLASS_ID_BITMAP_SIZE (1<<16) +struct _virNetworkObjList { + virObject parent; + + size_t count; + virNetworkObjPtr *objs; +}; + VIR_ENUM_IMPL(virNetworkForward, VIR_NETWORK_FORWARD_LAST, "none", "nat", "route", "bridge", "private", "vepa", "passthrough", "hostdev") diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 5725258..f7ab2cf 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -277,12 +277,6 @@ struct _virNetworkObj { typedef struct _virNetworkObjList virNetworkObjList; typedef virNetworkObjList *virNetworkObjListPtr; -struct _virNetworkObjList { - virObject parent; - - size_t count; - virNetworkObjPtr *objs; -}; typedef enum { VIR_NETWORK_TAINT_HOOK, /* Hook script was executed over -- 2.0.5

So far it's just a structure which happens to have 'Obj' in its name, but otherwise it not related to virObject at all. No reference counting, not virObjectLock(), nothing. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- cfg.mk | 2 - src/conf/network_conf.c | 126 ++++++++++++++++++++------------------ src/conf/network_conf.h | 8 +-- src/libvirt_private.syms | 4 +- src/network/bridge_driver.c | 56 ++++++++--------- src/parallels/parallels_network.c | 16 ++--- src/test/test_driver.c | 32 +++++----- tests/networkxml2conftest.c | 4 +- tests/objectlocking.ml | 2 - 9 files changed, 125 insertions(+), 125 deletions(-) diff --git a/cfg.mk b/cfg.mk index 07a794a..6885f9e 100644 --- a/cfg.mk +++ b/cfg.mk @@ -160,7 +160,6 @@ useless_free_options = \ --name=virNWFilterRuleDefFree \ --name=virNWFilterRuleInstFree \ --name=virNetworkDefFree \ - --name=virNetworkObjFree \ --name=virNodeDeviceDefFree \ --name=virNodeDeviceObjFree \ --name=virObjectUnref \ @@ -249,7 +248,6 @@ useless_free_options = \ # y virNetworkDefFree # n virNetworkFree (returns int) # n virNetworkFreeName (returns int) -# y virNetworkObjFree # n virNodeDevCapsDefFree FIXME # y virNodeDeviceDefFree # n virNodeDeviceFree (returns int) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index b3c0ac7..8f140d2 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -80,11 +80,19 @@ VIR_ENUM_IMPL(virNetworkForwardDriverName, VIR_ENUM_IMPL(virNetworkTaint, VIR_NETWORK_TAINT_LAST, "hook-script"); +static virClassPtr virNetworkObjClass; static virClassPtr virNetworkObjListClass; +static void virNetworkObjDispose(void *obj); static void virNetworkObjListDispose(void *obj); static int virNetworkObjOnceInit(void) { + if (!(virNetworkObjClass = virClassNew(virClassForObjectLockable(), + "virNetworkObj", + sizeof(virNetworkObj), + virNetworkObjDispose))) + return -1; + if (!(virNetworkObjListClass = virClassNew(virClassForObject(), "virNetworkObjList", sizeof(virNetworkObjList), @@ -96,6 +104,32 @@ static int virNetworkObjOnceInit(void) VIR_ONCE_GLOBAL_INIT(virNetworkObj) +virNetworkObjPtr +virNetworkObjNew(void) +{ + virNetworkObjPtr net; + + if (virNetworkObjInitialize() < 0) + return NULL; + + if (!(net = virObjectLockableNew(virNetworkObjClass))) + return NULL; + + if (!(net->class_id = virBitmapNew(CLASS_ID_BITMAP_SIZE))) + goto error; + + /* The first three class IDs are already taken */ + ignore_value(virBitmapSetBit(net->class_id, 0)); + ignore_value(virBitmapSetBit(net->class_id, 1)); + ignore_value(virBitmapSetBit(net->class_id, 2)); + + return net; + + error: + virObjectUnref(net); + return NULL; +} + virNetworkObjListPtr virNetworkObjListNew(void) { virNetworkObjListPtr nets; @@ -115,10 +149,10 @@ virNetworkObjPtr virNetworkObjFindByUUID(virNetworkObjListPtr nets, size_t i; for (i = 0; i < nets->count; i++) { - virNetworkObjLock(nets->objs[i]); + virObjectLock(nets->objs[i]); if (!memcmp(nets->objs[i]->def->uuid, uuid, VIR_UUID_BUFLEN)) return nets->objs[i]; - virNetworkObjUnlock(nets->objs[i]); + virObjectUnlock(nets->objs[i]); } return NULL; @@ -130,10 +164,10 @@ virNetworkObjPtr virNetworkObjFindByName(virNetworkObjListPtr nets, size_t i; for (i = 0; i < nets->count; i++) { - virNetworkObjLock(nets->objs[i]); + virObjectLock(nets->objs[i]); if (STREQ(nets->objs[i]->def->name, name)) return nets->objs[i]; - virNetworkObjUnlock(nets->objs[i]); + virObjectUnlock(nets->objs[i]); } return NULL; @@ -297,18 +331,14 @@ virNetworkDefFree(virNetworkDefPtr def) VIR_FREE(def); } -void virNetworkObjFree(virNetworkObjPtr net) +static void +virNetworkObjDispose(void *obj) { - if (!net) - return; + virNetworkObjPtr net = obj; virNetworkDefFree(net->def); virNetworkDefFree(net->newDef); virBitmapFree(net->class_id); - - virMutexDestroy(&net->lock); - - VIR_FREE(net); } static void @@ -318,7 +348,7 @@ virNetworkObjListDispose(void *obj) size_t i; for (i = 0; i < nets->count; i++) - virNetworkObjFree(nets->objs[i]); + virObjectUnref(nets->objs[i]); VIR_FREE(nets->objs); } @@ -409,32 +439,20 @@ virNetworkAssignDef(virNetworkObjListPtr nets, return network; } - if (VIR_ALLOC(network) < 0) + if (!(network = virNetworkObjNew())) return NULL; - if (virMutexInit(&network->lock) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot initialize mutex")); - VIR_FREE(network); - return NULL; - } - virNetworkObjLock(network); + virObjectLock(network); - if (VIR_APPEND_ELEMENT_COPY(nets->objs, nets->count, network) < 0 || - !(network->class_id = virBitmapNew(CLASS_ID_BITMAP_SIZE))) + if (VIR_APPEND_ELEMENT_COPY(nets->objs, nets->count, network) < 0) goto error; - /* The first three class IDs are already taken */ - ignore_value(virBitmapSetBit(network->class_id, 0)); - ignore_value(virBitmapSetBit(network->class_id, 1)); - ignore_value(virBitmapSetBit(network->class_id, 2)); - network->def = def; network->persistent = !live; return network; error: - virNetworkObjUnlock(network); - virNetworkObjFree(network); + virObjectUnlock(network); + virObjectUnref(network); return NULL; } @@ -602,17 +620,17 @@ void virNetworkRemoveInactive(virNetworkObjListPtr nets, { size_t i; - virNetworkObjUnlock(net); + virObjectUnlock(net); for (i = 0; i < nets->count; i++) { - virNetworkObjLock(nets->objs[i]); + virObjectLock(nets->objs[i]); if (nets->objs[i] == net) { - virNetworkObjUnlock(nets->objs[i]); - virNetworkObjFree(nets->objs[i]); + virObjectUnlock(nets->objs[i]); + virObjectUnref(nets->objs[i]); VIR_DELETE_ELEMENT(nets->objs, i, nets->count); break; } - virNetworkObjUnlock(nets->objs[i]); + virObjectUnlock(nets->objs[i]); } } @@ -3014,7 +3032,7 @@ virNetworkLoadAllState(virNetworkObjListPtr nets, continue; if ((net = virNetworkLoadState(nets, stateDir, entry->d_name))) - virNetworkObjUnlock(net); + virObjectUnlock(net); } closedir(dir); @@ -3055,7 +3073,7 @@ int virNetworkLoadAllConfigs(virNetworkObjListPtr nets, autostartDir, entry->d_name); if (net) - virNetworkObjUnlock(net); + virObjectUnlock(net); } closedir(dir); @@ -3110,12 +3128,12 @@ int virNetworkBridgeInUse(virNetworkObjListPtr nets, unsigned int ret = 0; for (i = 0; i < nets->count; i++) { - virNetworkObjLock(nets->objs[i]); + virObjectLock(nets->objs[i]); if (nets->objs[i]->def->bridge && STREQ(nets->objs[i]->def->bridge, bridge) && !(skipname && STREQ(nets->objs[i]->def->name, skipname))) ret = 1; - virNetworkObjUnlock(nets->objs[i]); + virObjectUnlock(nets->objs[i]); } return ret; @@ -4223,21 +4241,11 @@ virNetworkObjIsDuplicate(virNetworkObjListPtr nets, cleanup: if (net) - virNetworkObjUnlock(net); + virObjectUnlock(net); return ret; } -void virNetworkObjLock(virNetworkObjPtr obj) -{ - virMutexLock(&obj->lock); -} - -void virNetworkObjUnlock(virNetworkObjPtr obj) -{ - virMutexUnlock(&obj->lock); -} - #define MATCH(FLAG) (flags & (FLAG)) static bool virNetworkMatch(virNetworkObjPtr netobj, @@ -4289,21 +4297,21 @@ virNetworkObjListExport(virConnectPtr conn, for (i = 0; i < netobjs->count; i++) { virNetworkObjPtr netobj = netobjs->objs[i]; - virNetworkObjLock(netobj); + virObjectLock(netobj); if ((!filter || filter(conn, netobj->def)) && virNetworkMatch(netobj, flags)) { if (nets) { if (!(net = virGetNetwork(conn, netobj->def->name, netobj->def->uuid))) { - virNetworkObjUnlock(netobj); + virObjectUnlock(netobj); goto cleanup; } tmp_nets[nnets] = net; } nnets++; } - virNetworkObjUnlock(netobj); + virObjectUnlock(netobj); } if (tmp_nets) { @@ -4360,21 +4368,21 @@ virNetworkObjListGetNames(virNetworkObjListPtr nets, for (i = 0; i < nets->count && got < nnames; i++) { virNetworkObjPtr obj = nets->objs[i]; - virNetworkObjLock(obj); + virObjectLock(obj); if (filter && !filter(conn, obj->def)) { - virNetworkObjUnlock(obj); + virObjectUnlock(obj); continue; } if ((active && virNetworkObjIsActive(obj)) || (!active && !virNetworkObjIsActive(obj))) { if (VIR_STRDUP(names[got], obj->def->name) < 0) { - virNetworkObjUnlock(obj); + virObjectUnlock(obj); goto error; } got++; } - virNetworkObjUnlock(obj); + virObjectUnlock(obj); } return got; @@ -4396,16 +4404,16 @@ virNetworkObjListNumOfNetworks(virNetworkObjListPtr nets, for (i = 0; i < nets->count; i++) { virNetworkObjPtr obj = nets->objs[i]; - virNetworkObjLock(obj); + virObjectLock(obj); if (filter && !filter(conn, obj->def)) { - virNetworkObjUnlock(obj); + virObjectUnlock(obj); continue; } if ((active && virNetworkObjIsActive(obj)) || (!active && !virNetworkObjIsActive(obj))) count++; - virNetworkObjUnlock(obj); + virObjectUnlock(obj); } return count; diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index f7ab2cf..7f48b9a 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -258,7 +258,7 @@ struct _virNetworkDef { typedef struct _virNetworkObj virNetworkObj; typedef virNetworkObj *virNetworkObjPtr; struct _virNetworkObj { - virMutex lock; + virObjectLockable parent; pid_t dnsmasqPid; pid_t radvdPid; @@ -275,6 +275,8 @@ struct _virNetworkObj { unsigned int taint; }; +virNetworkObjPtr virNetworkObjNew(void); + typedef struct _virNetworkObjList virNetworkObjList; typedef virNetworkObjList *virNetworkObjListPtr; @@ -305,7 +307,6 @@ bool virNetworkObjTaint(virNetworkObjPtr obj, virNetworkTaintFlags taint); void virNetworkDefFree(virNetworkDefPtr def); -void virNetworkObjFree(virNetworkObjPtr net); typedef bool (*virNetworkObjListFilter)(virConnectPtr conn, virNetworkDefPtr def); @@ -412,9 +413,6 @@ int virNetworkObjIsDuplicate(virNetworkObjListPtr nets, virNetworkDefPtr def, bool check_active); -void virNetworkObjLock(virNetworkObjPtr obj); -void virNetworkObjUnlock(virNetworkObjPtr obj); - VIR_ENUM_DECL(virNetworkForward) # define VIR_CONNECT_LIST_NETWORKS_FILTERS_ACTIVE \ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a9ce2cd..660579c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -566,7 +566,6 @@ virNetworkLoadAllState; virNetworkObjAssignDef; virNetworkObjFindByName; virNetworkObjFindByUUID; -virNetworkObjFree; virNetworkObjGetPersistentDef; virNetworkObjIsDuplicate; virNetworkObjListExport; @@ -574,11 +573,10 @@ virNetworkObjListForEach; virNetworkObjListGetNames; virNetworkObjListNew; virNetworkObjListNumOfNetworks; -virNetworkObjLock; +virNetworkObjNew; virNetworkObjReplacePersistentDef; virNetworkObjSetDefTransient; virNetworkObjTaint; -virNetworkObjUnlock; virNetworkObjUnsetDefTransient; virNetworkObjUpdate; virNetworkRemoveInactive; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 0bde89d..2d4a6c5 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -348,9 +348,9 @@ networkUpdateAllState(virNetworkObjPtr obj, { int ret = -1; - virNetworkObjLock(obj); + virObjectLock(obj); if (!virNetworkObjIsActive(obj)) { - virNetworkObjUnlock(obj); + virObjectUnlock(obj); return 0; } @@ -407,7 +407,7 @@ networkUpdateAllState(virNetworkObjPtr obj, if (!obj->persistent && !obj->active) networkRemoveInactive(obj); else - virNetworkObjUnlock(obj); + virObjectUnlock(obj); return ret; } @@ -417,7 +417,7 @@ networkAutostartConfigs(virNetworkObjPtr net, { int ret = -1; - virNetworkObjLock(net); + virObjectLock(net); if (net->autostart && !virNetworkObjIsActive(net) && networkStartNetwork(net) < 0) @@ -425,7 +425,7 @@ networkAutostartConfigs(virNetworkObjPtr net, ret = 0; cleanup: - virNetworkObjUnlock(net); + virObjectUnlock(net); return ret; } @@ -1746,7 +1746,7 @@ networkRefreshDaemonsHelper(virNetworkObjPtr net, void *opaque ATTRIBUTE_UNUSED) { - virNetworkObjLock(net); + virObjectLock(net); if (virNetworkObjIsActive(net) && ((net->def->forward.type == VIR_NETWORK_FORWARD_NONE) || (net->def->forward.type == VIR_NETWORK_FORWARD_NAT) || @@ -1760,7 +1760,7 @@ networkRefreshDaemonsHelper(virNetworkObjPtr net, networkRefreshDhcpDaemon(net); networkRefreshRadvd(net); } - virNetworkObjUnlock(net); + virObjectUnlock(net); return 0; } @@ -1781,7 +1781,7 @@ networkReloadFirewallRulesHelper(virNetworkObjPtr net, void *opaque ATTRIBUTE_UNUSED) { - virNetworkObjLock(net); + virObjectLock(net); if (virNetworkObjIsActive(net) && ((net->def->forward.type == VIR_NETWORK_FORWARD_NONE) || (net->def->forward.type == VIR_NETWORK_FORWARD_NAT) || @@ -1794,7 +1794,7 @@ networkReloadFirewallRulesHelper(virNetworkObjPtr net, /* failed to add but already logged */ } } - virNetworkObjUnlock(net); + virObjectUnlock(net); return 0; } @@ -2496,7 +2496,7 @@ static virNetworkPtr networkLookupByUUID(virConnectPtr conn, cleanup: if (network) - virNetworkObjUnlock(network); + virObjectUnlock(network); return ret; } @@ -2522,7 +2522,7 @@ static virNetworkPtr networkLookupByName(virConnectPtr conn, cleanup: if (network) - virNetworkObjUnlock(network); + virObjectUnlock(network); return ret; } @@ -2670,7 +2670,7 @@ static int networkIsActive(virNetworkPtr net) cleanup: if (obj) - virNetworkObjUnlock(obj); + virObjectUnlock(obj); return ret; } @@ -2689,7 +2689,7 @@ static int networkIsPersistent(virNetworkPtr net) cleanup: if (obj) - virNetworkObjUnlock(obj); + virObjectUnlock(obj); return ret; } @@ -2959,7 +2959,7 @@ static virNetworkPtr networkCreateXML(virConnectPtr conn, const char *xml) if (event) virObjectEventStateQueue(driver->networkEventState, event); if (network) - virNetworkObjUnlock(network); + virObjectUnlock(network); networkDriverUnlock(); return ret; } @@ -3016,7 +3016,7 @@ static virNetworkPtr networkDefineXML(virConnectPtr conn, const char *xml) if (freeDef) virNetworkDefFree(def); if (network) - virNetworkObjUnlock(network); + virObjectUnlock(network); networkDriverUnlock(); return ret; } @@ -3077,7 +3077,7 @@ networkUndefine(virNetworkPtr net) if (event) virObjectEventStateQueue(driver->networkEventState, event); if (network) - virNetworkObjUnlock(network); + virObjectUnlock(network); networkDriverUnlock(); return ret; } @@ -3249,7 +3249,7 @@ networkUpdate(virNetworkPtr net, ret = 0; cleanup: if (network) - virNetworkObjUnlock(network); + virObjectUnlock(network); networkDriverUnlock(); return ret; } @@ -3284,7 +3284,7 @@ static int networkCreate(virNetworkPtr net) if (event) virObjectEventStateQueue(driver->networkEventState, event); if (network) - virNetworkObjUnlock(network); + virObjectUnlock(network); networkDriverUnlock(); return ret; } @@ -3335,7 +3335,7 @@ static int networkDestroy(virNetworkPtr net) if (event) virObjectEventStateQueue(driver->networkEventState, event); if (network) - virNetworkObjUnlock(network); + virObjectUnlock(network); networkDriverUnlock(); return ret; } @@ -3364,7 +3364,7 @@ static char *networkGetXMLDesc(virNetworkPtr net, cleanup: if (network) - virNetworkObjUnlock(network); + virObjectUnlock(network); return ret; } @@ -3389,7 +3389,7 @@ static char *networkGetBridgeName(virNetworkPtr net) { cleanup: if (network) - virNetworkObjUnlock(network); + virObjectUnlock(network); return bridge; } @@ -3410,7 +3410,7 @@ static int networkGetAutostart(virNetworkPtr net, cleanup: if (network) - virNetworkObjUnlock(network); + virObjectUnlock(network); return ret; } @@ -3478,7 +3478,7 @@ static int networkSetAutostart(virNetworkPtr net, VIR_FREE(configFile); VIR_FREE(autostartLink); if (network) - virNetworkObjUnlock(network); + virObjectUnlock(network); networkDriverUnlock(); return ret; } @@ -3651,7 +3651,7 @@ networkGetDHCPLeases(virNetworkPtr network, virJSONValueFree(leases_array); if (obj) - virNetworkObjUnlock(obj); + virObjectUnlock(obj); return rv; @@ -4124,7 +4124,7 @@ networkAllocateActualDevice(virDomainDefPtr dom, cleanup: if (network) - virNetworkObjUnlock(network); + virObjectUnlock(network); return ret; error: @@ -4327,7 +4327,7 @@ networkNotifyActualDevice(virDomainDefPtr dom, ret = 0; cleanup: if (network) - virNetworkObjUnlock(network); + virObjectUnlock(network); return ret; error: @@ -4477,7 +4477,7 @@ networkReleaseActualDevice(virDomainDefPtr dom, ret = 0; cleanup: if (network) - virNetworkObjUnlock(network); + virObjectUnlock(network); if (iface->type == VIR_DOMAIN_NET_TYPE_NETWORK) { virDomainActualNetDefFree(iface->data.network.actual); iface->data.network.actual = NULL; @@ -4581,7 +4581,7 @@ networkGetNetworkAddress(const char *netname, char **netaddr) ret = 0; cleanup: if (network) - virNetworkObjUnlock(network); + virObjectUnlock(network); return ret; } diff --git a/src/parallels/parallels_network.c b/src/parallels/parallels_network.c index e626ff6..3c879e4 100644 --- a/src/parallels/parallels_network.c +++ b/src/parallels/parallels_network.c @@ -230,7 +230,7 @@ parallelsLoadNetwork(parallelsConnPtr privconn, virJSONValuePtr jobj) goto cleanup; net->active = 1; net->autostart = 1; - virNetworkObjUnlock(net); + virObjectUnlock(net); return net; cleanup: @@ -265,7 +265,7 @@ parallelsAddRoutedNetwork(parallelsConnPtr privconn) } net->active = 1; net->autostart = 1; - virNetworkObjUnlock(net); + virObjectUnlock(net); return net; @@ -433,7 +433,7 @@ static virNetworkPtr parallelsNetworkLookupByUUID(virConnectPtr conn, cleanup: if (network) - virNetworkObjUnlock(network); + virObjectUnlock(network); return ret; } @@ -457,7 +457,7 @@ static virNetworkPtr parallelsNetworkLookupByName(virConnectPtr conn, cleanup: if (network) - virNetworkObjUnlock(network); + virObjectUnlock(network); return ret; } @@ -484,7 +484,7 @@ static char *parallelsNetworkGetXMLDesc(virNetworkPtr net, cleanup: if (network) - virNetworkObjUnlock(network); + virObjectUnlock(network); return ret; } @@ -505,7 +505,7 @@ static int parallelsNetworkIsActive(virNetworkPtr net) cleanup: if (obj) - virNetworkObjUnlock(obj); + virObjectUnlock(obj); return ret; } @@ -526,7 +526,7 @@ static int parallelsNetworkIsPersistent(virNetworkPtr net) cleanup: if (obj) - virNetworkObjUnlock(obj); + virObjectUnlock(obj); return ret; } @@ -551,7 +551,7 @@ static int parallelsNetworkGetAutostart(virNetworkPtr net, cleanup: if (network) - virNetworkObjUnlock(network); + virObjectUnlock(network); return ret; } diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 4b3aa24..3703d00 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -787,7 +787,7 @@ testOpenDefault(virConnectPtr conn) goto error; } netobj->active = 1; - virNetworkObjUnlock(netobj); + virObjectUnlock(netobj); if (!(interfacedef = virInterfaceDefParseString(defaultInterfaceXML))) goto error; @@ -1155,7 +1155,7 @@ testParseNetworks(testConnPtr privconn, } obj->active = 1; - virNetworkObjUnlock(obj); + virObjectUnlock(obj); } ret = 0; @@ -3508,7 +3508,7 @@ static virNetworkPtr testNetworkLookupByUUID(virConnectPtr conn, cleanup: if (net) - virNetworkObjUnlock(net); + virObjectUnlock(net); return ret; } @@ -3532,7 +3532,7 @@ static virNetworkPtr testNetworkLookupByName(virConnectPtr conn, cleanup: if (net) - virNetworkObjUnlock(net); + virObjectUnlock(net); return ret; } @@ -3621,7 +3621,7 @@ static int testNetworkIsActive(virNetworkPtr net) cleanup: if (obj) - virNetworkObjUnlock(obj); + virObjectUnlock(obj); return ret; } @@ -3642,7 +3642,7 @@ static int testNetworkIsPersistent(virNetworkPtr net) cleanup: if (obj) - virNetworkObjUnlock(obj); + virObjectUnlock(obj); return ret; } @@ -3675,7 +3675,7 @@ static virNetworkPtr testNetworkCreateXML(virConnectPtr conn, const char *xml) if (event) testObjectEventQueue(privconn, event); if (net) - virNetworkObjUnlock(net); + virObjectUnlock(net); testDriverUnlock(privconn); return ret; } @@ -3708,7 +3708,7 @@ virNetworkPtr testNetworkDefineXML(virConnectPtr conn, const char *xml) if (event) testObjectEventQueue(privconn, event); if (net) - virNetworkObjUnlock(net); + virObjectUnlock(net); testDriverUnlock(privconn); return ret; } @@ -3746,7 +3746,7 @@ static int testNetworkUndefine(virNetworkPtr network) if (event) testObjectEventQueue(privconn, event); if (privnet) - virNetworkObjUnlock(privnet); + virObjectUnlock(privnet); testDriverUnlock(privconn); return ret; } @@ -3796,7 +3796,7 @@ testNetworkUpdate(virNetworkPtr net, ret = 0; cleanup: if (network) - virNetworkObjUnlock(network); + virObjectUnlock(network); testDriverUnlock(privconn); return ret; } @@ -3833,7 +3833,7 @@ static int testNetworkCreate(virNetworkPtr network) if (event) testObjectEventQueue(privconn, event); if (privnet) - virNetworkObjUnlock(privnet); + virObjectUnlock(privnet); return ret; } @@ -3866,7 +3866,7 @@ static int testNetworkDestroy(virNetworkPtr network) if (event) testObjectEventQueue(privconn, event); if (privnet) - virNetworkObjUnlock(privnet); + virObjectUnlock(privnet); testDriverUnlock(privconn); return ret; } @@ -3893,7 +3893,7 @@ static char *testNetworkGetXMLDesc(virNetworkPtr network, cleanup: if (privnet) - virNetworkObjUnlock(privnet); + virObjectUnlock(privnet); return ret; } @@ -3922,7 +3922,7 @@ static char *testNetworkGetBridgeName(virNetworkPtr network) { cleanup: if (privnet) - virNetworkObjUnlock(privnet); + virObjectUnlock(privnet); return bridge; } @@ -3947,7 +3947,7 @@ static int testNetworkGetAutostart(virNetworkPtr network, cleanup: if (privnet) - virNetworkObjUnlock(privnet); + virObjectUnlock(privnet); return ret; } @@ -3972,7 +3972,7 @@ static int testNetworkSetAutostart(virNetworkPtr network, cleanup: if (privnet) - virNetworkObjUnlock(privnet); + virObjectUnlock(privnet); return ret; } diff --git a/tests/networkxml2conftest.c b/tests/networkxml2conftest.c index 280db30..6df161f 100644 --- a/tests/networkxml2conftest.c +++ b/tests/networkxml2conftest.c @@ -40,7 +40,7 @@ testCompareXMLToConfFiles(const char *inxml, const char *outconf, dnsmasqCapsPtr if (!(dev = virNetworkDefParseString(inXmlData))) goto fail; - if (VIR_ALLOC(obj) < 0) + if (!(obj = virNetworkObjNew())) goto fail; obj->def = dev; @@ -66,7 +66,7 @@ testCompareXMLToConfFiles(const char *inxml, const char *outconf, dnsmasqCapsPtr VIR_FREE(actual); VIR_FREE(pidfile); virCommandFree(cmd); - virNetworkObjFree(obj); + virObjectUnref(obj); dnsmasqContextFree(dctx); return ret; } diff --git a/tests/objectlocking.ml b/tests/objectlocking.ml index 2476731..2fa33e4 100644 --- a/tests/objectlocking.ml +++ b/tests/objectlocking.ml @@ -84,7 +84,6 @@ let lockedObjMethods = [ *) let objectLockMethods = [ "virDomainObjLock"; - "virNetworkObjLock"; "virStoragePoolObjLock"; "virNodeDevObjLock" ] @@ -99,7 +98,6 @@ let objectLockMethods = [ *) let objectUnlockMethods = [ "virDomainObjUnlock"; - "virNetworkObjUnlock"; "virStoragePoolObjUnlock"; "virNodeDevObjUnlock" ] -- 2.0.5

On 03.03.2015 12:05, Peter Krempa wrote: To get the order of locking right. Yep, I've saved that for a separate patch, which is not posted yet though. Not anymore. Any API that enters the remove function already has at least one reference (obtained via virNetworkObjFind*). So this merely decrease the refcount on the object since the list does no longer hold a reference to the object. Michal

On 03.03.2015 14:10, Peter Krempa wrote: Okay, considered this squashed in: diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 8f140d2..3ea8975 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -620,17 +620,13 @@ void virNetworkRemoveInactive(virNetworkObjListPtr nets, { size_t i; - virObjectUnlock(net); for (i = 0; i < nets->count; i++) { - virObjectLock(nets->objs[i]); if (nets->objs[i] == net) { - virObjectUnlock(nets->objs[i]); - virObjectUnref(nets->objs[i]); - VIR_DELETE_ELEMENT(nets->objs, i, nets->count); + virObjectUnlock(net); + virObjectUnref(net); break; } - virObjectUnlock(nets->objs[i]); } } Michal

On Thu, Feb 26, 2015 at 15:17:32 +0100, Michal Privoznik wrote: You should also convert the onle place in virNetworkLoadAllConfigs() to this new helper since it will also return a referenced object. I'll point out the problem later. ACK with virNetworkLoadAllConfigs tweaked too. Peter

On Thu, Feb 26, 2015 at 15:17:36 +0100, Michal Privoznik wrote: Too, many, commas. Hmm, okay. My comment to the previous patch is invalid as this would actually introduce a lock ordering problem. ACK

This patch turns both virNetworkObjFindByUUID() and virNetworkObjFindByName() to return an referenced object so that even if caller unlocks it, it's for sure that object won't disappear meanwhile. Especially if the object (in general) is locked and unlocked during the caller run. Moreover, this commit is nicely small, since the object unrefing can be done in virNetworkObjEndAPI(). Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/conf/network_conf.c | 11 +++++++---- src/parallels/parallels_network.c | 9 +++++---- src/test/test_driver.c | 4 ++-- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 5b0f36f..c620201 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -137,6 +137,7 @@ virNetworkObjEndAPI(virNetworkObjPtr *net) return; virObjectUnlock(*net); + virObjectUnref(*net); *net = NULL; } @@ -164,6 +165,7 @@ virNetworkObjPtr virNetworkObjFindByUUID(virNetworkObjListPtr nets, virObjectLock(nets->objs[i]); if (!memcmp(nets->objs[i]->def->uuid, uuid, VIR_UUID_BUFLEN)) { ret = nets->objs[i]; + virObjectRef(ret); break; } virObjectUnlock(nets->objs[i]); @@ -184,6 +186,7 @@ virNetworkObjPtr virNetworkObjFindByName(virNetworkObjListPtr nets, virObjectLock(nets->objs[i]); if (STREQ(nets->objs[i]->def->name, name)) { ret = nets->objs[i]; + virObjectRef(ret); break; } virObjectUnlock(nets->objs[i]); @@ -469,13 +472,13 @@ virNetworkAssignDef(virNetworkObjListPtr nets, network->def = def; network->persistent = !live; + virObjectRef(network); virObjectUnlock(nets); return network; error: virObjectUnlock(nets); - virObjectUnlock(network); - virObjectUnref(network); + virNetworkObjEndAPI(&network); return NULL; } @@ -3057,7 +3060,7 @@ virNetworkLoadAllState(virNetworkObjListPtr nets, continue; if ((net = virNetworkLoadState(nets, stateDir, entry->d_name))) - virObjectUnlock(net); + virNetworkObjEndAPI(&net); } closedir(dir); @@ -3098,7 +3101,7 @@ int virNetworkLoadAllConfigs(virNetworkObjListPtr nets, autostartDir, entry->d_name); if (net) - virObjectUnlock(net); + virNetworkObjEndAPI(&net); } closedir(dir); diff --git a/src/parallels/parallels_network.c b/src/parallels/parallels_network.c index 7618890..da29060 100644 --- a/src/parallels/parallels_network.c +++ b/src/parallels/parallels_network.c @@ -230,7 +230,6 @@ parallelsLoadNetwork(parallelsConnPtr privconn, virJSONValuePtr jobj) goto cleanup; net->active = 1; net->autostart = 1; - virObjectUnlock(net); return net; cleanup: @@ -265,7 +264,6 @@ parallelsAddRoutedNetwork(parallelsConnPtr privconn) } net->active = 1; net->autostart = 1; - virObjectUnlock(net); return net; @@ -277,7 +275,7 @@ parallelsAddRoutedNetwork(parallelsConnPtr privconn) static int parallelsLoadNetworks(parallelsConnPtr privconn) { virJSONValuePtr jobj, jobj2; - virNetworkObjPtr net; + virNetworkObjPtr net = NULL; int ret = -1; int count; size_t i; @@ -305,16 +303,19 @@ static int parallelsLoadNetworks(parallelsConnPtr privconn) net = parallelsLoadNetwork(privconn, jobj2); if (!net) goto cleanup; + else + virNetworkObjEndAPI(&net); } - if (!parallelsAddRoutedNetwork(privconn)) + if (!(net = parallelsAddRoutedNetwork(privconn))) goto cleanup; ret = 0; cleanup: virJSONValueFree(jobj); + virNetworkObjEndAPI(&net); return ret; } diff --git a/src/test/test_driver.c b/src/test/test_driver.c index a7a3848..bfa0e91 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -787,7 +787,7 @@ testOpenDefault(virConnectPtr conn) goto error; } netobj->active = 1; - virObjectUnlock(netobj); + virNetworkObjEndAPI(&netobj); if (!(interfacedef = virInterfaceDefParseString(defaultInterfaceXML))) goto error; @@ -1155,7 +1155,7 @@ testParseNetworks(testConnPtr privconn, } obj->active = 1; - virObjectUnlock(obj); + virNetworkObjEndAPI(&obj); } ret = 0; -- 2.0.5

Now that we have fine grained locks, there's no need to lock the whole driver. We can rely on self-locking APIs. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/network/bridge_driver.c | 45 +++------------------------------------------ 1 file changed, 3 insertions(+), 42 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index c2a992a..56debb3 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -129,9 +129,7 @@ networkObjFromNetwork(virNetworkPtr net) virNetworkObjPtr network; char uuidstr[VIR_UUID_STRING_BUFLEN]; - networkDriverLock(); network = virNetworkObjFindByUUID(driver->networks, net->uuid); - networkDriverUnlock(); if (!network) { virUUIDFormat(net->uuid, uuidstr); @@ -264,6 +262,8 @@ networkRemoveInactive(virNetworkObjPtr net) int ret = -1; + networkDriverLock(); + /* remove the (possibly) existing dnsmasq and radvd files */ if (!(dctx = dnsmasqContextNew(def->name, driver->dnsmasqStateDir))) { @@ -315,6 +315,7 @@ networkRemoveInactive(virNetworkObjPtr net) VIR_FREE(radvdpidbase); VIR_FREE(statusfile); dnsmasqContextFree(dctx); + networkDriverUnlock(); return ret; } @@ -701,11 +702,9 @@ networkStateAutoStart(void) if (!driver) return; - networkDriverLock(); virNetworkObjListForEach(driver->networks, networkAutostartConfigs, NULL); - networkDriverUnlock(); } /** @@ -2479,9 +2478,7 @@ static virNetworkPtr networkLookupByUUID(virConnectPtr conn, virNetworkObjPtr network; virNetworkPtr ret = NULL; - networkDriverLock(); network = virNetworkObjFindByUUID(driver->networks, uuid); - networkDriverUnlock(); if (!network) { virReportError(VIR_ERR_NO_NETWORK, _("no network with matching uuid '%s'"), @@ -2505,9 +2502,7 @@ static virNetworkPtr networkLookupByName(virConnectPtr conn, virNetworkObjPtr network; virNetworkPtr ret = NULL; - networkDriverLock(); network = virNetworkObjFindByName(driver->networks, name); - networkDriverUnlock(); if (!network) { virReportError(VIR_ERR_NO_NETWORK, _("no network with matching name '%s'"), name); @@ -2531,12 +2526,10 @@ static int networkConnectNumOfNetworks(virConnectPtr conn) if (virConnectNumOfNetworksEnsureACL(conn) < 0) return -1; - networkDriverLock(); nactive = virNetworkObjListNumOfNetworks(driver->networks, true, virConnectNumOfNetworksCheckACL, conn); - networkDriverUnlock(); return nactive; } @@ -2547,12 +2540,10 @@ static int networkConnectListNetworks(virConnectPtr conn, char **const names, in if (virConnectListNetworksEnsureACL(conn) < 0) return -1; - networkDriverLock(); got = virNetworkObjListGetNames(driver->networks, true, names, nnames, virConnectListNetworksCheckACL, conn); - networkDriverUnlock(); return got; } @@ -2564,12 +2555,10 @@ static int networkConnectNumOfDefinedNetworks(virConnectPtr conn) if (virConnectNumOfDefinedNetworksEnsureACL(conn) < 0) return -1; - networkDriverLock(); ninactive = virNetworkObjListNumOfNetworks(driver->networks, false, virConnectNumOfDefinedNetworksCheckACL, conn); - networkDriverUnlock(); return ninactive; } @@ -2580,12 +2569,10 @@ static int networkConnectListDefinedNetworks(virConnectPtr conn, char **const na if (virConnectListDefinedNetworksEnsureACL(conn) < 0) return -1; - networkDriverLock(); got = virNetworkObjListGetNames(driver->networks, false, names, nnames, virConnectListDefinedNetworksCheckACL, conn); - networkDriverUnlock(); return got; } @@ -2601,11 +2588,9 @@ networkConnectListAllNetworks(virConnectPtr conn, if (virConnectListAllNetworksEnsureACL(conn) < 0) goto cleanup; - networkDriverLock(); ret = virNetworkObjListExport(conn, driver->networks, nets, virConnectListAllNetworksCheckACL, flags); - networkDriverUnlock(); cleanup: return ret; @@ -2916,8 +2901,6 @@ static virNetworkPtr networkCreateXML(virConnectPtr conn, const char *xml) virNetworkPtr ret = NULL; virObjectEventPtr event = NULL; - networkDriverLock(); - if (!(def = virNetworkDefParseString(xml))) goto cleanup; @@ -2955,7 +2938,6 @@ static virNetworkPtr networkCreateXML(virConnectPtr conn, const char *xml) if (event) virObjectEventStateQueue(driver->networkEventState, event); virNetworkObjEndAPI(&network); - networkDriverUnlock(); return ret; } @@ -2967,8 +2949,6 @@ static virNetworkPtr networkDefineXML(virConnectPtr conn, const char *xml) virNetworkPtr ret = NULL; virObjectEventPtr event = NULL; - networkDriverLock(); - if (!(def = virNetworkDefParseString(xml))) goto cleanup; @@ -3011,7 +2991,6 @@ static virNetworkPtr networkDefineXML(virConnectPtr conn, const char *xml) if (freeDef) virNetworkDefFree(def); virNetworkObjEndAPI(&network); - networkDriverUnlock(); return ret; } @@ -3023,8 +3002,6 @@ networkUndefine(virNetworkPtr net) bool active = false; virObjectEventPtr event = NULL; - networkDriverLock(); - network = virNetworkObjFindByUUID(driver->networks, net->uuid); if (!network) { virReportError(VIR_ERR_NO_NETWORK, @@ -3071,7 +3048,6 @@ networkUndefine(virNetworkPtr net) if (event) virObjectEventStateQueue(driver->networkEventState, event); virNetworkObjEndAPI(&network); - networkDriverUnlock(); return ret; } @@ -3095,8 +3071,6 @@ networkUpdate(virNetworkPtr net, VIR_NETWORK_UPDATE_AFFECT_CONFIG, -1); - networkDriverLock(); - network = virNetworkObjFindByUUID(driver->networks, net->uuid); if (!network) { virReportError(VIR_ERR_NO_NETWORK, @@ -3242,7 +3216,6 @@ networkUpdate(virNetworkPtr net, ret = 0; cleanup: virNetworkObjEndAPI(&network); - networkDriverUnlock(); return ret; } @@ -3252,7 +3225,6 @@ static int networkCreate(virNetworkPtr net) int ret = -1; virObjectEventPtr event = NULL; - networkDriverLock(); network = virNetworkObjFindByUUID(driver->networks, net->uuid); if (!network) { @@ -3276,7 +3248,6 @@ static int networkCreate(virNetworkPtr net) if (event) virObjectEventStateQueue(driver->networkEventState, event); virNetworkObjEndAPI(&network); - networkDriverUnlock(); return ret; } @@ -3286,7 +3257,6 @@ static int networkDestroy(virNetworkPtr net) int ret = -1; virObjectEventPtr event = NULL; - networkDriverLock(); network = virNetworkObjFindByUUID(driver->networks, net->uuid); if (!network) { @@ -3326,7 +3296,6 @@ static int networkDestroy(virNetworkPtr net) if (event) virObjectEventStateQueue(driver->networkEventState, event); virNetworkObjEndAPI(&network); - networkDriverUnlock(); return ret; } @@ -3736,9 +3705,7 @@ networkAllocateActualDevice(virDomainDefPtr dom, virDomainActualNetDefFree(iface->data.network.actual); iface->data.network.actual = NULL; - networkDriverLock(); network = virNetworkObjFindByName(driver->networks, iface->data.network.name); - networkDriverUnlock(); if (!network) { virReportError(VIR_ERR_NO_NETWORK, _("no network with matching name '%s'"), @@ -4144,9 +4111,7 @@ networkNotifyActualDevice(virDomainDefPtr dom, if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) return 0; - networkDriverLock(); network = virNetworkObjFindByName(driver->networks, iface->data.network.name); - networkDriverUnlock(); if (!network) { virReportError(VIR_ERR_NO_NETWORK, _("no network with matching name '%s'"), @@ -4343,9 +4308,7 @@ networkReleaseActualDevice(virDomainDefPtr dom, if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) return 0; - networkDriverLock(); network = virNetworkObjFindByName(driver->networks, iface->data.network.name); - networkDriverUnlock(); if (!network) { virReportError(VIR_ERR_NO_NETWORK, _("no network with matching name '%s'"), @@ -4501,9 +4464,7 @@ networkGetNetworkAddress(const char *netname, char **netaddr) char *dev_name = NULL; *netaddr = NULL; - networkDriverLock(); network = virNetworkObjFindByName(driver->networks, netname); - networkDriverUnlock(); if (!network) { virReportError(VIR_ERR_NO_NETWORK, _("no network with matching name '%s'"), -- 2.0.5

Well, if 'everywhere' is defined as that part of the driver code that serves virNetwork* APIs. Again, we lower layers already have their locks, so there's no point doing big lock. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/test/test_driver.c | 56 +------------------------------------------------- 1 file changed, 1 insertion(+), 55 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index bfa0e91..de138c6 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -3495,10 +3495,7 @@ static virNetworkPtr testNetworkLookupByUUID(virConnectPtr conn, virNetworkObjPtr net; virNetworkPtr ret = NULL; - testDriverLock(privconn); net = virNetworkObjFindByUUID(privconn->networks, uuid); - testDriverUnlock(privconn); - if (net == NULL) { virReportError(VIR_ERR_NO_NETWORK, NULL); goto cleanup; @@ -3518,10 +3515,7 @@ static virNetworkPtr testNetworkLookupByName(virConnectPtr conn, virNetworkObjPtr net; virNetworkPtr ret = NULL; - testDriverLock(privconn); net = virNetworkObjFindByName(privconn->networks, name); - testDriverUnlock(privconn); - if (net == NULL) { virReportError(VIR_ERR_NO_NETWORK, NULL); goto cleanup; @@ -3540,11 +3534,8 @@ static int testConnectNumOfNetworks(virConnectPtr conn) testConnPtr privconn = conn->privateData; int numActive; - testDriverLock(privconn); numActive = virNetworkObjListNumOfNetworks(privconn->networks, true, NULL, conn); - testDriverUnlock(privconn); - return numActive; } @@ -3552,11 +3543,8 @@ static int testConnectListNetworks(virConnectPtr conn, char **const names, int n testConnPtr privconn = conn->privateData; int n; - testDriverLock(privconn); n = virNetworkObjListGetNames(privconn->networks, true, names, nnames, NULL, conn); - testDriverUnlock(privconn); - return n; } @@ -3565,11 +3553,8 @@ static int testConnectNumOfDefinedNetworks(virConnectPtr conn) testConnPtr privconn = conn->privateData; int numInactive; - testDriverLock(privconn); numInactive = virNetworkObjListNumOfNetworks(privconn->networks, false, NULL, conn); - testDriverUnlock(privconn); - return numInactive; } @@ -3577,11 +3562,8 @@ static int testConnectListDefinedNetworks(virConnectPtr conn, char **const names testConnPtr privconn = conn->privateData; int n; - testDriverLock(privconn); n = virNetworkObjListGetNames(privconn->networks, false, names, nnames, NULL, conn); - testDriverUnlock(privconn); - return n; } @@ -3591,15 +3573,10 @@ testConnectListAllNetworks(virConnectPtr conn, unsigned int flags) { testConnPtr privconn = conn->privateData; - int ret = -1; virCheckFlags(VIR_CONNECT_LIST_NETWORKS_FILTERS_ALL, -1); - testDriverLock(privconn); - ret = virNetworkObjListExport(conn, privconn->networks, nets, NULL, flags); - testDriverUnlock(privconn); - - return ret; + return virNetworkObjListExport(conn, privconn->networks, nets, NULL, flags); } static int testNetworkIsActive(virNetworkPtr net) @@ -3608,9 +3585,7 @@ static int testNetworkIsActive(virNetworkPtr net) virNetworkObjPtr obj; int ret = -1; - testDriverLock(privconn); obj = virNetworkObjFindByUUID(privconn->networks, net->uuid); - testDriverUnlock(privconn); if (!obj) { virReportError(VIR_ERR_NO_NETWORK, NULL); goto cleanup; @@ -3628,9 +3603,7 @@ static int testNetworkIsPersistent(virNetworkPtr net) virNetworkObjPtr obj; int ret = -1; - testDriverLock(privconn); obj = virNetworkObjFindByUUID(privconn->networks, net->uuid); - testDriverUnlock(privconn); if (!obj) { virReportError(VIR_ERR_NO_NETWORK, NULL); goto cleanup; @@ -3651,7 +3624,6 @@ static virNetworkPtr testNetworkCreateXML(virConnectPtr conn, const char *xml) virNetworkPtr ret = NULL; virObjectEventPtr event = NULL; - testDriverLock(privconn); if ((def = virNetworkDefParseString(xml)) == NULL) goto cleanup; @@ -3671,7 +3643,6 @@ static virNetworkPtr testNetworkCreateXML(virConnectPtr conn, const char *xml) if (event) testObjectEventQueue(privconn, event); virNetworkObjEndAPI(&net); - testDriverUnlock(privconn); return ret; } @@ -3684,7 +3655,6 @@ virNetworkPtr testNetworkDefineXML(virConnectPtr conn, const char *xml) virNetworkPtr ret = NULL; virObjectEventPtr event = NULL; - testDriverLock(privconn); if ((def = virNetworkDefParseString(xml)) == NULL) goto cleanup; @@ -3703,7 +3673,6 @@ virNetworkPtr testNetworkDefineXML(virConnectPtr conn, const char *xml) if (event) testObjectEventQueue(privconn, event); virNetworkObjEndAPI(&net); - testDriverUnlock(privconn); return ret; } @@ -3714,7 +3683,6 @@ static int testNetworkUndefine(virNetworkPtr network) int ret = -1; virObjectEventPtr event = NULL; - testDriverLock(privconn); privnet = virNetworkObjFindByName(privconn->networks, network->name); if (privnet == NULL) { @@ -3739,7 +3707,6 @@ static int testNetworkUndefine(virNetworkPtr network) cleanup: if (event) testObjectEventQueue(privconn, event); - virNetworkObjEndAPI(&privnet); testDriverUnlock(privconn); return ret; } @@ -3760,8 +3727,6 @@ testNetworkUpdate(virNetworkPtr net, VIR_NETWORK_UPDATE_AFFECT_CONFIG, -1); - testDriverLock(privconn); - network = virNetworkObjFindByUUID(privconn->networks, net->uuid); if (!network) { virReportError(VIR_ERR_NO_NETWORK, @@ -3789,7 +3754,6 @@ testNetworkUpdate(virNetworkPtr net, ret = 0; cleanup: virNetworkObjEndAPI(&network); - testDriverUnlock(privconn); return ret; } @@ -3800,10 +3764,7 @@ static int testNetworkCreate(virNetworkPtr network) int ret = -1; virObjectEventPtr event = NULL; - testDriverLock(privconn); privnet = virNetworkObjFindByName(privconn->networks, network->name); - testDriverUnlock(privconn); - if (privnet == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); goto cleanup; @@ -3835,9 +3796,7 @@ static int testNetworkDestroy(virNetworkPtr network) int ret = -1; virObjectEventPtr event = NULL; - testDriverLock(privconn); privnet = virNetworkObjFindByName(privconn->networks, network->name); - if (privnet == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); goto cleanup; @@ -3857,7 +3816,6 @@ static int testNetworkDestroy(virNetworkPtr network) if (event) testObjectEventQueue(privconn, event); virNetworkObjEndAPI(&privnet); - testDriverUnlock(privconn); return ret; } @@ -3870,10 +3828,7 @@ static char *testNetworkGetXMLDesc(virNetworkPtr network, virCheckFlags(0, NULL); - testDriverLock(privconn); privnet = virNetworkObjFindByName(privconn->networks, network->name); - testDriverUnlock(privconn); - if (privnet == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); goto cleanup; @@ -3891,10 +3846,7 @@ static char *testNetworkGetBridgeName(virNetworkPtr network) { char *bridge = NULL; virNetworkObjPtr privnet; - testDriverLock(privconn); privnet = virNetworkObjFindByName(privconn->networks, network->name); - testDriverUnlock(privconn); - if (privnet == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); goto cleanup; @@ -3921,10 +3873,7 @@ static int testNetworkGetAutostart(virNetworkPtr network, virNetworkObjPtr privnet; int ret = -1; - testDriverLock(privconn); privnet = virNetworkObjFindByName(privconn->networks, network->name); - testDriverUnlock(privconn); - if (privnet == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); goto cleanup; @@ -3945,10 +3894,7 @@ static int testNetworkSetAutostart(virNetworkPtr network, virNetworkObjPtr privnet; int ret = -1; - testDriverLock(privconn); privnet = virNetworkObjFindByName(privconn->networks, network->name); - testDriverUnlock(privconn); - if (privnet == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); goto cleanup; -- 2.0.5