[libvirt PATCH 00/16] various network driver cleanups
These all bubbled to the top of a series that adds support for an nftables backend to the network driver, but are valid patches by themselves, so I'm sending them separately to make the other series look shorter. Laine Stump (16): conf: replace explicit virNetworkDefFree() with g_autoptr(virNetworkDef) esx: replace explicit virNetworkDefFree() with g_autoptr(virNetworkDef) network: replace explicit virNetworkDefFree() with g_autoptr(virNetworkDef) qemu: replace explicit virNetworkDefFree() with g_autoptr(virNetworkDef) test driver: replace explicit virNetworkDefFree() with g_autoptr(virNetworkDef) vbox: replace explicit virNetworkDefFree() with g_autoptr(virNetworkDef) tests: replace explicit virNetworkDefFree() with g_autoptr(virNetworkDef) conf: remove superfluous cleanup: labels and ret return variables qemu: remove superfluous cleanup: labels and ret return variables tests: remove superfluous cleanup: labels and ret return variables network: move driver state struct into bridge_driver_conf.h network: create separate config object for virNetworkDriverState util: replace g_snprintf with g_autofreed g_strdup_printf in viriptables.c util: remove unused function virFirewallApplyRuleFirewallD() util: make virFirewallRuleToString() global util: don't use virFirewallRuleToString() to log the rule being applied po/POTFILES | 1 + src/conf/domain_conf.c | 26 +-- src/conf/virnetworkobj.c | 72 ++++----- src/esx/esx_network_driver.c | 6 +- src/libvirt_private.syms | 1 + src/network/bridge_driver.c | 233 ++++++++++++--------------- src/network/bridge_driver_conf.c | 165 +++++++++++++++++++ src/network/bridge_driver_conf.h | 82 ++++++++++ src/network/bridge_driver_platform.h | 43 +---- src/network/meson.build | 1 + src/qemu/qemu_process.c | 24 ++- src/test/test_driver.c | 6 +- src/util/virfirewall.c | 28 ++-- src/util/virfirewall.h | 3 + src/util/viriptables.c | 15 +- src/vbox/vbox_network.c | 6 +- tests/networkxml2firewalltest.c | 15 +- tests/networkxml2xmltest.c | 3 +- tests/networkxml2xmlupdatetest.c | 8 +- 19 files changed, 443 insertions(+), 295 deletions(-) create mode 100644 src/network/bridge_driver_conf.c create mode 100644 src/network/bridge_driver_conf.h -- 2.37.1
Signed-off-by: Laine Stump <laine@redhat.com> --- src/conf/domain_conf.c | 3 +-- src/conf/virnetworkobj.c | 21 +++++++++------------ 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2fc94b40ef..f66a19311b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -29891,7 +29891,7 @@ virDomainNetBandwidthUpdate(virDomainNetDef *iface, int virDomainNetResolveActualType(virDomainNetDef *iface) { - virNetworkDef *def = NULL; + g_autoptr(virNetworkDef) def = NULL; int ret = -1; g_autofree char *xml = NULL; g_autoptr(virConnect) conn = NULL; @@ -29961,7 +29961,6 @@ virDomainNetResolveActualType(virDomainNetDef *iface) } cleanup: - virNetworkDefFree(def); return ret; } diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c index cc3b93db6d..3896f116fc 100644 --- a/src/conf/virnetworkobj.c +++ b/src/conf/virnetworkobj.c @@ -843,7 +843,7 @@ virNetworkLoadState(virNetworkObjList *nets, virNetworkXMLOption *xmlopt) { g_autofree char *configFile = NULL; - virNetworkDef *def = NULL; + g_autoptr(virNetworkDef) def = NULL; virNetworkObj *obj = NULL; g_autoptr(xmlDoc) xml = NULL; xmlNodePtr node = NULL; @@ -929,6 +929,7 @@ virNetworkLoadState(virNetworkObjList *nets, VIR_NETWORK_OBJ_LIST_ADD_LIVE))) goto error; /* do not put any "goto error" below this comment */ + def = NULL; /* assign status data stored in the network object */ if (classIdMap) { @@ -945,7 +946,6 @@ virNetworkLoadState(virNetworkObjList *nets, return obj; error: - virNetworkDefFree(def); return NULL; } @@ -958,7 +958,7 @@ virNetworkLoadConfig(virNetworkObjList *nets, virNetworkXMLOption *xmlopt) { char *configFile = NULL, *autostartLink = NULL; - virNetworkDef *def = NULL; + g_autoptr(virNetworkDef) def = NULL; virNetworkObj *obj; bool saveConfig = false; int autostart; @@ -1026,6 +1026,8 @@ virNetworkLoadConfig(virNetworkObjList *nets, if (!(obj = virNetworkObjAssignDef(nets, def, 0))) goto error; + def = NULL; + obj->autostart = (autostart == 1); VIR_FREE(configFile); @@ -1036,7 +1038,6 @@ virNetworkLoadConfig(virNetworkObjList *nets, error: VIR_FREE(configFile); VIR_FREE(autostartLink); - virNetworkDefFree(def); return NULL; } @@ -1213,15 +1214,15 @@ virNetworkObjUpdate(virNetworkObj *obj, unsigned int flags) /* virNetworkUpdateFlags */ { int ret = -1; - virNetworkDef *livedef = NULL; - virNetworkDef *configdef = NULL; + g_autoptr(virNetworkDef) livedef = NULL; + g_autoptr(virNetworkDef) configdef = NULL; /* normalize config data, and check for common invalid requests. */ if (virNetworkObjConfigChangeSetup(obj, xmlopt, flags) < 0) goto cleanup; if (flags & VIR_NETWORK_UPDATE_AFFECT_LIVE) { - virNetworkDef *checkdef; + g_autoptr(virNetworkDef) checkdef = NULL; /* work on a copy of the def */ if (!(livedef = virNetworkDefCopy(obj->def, xmlopt, 0))) @@ -1235,11 +1236,10 @@ virNetworkObjUpdate(virNetworkObj *obj, */ if (!(checkdef = virNetworkDefCopy(livedef, xmlopt, 0))) goto cleanup; - virNetworkDefFree(checkdef); } if (flags & VIR_NETWORK_UPDATE_AFFECT_CONFIG) { - virNetworkDef *checkdef; + g_autoptr(virNetworkDef) checkdef = NULL; /* work on a copy of the def */ if (!(configdef = virNetworkDefCopy(virNetworkObjGetPersistentDef(obj), @@ -1256,7 +1256,6 @@ virNetworkObjUpdate(virNetworkObj *obj, VIR_NETWORK_XML_INACTIVE))) { goto cleanup; } - virNetworkDefFree(checkdef); } if (configdef) { @@ -1273,8 +1272,6 @@ virNetworkObjUpdate(virNetworkObj *obj, ret = 0; cleanup: - virNetworkDefFree(livedef); - virNetworkDefFree(configdef); return ret; } -- 2.37.1
Signed-off-by: Laine Stump <laine@redhat.com> --- src/esx/esx_network_driver.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c index 290c0ad56f..bf9630ce9d 100644 --- a/src/esx/esx_network_driver.c +++ b/src/esx/esx_network_driver.c @@ -277,7 +277,7 @@ esxNetworkDefineXMLFlags(virConnectPtr conn, const char *xml, { virNetworkPtr network = NULL; esxPrivate *priv = conn->privateData; - virNetworkDef *def = NULL; + g_autoptr(virNetworkDef) def = NULL; esxVI_HostVirtualSwitch *hostVirtualSwitch = NULL; esxVI_HostPortGroup *hostPortGroupList = NULL; esxVI_HostPortGroup *hostPortGroup = NULL; @@ -483,7 +483,6 @@ esxNetworkDefineXMLFlags(virConnectPtr conn, const char *xml, network = virGetNetwork(conn, hostVirtualSwitch->name, md5); cleanup: - virNetworkDefFree(def); esxVI_HostVirtualSwitch_Free(&hostVirtualSwitch); esxVI_HostPortGroup_Free(&hostPortGroupList); esxVI_HostVirtualSwitchSpec_Free(&hostVirtualSwitchSpec); @@ -658,7 +657,7 @@ esxNetworkGetXMLDesc(virNetworkPtr network_, unsigned int flags) esxVI_String *networkNameList = NULL; esxVI_String *hostPortGroupKey = NULL; esxVI_String *networkName = NULL; - virNetworkDef *def; + g_autoptr(virNetworkDef) def = NULL; if (esxVI_EnsureSession(priv->primary) < 0) return NULL; @@ -812,7 +811,6 @@ esxNetworkGetXMLDesc(virNetworkPtr network_, unsigned int flags) esxVI_String_Free(&propertyNameList); esxVI_ObjectContent_Free(&networkList); esxVI_String_Free(&networkNameList); - virNetworkDefFree(def); return xml; } -- 2.37.1
Signed-off-by: Laine Stump <laine@redhat.com> --- src/network/bridge_driver.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index f6538d2638..733abaa719 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -3051,7 +3051,7 @@ networkCreateXMLFlags(virConnectPtr conn, unsigned int flags) { virNetworkDriverState *driver = networkGetDriver(); - virNetworkDef *newDef; + g_autoptr(virNetworkDef) newDef = NULL; virNetworkObj *obj = NULL; virNetworkDef *def; virNetworkPtr net = NULL; @@ -3077,6 +3077,7 @@ networkCreateXMLFlags(virConnectPtr conn, VIR_NETWORK_OBJ_LIST_ADD_LIVE | VIR_NETWORK_OBJ_LIST_ADD_CHECK_LIVE))) goto cleanup; + newDef = NULL; def = virNetworkObjGetDef(obj); @@ -3094,7 +3095,6 @@ networkCreateXMLFlags(virConnectPtr conn, net = virGetNetwork(conn, def->name, def->uuid); cleanup: - virNetworkDefFree(newDef); virObjectEventStateQueue(driver->networkEventState, event); virNetworkObjEndAPI(&obj); return net; @@ -3115,8 +3115,8 @@ networkDefineXMLFlags(virConnectPtr conn, unsigned int flags) { virNetworkDriverState *driver = networkGetDriver(); - virNetworkDef *def = NULL; - bool freeDef = true; + g_autoptr(virNetworkDef) def = NULL; + virNetworkDef *defAlias; virNetworkObj *obj = NULL; virNetworkPtr net = NULL; virObjectEvent *event = NULL; @@ -3127,6 +3127,8 @@ networkDefineXMLFlags(virConnectPtr conn, !!(flags & VIR_NETWORK_DEFINE_VALIDATE)))) goto cleanup; + defAlias = def; /* so we can still ref the object after nullifying def */ + if (virNetworkDefineXMLFlagsEnsureACL(conn, def) < 0) goto cleanup; @@ -3136,11 +3138,11 @@ networkDefineXMLFlags(virConnectPtr conn, if (!(obj = virNetworkObjAssignDef(driver->networks, def, 0))) goto cleanup; - /* def was assigned to network object */ - freeDef = false; + /* def was assigned to network object so don't autofree */ + def = NULL; if (virNetworkSaveConfig(driver->networkConfigDir, - def, network_driver->xmlopt) < 0) { + defAlias, network_driver->xmlopt) < 0) { if (!virNetworkObjIsActive(obj)) { virNetworkObjRemoveInactive(driver->networks, obj); goto cleanup; @@ -3153,17 +3155,15 @@ networkDefineXMLFlags(virConnectPtr conn, goto cleanup; } - event = virNetworkEventLifecycleNew(def->name, def->uuid, + event = virNetworkEventLifecycleNew(defAlias->name, defAlias->uuid, VIR_NETWORK_EVENT_DEFINED, 0); - VIR_INFO("Defining network '%s'", def->name); - net = virGetNetwork(conn, def->name, def->uuid); + VIR_INFO("Defining network '%s'", defAlias->name); + net = virGetNetwork(conn, defAlias->name, defAlias->uuid); cleanup: virObjectEventStateQueue(driver->networkEventState, event); - if (freeDef) - virNetworkDefFree(def); virNetworkObjEndAPI(&obj); return net; } -- 2.37.1
Signed-off-by: Laine Stump <laine@redhat.com> --- src/qemu/qemu_process.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 5c8413a6b6..002eab43be 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4828,7 +4828,7 @@ qemuProcessGetNetworkAddress(const char *netname, g_autoptr(virConnect) conn = NULL; int ret = -1; g_autoptr(virNetwork) net = NULL; - virNetworkDef *netdef = NULL; + g_autoptr(virNetworkDef) netdef = NULL; virNetworkIPDef *ipdef; virSocketAddr addr; virSocketAddr *addrptr = NULL; @@ -4912,7 +4912,6 @@ qemuProcessGetNetworkAddress(const char *netname, ret = 0; cleanup: - virNetworkDefFree(netdef); return ret; } -- 2.37.1
Signed-off-by: Laine Stump <laine@redhat.com> --- src/test/test_driver.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 24ff6e8967..641a141b6a 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -5561,7 +5561,7 @@ testNetworkCreateXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) { testDriver *privconn = conn->privateData; - virNetworkDef *newDef; + g_autoptr(virNetworkDef) newDef = NULL; virNetworkObj *obj = NULL; virNetworkDef *def; virNetworkPtr net = NULL; @@ -5588,7 +5588,6 @@ testNetworkCreateXMLFlags(virConnectPtr conn, const char *xml, net = virGetNetwork(conn, def->name, def->uuid); cleanup: - virNetworkDefFree(newDef); virObjectEventStateQueue(privconn->eventState, event); virNetworkObjEndAPI(&obj); return net; @@ -5608,7 +5607,7 @@ testNetworkDefineXMLFlags(virConnectPtr conn, unsigned int flags) { testDriver *privconn = conn->privateData; - virNetworkDef *newDef; + g_autoptr(virNetworkDef) newDef = NULL; virNetworkObj *obj = NULL; virNetworkDef *def; virNetworkPtr net = NULL; @@ -5632,7 +5631,6 @@ testNetworkDefineXMLFlags(virConnectPtr conn, net = virGetNetwork(conn, def->name, def->uuid); cleanup: - virNetworkDefFree(newDef); virObjectEventStateQueue(privconn->eventState, event); virNetworkObjEndAPI(&obj); return net; -- 2.37.1
Signed-off-by: Laine Stump <laine@redhat.com> --- src/vbox/vbox_network.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/vbox/vbox_network.c b/src/vbox/vbox_network.c index f687ebbf1a..885fd48321 100644 --- a/src/vbox/vbox_network.c +++ b/src/vbox/vbox_network.c @@ -374,7 +374,7 @@ vboxNetworkDefineCreateXML(virConnectPtr conn, const char *xml, bool start, PRUnichar *networkNameUtf16 = NULL; char *networkNameUtf8 = NULL; IHostNetworkInterface *networkInterface = NULL; - virNetworkDef *def = NULL; + g_autoptr(virNetworkDef) def = NULL; virNetworkIPDef *ipdef = NULL; unsigned char uuid[VIR_UUID_BUFLEN]; vboxIID vboxnetiid; @@ -556,7 +556,6 @@ vboxNetworkDefineCreateXML(virConnectPtr conn, const char *xml, bool start, VBOX_UTF8_FREE(networkInterfaceNameUtf8); VBOX_UTF16_FREE(networkInterfaceNameUtf16); VBOX_RELEASE(host); - virNetworkDefFree(def); return ret; } @@ -781,7 +780,7 @@ vboxSocketParseAddrUtf16(struct _vboxDriver *data, const PRUnichar *utf16, static char *vboxNetworkGetXMLDesc(virNetworkPtr network, unsigned int flags) { struct _vboxDriver *data = network->conn->privateData; - virNetworkDef *def = NULL; + g_autoptr(virNetworkDef) def = NULL; virNetworkIPDef *ipdef = NULL; char *networkNameUtf8 = NULL; PRUnichar *networkInterfaceNameUtf16 = NULL; @@ -926,7 +925,6 @@ static char *vboxNetworkGetXMLDesc(virNetworkPtr network, unsigned int flags) VBOX_RELEASE(networkInterface); VBOX_UTF16_FREE(networkInterfaceNameUtf16); VBOX_RELEASE(host); - virNetworkDefFree(def); VIR_FREE(networkNameUtf8); VBOX_RELEASE(dhcpServer); return ret; -- 2.37.1
Signed-off-by: Laine Stump <laine@redhat.com> --- tests/networkxml2firewalltest.c | 3 +-- tests/networkxml2xmltest.c | 3 +-- tests/networkxml2xmlupdatetest.c | 3 +-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/tests/networkxml2firewalltest.c b/tests/networkxml2firewalltest.c index 2648115a12..f895ca8c56 100644 --- a/tests/networkxml2firewalltest.c +++ b/tests/networkxml2firewalltest.c @@ -89,7 +89,7 @@ static int testCompareXMLToArgvFiles(const char *xml, { g_autofree char *actualargv = NULL; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; - virNetworkDef *def = NULL; + g_autoptr(virNetworkDef) def = NULL; int ret = -1; char *actual; g_autoptr(virCommandDryRunToken) dryRunToken = virCommandDryRunTokenNew(); @@ -117,7 +117,6 @@ static int testCompareXMLToArgvFiles(const char *xml, ret = 0; cleanup: - virNetworkDefFree(def); return ret; } diff --git a/tests/networkxml2xmltest.c b/tests/networkxml2xmltest.c index 5f39162ef1..521f058acc 100644 --- a/tests/networkxml2xmltest.c +++ b/tests/networkxml2xmltest.c @@ -27,7 +27,7 @@ testCompareXMLToXMLFiles(const char *inxml, const char *outxml, g_autofree char *actual = NULL; int ret; testCompareNetXML2XMLResult result = TEST_COMPARE_NET_XML2XML_RESULT_SUCCESS; - virNetworkDef *dev = NULL; + g_autoptr(virNetworkDef) dev = NULL; g_autoptr(virNetworkXMLOption) xmlopt = NULL; if (!(xmlopt = networkDnsmasqCreateXMLConf())) @@ -68,7 +68,6 @@ testCompareXMLToXMLFiles(const char *inxml, const char *outxml, } virResetLastError(); - virNetworkDefFree(dev); return ret; } diff --git a/tests/networkxml2xmlupdatetest.c b/tests/networkxml2xmlupdatetest.c index c50c6c8558..4926609f90 100644 --- a/tests/networkxml2xmlupdatetest.c +++ b/tests/networkxml2xmlupdatetest.c @@ -20,7 +20,7 @@ testCompareXMLToXMLFiles(const char *netxml, const char *updatexml, g_autofree char *updateXmlData = NULL; g_autofree char *actual = NULL; int ret = -1; - virNetworkDef *def = NULL; + g_autoptr(virNetworkDef) def = NULL; if (virTestLoadFile(updatexml, &updateXmlData) < 0) goto error; @@ -53,7 +53,6 @@ testCompareXMLToXMLFiles(const char *netxml, const char *updatexml, } } error: - virNetworkDefFree(def); return ret; } -- 2.37.1
After converting virNetworkDef * to g_autoptr(virNetworkDef) the cleanup codepath was empty, so it has been removed. Signed-off-by: Laine Stump <laine@redhat.com> --- src/conf/domain_conf.c | 23 +++++++++--------- src/conf/virnetworkobj.c | 51 ++++++++++++++++++---------------------- 2 files changed, 35 insertions(+), 39 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f66a19311b..610bbcfd58 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -29892,7 +29892,6 @@ int virDomainNetResolveActualType(virDomainNetDef *iface) { g_autoptr(virNetworkDef) def = NULL; - int ret = -1; g_autofree char *xml = NULL; g_autoptr(virConnect) conn = NULL; g_autoptr(virNetwork) net = NULL; @@ -29907,13 +29906,13 @@ virDomainNetResolveActualType(virDomainNetDef *iface) return -1; if (!(net = virNetworkLookupByName(conn, iface->data.network.name))) - goto cleanup; + return -1; if (!(xml = virNetworkGetXMLDesc(net, 0))) - goto cleanup; + return -1; if (!(def = virNetworkDefParseString(xml, NULL, false))) - goto cleanup; + return -1; switch ((virNetworkForwardType) def->forward.type) { case VIR_NETWORK_FORWARD_NONE: @@ -29924,11 +29923,11 @@ virDomainNetResolveActualType(virDomainNetDef *iface) * NETWORK; we just keep the info from the portgroup in * iface->data.network.actual */ - ret = VIR_DOMAIN_NET_TYPE_NETWORK; + return VIR_DOMAIN_NET_TYPE_NETWORK; break; case VIR_NETWORK_FORWARD_HOSTDEV: - ret = VIR_DOMAIN_NET_TYPE_HOSTDEV; + return VIR_DOMAIN_NET_TYPE_HOSTDEV; break; case VIR_NETWORK_FORWARD_BRIDGE: @@ -29936,7 +29935,7 @@ virDomainNetResolveActualType(virDomainNetDef *iface) /* <forward type='bridge'/> <bridge name='xxx'/> * is VIR_DOMAIN_NET_TYPE_BRIDGE */ - ret = VIR_DOMAIN_NET_TYPE_BRIDGE; + return VIR_DOMAIN_NET_TYPE_BRIDGE; break; } @@ -29951,17 +29950,19 @@ virDomainNetResolveActualType(virDomainNetDef *iface) /* <forward type='bridge|private|vepa|passthrough'> are all * VIR_DOMAIN_NET_TYPE_DIRECT. */ - ret = VIR_DOMAIN_NET_TYPE_DIRECT; + return VIR_DOMAIN_NET_TYPE_DIRECT; break; case VIR_NETWORK_FORWARD_LAST: default: virReportEnumRangeError(virNetworkForwardType, def->forward.type); - goto cleanup; + return -1; } - cleanup: - return ret; + /* this line is unreachable due to the preceding switch, but the compiler + * requires some kind of return at the end of the function. + */ + return VIR_NETWORK_FORWARD_NONE; } diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c index 3896f116fc..b4c61fff85 100644 --- a/src/conf/virnetworkobj.c +++ b/src/conf/virnetworkobj.c @@ -856,28 +856,28 @@ virNetworkLoadState(virNetworkObjList *nets, if ((configFile = virNetworkConfigFile(stateDir, name)) == NULL) - goto error; + return NULL; if (!(xml = virXMLParseCtxt(configFile, NULL, _("(network status)"), &ctxt))) - goto error; + return NULL; if (!(node = virXPathNode("//network", ctxt))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not find any 'network' element in status file")); - goto error; + return NULL; } /* parse the definition first */ ctxt->node = node; if (!(def = virNetworkDefParseXML(ctxt, xmlopt))) - goto error; + return NULL; if (STRNEQ(name, def->name)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Network config filename '%s'" " does not match network name '%s'"), configFile, def->name); - goto error; + return NULL; } /* now parse possible status data */ @@ -893,7 +893,7 @@ virNetworkLoadState(virNetworkObjList *nets, if ((classIdStr = virXPathString("string(./class_id[1]/@bitmap)", ctxt))) { if (!(classIdMap = virBitmapParseUnlimited(classIdStr))) - goto error; + return NULL; } floor_sum = virXPathString("string(./floor[1]/@sum)", ctxt); @@ -902,11 +902,11 @@ virNetworkLoadState(virNetworkObjList *nets, virReportError(VIR_ERR_INTERNAL_ERROR, _("Malformed 'floor_sum' attribute: %s"), floor_sum); - goto error; + return NULL; } if ((n = virXPathNodeSet("./taint", ctxt, &nodes)) < 0) - goto error; + return NULL; for (i = 0; i < n; i++) { g_autofree char *str = virXMLPropString(nodes[i], "flag"); @@ -915,7 +915,7 @@ virNetworkLoadState(virNetworkObjList *nets, if (flag < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unknown taint flag %s"), str); - goto error; + return NULL; } /* Compute taint mask here. The network object does not * exist yet, so we can't use virNetworkObjtTaint. */ @@ -925,10 +925,9 @@ virNetworkLoadState(virNetworkObjList *nets, } /* create the object */ - if (!(obj = virNetworkObjAssignDef(nets, def, - VIR_NETWORK_OBJ_LIST_ADD_LIVE))) - goto error; - /* do not put any "goto error" below this comment */ + if (!(obj = virNetworkObjAssignDef(nets, def, VIR_NETWORK_OBJ_LIST_ADD_LIVE))) + return NULL; + def = NULL; /* assign status data stored in the network object */ @@ -944,9 +943,6 @@ virNetworkLoadState(virNetworkObjList *nets, obj->active = true; /* network with a state file is by definition active */ return obj; - - error: - return NULL; } @@ -1213,29 +1209,29 @@ virNetworkObjUpdate(virNetworkObj *obj, virNetworkXMLOption *xmlopt, unsigned int flags) /* virNetworkUpdateFlags */ { - int ret = -1; g_autoptr(virNetworkDef) livedef = NULL; g_autoptr(virNetworkDef) configdef = NULL; /* normalize config data, and check for common invalid requests. */ if (virNetworkObjConfigChangeSetup(obj, xmlopt, flags) < 0) - goto cleanup; + return -1; if (flags & VIR_NETWORK_UPDATE_AFFECT_LIVE) { g_autoptr(virNetworkDef) checkdef = NULL; /* work on a copy of the def */ if (!(livedef = virNetworkDefCopy(obj->def, xmlopt, 0))) - goto cleanup; + return -1; + if (virNetworkDefUpdateSection(livedef, command, section, parentIndex, xml, flags) < 0) { - goto cleanup; + return -1; } /* run a final format/parse cycle to make sure we didn't * add anything illegal to the def */ if (!(checkdef = virNetworkDefCopy(livedef, xmlopt, 0))) - goto cleanup; + return -1; } if (flags & VIR_NETWORK_UPDATE_AFFECT_CONFIG) { @@ -1245,23 +1241,24 @@ virNetworkObjUpdate(virNetworkObj *obj, if (!(configdef = virNetworkDefCopy(virNetworkObjGetPersistentDef(obj), xmlopt, VIR_NETWORK_XML_INACTIVE))) { - goto cleanup; + return -1; } if (virNetworkDefUpdateSection(configdef, command, section, parentIndex, xml, flags) < 0) { - goto cleanup; + return -1; } if (!(checkdef = virNetworkDefCopy(configdef, xmlopt, VIR_NETWORK_XML_INACTIVE))) { - goto cleanup; + return -1; } } if (configdef) { /* successfully modified copy, now replace original */ if (virNetworkObjReplacePersistentDef(obj, configdef) < 0) - goto cleanup; + return -1; + configdef = NULL; } if (livedef) { @@ -1270,9 +1267,7 @@ virNetworkObjUpdate(virNetworkObj *obj, obj->def = g_steal_pointer(&livedef); } - ret = 0; - cleanup: - return ret; + return 0; } -- 2.37.1
After converting virNetworkDef * to g_autoptr(virNetworkDef) the cleanup codepath was empty, so it has been removed. Signed-off-by: Laine Stump <laine@redhat.com> --- src/qemu/qemu_process.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 002eab43be..68c2f45535 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4826,7 +4826,6 @@ qemuProcessGetNetworkAddress(const char *netname, char **netaddr) { g_autoptr(virConnect) conn = NULL; - int ret = -1; g_autoptr(virNetwork) net = NULL; g_autoptr(virNetworkDef) netdef = NULL; virNetworkIPDef *ipdef; @@ -4842,15 +4841,15 @@ qemuProcessGetNetworkAddress(const char *netname, net = virNetworkLookupByName(conn, netname); if (!net) - goto cleanup; + return -1; xml = virNetworkGetXMLDesc(net, 0); if (!xml) - goto cleanup; + return -1; netdef = virNetworkDefParseString(xml, NULL, false); if (!netdef) - goto cleanup; + return -1; switch ((virNetworkForwardType) netdef->forward.type) { case VIR_NETWORK_FORWARD_NONE: @@ -4862,7 +4861,7 @@ qemuProcessGetNetworkAddress(const char *netname, virReportError(VIR_ERR_INTERNAL_ERROR, _("network '%s' doesn't have an IP address"), netdef->name); - goto cleanup; + return -1; } addrptr = &ipdef->address; break; @@ -4886,7 +4885,7 @@ qemuProcessGetNetworkAddress(const char *netname, virReportError(VIR_ERR_INTERNAL_ERROR, _("network '%s' has no associated interface or bridge"), netdef->name); - goto cleanup; + return -1; } break; @@ -4896,23 +4895,21 @@ qemuProcessGetNetworkAddress(const char *netname, case VIR_NETWORK_FORWARD_LAST: default: virReportEnumRangeError(virNetworkForwardType, netdef->forward.type); - goto cleanup; + return -1; } if (dev_name) { if (virNetDevIPAddrGet(dev_name, &addr) < 0) - goto cleanup; + return -1; addrptr = &addr; } if (!(addrptr && (*netaddr = virSocketAddrFormat(addrptr)))) { - goto cleanup; + return -1; } - ret = 0; - cleanup: - return ret; + return 0; } -- 2.37.1
After converting virNetworkDef * to g_autoptr(virNetworkDef) the cleanup codepath was empty, so it has been removed. Signed-off-by: Laine Stump <laine@redhat.com> --- tests/networkxml2firewalltest.c | 12 ++++-------- tests/networkxml2xmlupdatetest.c | 5 ++--- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/tests/networkxml2firewalltest.c b/tests/networkxml2firewalltest.c index f895ca8c56..ca793fd4ea 100644 --- a/tests/networkxml2firewalltest.c +++ b/tests/networkxml2firewalltest.c @@ -90,17 +90,16 @@ static int testCompareXMLToArgvFiles(const char *xml, g_autofree char *actualargv = NULL; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; g_autoptr(virNetworkDef) def = NULL; - int ret = -1; char *actual; g_autoptr(virCommandDryRunToken) dryRunToken = virCommandDryRunTokenNew(); virCommandSetDryRun(dryRunToken, &buf, true, true, testCommandDryRun, NULL); if (!(def = virNetworkDefParseFile(xml, NULL))) - goto cleanup; + return -1; if (networkAddFirewallRules(def) < 0) - goto cleanup; + return -1; actual = actualargv = virBufferContentAndReset(&buf); @@ -112,12 +111,9 @@ static int testCompareXMLToArgvFiles(const char *xml, actual += strlen(baseargs); if (virTestCompareToFileFull(actual, cmdline, false) < 0) - goto cleanup; + return -1; - ret = 0; - - cleanup: - return ret; + return 0; } struct testInfo { diff --git a/tests/networkxml2xmlupdatetest.c b/tests/networkxml2xmlupdatetest.c index 4926609f90..58b6f70c96 100644 --- a/tests/networkxml2xmlupdatetest.c +++ b/tests/networkxml2xmlupdatetest.c @@ -23,7 +23,7 @@ testCompareXMLToXMLFiles(const char *netxml, const char *updatexml, g_autoptr(virNetworkDef) def = NULL; if (virTestLoadFile(updatexml, &updateXmlData) < 0) - goto error; + return -1; if (!(def = virNetworkDefParseFile(netxml, NULL))) goto fail; @@ -37,7 +37,7 @@ testCompareXMLToXMLFiles(const char *netxml, const char *updatexml, if (!expectFailure) { if (virTestCompareToFile(actual, outxml) < 0) - goto error; + return -1; } ret = 0; @@ -52,7 +52,6 @@ testCompareXMLToXMLFiles(const char *netxml, const char *updatexml, ret = 0; } } - error: return ret; } -- 2.37.1
This is more similar to lxc and qemu drivers, where the driver state struct is defined along with a config struct in ${driver}_conf.h Signed-off-by: Laine Stump <laine@redhat.com> --- src/network/bridge_driver.c | 25 ----------- src/network/bridge_driver_conf.c | 56 +++++++++++++++++++++++ src/network/bridge_driver_conf.h | 67 ++++++++++++++++++++++++++++ src/network/bridge_driver_platform.h | 43 +++--------------- src/network/meson.build | 1 + 5 files changed, 129 insertions(+), 63 deletions(-) create mode 100644 src/network/bridge_driver_conf.c create mode 100644 src/network/bridge_driver_conf.h diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 733abaa719..3c36a66ec7 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -95,31 +95,6 @@ networkGetDriver(void) } -static dnsmasqCaps * -networkGetDnsmasqCaps(virNetworkDriverState *driver) -{ - VIR_LOCK_GUARD lock = virLockGuardLock(&driver->lock); - return virObjectRef(driver->dnsmasqCaps); -} - - -static int -networkDnsmasqCapsRefresh(virNetworkDriverState *driver) -{ - dnsmasqCaps *caps; - - if (!(caps = dnsmasqCapsNewFromBinary())) - return -1; - - VIR_WITH_MUTEX_LOCK_GUARD(&driver->lock) { - virObjectUnref(driver->dnsmasqCaps); - driver->dnsmasqCaps = caps; - } - - return 0; -} - - extern virXMLNamespace networkDnsmasqXMLNamespace; typedef struct _networkDnsmasqXmlNsDef networkDnsmasqXmlNsDef; diff --git a/src/network/bridge_driver_conf.c b/src/network/bridge_driver_conf.c new file mode 100644 index 0000000000..b6c059e1e9 --- /dev/null +++ b/src/network/bridge_driver_conf.c @@ -0,0 +1,56 @@ +/* + * Copyright (C) 2022 Red Hat, Inc. + * + * bridge_driver__conf.c: network.conf config file inspection + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + */ + +/* includes */ +#include <config.h> +#include "virerror.h" +#include "datatypes.h" +#include "virlog.h" +#include "bridge_driver_conf.h" + +#define VIR_FROM_THIS VIR_FROM_NETWORK + +VIR_LOG_INIT("network.bridge_driver"); + + +dnsmasqCaps * +networkGetDnsmasqCaps(virNetworkDriverState *driver) +{ + VIR_LOCK_GUARD lock = virLockGuardLock(&driver->lock); + return virObjectRef(driver->dnsmasqCaps); +} + + +int +networkDnsmasqCapsRefresh(virNetworkDriverState *driver) +{ + dnsmasqCaps *caps; + + if (!(caps = dnsmasqCapsNewFromBinary())) + return -1; + + VIR_WITH_MUTEX_LOCK_GUARD(&driver->lock) { + virObjectUnref(driver->dnsmasqCaps); + driver->dnsmasqCaps = caps; + } + + return 0; +} diff --git a/src/network/bridge_driver_conf.h b/src/network/bridge_driver_conf.h new file mode 100644 index 0000000000..f4307bbdba --- /dev/null +++ b/src/network/bridge_driver_conf.h @@ -0,0 +1,67 @@ +/* + * bridge_driver_conf.h: network bridge driver state and config objects + * + * Copyright (C) 2006-2013 Red Hat, Inc. + * Copyright (C) 2006 Daniel P. Berrange + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#pragma once + +#include "internal.h" +#include "virthread.h" +#include "virdnsmasq.h" +#include "virnetworkobj.h" +#include "object_event.h" + +/* Main driver state */ +struct _virNetworkDriverState { + virMutex lock; + + /* Read-only */ + bool privileged; + + /* pid file FD, ensures two copies of the driver can't use the same root */ + int lockFD; + + /* Immutable pointer, self-locking APIs */ + virNetworkObjList *networks; + + /* Immutable pointers, Immutable objects */ + char *networkConfigDir; + char *networkAutostartDir; + char *stateDir; + char *pidDir; + char *dnsmasqStateDir; + + /* Require lock to get a reference on the object, + * lockless access thereafter + */ + dnsmasqCaps *dnsmasqCaps; + + /* Immutable pointer, self-locking APIs */ + virObjectEventState *networkEventState; + + virNetworkXMLOption *xmlopt; +}; + +typedef struct _virNetworkDriverState virNetworkDriverState; + +dnsmasqCaps * +networkGetDnsmasqCaps(virNetworkDriverState *driver); + +int +networkDnsmasqCapsRefresh(virNetworkDriverState *driver); diff --git a/src/network/bridge_driver_platform.h b/src/network/bridge_driver_platform.h index de7cbc1195..b720d343be 100644 --- a/src/network/bridge_driver_platform.h +++ b/src/network/bridge_driver_platform.h @@ -21,46 +21,13 @@ #pragma once -#include "internal.h" -#include "virthread.h" -#include "virdnsmasq.h" -#include "virnetworkobj.h" -#include "object_event.h" +#include "network_conf.h" +#include "bridge_driver_conf.h" -/* Main driver state */ -struct _virNetworkDriverState { - virMutex lock; +void networkPreReloadFirewallRules(virNetworkDriverState *driver, + bool startup, + bool force); - /* Read-only */ - bool privileged; - - /* pid file FD, ensures two copies of the driver can't use the same root */ - int lockFD; - - /* Immutable pointer, self-locking APIs */ - virNetworkObjList *networks; - - /* Immutable pointers, Immutable objects */ - char *networkConfigDir; - char *networkAutostartDir; - char *stateDir; - char *pidDir; - char *dnsmasqStateDir; - - /* Require lock to get a reference on the object, - * lockless access thereafter - */ - dnsmasqCaps *dnsmasqCaps; - - /* Immutable pointer, self-locking APIs */ - virObjectEventState *networkEventState; - - virNetworkXMLOption *xmlopt; -}; - -typedef struct _virNetworkDriverState virNetworkDriverState; - -void networkPreReloadFirewallRules(virNetworkDriverState *driver, bool startup, bool force); void networkPostReloadFirewallRules(bool startup); int networkCheckRouteCollision(virNetworkDef *def); diff --git a/src/network/meson.build b/src/network/meson.build index b5eff0c3ab..395074a0a0 100644 --- a/src/network/meson.build +++ b/src/network/meson.build @@ -1,5 +1,6 @@ network_driver_sources = [ 'bridge_driver.c', + 'bridge_driver_conf.c', 'bridge_driver_platform.c', ] -- 2.37.1
Similar to the other drivers, virNetworkDriverState now has a virObject-derived object called virNetworkDriverConfig which is used for config items. As a starting point, the directory paths used by the network driver are moved there (again, parallelling what is done for other drivers). Using items in virNetworkDriverConfig is (yes, again) similar to using items in the other drivers' config - anything in the config object is immutable (once initialized), so the state object only needs to be locked while getting a reference to the config object, and then the members of the config object can be safely used until the config object is unrefed. Signed-off-by: Laine Stump <laine@redhat.com> --- po/POTFILES | 1 + src/network/bridge_driver.c | 184 +++++++++++++++---------------- src/network/bridge_driver_conf.c | 111 ++++++++++++++++++- src/network/bridge_driver_conf.h | 31 ++++-- 4 files changed, 224 insertions(+), 103 deletions(-) diff --git a/po/POTFILES b/po/POTFILES index 9621efb0d3..d32105f7d5 100644 --- a/po/POTFILES +++ b/po/POTFILES @@ -137,6 +137,7 @@ src/lxc/lxc_hostdev.c src/lxc/lxc_native.c src/lxc/lxc_process.c src/network/bridge_driver.c +src/network/bridge_driver_conf.c src/network/bridge_driver_linux.c src/network/leaseshelper.c src/node_device/node_device_driver.c diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 3c36a66ec7..7c7812e276 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -302,26 +302,26 @@ networkRunHook(virNetworkObj *obj, static char * -networkDnsmasqLeaseFileNameDefault(virNetworkDriverState *driver, +networkDnsmasqLeaseFileNameDefault(virNetworkDriverConfig *cfg, const char *netname) { - return g_strdup_printf("%s/%s.leases", driver->dnsmasqStateDir, netname); + return g_strdup_printf("%s/%s.leases", cfg->dnsmasqStateDir, netname); } static char * -networkDnsmasqLeaseFileNameCustom(virNetworkDriverState *driver, +networkDnsmasqLeaseFileNameCustom(virNetworkDriverConfig *cfg, const char *bridge) { - return g_strdup_printf("%s/%s.status", driver->dnsmasqStateDir, bridge); + return g_strdup_printf("%s/%s.status", cfg->dnsmasqStateDir, bridge); } static char * -networkDnsmasqConfigFileName(virNetworkDriverState *driver, +networkDnsmasqConfigFileName(virNetworkDriverConfig *cfg, const char *netname) { - return g_strdup_printf("%s/%s.conf", driver->dnsmasqStateDir, netname); + return g_strdup_printf("%s/%s.conf", cfg->dnsmasqStateDir, netname); } @@ -330,6 +330,7 @@ static int networkRemoveInactive(virNetworkDriverState *driver, virNetworkObj *obj) { + g_autoptr(virNetworkDriverConfig) cfg = virNetworkDriverGetConfig(driver); g_autofree char *leasefile = NULL; g_autofree char *customleasefile = NULL; g_autofree char *configfile = NULL; @@ -340,23 +341,23 @@ networkRemoveInactive(virNetworkDriverState *driver, /* remove the (possibly) existing dnsmasq files */ if (!(dctx = dnsmasqContextNew(def->name, - driver->dnsmasqStateDir))) { + cfg->dnsmasqStateDir))) { return -1; } - if (!(leasefile = networkDnsmasqLeaseFileNameDefault(driver, def->name))) + if (!(leasefile = networkDnsmasqLeaseFileNameDefault(cfg, def->name))) return -1; - if (!(customleasefile = networkDnsmasqLeaseFileNameCustom(driver, def->bridge))) + if (!(customleasefile = networkDnsmasqLeaseFileNameCustom(cfg, def->bridge))) return -1; - if (!(configfile = networkDnsmasqConfigFileName(driver, def->name))) + if (!(configfile = networkDnsmasqConfigFileName(cfg, def->name))) return -1; - if (!(statusfile = virNetworkConfigFile(driver->stateDir, def->name))) + if (!(statusfile = virNetworkConfigFile(cfg->stateDir, def->name))) return -1; - if (!(macMapFile = virMacMapFileName(driver->dnsmasqStateDir, def->bridge))) + if (!(macMapFile = virMacMapFileName(cfg->dnsmasqStateDir, def->bridge))) return -1; /* dnsmasq */ @@ -419,14 +420,14 @@ networkUpdatePort(virNetworkPortDef *port, } static int -networkSetMacMap(virNetworkDriverState *driver, +networkSetMacMap(virNetworkDriverConfig *cfg, virNetworkObj *obj) { virNetworkDef *def = virNetworkObjGetDef(obj); g_autoptr(virMacMap) macmap = NULL; g_autofree char *macMapFile = NULL; - if (!(macMapFile = virMacMapFileName(driver->dnsmasqStateDir, + if (!(macMapFile = virMacMapFileName(cfg->dnsmasqStateDir, def->bridge))) return -1; if (!(macmap = virMacMapNew(macMapFile))) @@ -442,6 +443,7 @@ networkUpdateState(virNetworkObj *obj, { virNetworkDef *def; virNetworkDriverState *driver = opaque; + g_autoptr(virNetworkDriverConfig) cfg = virNetworkDriverGetConfig(driver); g_autoptr(dnsmasqCaps) dnsmasq_caps = networkGetDnsmasqCaps(driver); VIR_LOCK_GUARD lock = virObjectLockGuard(obj); @@ -493,10 +495,10 @@ networkUpdateState(virNetworkObj *obj, if (virNetworkObjIsActive(obj) && def->ips && (def->nips > 0)) { pid_t dnsmasqPid; - if (networkSetMacMap(driver, obj) < 0) + if (networkSetMacMap(cfg, obj) < 0) return -1; - ignore_value(virPidFileReadIfAlive(driver->pidDir, + ignore_value(virPidFileReadIfAlive(cfg->pidDir, def->name, &dnsmasqPid, dnsmasqCapsGetBinaryPath(dnsmasq_caps))); @@ -576,8 +578,7 @@ networkStateInitialize(bool privileged, virStateInhibitCallback callback G_GNUC_UNUSED, void *opaque G_GNUC_UNUSED) { - g_autofree char *configdir = NULL; - g_autofree char *rundir = NULL; + virNetworkDriverConfig *cfg; bool autostart = true; #ifdef WITH_FIREWALLD GDBusConnection *sysbus = NULL; @@ -602,36 +603,11 @@ networkStateInitialize(bool privileged, if (!(network_driver->xmlopt = networkDnsmasqCreateXMLConf())) goto error; - /* configuration/state paths are one of - * ~/.config/libvirt/... (session/unprivileged) - * /etc/libvirt/... && /var/(run|lib)/libvirt/... (system/privileged). - */ - if (privileged) { - network_driver->networkConfigDir = g_strdup(SYSCONFDIR "/libvirt/qemu/networks"); - network_driver->networkAutostartDir = g_strdup(SYSCONFDIR "/libvirt/qemu/networks/autostart"); - network_driver->stateDir = g_strdup(RUNSTATEDIR "/libvirt/network"); - network_driver->pidDir = g_strdup(RUNSTATEDIR "/libvirt/network"); - network_driver->dnsmasqStateDir = g_strdup(LOCALSTATEDIR "/lib/libvirt/dnsmasq"); - } else { - configdir = virGetUserConfigDirectory(); - rundir = virGetUserRuntimeDirectory(); - - network_driver->networkConfigDir = g_strdup_printf("%s/qemu/networks", configdir); - network_driver->networkAutostartDir = g_strdup_printf("%s/qemu/networks/autostart", configdir); - network_driver->stateDir = g_strdup_printf("%s/network/lib", rundir); - network_driver->pidDir = g_strdup_printf("%s/network/run", rundir); - network_driver->dnsmasqStateDir = g_strdup_printf("%s/dnsmasq/lib", rundir); - } - - if (g_mkdir_with_parents(network_driver->stateDir, 0777) < 0) { - virReportSystemError(errno, - _("cannot create directory %s"), - network_driver->stateDir); + if (!(network_driver->config = cfg = virNetworkDriverConfigNew(privileged))) goto error; - } if ((network_driver->lockFD = - virPidFileAcquire(network_driver->stateDir, "driver", + virPidFileAcquire(cfg->stateDir, "driver", false, getpid())) < 0) goto error; @@ -642,13 +618,13 @@ networkStateInitialize(bool privileged, goto error; if (virNetworkObjLoadAllState(network_driver->networks, - network_driver->stateDir, + cfg->stateDir, network_driver->xmlopt) < 0) goto error; if (virNetworkObjLoadAllConfigs(network_driver->networks, - network_driver->networkConfigDir, - network_driver->networkAutostartDir, + cfg->networkConfigDir, + cfg->networkAutostartDir, network_driver->xmlopt) < 0) goto error; @@ -665,7 +641,7 @@ networkStateInitialize(bool privileged, networkReloadFirewallRules(network_driver, true, false); networkRefreshDaemons(network_driver); - if (virDriverShouldAutostart(network_driver->stateDir, &autostart) < 0) + if (virDriverShouldAutostart(cfg->stateDir, &autostart) < 0) goto error; if (autostart) { @@ -722,15 +698,19 @@ networkStateInitialize(bool privileged, static int networkStateReload(void) { + g_autoptr(virNetworkDriverConfig) cfg = NULL; + if (!network_driver) return 0; + cfg = virNetworkDriverGetConfig(network_driver); + virNetworkObjLoadAllState(network_driver->networks, - network_driver->stateDir, + cfg->stateDir, network_driver->xmlopt); virNetworkObjLoadAllConfigs(network_driver->networks, - network_driver->networkConfigDir, - network_driver->networkAutostartDir, + cfg->networkConfigDir, + cfg->networkAutostartDir, network_driver->xmlopt); networkReloadFirewallRules(network_driver, false, false); networkRefreshDaemons(network_driver); @@ -758,16 +738,14 @@ networkStateCleanup(void) /* free inactive networks */ virObjectUnref(network_driver->networks); - if (network_driver->lockFD != -1) - virPidFileRelease(network_driver->stateDir, "driver", - network_driver->lockFD); + if (network_driver->lockFD != -1) { + g_autoptr(virNetworkDriverConfig) cfg = virNetworkDriverGetConfig(network_driver); - g_free(network_driver->networkConfigDir); - g_free(network_driver->networkAutostartDir); - g_free(network_driver->stateDir); - g_free(network_driver->pidDir); - g_free(network_driver->dnsmasqStateDir); + virPidFileRelease(cfg->stateDir, "driver", + network_driver->lockFD); + } + virObjectUnref(network_driver->config); virObjectUnref(network_driver->dnsmasqCaps); virMutexDestroy(&network_driver->lock); @@ -1431,6 +1409,7 @@ networkBuildDhcpDaemonCommandLine(virNetworkDriverState *driver, char *pidfile, dnsmasqContext *dctx) { + g_autoptr(virNetworkDriverConfig) cfg = virNetworkDriverGetConfig(driver); virNetworkDef *def = virNetworkObjGetDef(obj); g_autoptr(dnsmasqCaps) dnsmasq_caps = networkGetDnsmasqCaps(driver); g_autoptr(virCommand) cmd = NULL; @@ -1448,7 +1427,7 @@ networkBuildDhcpDaemonCommandLine(virNetworkDriverState *driver, return -1; /* construct the filename */ - if (!(configfile = networkDnsmasqConfigFileName(driver, def->name))) + if (!(configfile = networkDnsmasqConfigFileName(cfg, def->name))) return -1; /* Write the file */ @@ -1481,6 +1460,7 @@ static int networkStartDhcpDaemon(virNetworkDriverState *driver, virNetworkObj *obj) { + g_autoptr(virNetworkDriverConfig) cfg = virNetworkDriverGetConfig(driver); virNetworkDef *def = virNetworkObjGetDef(obj); virNetworkIPDef *ipdef; size_t i; @@ -1506,24 +1486,22 @@ networkStartDhcpDaemon(virNetworkDriverState *driver, if (!needDnsmasq && def->dns.enable == VIR_TRISTATE_BOOL_NO) return 0; - if (g_mkdir_with_parents(driver->pidDir, 0777) < 0) { - virReportSystemError(errno, - _("cannot create directory %s"), - driver->pidDir); + if (g_mkdir_with_parents(cfg->pidDir, 0777) < 0) { + virReportSystemError(errno, _("cannot create directory %s"), cfg->pidDir); return -1; } - if (!(pidfile = virPidFileBuildPath(driver->pidDir, def->name))) + if (!(pidfile = virPidFileBuildPath(cfg->pidDir, def->name))) return -1; - if (g_mkdir_with_parents(driver->dnsmasqStateDir, 0777) < 0) { + if (g_mkdir_with_parents(cfg->dnsmasqStateDir, 0777) < 0) { virReportSystemError(errno, _("cannot create directory %s"), - driver->dnsmasqStateDir); + cfg->dnsmasqStateDir); return -1; } - dctx = dnsmasqContextNew(def->name, driver->dnsmasqStateDir); + dctx = dnsmasqContextNew(def->name, cfg->dnsmasqStateDir); if (dctx == NULL) return -1; @@ -1547,7 +1525,7 @@ networkStartDhcpDaemon(virNetworkDriverState *driver, * pid */ - if (virPidFileRead(driver->pidDir, def->name, &dnsmasqPid) < 0) + if (virPidFileRead(cfg->pidDir, def->name, &dnsmasqPid) < 0) return -1; virNetworkObjSetDnsmasqPid(obj, dnsmasqPid); @@ -1567,6 +1545,7 @@ static int networkRefreshDhcpDaemon(virNetworkDriverState *driver, virNetworkObj *obj) { + g_autoptr(virNetworkDriverConfig) cfg = virNetworkDriverGetConfig(driver); virNetworkDef *def = virNetworkObjGetDef(obj); size_t i; pid_t dnsmasqPid; @@ -1585,7 +1564,7 @@ networkRefreshDhcpDaemon(virNetworkDriverState *driver, return networkStartDhcpDaemon(driver, obj); VIR_INFO("Refreshing dnsmasq for network %s", def->bridge); - if (!(dctx = dnsmasqContextNew(def->name, driver->dnsmasqStateDir))) + if (!(dctx = dnsmasqContextNew(def->name, cfg->dnsmasqStateDir))) return -1; /* Look for first IPv4 address that has dhcp defined. @@ -1917,6 +1896,7 @@ static int networkStartNetworkVirtual(virNetworkDriverState *driver, virNetworkObj *obj) { + g_autoptr(virNetworkDriverConfig) cfg = virNetworkDriverGetConfig(driver); virNetworkDef *def = virNetworkObjGetDef(obj); size_t i; bool v4present = false, v6present = false; @@ -2028,7 +2008,7 @@ networkStartNetworkVirtual(virNetworkDriverState *driver, /* start dnsmasq if there are any IP addresses (v4 or v6) */ if (v4present || v6present) { - if (networkSetMacMap(driver, obj) < 0) + if (networkSetMacMap(cfg, obj) < 0) goto error; if (networkStartDhcpDaemon(driver, obj) < 0) @@ -2278,6 +2258,7 @@ static int networkStartNetwork(virNetworkDriverState *driver, virNetworkObj *obj) { + g_autoptr(virNetworkDriverConfig) cfg = virNetworkDriverGetConfig(driver); virNetworkDef *def = virNetworkObjGetDef(obj); int ret = -1; @@ -2291,7 +2272,7 @@ networkStartNetwork(virNetworkDriverState *driver, VIR_DEBUG("Beginning network startup process"); - virNetworkObjDeleteAllPorts(obj, driver->stateDir); + virNetworkObjDeleteAllPorts(obj, cfg->stateDir); VIR_DEBUG("Setting current network def as transient"); if (virNetworkObjSetDefTransient(obj, true, network_driver->xmlopt) < 0) @@ -2352,7 +2333,7 @@ networkStartNetwork(virNetworkDriverState *driver, * is setup. */ VIR_DEBUG("Writing network status to disk"); - if (virNetworkObjSaveStatus(driver->stateDir, + if (virNetworkObjSaveStatus(cfg->stateDir, obj, network_driver->xmlopt) < 0) goto cleanup; @@ -2377,6 +2358,7 @@ static int networkShutdownNetwork(virNetworkDriverState *driver, virNetworkObj *obj) { + g_autoptr(virNetworkDriverConfig) cfg = virNetworkDriverGetConfig(driver); virNetworkDef *def = virNetworkObjGetDef(obj); int ret = 0; g_autofree char *stateFile = NULL; @@ -2386,7 +2368,7 @@ networkShutdownNetwork(virNetworkDriverState *driver, if (!virNetworkObjIsActive(obj)) return 0; - stateFile = virNetworkConfigFile(driver->stateDir, def->name); + stateFile = virNetworkConfigFile(cfg->stateDir, def->name); if (!stateFile) return -1; @@ -3090,6 +3072,7 @@ networkDefineXMLFlags(virConnectPtr conn, unsigned int flags) { virNetworkDriverState *driver = networkGetDriver(); + g_autoptr(virNetworkDriverConfig) cfg = virNetworkDriverGetConfig(driver); g_autoptr(virNetworkDef) def = NULL; virNetworkDef *defAlias; virNetworkObj *obj = NULL; @@ -3116,7 +3099,7 @@ networkDefineXMLFlags(virConnectPtr conn, /* def was assigned to network object so don't autofree */ def = NULL; - if (virNetworkSaveConfig(driver->networkConfigDir, + if (virNetworkSaveConfig(cfg->networkConfigDir, defAlias, network_driver->xmlopt) < 0) { if (!virNetworkObjIsActive(obj)) { virNetworkObjRemoveInactive(driver->networks, obj); @@ -3156,6 +3139,7 @@ static int networkUndefine(virNetworkPtr net) { virNetworkDriverState *driver = networkGetDriver(); + g_autoptr(virNetworkDriverConfig) cfg = virNetworkDriverGetConfig(driver); virNetworkObj *obj; virNetworkDef *def; int ret = -1; @@ -3179,8 +3163,8 @@ networkUndefine(virNetworkPtr net) } /* remove autostart link */ - if (virNetworkObjDeleteConfig(driver->networkConfigDir, - driver->networkAutostartDir, + if (virNetworkObjDeleteConfig(cfg->networkConfigDir, + cfg->networkAutostartDir, obj) < 0) goto cleanup; @@ -3219,6 +3203,7 @@ networkUpdate(virNetworkPtr net, unsigned int flags) { virNetworkDriverState *driver = networkGetDriver(); + g_autoptr(virNetworkDriverConfig) cfg = virNetworkDriverGetConfig(driver); virNetworkObj *obj = NULL; virNetworkDef *def; int isActive, ret = -1; @@ -3320,7 +3305,7 @@ networkUpdate(virNetworkPtr net, if (flags & VIR_NETWORK_UPDATE_AFFECT_CONFIG) { /* save updated persistent config to disk */ - if (virNetworkSaveConfig(driver->networkConfigDir, + if (virNetworkSaveConfig(cfg->networkConfigDir, virNetworkObjGetPersistentDef(obj), network_driver->xmlopt) < 0) { goto cleanup; @@ -3381,7 +3366,7 @@ networkUpdate(virNetworkPtr net, } /* save current network state to disk */ - if ((ret = virNetworkObjSaveStatus(driver->stateDir, + if ((ret = virNetworkObjSaveStatus(cfg->stateDir, obj, network_driver->xmlopt)) < 0) goto cleanup; } @@ -3433,6 +3418,7 @@ static int networkDestroy(virNetworkPtr net) { virNetworkDriverState *driver = networkGetDriver(); + g_autoptr(virNetworkDriverConfig) cfg = virNetworkDriverGetConfig(driver); virNetworkObj *obj; virNetworkDef *def; int ret = -1; @@ -3455,7 +3441,7 @@ networkDestroy(virNetworkPtr net) if ((ret = networkShutdownNetwork(driver, obj)) < 0) goto cleanup; - virNetworkObjDeleteAllPorts(obj, driver->stateDir); + virNetworkObjDeleteAllPorts(obj, cfg->stateDir); /* @def replaced in virNetworkObjUnsetDefTransient */ def = virNetworkObjGetDef(obj); @@ -3567,6 +3553,7 @@ networkSetAutostart(virNetworkPtr net, int autostart) { virNetworkDriverState *driver = networkGetDriver(); + g_autoptr(virNetworkDriverConfig) cfg = virNetworkDriverGetConfig(driver); virNetworkObj *obj; virNetworkDef *def; g_autofree char *configFile = NULL; @@ -3591,18 +3578,18 @@ networkSetAutostart(virNetworkPtr net, new_autostart = (autostart != 0); cur_autostart = virNetworkObjIsAutostart(obj); if (cur_autostart != new_autostart) { - if ((configFile = virNetworkConfigFile(driver->networkConfigDir, + if ((configFile = virNetworkConfigFile(cfg->networkConfigDir, def->name)) == NULL) goto cleanup; - if ((autostartLink = virNetworkConfigFile(driver->networkAutostartDir, + if ((autostartLink = virNetworkConfigFile(cfg->networkAutostartDir, def->name)) == NULL) goto cleanup; if (new_autostart) { - if (g_mkdir_with_parents(driver->networkAutostartDir, 0777) < 0) { + if (g_mkdir_with_parents(cfg->networkAutostartDir, 0777) < 0) { virReportSystemError(errno, _("cannot create autostart directory '%s'"), - driver->networkAutostartDir); + cfg->networkAutostartDir); goto cleanup; } @@ -3639,6 +3626,7 @@ networkGetDHCPLeases(virNetworkPtr net, unsigned int flags) { virNetworkDriverState *driver = networkGetDriver(); + g_autoptr(virNetworkDriverConfig) cfg = virNetworkDriverGetConfig(driver); size_t i; size_t nleases = 0; int rv = -1; @@ -3669,7 +3657,7 @@ networkGetDHCPLeases(virNetworkPtr net, goto cleanup; /* Retrieve custom leases file location */ - custom_lease_file = networkDnsmasqLeaseFileNameCustom(driver, def->bridge); + custom_lease_file = networkDnsmasqLeaseFileNameCustom(cfg, def->bridge); /* Read entire contents */ if (virFileReadAllQuiet(custom_lease_file, @@ -3868,6 +3856,7 @@ networkAllocatePort(virNetworkObj *obj, virNetworkPortDef *port) { virNetworkDriverState *driver = networkGetDriver(); + g_autoptr(virNetworkDriverConfig) cfg = virNetworkDriverGetConfig(driver); virNetworkDef *netdef = NULL; virPortGroupDef *portgroup = NULL; virNetworkForwardIfDef *dev = NULL; @@ -4124,7 +4113,7 @@ networkAllocatePort(virNetworkObj *obj, &port->class_id) < 0) return -1; - if (virNetworkObjMacMgrAdd(obj, driver->dnsmasqStateDir, + if (virNetworkObjMacMgrAdd(obj, cfg->dnsmasqStateDir, port->ownername, &port->mac) < 0) return -1; @@ -4321,6 +4310,7 @@ networkReleasePort(virNetworkObj *obj, virNetworkPortDef *port) { virNetworkDriverState *driver = networkGetDriver(); + g_autoptr(virNetworkDriverConfig) cfg = virNetworkDriverGetConfig(driver); virNetworkDef *netdef; virNetworkForwardIfDef *dev = NULL; size_t i; @@ -4404,7 +4394,7 @@ networkReleasePort(virNetworkObj *obj, return -1; } - virNetworkObjMacMgrDel(obj, driver->dnsmasqStateDir, port->ownername, &port->mac); + virNetworkObjMacMgrDel(obj, cfg->dnsmasqStateDir, port->ownername, &port->mac); netdef->connections--; if (dev) @@ -4560,6 +4550,7 @@ networkPlugBandwidthImpl(virNetworkObj *obj, unsigned long long new_rate) { virNetworkDriverState *driver = networkGetDriver(); + g_autoptr(virNetworkDriverConfig) cfg = virNetworkDriverGetConfig(driver); virNetworkDef *def = virNetworkObjGetDef(obj); virBitmap *classIdMap = virNetworkObjGetClassIdMap(obj); unsigned long long tmp_floor_sum = virNetworkObjGetFloorSum(obj); @@ -4586,7 +4577,7 @@ networkPlugBandwidthImpl(virNetworkObj *obj, tmp_floor_sum += ifaceBand->in->floor; virNetworkObjSetFloorSum(obj, tmp_floor_sum); /* update status file */ - if (virNetworkObjSaveStatus(driver->stateDir, obj, network_driver->xmlopt) < 0) { + if (virNetworkObjSaveStatus(cfg->stateDir, obj, network_driver->xmlopt) < 0) { ignore_value(virBitmapClearBit(classIdMap, next_id)); tmp_floor_sum -= ifaceBand->in->floor; virNetworkObjSetFloorSum(obj, tmp_floor_sum); @@ -4643,6 +4634,7 @@ networkUnplugBandwidth(virNetworkObj *obj, virBitmap *classIdMap = virNetworkObjGetClassIdMap(obj); unsigned long long tmp_floor_sum = virNetworkObjGetFloorSum(obj); virNetworkDriverState *driver = networkGetDriver(); + g_autoptr(virNetworkDriverConfig) cfg = virNetworkDriverGetConfig(driver); int ret = 0; unsigned long long new_rate; @@ -4668,7 +4660,7 @@ networkUnplugBandwidth(virNetworkObj *obj, /* return class ID */ ignore_value(virBitmapClearBit(classIdMap, *class_id)); /* update status file */ - if (virNetworkObjSaveStatus(driver->stateDir, + if (virNetworkObjSaveStatus(cfg->stateDir, obj, network_driver->xmlopt) < 0) { tmp_floor_sum += ifaceBand->in->floor; virNetworkObjSetFloorSum(obj, tmp_floor_sum); @@ -4713,6 +4705,7 @@ networkUpdatePortBandwidth(virNetworkObj *obj, virNetDevBandwidth *newBandwidth) { virNetworkDriverState *driver = networkGetDriver(); + g_autoptr(virNetworkDriverConfig) cfg = virNetworkDriverGetConfig(driver); virNetworkDef *def; unsigned long long tmp_floor_sum; unsigned long long new_rate = 0; @@ -4761,7 +4754,7 @@ networkUpdatePortBandwidth(virNetworkObj *obj, if (virNetDevBandwidthUpdateRate(def->bridge, 2, def->bandwidth, new_rate) < 0 || - virNetworkObjSaveStatus(driver->stateDir, + virNetworkObjSaveStatus(cfg->stateDir, obj, network_driver->xmlopt) < 0) { /* Ouch, rollback */ tmp_floor_sum -= new_floor; @@ -4835,6 +4828,7 @@ networkPortCreateXML(virNetworkPtr net, unsigned int flags) { virNetworkDriverState *driver = networkGetDriver(); + g_autoptr(virNetworkDriverConfig) cfg = virNetworkDriverGetConfig(driver); virNetworkObj *obj; virNetworkDef *def; g_autoptr(virNetworkPortDef) portdef = NULL; @@ -4883,7 +4877,7 @@ networkPortCreateXML(virNetworkPtr net, if (rc < 0) goto cleanup; - if (virNetworkObjAddPort(obj, portdef, driver->stateDir) < 0) { + if (virNetworkObjAddPort(obj, portdef, cfg->stateDir) < 0) { virErrorPtr save_err; virErrorPreserveLast(&save_err); @@ -4944,6 +4938,7 @@ networkPortDelete(virNetworkPortPtr port, unsigned int flags) { virNetworkDriverState *driver = networkGetDriver(); + g_autoptr(virNetworkDriverConfig) cfg = virNetworkDriverGetConfig(driver); virNetworkObj *obj; virNetworkDef *def; virNetworkPortDef *portdef; @@ -4972,7 +4967,7 @@ networkPortDelete(virNetworkPortPtr port, if (networkReleasePort(obj, portdef) < 0) goto cleanup; - virNetworkObjDeletePort(obj, port->uuid, driver->stateDir); + virNetworkObjDeletePort(obj, port->uuid, cfg->stateDir); ret = 0; cleanup: @@ -4988,6 +4983,7 @@ networkPortSetParameters(virNetworkPortPtr port, unsigned int flags) { virNetworkDriverState *driver = networkGetDriver(); + g_autoptr(virNetworkDriverConfig) cfg = virNetworkDriverGetConfig(driver); virNetworkObj *obj; virNetworkDef *def; virNetworkPortDef *portdef; @@ -5016,7 +5012,7 @@ networkPortSetParameters(virNetworkPortPtr port, goto cleanup; } - if (!(dir = virNetworkObjGetPortStatusDir(obj, driver->stateDir))) + if (!(dir = virNetworkObjGetPortStatusDir(obj, cfg->stateDir))) goto cleanup; bandwidth = g_new0(virNetDevBandwidth, 1); diff --git a/src/network/bridge_driver_conf.c b/src/network/bridge_driver_conf.c index b6c059e1e9..664e42bf99 100644 --- a/src/network/bridge_driver_conf.c +++ b/src/network/bridge_driver_conf.c @@ -21,9 +21,11 @@ /* includes */ #include <config.h> -#include "virerror.h" +#include "configmake.h" #include "datatypes.h" #include "virlog.h" +#include "virerror.h" +#include "virutil.h" #include "bridge_driver_conf.h" #define VIR_FROM_THIS VIR_FROM_NETWORK @@ -31,6 +33,22 @@ VIR_LOG_INIT("network.bridge_driver"); +static virClass *virNetworkDriverConfigClass; +static void virNetworkDriverConfigDispose(void *obj); + +static int +virNetworkConfigOnceInit(void) +{ + if (!VIR_CLASS_NEW(virNetworkDriverConfig, virClassForObject())) + return -1; + + return 0; +} + + +VIR_ONCE_GLOBAL_INIT(virNetworkConfig); + + dnsmasqCaps * networkGetDnsmasqCaps(virNetworkDriverState *driver) { @@ -39,6 +57,97 @@ networkGetDnsmasqCaps(virNetworkDriverState *driver) } +static int +virNetworkLoadDriverConfig(virNetworkDriverConfig *cfg G_GNUC_UNUSED, + const char *filename) +{ + g_autoptr(virConf) conf = NULL; + + /* if file doesn't exist or is unreadable, ignore the "error" */ + if (access(filename, R_OK) == -1) + return 0; + + conf = virConfReadFile(filename, 0); + if (!conf) + return -1; + + /* use virConfGetValue*(conf, ...) functions to read any settings into cfg */ + + return 0; +} + + +virNetworkDriverConfig * +virNetworkDriverConfigNew(bool privileged) +{ + g_autoptr(virNetworkDriverConfig) cfg = NULL; + g_autofree char *configdir = NULL; + g_autofree char *configfile = NULL; + + if (virNetworkConfigInitialize() < 0) + return NULL; + + if (!(cfg = virObjectNew(virNetworkDriverConfigClass))) + return NULL; + + /* configuration/state paths are one of + * ~/.config/libvirt/... (session/unprivileged) + * /etc/libvirt/... && /var/(run|lib)/libvirt/... (system/privileged). + */ + if (privileged) { + configdir = g_strdup(SYSCONFDIR "/libvirt"); + cfg->networkConfigDir = g_strdup(SYSCONFDIR "/libvirt/qemu/networks"); + cfg->networkAutostartDir = g_strdup(SYSCONFDIR "/libvirt/qemu/networks/autostart"); + cfg->stateDir = g_strdup(RUNSTATEDIR "/libvirt/network"); + cfg->pidDir = g_strdup(RUNSTATEDIR "/libvirt/network"); + cfg->dnsmasqStateDir = g_strdup(LOCALSTATEDIR "/lib/libvirt/dnsmasq"); + } else { + g_autofree char *rundir = virGetUserRuntimeDirectory(); + + configdir = virGetUserConfigDirectory(); + cfg->networkConfigDir = g_strdup_printf("%s/qemu/networks", configdir); + cfg->networkAutostartDir = g_strdup_printf("%s/qemu/networks/autostart", configdir); + cfg->stateDir = g_strdup_printf("%s/network/lib", rundir); + cfg->pidDir = g_strdup_printf("%s/network/run", rundir); + cfg->dnsmasqStateDir = g_strdup_printf("%s/dnsmasq/lib", rundir); + } + + configfile = g_strconcat(configdir, "/network.conf", NULL); + + if (virNetworkLoadDriverConfig(cfg, configfile) < 0) + return NULL; + + if (g_mkdir_with_parents(cfg->stateDir, 0777) < 0) { + virReportSystemError(errno, _("cannot create directory %s"), cfg->stateDir); + return NULL; + } + + return g_steal_pointer(&cfg); +} + + +virNetworkDriverConfig * +virNetworkDriverGetConfig(virNetworkDriverState *driver) +{ + VIR_LOCK_GUARD lock = virLockGuardLock(&driver->lock); + return virObjectRef(driver->config); +} + + +static void +virNetworkDriverConfigDispose(void *obj) +{ + virNetworkDriverConfig *cfg = obj; + + g_free(cfg->networkConfigDir); + g_free(cfg->networkAutostartDir); + g_free(cfg->stateDir); + g_free(cfg->pidDir); + g_free(cfg->dnsmasqStateDir); +} + + + int networkDnsmasqCapsRefresh(virNetworkDriverState *driver) { diff --git a/src/network/bridge_driver_conf.h b/src/network/bridge_driver_conf.h index f4307bbdba..426c16198d 100644 --- a/src/network/bridge_driver_conf.h +++ b/src/network/bridge_driver_conf.h @@ -27,26 +27,38 @@ #include "virnetworkobj.h" #include "object_event.h" +typedef struct _virNetworkDriverConfig virNetworkDriverConfig; +struct _virNetworkDriverConfig { + virObject parent; + + /* Immutable pointers, Immutable objects */ + char *networkConfigDir; + char *networkAutostartDir; + char *stateDir; + char *pidDir; + char *dnsmasqStateDir; +}; + +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virNetworkDriverConfig, virObjectUnref); + /* Main driver state */ +typedef struct _virNetworkDriverState virNetworkDriverState; struct _virNetworkDriverState { virMutex lock; /* Read-only */ bool privileged; + /* Require lock to get reference on 'config', + * then lockless thereafter */ + virNetworkDriverConfig *config; + /* pid file FD, ensures two copies of the driver can't use the same root */ int lockFD; /* Immutable pointer, self-locking APIs */ virNetworkObjList *networks; - /* Immutable pointers, Immutable objects */ - char *networkConfigDir; - char *networkAutostartDir; - char *stateDir; - char *pidDir; - char *dnsmasqStateDir; - /* Require lock to get a reference on the object, * lockless access thereafter */ @@ -58,7 +70,10 @@ struct _virNetworkDriverState { virNetworkXMLOption *xmlopt; }; -typedef struct _virNetworkDriverState virNetworkDriverState; +virNetworkDriverConfig * +virNetworkDriverConfigNew(bool privileged); +virNetworkDriverConfig * +virNetworkDriverGetConfig(virNetworkDriverState *driver); dnsmasqCaps * networkGetDnsmasqCaps(virNetworkDriverState *driver); -- 2.37.1
Signed-off-by: Laine Stump <laine@redhat.com> --- src/util/viriptables.c | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/src/util/viriptables.c b/src/util/viriptables.c index e9886a200e..6275760de4 100644 --- a/src/util/viriptables.c +++ b/src/util/viriptables.c @@ -163,10 +163,7 @@ iptablesInput(virFirewall *fw, int action, int tcp) { - char portstr[32]; - - g_snprintf(portstr, sizeof(portstr), "%d", port); - portstr[sizeof(portstr) - 1] = '\0'; + g_autofree char *portstr = g_strdup_printf("%d", port); virFirewallAddRule(fw, layer, "--table", "filter", @@ -187,10 +184,7 @@ iptablesOutput(virFirewall *fw, int action, int tcp) { - char portstr[32]; - - g_snprintf(portstr, sizeof(portstr), "%d", port); - portstr[sizeof(portstr) - 1] = '\0'; + g_autofree char *portstr = g_strdup_printf("%d", port); virFirewallAddRule(fw, layer, "--table", "filter", @@ -1028,10 +1022,7 @@ iptablesOutputFixUdpChecksum(virFirewall *fw, int port, int action) { - char portstr[32]; - - g_snprintf(portstr, sizeof(portstr), "%d", port); - portstr[sizeof(portstr) - 1] = '\0'; + g_autofree char *portstr = g_strdup_printf("%d", port); virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4, "--table", "mangle", -- 2.37.1
This was a wrapper to call a function in virfirewalld.c that sends an iptables passthrough rule to firewalld. It hasn't been used in a year or two, and won't ever be used in the future since passthrough rules are only supported for iptables, and we've determined that we shouldn't use iptables passthrough rules. Signed-off-by: Laine Stump <laine@redhat.com> --- src/util/virfirewall.c | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c index 3183e0aec7..0a9ba9ad5c 100644 --- a/src/util/virfirewall.c +++ b/src/util/virfirewall.c @@ -523,16 +523,6 @@ virFirewallApplyRuleDirect(virFirewallRule *rule, } -static int G_GNUC_UNUSED -virFirewallApplyRuleFirewallD(virFirewallRule *rule, - bool ignoreErrors, - char **output) -{ - /* wrapper necessary because virFirewallRule is a private struct */ - return virFirewallDApplyRule(rule->layer, rule->args, rule->argsLen, ignoreErrors, output); -} - - static int virFirewallApplyRule(virFirewall *firewall, virFirewallRule *rule, -- 2.37.1
Although the next commit will eliminate the one current use of virFirewallRuleToString(), a future commit will once again have a use for it, but in a different source file so it will need to be a global function rather than static. Make that change now so that we don't get a compile error from having an unused static function in the next commit. (The arg list is also changed to include the name of the command as a separate argument rather than just assuming that it can be derived from the rule's layer (which is correct for iptables, but won't be correct for nftables)). Signed-off-by: Laine Stump <laine@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virfirewall.c | 13 ++++++++----- src/util/virfirewall.h | 3 +++ 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ac2802095e..f739259375 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2352,6 +2352,7 @@ virFirewallRuleAddArgFormat; virFirewallRuleAddArgList; virFirewallRuleAddArgSet; virFirewallRuleGetArgCount; +virFirewallRuleToString; virFirewallStartRollback; virFirewallStartTransaction; diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c index 0a9ba9ad5c..247430be2e 100644 --- a/src/util/virfirewall.c +++ b/src/util/virfirewall.c @@ -461,14 +461,14 @@ void virFirewallStartRollback(virFirewall *firewall, } -static char * -virFirewallRuleToString(virFirewallRule *rule) +char * +virFirewallRuleToString(const char *cmd, + virFirewallRule *rule) { - const char *bin = virFirewallLayerCommandTypeToString(rule->layer); g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; size_t i; - virBufferAdd(&buf, bin, -1); + virBufferAdd(&buf, cmd, -1); for (i = 0; i < rule->argsLen; i++) { virBufferAddLit(&buf, " "); virBufferAdd(&buf, rule->args[i], -1); @@ -477,6 +477,7 @@ virFirewallRuleToString(virFirewallRule *rule) return virBufferContentAndReset(&buf); } + static int virFirewallApplyRuleDirect(virFirewallRule *rule, bool ignoreErrors, @@ -529,8 +530,10 @@ virFirewallApplyRule(virFirewall *firewall, bool ignoreErrors) { g_autofree char *output = NULL; - g_autofree char *str = virFirewallRuleToString(rule); g_auto(GStrv) lines = NULL; + g_autofree char *str + = virFirewallRuleToString(virFirewallLayerCommandTypeToString(rule->layer), rule); + VIR_INFO("Applying rule '%s'", NULLSTR(str)); if (rule->ignoreErrors) diff --git a/src/util/virfirewall.h b/src/util/virfirewall.h index 7448825dbc..187748b2bf 100644 --- a/src/util/virfirewall.h +++ b/src/util/virfirewall.h @@ -89,6 +89,9 @@ void virFirewallRuleAddArgList(virFirewall *firewall, size_t virFirewallRuleGetArgCount(virFirewallRule *rule); +char *virFirewallRuleToString(const char *cmd, + virFirewallRule *rule); + typedef enum { /* Ignore all errors when applying rules, so no * rollback block will be required */ -- 2.37.1
Instead of separately building the commandline into a string to log, just wait a few lines until we've built the virCommand object, and call virCommandToString, which does the same thing. (As a bonus, we were already calling virCommandToString to put the commandline in a string in case of a failure when running it - from the point of view of *that* usage, we're just moving the call to virCommandToString *up* a few lines, i.e. we now only construct the commandline string once.) Signed-off-by: Laine Stump <laine@redhat.com> --- src/util/virfirewall.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c index 247430be2e..fbb0e438b3 100644 --- a/src/util/virfirewall.c +++ b/src/util/virfirewall.c @@ -486,6 +486,7 @@ virFirewallApplyRuleDirect(virFirewallRule *rule, size_t i; const char *bin = virFirewallLayerCommandTypeToString(rule->layer); g_autoptr(virCommand) cmd = NULL; + g_autofree char *cmdStr = NULL; int status; g_autofree char *error = NULL; @@ -501,6 +502,9 @@ virFirewallApplyRuleDirect(virFirewallRule *rule, for (i = 0; i < rule->argsLen; i++) virCommandAddArg(cmd, rule->args[i]); + cmdStr = virCommandToString(cmd, false); + VIR_INFO("Applying rule '%s'", NULLSTR(cmdStr)); + virCommandSetOutputBuffer(cmd, output); virCommandSetErrorBuffer(cmd, &error); @@ -511,10 +515,9 @@ virFirewallApplyRuleDirect(virFirewallRule *rule, if (ignoreErrors) { VIR_DEBUG("Ignoring error running command"); } else { - g_autofree char *args = virCommandToString(cmd, false); virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to apply firewall rules %s: %s"), - NULLSTR(args), NULLSTR(error)); + NULLSTR(cmdStr), NULLSTR(error)); VIR_FREE(*output); return -1; } @@ -531,10 +534,6 @@ virFirewallApplyRule(virFirewall *firewall, { g_autofree char *output = NULL; g_auto(GStrv) lines = NULL; - g_autofree char *str - = virFirewallRuleToString(virFirewallLayerCommandTypeToString(rule->layer), rule); - - VIR_INFO("Applying rule '%s'", NULLSTR(str)); if (rule->ignoreErrors) ignoreErrors = rule->ignoreErrors; -- 2.37.1
On a Monday in 2022, Laine Stump wrote:
These all bubbled to the top of a series that adds support for an nftables backend to the network driver, but are valid patches by themselves, so I'm sending them separately to make the other series look shorter.
Laine Stump (16): conf: replace explicit virNetworkDefFree() with g_autoptr(virNetworkDef) esx: replace explicit virNetworkDefFree() with g_autoptr(virNetworkDef) network: replace explicit virNetworkDefFree() with g_autoptr(virNetworkDef) qemu: replace explicit virNetworkDefFree() with g_autoptr(virNetworkDef) test driver: replace explicit virNetworkDefFree() with g_autoptr(virNetworkDef) vbox: replace explicit virNetworkDefFree() with g_autoptr(virNetworkDef) tests: replace explicit virNetworkDefFree() with g_autoptr(virNetworkDef) conf: remove superfluous cleanup: labels and ret return variables qemu: remove superfluous cleanup: labels and ret return variables tests: remove superfluous cleanup: labels and ret return variables network: move driver state struct into bridge_driver_conf.h network: create separate config object for virNetworkDriverState util: replace g_snprintf with g_autofreed g_strdup_printf in viriptables.c util: remove unused function virFirewallApplyRuleFirewallD() util: make virFirewallRuleToString() global util: don't use virFirewallRuleToString() to log the rule being applied
po/POTFILES | 1 + src/conf/domain_conf.c | 26 +-- src/conf/virnetworkobj.c | 72 ++++----- src/esx/esx_network_driver.c | 6 +- src/libvirt_private.syms | 1 + src/network/bridge_driver.c | 233 ++++++++++++--------------- src/network/bridge_driver_conf.c | 165 +++++++++++++++++++ src/network/bridge_driver_conf.h | 82 ++++++++++ src/network/bridge_driver_platform.h | 43 +---- src/network/meson.build | 1 + src/qemu/qemu_process.c | 24 ++- src/test/test_driver.c | 6 +- src/util/virfirewall.c | 28 ++-- src/util/virfirewall.h | 3 + src/util/viriptables.c | 15 +- src/vbox/vbox_network.c | 6 +- tests/networkxml2firewalltest.c | 15 +- tests/networkxml2xmltest.c | 3 +- tests/networkxml2xmlupdatetest.c | 8 +- 19 files changed, 443 insertions(+), 295 deletions(-) create mode 100644 src/network/bridge_driver_conf.c create mode 100644 src/network/bridge_driver_conf.h
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Laine Stump