On 6/26/23 15:55, K Shiva wrote:
- The definition of struct virNetworkObj has been moved from
virnetworkobj.c to its header as it was needed by network_event.h.
- Added functions to parse and save the XML along with helper functions
that resolve the live/persistent state of the network.
Signed-off-by: K Shiva <shiva_kr(a)riseup.net>
---
src/conf/network_conf.c | 3 +
src/conf/network_conf.h | 2 +
src/conf/virnetworkobj.c | 347 ++++++++++++++++++++++++++++++++++++---
src/conf/virnetworkobj.h | 56 +++++++
4 files changed, 386 insertions(+), 22 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 73788b6d87..84952db041 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -2546,6 +2546,9 @@ virNetworkSaveXML(const char *configDir,
char uuidstr[VIR_UUID_STRING_BUFLEN];
g_autofree char *configFile = NULL;
+ if (!configDir)
+ return 0;
+
if ((configFile = virNetworkConfigFile(configDir, def->name)) == NULL)
return -1;
This seems orthogonal to the rest of the changes. I'm not sure why it's
needed but definitely does not belong into this patch.
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 2b2e9d15f0..5a1bdb1284 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -249,6 +249,8 @@ struct _virNetworkDef {
unsigned char uuid[VIR_UUID_BUFLEN];
bool uuid_specified;
char *name;
+ char *title;
+ char *description;
int connections; /* # of guest interfaces connected to this network */
char *bridge; /* Name of bridge device */
diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c
index b8b86da06f..82f90937bc 100644
--- a/src/conf/virnetworkobj.c
+++ b/src/conf/virnetworkobj.c
@@ -39,28 +39,6 @@ VIR_LOG_INIT("conf.virnetworkobj");
* that big. */
#define INIT_CLASS_ID_BITMAP_SIZE (1<<4)
-struct _virNetworkObj {
- virObjectLockable parent;
-
- pid_t dnsmasqPid;
- bool active;
- bool autostart;
- bool persistent;
-
- virNetworkDef *def; /* The current definition */
- virNetworkDef *newDef; /* New definition to activate at shutdown */
-
- virBitmap *classIdMap; /* bitmap of class IDs for QoS */
- unsigned long long floor_sum; /* sum of all 'floor'-s of attached NICs */
-
- unsigned int taint;
-
- /* Immutable pointer, self locking APIs */
- virMacMap *macmap;
-
- GHashTable *ports; /* uuid -> virNetworkPortDef **/
-};
-
struct _virNetworkObjList {
virObjectRWLockable parent;
@@ -1822,3 +1800,328 @@ virNetworkObjLoadAllPorts(virNetworkObj *net,
return 0;
}
+
+
+/**
+ * virNetworkObjUpdateModificationImpact:
+ *
+ * @net: network object
+ * @flags: flags to update the modification impact on
+ *
+ * Resolves virNetworkUpdateFlags in @flags so that they correctly
+ * apply to the actual state of @net. @flags may be modified after call to this
+ * function.
+ *
+ * Returns 0 on success if @flags point to a valid combination for @net or -1 on
+ * error.
+ */
+int
+virNetworkObjUpdateModificationImpact(virNetworkObj *net,
+ unsigned int *flags)
+{
+ bool isActive = virNetworkObjIsActive(net);
+
+ if ((*flags & (VIR_NETWORK_UPDATE_AFFECT_LIVE |
VIR_NETWORK_UPDATE_AFFECT_CONFIG)) ==
+ VIR_NETWORK_UPDATE_AFFECT_CURRENT) {
+ if (isActive)
+ *flags |= VIR_NETWORK_UPDATE_AFFECT_LIVE;
+ else
+ *flags |= VIR_NETWORK_UPDATE_AFFECT_CONFIG;
+ }
+
+ if (!isActive && (*flags & VIR_NETWORK_UPDATE_AFFECT_LIVE)) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("network is not running"));
+ return -1;
+ }
+
+ if (!net->persistent && (*flags & VIR_NETWORK_UPDATE_AFFECT_CONFIG))
{
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("transient networks do not have any "
+ "persistent config"));
+ return -1;
+ }
+
+ return 0;
+}
+
+
+/**
+ * virNetworkObjGetDefs:
+ *
+ * @net: network object
+ * @flags: for virNetworkUpdateFlags
+ * @liveDef: Set the pointer to the live definition of @net.
+ * @persDef: Set the pointer to the config definition of @net.
+ *
+ * Helper function to resolve @flags and retrieve correct network pointer
+ * objects. This function should be used only when the network driver
+ * creates net->newDef once the network has started.
+ *
+ * If @liveDef or @persDef are set it implies that @flags request modification
+ * thereof.
+ *
+ * Returns 0 on success and sets @liveDef and @persDef; -1 if @flags are
+ * inappropriate.
+ */
+int
+virNetworkObjGetDefs(virNetworkObj *net,
+ unsigned int flags,
+ virNetworkDef **liveDef,
+ virNetworkDef **persDef)
+{
+ if (liveDef)
+ *liveDef = NULL;
+
+ if (persDef)
+ *persDef = NULL;
+
+ if (virNetworkObjUpdateModificationImpact(net, &flags) < 0)
+ return -1;
+
+ if (virNetworkObjIsActive(net)) {
+ if (liveDef && (flags & VIR_NETWORK_UPDATE_AFFECT_LIVE))
+ *liveDef = net->def;
+
+ if (persDef && (flags & VIR_NETWORK_UPDATE_AFFECT_CONFIG))
+ *persDef = net->newDef;
+ } else {
+ if (persDef)
+ *persDef = net->def;
+ }
+
+ return 0;
+}
+
+
+/**
+ * virNetworkObjGetOneDefState:
+ *
+ * @net: Network object
+ * @flags: for virNetworkUpdateFlags
+ * @live: set to true if live config was returned (may be omitted)
+ *
+ * Helper function to resolve @flags and return the correct network pointer
+ * object. This function returns one of @net->def or @net->persistentDef
+ * according to @flags. @live is set to true if the live net config will be
+ * returned. This helper should be used only in APIs that guarantee
+ * that @flags contains exactly one of VIR_NETWORK_UPDATE_AFFECT_LIVE or
+ * VIR_NETWORK_UPDATE_AFFECT_CONFIG and not both.
+ *
+ * Returns the correct definition pointer or NULL on error.
+ */
+virNetworkDef *
+virNetworkObjGetOneDefState(virNetworkObj *net,
+ unsigned int flags,
+ bool *live)
+{
+ if (flags & VIR_NETWORK_UPDATE_AFFECT_LIVE &&
+ flags & VIR_NETWORK_UPDATE_AFFECT_CONFIG) {
+ virReportInvalidArg(flags, "%s",
+ _("Flags 'VIR_NETWORK_UPDATE_AFFECT_LIVE' and
"
+ "'VIR_NETWORK_UPDATE_AFFECT_CONFIG' are
mutually "
+ "exclusive"));
+ return NULL;
+ }
+
+ if (virNetworkObjUpdateModificationImpact(net, &flags) < 0)
+ return NULL;
+
+ if (live)
+ *live = flags & VIR_NETWORK_UPDATE_AFFECT_LIVE;
+
+ if (virNetworkObjIsActive(net) && flags &
VIR_NETWORK_UPDATE_AFFECT_CONFIG)
+ return net->newDef;
+
+ return net->def;
+}
+
+
+/**
+ * virNetworkObjGetOneDef:
+ *
+ * @net: Network object
+ * @flags: for virNetworkUpdateFlags
+ *
+ * Helper function to resolve @flags and return the correct network pointer
+ * object. This function returns one of @net->def or @net->persistentDef
+ * according to @flags. This helper should be used only in APIs that guarantee
+ * that @flags contains exactly one of VIR_NETWORK_UPDATE_AFFECT_LIVE or
+ * VIR_NETWORK_UPDATE_AFFECT_CONFIG and not both.
+ *
+ * Returns the correct definition pointer or NULL on error.
+ */
+virNetworkDef *
+virNetworkObjGetOneDef(virNetworkObj *net,
+ unsigned int flags)
+{
+ return virNetworkObjGetOneDefState(net, flags, NULL);
+}
+
+
+char *
+virNetworkObjGetMetadata(virNetworkObj *net,
+ int type,
+ const char *uri,
+ unsigned int flags)
+{
+ virNetworkDef *def;
+ char *ret = NULL;
+
+ virCheckFlags(VIR_NETWORK_UPDATE_AFFECT_LIVE |
+ VIR_NETWORK_UPDATE_AFFECT_CONFIG, NULL);
+
+ if (type >= VIR_NETWORK_METADATA_LAST) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("unknown metadata type '%1$d'"), type);
+ return NULL;
+ }
+
+ if (!(def = virNetworkObjGetOneDef(net, flags)))
+ return NULL;
+
+ switch ((virNetworkMetadataType) type) {
+ case VIR_NETWORK_METADATA_DESCRIPTION:
+ ret = g_strdup(def->description);
+ break;
+
+ case VIR_NETWORK_METADATA_TITLE:
+ ret = g_strdup(def->title);
+ break;
+
+ case VIR_NETWORK_METADATA_ELEMENT:
+ if (!def->metadata)
+ break;
+
+ if (virXMLExtractNamespaceXML(def->metadata, uri, &ret) < 0)
+ return NULL;
+ break;
+
+ case VIR_NETWORK_METADATA_LAST:
+ break;
+ }
+
+ if (!ret)
+ virReportError(VIR_ERR_NO_NETWORK_METADATA, "%s",
+ _("Requested metadata element is not present"));
+
+ return ret;
+}
+
+
+static int
+virNetworkDefSetMetadata(virNetworkDef *def,
+ int type,
+ const char *metadata,
+ const char *key,
+ const char *uri)
+{
+ g_autoptr(xmlDoc) doc = NULL;
+ xmlNodePtr old;
+ g_autoptr(xmlNode) new = NULL;
+
+ if (type >= VIR_NETWORK_METADATA_LAST) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("unknown metadata type '%1$d'"), type);
+ return -1;
+ }
+
+ switch ((virNetworkMetadataType) type) {
+ case VIR_NETWORK_METADATA_DESCRIPTION:
+ g_clear_pointer(&def->description, g_free);
+
+ if (STRNEQ_NULLABLE(metadata, ""))
+ def->description = g_strdup(metadata);
+ break;
+
+ case VIR_NETWORK_METADATA_TITLE:
+ g_clear_pointer(&def->title, g_free);
+
+ if (STRNEQ_NULLABLE(metadata, ""))
+ def->title = g_strdup(metadata);
+ break;
+
+ case VIR_NETWORK_METADATA_ELEMENT:
+ if (metadata) {
+
+ /* parse and modify the xml from the user */
+ if (!(doc = virXMLParseStringCtxt(metadata, _("(metadata_xml)"),
NULL)))
+ return -1;
+
+ if (virXMLInjectNamespace(doc->children, uri, key) < 0)
+ return -1;
+
+ /* create the root node if needed */
+ if (!def->metadata)
+ def->metadata = virXMLNewNode(NULL, "metadata");
+
+ if (!(new = xmlCopyNode(doc->children, 1))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Failed to copy XML node"));
+ return -1;
+ }
+ }
+
+ /* remove possible other nodes sharing the namespace */
+ while ((old = virXMLFindChildNodeByNs(def->metadata, uri))) {
+ xmlUnlinkNode(old);
+ xmlFreeNode(old);
+ }
+
+ if (new) {
+ if (!(xmlAddChild(def->metadata, new))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("failed to add metadata to XML document"));
+ return -1;
+ }
+ new = NULL;
+ }
+ break;
+
+ case VIR_NETWORK_METADATA_LAST:
+ break;
+ }
+
+ return 0;
+}
+
+
+int
+virNetworkObjSetMetadata(virNetworkObj *net,
+ int type,
+ const char *metadata,
+ const char *key,
+ const char *uri,
+ virNetworkXMLOption *xmlopt,
+ const char *stateDir,
+ const char *configDir,
+ unsigned int flags)
+{
+ virNetworkDef *def;
+ virNetworkDef *persistentDef;
+
+ virCheckFlags(VIR_NETWORK_UPDATE_AFFECT_LIVE |
+ VIR_NETWORK_UPDATE_AFFECT_CONFIG, -1);
+
+ if (virNetworkObjGetDefs(net, flags, &def, &persistentDef) < 0)
+ return -1;
+
+ if (def) {
+ if (virNetworkDefSetMetadata(def, type, metadata, key, uri) < 0)
+ return -1;
+
+ if (virNetworkObjSaveStatus(stateDir, net, xmlopt) < 0)
+ return -1;
+ }
+
+ if (persistentDef) {
+ if (virNetworkDefSetMetadata(persistentDef, type, metadata, key,
+ uri) < 0)
+ return -1;
+
+ if (virNetworkSaveConfig(configDir, persistentDef, xmlopt) < 0)
+ return -1;
+ }
+
+ return 0;
+}
diff --git a/src/conf/virnetworkobj.h b/src/conf/virnetworkobj.h
index 7d34fa3204..d17a43d7bb 100644
--- a/src/conf/virnetworkobj.h
+++ b/src/conf/virnetworkobj.h
@@ -26,6 +26,28 @@
typedef struct _virNetworkObj virNetworkObj;
+struct _virNetworkObj {
+ virObjectLockable parent;
+
+ pid_t dnsmasqPid;
+ bool active;
+ bool autostart;
+ bool persistent;
+
+ virNetworkDef *def; /* The current definition */
+ virNetworkDef *newDef; /* New definition to activate at shutdown */
+
+ virBitmap *classIdMap; /* bitmap of class IDs for QoS */
+ unsigned long long floor_sum; /* sum of all 'floor'-s of attached NICs */
+
+ unsigned int taint;
+
+ /* Immutable pointer, self locking APIs */
+ virMacMap *macmap;
+
+ GHashTable *ports; /* uuid -> virNetworkPortDef **/
+};
+
No, the struct was hidden for a reason: we don't want the rest of the
code to fiddle with struct members directly but use so called getter and
setter functions (functions which get/set value of individual struct
members), e.g. virNetworkObjIsActive() / virNetworkObjSetActive(). This
way, code is more auditable, i.e. it's easy to see where a given struct
member is set. Not only that, but individual getters/setters might
perform additional tasks tied to the struct member, e.g. lock the object.
virNetworkObj *
virNetworkObjNew(void);
@@ -258,3 +280,37 @@ virNetworkObjListNumOfNetworks(virNetworkObjList *nets,
void
virNetworkObjListPrune(virNetworkObjList *nets,
unsigned int flags);
+
+int virNetworkObjUpdateModificationImpact(virNetworkObj *net,
+ unsigned int *flags);
+
+int
+virNetworkObjGetDefs(virNetworkObj *net,
+ unsigned int flags,
+ virNetworkDef **liveDef,
+ virNetworkDef **persDef);
+
+virNetworkDef *
+virNetworkObjGetOneDefState(virNetworkObj *net,
+ unsigned int flags,
+ bool *state);
+virNetworkDef *
+virNetworkObjGetOneDef(virNetworkObj *net,
+ unsigned int flags);
Neither of these ^^^ functions is called from outside of
virnetworkobj.c. What's the reason for their exposure?
+
+char *
+virNetworkObjGetMetadata(virNetworkObj *network,
+ int type,
+ const char *uri,
+ unsigned int flags);
+
+int
+virNetworkObjSetMetadata(virNetworkObj *network,
+ int type,
+ const char *metadata,
+ const char *key,
+ const char *uri,
+ virNetworkXMLOption *xmlopt,
+ const char *stateDir,
+ const char *configDir,
+ unsigned int flags);
BTW: this is where I stop my review, as these patches need to be split
differently.
Michal