[libvirt PATCH 0/4] Introduce new Metadata fields for Network object with corresponding APIs
This commit introduces <title> and <description> fields to the XML schema of the Network object. It also adds public Get/Set APIs for modifying all metadata fields of the same, along with a test program. K Shiva Kiran (4): Add <title> and <description> for Network Objects Adding Public Get and Set APIs for Network Metadata Add virNetworkObj Get and Set Methods for Metadata Add Test driver and testcase for Network Metadata change APIs docs/formatnetwork.rst | 11 + include/libvirt/libvirt-network.h | 29 +++ include/libvirt/virterror.h | 1 + src/conf/network_conf.c | 21 ++ src/conf/network_conf.h | 2 + src/conf/schemas/basictypes.rng | 15 ++ src/conf/schemas/domaincommon.rng | 15 -- src/conf/schemas/network.rng | 10 + src/conf/virnetworkobj.c | 325 ++++++++++++++++++++++++++++++ src/conf/virnetworkobj.h | 17 ++ src/driver-network.h | 16 ++ src/libvirt-network.c | 167 +++++++++++++++ src/libvirt_public.syms | 6 + src/remote/remote_driver.c | 2 + src/remote/remote_protocol.x | 36 +++- src/remote_protocol-structs | 19 ++ src/test/test_driver.c | 67 ++++++ src/util/virerror.c | 3 + tests/meson.build | 1 + tests/networkmetadatatest.c | 297 +++++++++++++++++++++++++++ 20 files changed, 1044 insertions(+), 16 deletions(-) create mode 100644 tests/networkmetadatatest.c -- 2.41.0
From: K Shiva <shiva_kr@riseup.net> This patch adds new elements <title> and <description> to the Network XML. - The <title> attribute holds a short title defined by the user and cannot contain newlines. - The <description> attribute holds any documentation that the user wants to store. - Schema definitions of <title> and <description> have been moved from domaincommon.rng to basictypes.rng for use by network and future objects. Signed-off-by: K Shiva Kiran <shiva_kr@riseup.net> --- docs/formatnetwork.rst | 11 +++++++++++ src/conf/network_conf.c | 18 ++++++++++++++++++ src/conf/network_conf.h | 2 ++ src/conf/schemas/basictypes.rng | 15 +++++++++++++++ src/conf/schemas/domaincommon.rng | 15 --------------- src/conf/schemas/network.rng | 10 ++++++++++ 6 files changed, 56 insertions(+), 15 deletions(-) diff --git a/docs/formatnetwork.rst b/docs/formatnetwork.rst index b5dc29db07..b3a24ce1a4 100644 --- a/docs/formatnetwork.rst +++ b/docs/formatnetwork.rst @@ -30,6 +30,8 @@ The first elements provide basic metadata about the virtual network. <network ipv6='yes' trustGuestRxFilters='no'> <name>default</name> <uuid>3e3fce45-4f53-4fa7-bb32-11f34168b82b</uuid> + <title>A short description - title - of the network</title> + <description>Some human readable description</description> <metadata> <app1:foo xmlns:app1="http://app1.org/app1/">..</app1:foo> <app2:bar xmlns:app2="http://app1.org/app2/">..</app2:bar> @@ -47,6 +49,7 @@ The first elements provide basic metadata about the virtual network. the virtual network. The format must be RFC 4122 compliant, eg ``3e3fce45-4f53-4fa7-bb32-11f34168b82b``. If omitted when defining/creating a new network, a random UUID is generated. :since:`Since 0.3.0` +``metadata`` The ``metadata`` node can be used by applications to store custom metadata in the form of XML nodes/trees. Applications must use custom namespaces on their XML nodes/trees, with only one top-level element per namespace (if the @@ -65,6 +68,14 @@ The first elements provide basic metadata about the virtual network. documentation for more details. Note that an explicit setting of this attribute in a portgroup or the individual domain interface will override the setting in the network. +``title`` + The optional element ``title`` provides space for a short description of the + network. The title should not contain any newlines. :since:`Since 9.6.0` . +``description`` + The content of the ``description`` element provides a human readable + description of the network. This data is not used by libvirt in any + way, it can contain any information the user wants. :since:`Since 9.6.0` + Connectivity ~~~~~~~~~~~~ diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 73788b6d87..427635250c 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -281,6 +281,8 @@ virNetworkDefFree(virNetworkDef *def) virNetDevBandwidthFree(def->bandwidth); virNetDevVlanClear(&def->vlan); + g_free(def->title); + g_free(def->description); xmlFreeNode(def->metadata); if (def->namespaceData && def->ns.free) @@ -1599,6 +1601,17 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt, def->uuid_specified = true; } + /* Extract short description of network (title) */ + def->title = virXPathString("string(./title[1])", ctxt); + if (def->title && strchr(def->title, '\n')) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Network title can't contain newlines")); + return NULL; + } + + /* Extract documentation if present */ + def->description = virXPathString("string(./description[1])", ctxt); + /* check if definitions with no IPv6 gateway addresses is to * allow guest-to-guest communications. */ @@ -2311,6 +2324,11 @@ virNetworkDefFormatBuf(virBuffer *buf, virUUIDFormat(uuid, uuidstr); virBufferAsprintf(buf, "<uuid>%s</uuid>\n", uuidstr); + virBufferEscapeString(buf, "<title>%s</title>\n", def->title); + + virBufferEscapeString(buf, "<description>%s</description>\n", + def->description); + if (virXMLFormatMetadata(buf, def->metadata) < 0) return -1; 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/schemas/basictypes.rng b/src/conf/schemas/basictypes.rng index 2d6f1a2c84..26eb538077 100644 --- a/src/conf/schemas/basictypes.rng +++ b/src/conf/schemas/basictypes.rng @@ -610,6 +610,21 @@ </choice> </define> + <!-- + title and description element, may be placed anywhere under the root + --> + <define name="title"> + <element name="title"> + <ref name="objectNameWithSlash"/> + </element> + </define> + + <define name="description"> + <element name="description"> + <text/> + </element> + </define> + <define name="metadata"> <element name="metadata"> <zeroOrMore> diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index c2f56b0490..2556ac01ed 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -8,21 +8,6 @@ <include href="nwfilter_params.rng"/> <include href="privatedata.rng"/> - <!-- - description and title element, may be placed anywhere under the root - --> - <define name="description"> - <element name="description"> - <text/> - </element> - </define> - - <define name="title"> - <element name="title"> - <ref name="objectNameWithSlash"/> - </element> - </define> - <define name="createMode"> <data type="unsignedInt"> <param name="pattern">0[0-7]{3}|[0-7]{1,3}</param> diff --git a/src/conf/schemas/network.rng b/src/conf/schemas/network.rng index 4317572208..cda174ab4b 100644 --- a/src/conf/schemas/network.rng +++ b/src/conf/schemas/network.rng @@ -37,6 +37,16 @@ <text/> </element> + <!-- <title> element --> + <optional> + <ref name="title"/> + </optional> + + <!-- <description> element --> + <optional> + <ref name="description"/> + </optional> + <!-- <metadata> element --> <optional> <ref name="metadata"/> -- 2.41.0
This patch introduces public Get and Set APIs for modifying <title>, <description> and <metadata> elements of the Network object. - Added enum to select one of the above elements to operate on. - Added error code and messages for missing metadata. - Added public API implementation. - Added driver support. - Defined wire protocol format. Signed-off-by: K Shiva Kiran <shiva_kr@riseup.net> --- include/libvirt/libvirt-network.h | 29 ++++++ include/libvirt/virterror.h | 1 + src/driver-network.h | 16 +++ src/libvirt-network.c | 167 ++++++++++++++++++++++++++++++ src/libvirt_public.syms | 6 ++ src/remote/remote_driver.c | 2 + src/remote/remote_protocol.x | 36 ++++++- src/remote_protocol-structs | 19 ++++ src/util/virerror.c | 3 + 9 files changed, 278 insertions(+), 1 deletion(-) diff --git a/include/libvirt/libvirt-network.h b/include/libvirt/libvirt-network.h index 90cde0cf24..147dc8c6b8 100644 --- a/include/libvirt/libvirt-network.h +++ b/include/libvirt/libvirt-network.h @@ -547,4 +547,33 @@ virNetworkPortFree(virNetworkPortPtr port); int virNetworkPortRef(virNetworkPortPtr port); +/** + * virNetworkMetadataType: + * + * Since: 9.6.0 + */ +typedef enum { + VIR_NETWORK_METADATA_DESCRIPTION = 0, /* Operate on <description> (Since: 9.6.0) */ + VIR_NETWORK_METADATA_TITLE = 1, /* Operate on <title> (Since: 9.6.0) */ + VIR_NETWORK_METADATA_ELEMENT = 2, /* Operate on <metadata> (Since: 9.6.0) */ + +# ifdef VIR_ENUM_SENTINELS + VIR_NETWORK_METADATA_LAST /* (Since: 9.6.0) */ +# endif +} virNetworkMetadataType; + +int +virNetworkSetMetadata(virNetworkPtr network, + int type, + const char *metadata, + const char *key, + const char *uri, + unsigned int flags); + +char * +virNetworkGetMetadata(virNetworkPtr network, + int type, + const char *uri, + unsigned int flags); + #endif /* LIBVIRT_NETWORK_H */ diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index df13e4f11e..55bc4431d9 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -348,6 +348,7 @@ typedef enum { VIR_ERR_NO_HOSTNAME = 108, /* no domain's hostname found (Since: 6.1.0) */ VIR_ERR_CHECKPOINT_INCONSISTENT = 109, /* checkpoint can't be used (Since: 6.10.0) */ VIR_ERR_MULTIPLE_DOMAINS = 110, /* more than one matching domain found (Since: 7.1.0) */ + VIR_ERR_NO_NETWORK_METADATA = 111, /* Network metadata is not present (Since: 9.6.0) */ # ifdef VIR_ENUM_SENTINELS VIR_ERR_NUMBER_LAST /* (Since: 5.0.0) */ diff --git a/src/driver-network.h b/src/driver-network.h index 99efd4c8aa..1d19b013c9 100644 --- a/src/driver-network.h +++ b/src/driver-network.h @@ -161,6 +161,20 @@ typedef int virNetworkPortPtr **ports, unsigned int flags); +typedef int +(*virDrvNetworkSetMetadata)(virNetworkPtr network, + int type, + const char *metadata, + const char *key, + const char *uri, + unsigned int flags); + +typedef char * +(*virDrvNetworkGetMetadata)(virNetworkPtr network, + int type, + const char *uri, + unsigned int flags); + typedef struct _virNetworkDriver virNetworkDriver; /** @@ -202,4 +216,6 @@ struct _virNetworkDriver { virDrvNetworkPortGetParameters networkPortGetParameters; virDrvNetworkPortDelete networkPortDelete; virDrvNetworkListAllPorts networkListAllPorts; + virDrvNetworkSetMetadata networkSetMetadata; + virDrvNetworkGetMetadata networkGetMetadata; }; diff --git a/src/libvirt-network.c b/src/libvirt-network.c index 236dfe2f5d..2dd11cf205 100644 --- a/src/libvirt-network.c +++ b/src/libvirt-network.c @@ -1915,3 +1915,170 @@ virNetworkPortRef(virNetworkPortPtr port) virObjectRef(port); return 0; } + + +/** + * virNetworkSetMetadata: + * @network: a network object + * @type: type of metadata, from virNetworkMetadataType + * @metadata: new metadata text + * @key: XML namespace key, or NULL + * @uri: XML namespace URI, or NULL + * @flags: bitwise-OR of virNetworkUpdateFlags + * + * Sets the appropriate network element given by @type to the + * value of @metadata. A @type of VIR_NETWORK_METADATA_DESCRIPTION + * is free-form text; VIR_NETWORK_METADATA_TITLE is free-form, but no + * newlines are permitted, and should be short (although the length is + * not enforced). For these two options @key and @uri are irrelevant and + * must be set to NULL. + * + * For type VIR_NETWORK_METADATA_ELEMENT @metadata must be well-formed + * XML belonging to namespace defined by @uri with local name @key. + * + * Passing NULL for @metadata says to remove that element from the + * network XML (passing the empty string leaves the element present). + * + * The resulting metadata will be present in virNetworkGetXMLDesc(), + * as well as quick access through virNetworkGetMetadata(). + * + * @flags controls whether the live network state, persistent configuration, + * or both will be modified. + * + * Returns 0 on success, -1 in case of failure. + * + * Since: 9.6.0 + */ +int +virNetworkSetMetadata(virNetworkPtr network, + int type, + const char *metadata, + const char *key, + const char *uri, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DEBUG("network=%p, type=%d, metadata='%s', key='%s', uri='%s', flags=0x%x", + network, type, NULLSTR(metadata), NULLSTR(key), NULLSTR(uri), + flags); + + virResetLastError(); + + virCheckNetworkReturn(network, -1); + conn = network->conn; + + virCheckReadOnlyGoto(conn->flags, error); + + switch (type) { + case VIR_NETWORK_METADATA_TITLE: + if (metadata && strchr(metadata, '\n')) { + virReportInvalidArg(metadata, "%s", + _("metadata title can't contain " + "newlines")); + goto error; + } + G_GNUC_FALLTHROUGH; + case VIR_NETWORK_METADATA_DESCRIPTION: + virCheckNullArgGoto(uri, error); + virCheckNullArgGoto(key, error); + break; + case VIR_NETWORK_METADATA_ELEMENT: + virCheckNonNullArgGoto(uri, error); + if (metadata) + virCheckNonNullArgGoto(key, error); + break; + default: + /* For future expansion */ + break; + } + + if (conn->networkDriver->networkSetMetadata) { + int ret; + ret = conn->networkDriver->networkSetMetadata(network, type, metadata, key, uri, + flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(network->conn); + return -1; +} + + +/** + * virNetworkGetMetadata: + * @network: a network object + * @type: type of metadata, from virNetworkMetadataType + * @uri: XML namespace identifier + * @flags: bitwise-OR of virNetworkUpdateFlags + * + * Retrieves the appropriate network element given by @type. + * If VIR_NETWORK_METADATA_ELEMENT is requested parameter @uri + * must be set to the name of the namespace the requested elements + * belong to, otherwise must be NULL. + * + * If an element of the network XML is not present, the resulting + * error will be VIR_ERR_NO_NETWORK_METADATA. This method forms + * a shortcut for seeing information from virNetworkSetMetadata() + * without having to go through virNetworkGetXMLDesc(). + * + * @flags controls whether the live network state or persistent + * configuration will be queried. + * + * Returns the metadata string on success (caller must free), + * or NULL in case of failure. + * + * Since: 9.6.0 + */ +char * +virNetworkGetMetadata(virNetworkPtr network, + int type, + const char *uri, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DEBUG("network=%p, type=%d, uri='%s', flags=0x%x", + network, type, NULLSTR(uri), flags); + + virResetLastError(); + + virCheckNetworkReturn(network, NULL); + + VIR_EXCLUSIVE_FLAGS_GOTO(VIR_NETWORK_UPDATE_AFFECT_LIVE, + VIR_NETWORK_UPDATE_AFFECT_CONFIG, + error); + + switch (type) { + case VIR_NETWORK_METADATA_TITLE: + case VIR_NETWORK_METADATA_DESCRIPTION: + virCheckNullArgGoto(uri, error); + break; + case VIR_NETWORK_METADATA_ELEMENT: + virCheckNonNullArgGoto(uri, error); + break; + default: + /* For future expansion */ + break; + } + + conn = network->conn; + + if (conn->networkDriver->networkGetMetadata) { + char *ret; + if (!(ret = conn->networkDriver->networkGetMetadata(network, type, uri, flags))) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(network->conn); + return NULL; +} diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 80742f268e..4523700a0f 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -932,4 +932,10 @@ LIBVIRT_9.0.0 { virDomainFDAssociate; } LIBVIRT_8.5.0; +LIBVIRT_9.6.0 { + global: + virNetworkGetMetadata; + virNetworkSetMetadata; +} LIBVIRT_9.0.0; + # .... define new API here using predicted next version number .... diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 65ec239fb7..4e277f668a 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8160,6 +8160,8 @@ static virNetworkDriver network_driver = { .networkPortSetParameters = remoteNetworkPortSetParameters, /* 5.5.0 */ .networkPortGetParameters = remoteNetworkPortGetParameters, /* 5.5.0 */ .networkPortDelete = remoteNetworkPortDelete, /* 5.5.0 */ + .networkSetMetadata = remoteNetworkSetMetadata, /* 9.6.0 */ + .networkGetMetadata = remoteNetworkGetMetadata, /* 9.6.0 */ }; static virInterfaceDriver interface_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 5d86a51116..7ff059e393 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -3323,6 +3323,26 @@ struct remote_network_event_lifecycle_msg { int detail; }; +struct remote_network_set_metadata_args { + remote_nonnull_network network; + int type; + remote_string metadata; + remote_string key; + remote_string uri; + unsigned int flags; +}; + +struct remote_network_get_metadata_args { + remote_nonnull_network network; + int type; + remote_string uri; + unsigned int flags; +}; + +struct remote_network_get_metadata_ret { + remote_nonnull_string metadata; +}; + struct remote_connect_storage_pool_event_register_any_args { int eventID; remote_storage_pool pool; @@ -6974,5 +6994,19 @@ enum remote_procedure { * @generate: none * @acl: domain:write */ - REMOTE_PROC_DOMAIN_FD_ASSOCIATE = 443 + REMOTE_PROC_DOMAIN_FD_ASSOCIATE = 443, + + /** + * @generate: both + * @acl: network:write + * @acl: network:save:!VIR_NETWORK_UPDATE_AFFECT_CONFIG|VIR_NETWORK_UPDATE_AFFECT_LIVE + * @acl: network:save:VIR_NETWORK_UPDATE_AFFECT_CONFIG + */ + REMOTE_PROC_NETWORK_SET_METADATA = 444, + + /** + * @generate: both + * @acl: network:read + */ + REMOTE_PROC_NETWORK_GET_METADATA = 445 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 3c6c230a16..14898a0bc7 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -3130,6 +3130,23 @@ struct remote_network_port_delete_args { remote_nonnull_network_port port; u_int flags; }; +struct remote_network_set_metadata_args { + remote_nonnull_network network; + int type; + remote_string metadata; + remote_string key; + remote_string uri; + u_int flags; +}; +struct remote_network_get_metadata_args { + remote_nonnull_network network; + int type; + remote_string uri; + u_int flags; +}; +struct remote_network_get_metadata_ret { + remote_nonnull_string metadata; +}; struct remote_domain_checkpoint_create_xml_args { remote_nonnull_domain dom; remote_nonnull_string xml_desc; @@ -3717,4 +3734,6 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_RESTORE_PARAMS = 441, REMOTE_PROC_DOMAIN_ABORT_JOB_FLAGS = 442, REMOTE_PROC_DOMAIN_FD_ASSOCIATE = 443, + REMOTE_PROC_NETWORK_SET_METADATA = 444, + REMOTE_PROC_NETWORK_GET_METADATA = 445 }; diff --git a/src/util/virerror.c b/src/util/virerror.c index 453f19514e..227a182417 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -1287,6 +1287,9 @@ static const virErrorMsgTuple virErrorMsgStrings[] = { [VIR_ERR_MULTIPLE_DOMAINS] = { N_("multiple matching domains found"), N_("multiple matching domains found: %1$s") }, + [VIR_ERR_NO_NETWORK_METADATA] = { + N_("metadata not found"), + N_("metadata not found: %1$s") }, }; G_STATIC_ASSERT(G_N_ELEMENTS(virErrorMsgStrings) == VIR_ERR_NUMBER_LAST); -- 2.41.0
On 7/11/23 08:47, K Shiva Kiran wrote:
This patch introduces public Get and Set APIs for modifying <title>, <description> and <metadata> elements of the Network object.
- Added enum to select one of the above elements to operate on. - Added error code and messages for missing metadata. - Added public API implementation. - Added driver support. - Defined wire protocol format.
Signed-off-by: K Shiva Kiran <shiva_kr@riseup.net> --- include/libvirt/libvirt-network.h | 29 ++++++ include/libvirt/virterror.h | 1 + src/driver-network.h | 16 +++ src/libvirt-network.c | 167 ++++++++++++++++++++++++++++++ src/libvirt_public.syms | 6 ++ src/remote/remote_driver.c | 2 + src/remote/remote_protocol.x | 36 ++++++- src/remote_protocol-structs | 19 ++++ src/util/virerror.c | 3 + 9 files changed, 278 insertions(+), 1 deletion(-)
We usually introduce public APIs in one patch and implement remote driver in another. But I guess I can live with this.
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 3c6c230a16..14898a0bc7 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -3130,6 +3130,23 @@ struct remote_network_port_delete_args { remote_nonnull_network_port port; u_int flags; }; +struct remote_network_set_metadata_args { + remote_nonnull_network network; + int type; + remote_string metadata; + remote_string key; + remote_string uri; + u_int flags; +}; +struct remote_network_get_metadata_args { + remote_nonnull_network network; + int type; + remote_string uri; + u_int flags; +}; +struct remote_network_get_metadata_ret { + remote_nonnull_string metadata; +};
This is misplaced.
struct remote_domain_checkpoint_create_xml_args { remote_nonnull_domain dom; remote_nonnull_string xml_desc; @@ -3717,4 +3734,6 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_RESTORE_PARAMS = 441, REMOTE_PROC_DOMAIN_ABORT_JOB_FLAGS = 442, REMOTE_PROC_DOMAIN_FD_ASSOCIATE = 443, + REMOTE_PROC_NETWORK_SET_METADATA = 444, + REMOTE_PROC_NETWORK_GET_METADATA = 445
Here we want the trailing comma.
};
Squash in the following: diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 14898a0bc7..c07e0af1e6 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2687,6 +2687,23 @@ struct remote_network_event_lifecycle_msg { int event; int detail; }; +struct remote_network_set_metadata_args { + remote_nonnull_network network; + int type; + remote_string metadata; + remote_string key; + remote_string uri; + u_int flags; +}; +struct remote_network_get_metadata_args { + remote_nonnull_network network; + int type; + remote_string uri; + u_int flags; +}; +struct remote_network_get_metadata_ret { + remote_nonnull_string metadata; +}; struct remote_connect_storage_pool_event_register_any_args { int eventID; remote_storage_pool pool; @@ -3130,23 +3147,6 @@ struct remote_network_port_delete_args { remote_nonnull_network_port port; u_int flags; }; -struct remote_network_set_metadata_args { - remote_nonnull_network network; - int type; - remote_string metadata; - remote_string key; - remote_string uri; - u_int flags; -}; -struct remote_network_get_metadata_args { - remote_nonnull_network network; - int type; - remote_string uri; - u_int flags; -}; -struct remote_network_get_metadata_ret { - remote_nonnull_string metadata; -}; struct remote_domain_checkpoint_create_xml_args { remote_nonnull_domain dom; remote_nonnull_string xml_desc; @@ -3735,5 +3735,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_ABORT_JOB_FLAGS = 442, REMOTE_PROC_DOMAIN_FD_ASSOCIATE = 443, REMOTE_PROC_NETWORK_SET_METADATA = 444, - REMOTE_PROC_NETWORK_GET_METADATA = 445 + REMOTE_PROC_NETWORK_GET_METADATA = 445, }; Michal
From 61d699de0603ca49a67cc898a219cce2613b82ba Mon Sep 17 00:00:00 2001 From: K Shiva Kiran <shiva_kr@riseup.net> Date: Tue, 27 Jun 2023 20:48:52 +0530 Subject: [libvirt PATCH v2 2/4] Adding Public Get and Set APIs for Network Metadata This patch introduces public Get and Set APIs for modifying <title>, <description> and <metadata> elements of the Network object. - Added enum to select one of the above elements to operate on. - Added error code and messages for missing metadata. - Added public API implementation. - Added driver support. - Defined wire protocol format. --- This is a v2 of: https://listman.redhat.com/archives/libvir-list/2023-July/240666.html include/libvirt/libvirt-network.h | 29 ++++++ include/libvirt/virterror.h | 1 + src/driver-network.h | 16 +++ src/libvirt-network.c | 167 ++++++++++++++++++++++++++++++ src/libvirt_public.syms | 6 ++ src/remote/remote_driver.c | 2 + src/remote/remote_protocol.x | 36 ++++++- src/remote_protocol-structs | 19 ++++ src/util/virerror.c | 3 + 9 files changed, 278 insertions(+), 1 deletion(-) diff --git a/include/libvirt/libvirt-network.h b/include/libvirt/libvirt-network.h index 90cde0cf24..147dc8c6b8 100644 --- a/include/libvirt/libvirt-network.h +++ b/include/libvirt/libvirt-network.h @@ -547,4 +547,33 @@ virNetworkPortFree(virNetworkPortPtr port); int virNetworkPortRef(virNetworkPortPtr port); +/** + * virNetworkMetadataType: + * + * Since: 9.6.0 + */ +typedef enum { + VIR_NETWORK_METADATA_DESCRIPTION = 0, /* Operate on <description> (Since: 9.6.0) */ + VIR_NETWORK_METADATA_TITLE = 1, /* Operate on <title> (Since: 9.6.0) */ + VIR_NETWORK_METADATA_ELEMENT = 2, /* Operate on <metadata> (Since: 9.6.0) */ + +# ifdef VIR_ENUM_SENTINELS + VIR_NETWORK_METADATA_LAST /* (Since: 9.6.0) */ +# endif +} virNetworkMetadataType; + +int +virNetworkSetMetadata(virNetworkPtr network, + int type, + const char *metadata, + const char *key, + const char *uri, + unsigned int flags); + +char * +virNetworkGetMetadata(virNetworkPtr network, + int type, + const char *uri, + unsigned int flags); + #endif /* LIBVIRT_NETWORK_H */ diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index df13e4f11e..55bc4431d9 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -348,6 +348,7 @@ typedef enum { VIR_ERR_NO_HOSTNAME = 108, /* no domain's hostname found (Since: 6.1.0) */ VIR_ERR_CHECKPOINT_INCONSISTENT = 109, /* checkpoint can't be used (Since: 6.10.0) */ VIR_ERR_MULTIPLE_DOMAINS = 110, /* more than one matching domain found (Since: 7.1.0) */ + VIR_ERR_NO_NETWORK_METADATA = 111, /* Network metadata is not present (Since: 9.6.0) */ # ifdef VIR_ENUM_SENTINELS VIR_ERR_NUMBER_LAST /* (Since: 5.0.0) */ diff --git a/src/driver-network.h b/src/driver-network.h index 99efd4c8aa..1d19b013c9 100644 --- a/src/driver-network.h +++ b/src/driver-network.h @@ -161,6 +161,20 @@ typedef int virNetworkPortPtr **ports, unsigned int flags); +typedef int +(*virDrvNetworkSetMetadata)(virNetworkPtr network, + int type, + const char *metadata, + const char *key, + const char *uri, + unsigned int flags); + +typedef char * +(*virDrvNetworkGetMetadata)(virNetworkPtr network, + int type, + const char *uri, + unsigned int flags); + typedef struct _virNetworkDriver virNetworkDriver; /** @@ -202,4 +216,6 @@ struct _virNetworkDriver { virDrvNetworkPortGetParameters networkPortGetParameters; virDrvNetworkPortDelete networkPortDelete; virDrvNetworkListAllPorts networkListAllPorts; + virDrvNetworkSetMetadata networkSetMetadata; + virDrvNetworkGetMetadata networkGetMetadata; }; diff --git a/src/libvirt-network.c b/src/libvirt-network.c index 236dfe2f5d..2dd11cf205 100644 --- a/src/libvirt-network.c +++ b/src/libvirt-network.c @@ -1915,3 +1915,170 @@ virNetworkPortRef(virNetworkPortPtr port) virObjectRef(port); return 0; } + + +/** + * virNetworkSetMetadata: + * @network: a network object + * @type: type of metadata, from virNetworkMetadataType + * @metadata: new metadata text + * @key: XML namespace key, or NULL + * @uri: XML namespace URI, or NULL + * @flags: bitwise-OR of virNetworkUpdateFlags + * + * Sets the appropriate network element given by @type to the + * value of @metadata. A @type of VIR_NETWORK_METADATA_DESCRIPTION + * is free-form text; VIR_NETWORK_METADATA_TITLE is free-form, but no + * newlines are permitted, and should be short (although the length is + * not enforced). For these two options @key and @uri are irrelevant and + * must be set to NULL. + * + * For type VIR_NETWORK_METADATA_ELEMENT @metadata must be well-formed + * XML belonging to namespace defined by @uri with local name @key. + * + * Passing NULL for @metadata says to remove that element from the + * network XML (passing the empty string leaves the element present). + * + * The resulting metadata will be present in virNetworkGetXMLDesc(), + * as well as quick access through virNetworkGetMetadata(). + * + * @flags controls whether the live network state, persistent configuration, + * or both will be modified. + * + * Returns 0 on success, -1 in case of failure. + * + * Since: 9.6.0 + */ +int +virNetworkSetMetadata(virNetworkPtr network, + int type, + const char *metadata, + const char *key, + const char *uri, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DEBUG("network=%p, type=%d, metadata='%s', key='%s', uri='%s', flags=0x%x", + network, type, NULLSTR(metadata), NULLSTR(key), NULLSTR(uri), + flags); + + virResetLastError(); + + virCheckNetworkReturn(network, -1); + conn = network->conn; + + virCheckReadOnlyGoto(conn->flags, error); + + switch (type) { + case VIR_NETWORK_METADATA_TITLE: + if (metadata && strchr(metadata, '\n')) { + virReportInvalidArg(metadata, "%s", + _("metadata title can't contain " + "newlines")); + goto error; + } + G_GNUC_FALLTHROUGH; + case VIR_NETWORK_METADATA_DESCRIPTION: + virCheckNullArgGoto(uri, error); + virCheckNullArgGoto(key, error); + break; + case VIR_NETWORK_METADATA_ELEMENT: + virCheckNonNullArgGoto(uri, error); + if (metadata) + virCheckNonNullArgGoto(key, error); + break; + default: + /* For future expansion */ + break; + } + + if (conn->networkDriver->networkSetMetadata) { + int ret; + ret = conn->networkDriver->networkSetMetadata(network, type, metadata, key, uri, + flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(network->conn); + return -1; +} + + +/** + * virNetworkGetMetadata: + * @network: a network object + * @type: type of metadata, from virNetworkMetadataType + * @uri: XML namespace identifier + * @flags: bitwise-OR of virNetworkUpdateFlags + * + * Retrieves the appropriate network element given by @type. + * If VIR_NETWORK_METADATA_ELEMENT is requested parameter @uri + * must be set to the name of the namespace the requested elements + * belong to, otherwise must be NULL. + * + * If an element of the network XML is not present, the resulting + * error will be VIR_ERR_NO_NETWORK_METADATA. This method forms + * a shortcut for seeing information from virNetworkSetMetadata() + * without having to go through virNetworkGetXMLDesc(). + * + * @flags controls whether the live network state or persistent + * configuration will be queried. + * + * Returns the metadata string on success (caller must free), + * or NULL in case of failure. + * + * Since: 9.6.0 + */ +char * +virNetworkGetMetadata(virNetworkPtr network, + int type, + const char *uri, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DEBUG("network=%p, type=%d, uri='%s', flags=0x%x", + network, type, NULLSTR(uri), flags); + + virResetLastError(); + + virCheckNetworkReturn(network, NULL); + + VIR_EXCLUSIVE_FLAGS_GOTO(VIR_NETWORK_UPDATE_AFFECT_LIVE, + VIR_NETWORK_UPDATE_AFFECT_CONFIG, + error); + + switch (type) { + case VIR_NETWORK_METADATA_TITLE: + case VIR_NETWORK_METADATA_DESCRIPTION: + virCheckNullArgGoto(uri, error); + break; + case VIR_NETWORK_METADATA_ELEMENT: + virCheckNonNullArgGoto(uri, error); + break; + default: + /* For future expansion */ + break; + } + + conn = network->conn; + + if (conn->networkDriver->networkGetMetadata) { + char *ret; + if (!(ret = conn->networkDriver->networkGetMetadata(network, type, uri, flags))) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(network->conn); + return NULL; +} diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 80742f268e..4523700a0f 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -932,4 +932,10 @@ LIBVIRT_9.0.0 { virDomainFDAssociate; } LIBVIRT_8.5.0; +LIBVIRT_9.6.0 { + global: + virNetworkGetMetadata; + virNetworkSetMetadata; +} LIBVIRT_9.0.0; + # .... define new API here using predicted next version number .... diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 65ec239fb7..4e277f668a 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8160,6 +8160,8 @@ static virNetworkDriver network_driver = { .networkPortSetParameters = remoteNetworkPortSetParameters, /* 5.5.0 */ .networkPortGetParameters = remoteNetworkPortGetParameters, /* 5.5.0 */ .networkPortDelete = remoteNetworkPortDelete, /* 5.5.0 */ + .networkSetMetadata = remoteNetworkSetMetadata, /* 9.6.0 */ + .networkGetMetadata = remoteNetworkGetMetadata, /* 9.6.0 */ }; static virInterfaceDriver interface_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 5d86a51116..7ff059e393 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -3323,6 +3323,26 @@ struct remote_network_event_lifecycle_msg { int detail; }; +struct remote_network_set_metadata_args { + remote_nonnull_network network; + int type; + remote_string metadata; + remote_string key; + remote_string uri; + unsigned int flags; +}; + +struct remote_network_get_metadata_args { + remote_nonnull_network network; + int type; + remote_string uri; + unsigned int flags; +}; + +struct remote_network_get_metadata_ret { + remote_nonnull_string metadata; +}; + struct remote_connect_storage_pool_event_register_any_args { int eventID; remote_storage_pool pool; @@ -6974,5 +6994,19 @@ enum remote_procedure { * @generate: none * @acl: domain:write */ - REMOTE_PROC_DOMAIN_FD_ASSOCIATE = 443 + REMOTE_PROC_DOMAIN_FD_ASSOCIATE = 443, + + /** + * @generate: both + * @acl: network:write + * @acl: network:save:!VIR_NETWORK_UPDATE_AFFECT_CONFIG|VIR_NETWORK_UPDATE_AFFECT_LIVE + * @acl: network:save:VIR_NETWORK_UPDATE_AFFECT_CONFIG + */ + REMOTE_PROC_NETWORK_SET_METADATA = 444, + + /** + * @generate: both + * @acl: network:read + */ + REMOTE_PROC_NETWORK_GET_METADATA = 445 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 3c6c230a16..c07e0af1e6 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2687,6 +2687,23 @@ struct remote_network_event_lifecycle_msg { int event; int detail; }; +struct remote_network_set_metadata_args { + remote_nonnull_network network; + int type; + remote_string metadata; + remote_string key; + remote_string uri; + u_int flags; +}; +struct remote_network_get_metadata_args { + remote_nonnull_network network; + int type; + remote_string uri; + u_int flags; +}; +struct remote_network_get_metadata_ret { + remote_nonnull_string metadata; +}; struct remote_connect_storage_pool_event_register_any_args { int eventID; remote_storage_pool pool; @@ -3717,4 +3734,6 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_RESTORE_PARAMS = 441, REMOTE_PROC_DOMAIN_ABORT_JOB_FLAGS = 442, REMOTE_PROC_DOMAIN_FD_ASSOCIATE = 443, + REMOTE_PROC_NETWORK_SET_METADATA = 444, + REMOTE_PROC_NETWORK_GET_METADATA = 445, }; diff --git a/src/util/virerror.c b/src/util/virerror.c index 453f19514e..227a182417 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -1287,6 +1287,9 @@ static const virErrorMsgTuple virErrorMsgStrings[] = { [VIR_ERR_MULTIPLE_DOMAINS] = { N_("multiple matching domains found"), N_("multiple matching domains found: %1$s") }, + [VIR_ERR_NO_NETWORK_METADATA] = { + N_("metadata not found"), + N_("metadata not found: %1$s") }, }; G_STATIC_ASSERT(G_N_ELEMENTS(virErrorMsgStrings) == VIR_ERR_NUMBER_LAST); -- 2.41.0
On 19/07/23 14:48, Michal Prívozník wrote:
On 7/11/23 08:47, K Shiva Kiran wrote:
This patch introduces public Get and Set APIs for modifying <title>, <description> and <metadata> elements of the Network object.
- Added enum to select one of the above elements to operate on. - Added error code and messages for missing metadata. - Added public API implementation. - Added driver support. - Defined wire protocol format.
Signed-off-by: K Shiva Kiran <shiva_kr@riseup.net> --- include/libvirt/libvirt-network.h | 29 ++++++ include/libvirt/virterror.h | 1 + src/driver-network.h | 16 +++ src/libvirt-network.c | 167 ++++++++++++++++++++++++++++++ src/libvirt_public.syms | 6 ++ src/remote/remote_driver.c | 2 + src/remote/remote_protocol.x | 36 ++++++- src/remote_protocol-structs | 19 ++++ src/util/virerror.c | 3 + 9 files changed, 278 insertions(+), 1 deletion(-)
We usually introduce public APIs in one patch and implement remote driver in another. But I guess I can live with this.
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 3c6c230a16..14898a0bc7 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -3130,6 +3130,23 @@ struct remote_network_port_delete_args { remote_nonnull_network_port port; u_int flags; }; +struct remote_network_set_metadata_args { + remote_nonnull_network network; + int type; + remote_string metadata; + remote_string key; + remote_string uri; + u_int flags; +}; +struct remote_network_get_metadata_args { + remote_nonnull_network network; + int type; + remote_string uri; + u_int flags; +}; +struct remote_network_get_metadata_ret { + remote_nonnull_string metadata; +}; This is misplaced.
struct remote_domain_checkpoint_create_xml_args { remote_nonnull_domain dom; remote_nonnull_string xml_desc; @@ -3717,4 +3734,6 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_RESTORE_PARAMS = 441, REMOTE_PROC_DOMAIN_ABORT_JOB_FLAGS = 442, REMOTE_PROC_DOMAIN_FD_ASSOCIATE = 443, + REMOTE_PROC_NETWORK_SET_METADATA = 444, + REMOTE_PROC_NETWORK_GET_METADATA = 445 Here we want the trailing comma.
}; Squash in the following:
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 14898a0bc7..c07e0af1e6 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2687,6 +2687,23 @@ struct remote_network_event_lifecycle_msg { int event; int detail; }; +struct remote_network_set_metadata_args { + remote_nonnull_network network; + int type; + remote_string metadata; + remote_string key; + remote_string uri; + u_int flags; +}; +struct remote_network_get_metadata_args { + remote_nonnull_network network; + int type; + remote_string uri; + u_int flags; +}; +struct remote_network_get_metadata_ret { + remote_nonnull_string metadata; +}; struct remote_connect_storage_pool_event_register_any_args { int eventID; remote_storage_pool pool; @@ -3130,23 +3147,6 @@ struct remote_network_port_delete_args { remote_nonnull_network_port port; u_int flags; }; -struct remote_network_set_metadata_args { - remote_nonnull_network network; - int type; - remote_string metadata; - remote_string key; - remote_string uri; - u_int flags; -}; -struct remote_network_get_metadata_args { - remote_nonnull_network network; - int type; - remote_string uri; - u_int flags; -}; -struct remote_network_get_metadata_ret { - remote_nonnull_string metadata; -}; struct remote_domain_checkpoint_create_xml_args { remote_nonnull_domain dom; remote_nonnull_string xml_desc; @@ -3735,5 +3735,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_ABORT_JOB_FLAGS = 442, REMOTE_PROC_DOMAIN_FD_ASSOCIATE = 443, REMOTE_PROC_NETWORK_SET_METADATA = 444, - REMOTE_PROC_NETWORK_GET_METADATA = 445 + REMOTE_PROC_NETWORK_GET_METADATA = 445, };
Michal
Squashed in v2: https://listman.redhat.com/archives/libvir-list/2023-July/240830.html I realize it's a mistake to send a v2 patch as a reply to the same thread and won't be repeating it. Shiva
- Introduces virNetworkObjGetMetadata() and virNetworkObjSetMetadata(). - These functions implement common behaviour that can be reused by network drivers that use the virNetworkObj struct. - Also adds helper functions that resolve the live/persistent state of the network before setting metadata. Signed-off-by: K Shiva Kiran <shiva_kr@riseup.net> --- src/conf/virnetworkobj.c | 325 +++++++++++++++++++++++++++++++++++++++ src/conf/virnetworkobj.h | 17 ++ 2 files changed, 342 insertions(+) diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c index b8b86da06f..41d0a1d971 100644 --- a/src/conf/virnetworkobj.c +++ b/src/conf/virnetworkobj.c @@ -1822,3 +1822,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. + */ +static 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. + */ +static 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. + */ +static 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. + */ +static 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..a4ed28b1bc 100644 --- a/src/conf/virnetworkobj.h +++ b/src/conf/virnetworkobj.h @@ -258,3 +258,20 @@ virNetworkObjListNumOfNetworks(virNetworkObjList *nets, void virNetworkObjListPrune(virNetworkObjList *nets, unsigned int flags); + +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); -- 2.41.0
On 7/11/23 08:47, K Shiva Kiran wrote:
- Introduces virNetworkObjGetMetadata() and virNetworkObjSetMetadata(). - These functions implement common behaviour that can be reused by network drivers that use the virNetworkObj struct. - Also adds helper functions that resolve the live/persistent state of the network before setting metadata.
Signed-off-by: K Shiva Kiran <shiva_kr@riseup.net> --- src/conf/virnetworkobj.c | 325 +++++++++++++++++++++++++++++++++++++++ src/conf/virnetworkobj.h | 17 ++ 2 files changed, 342 insertions(+)
diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c index b8b86da06f..41d0a1d971 100644 --- a/src/conf/virnetworkobj.c +++ b/src/conf/virnetworkobj.c @@ -1822,3 +1822,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. + */ +static 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"));
I'm going to mention it here, but it applies to the all of your patches, and all the new code in fact. We made an exemption for error messages and thus they don't need to be broken at 80 chars limit. In fact, they shouldn't. The reason is simple: easier grep as one doesn't have to try and guess how is the message broken into individual substrings. Of course, you will find plenty of "bad" examples throughout the code, but that's because it is an old code. Whenever we rewrite something or introduce new code this exception applies and the old code is, well, gradually fixed.
+ return -1; + } + + return 0;
This code is basically the same as in networkUpdate(). The first part that sets _LIVE or _CONFIG is there verbatim, the second one is hidden under virNetworkObjConfigChangeSetup(). There's one extra step that the function does - it calls virNetworkObjSetDefTransient() but I don't think that's necessary. Either the network is active and virNetworkObjSetDefTransient() was already called, or is inactive and thus it's a NOP. IOW, the call to virNetworkObjSetDefTransient() can be removed. After this, virNetworkObjUpdateModificationImpact() could be exported (just like corresponding virDomain* sibling function is) so that it can be called from both src/conf/virnetworkobj.c and src/network/bridge_driver.c. Because I think we can get away with the networkUpdate() doing the following: networkUpdate() { ... virNetworkObjUpdateModificationImpact(); if (isActive && (flags & VIR_NETWORK_UPDATE_AFFECT_LIVE)) { /* This is the check that possibly calls networkRemoveFirewallRules() and sets needFirewallRefresh = true */ } virNetworkObjUpdate(); ... } BTW: when cooking the patch, don't forget the same pattern is copied to our test driver (src/test/test_driver.c). Michal
On 19/07/23 14:48, Michal Prívozník wrote:
- Introduces virNetworkObjGetMetadata() and virNetworkObjSetMetadata(). - These functions implement common behaviour that can be reused by network drivers that use the virNetworkObj struct. - Also adds helper functions that resolve the live/persistent state of the network before setting metadata.
Signed-off-by: K Shiva Kiran<shiva_kr@riseup.net> --- src/conf/virnetworkobj.c | 325 +++++++++++++++++++++++++++++++++++++++ src/conf/virnetworkobj.h | 17 ++ 2 files changed, 342 insertions(+)
diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c index b8b86da06f..41d0a1d971 100644 --- a/src/conf/virnetworkobj.c +++ b/src/conf/virnetworkobj.c @@ -1822,3 +1822,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. + */ +static 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")); I'm going to mention it here, but it applies to the all of your patches, and all the new code in fact. We made an exemption for error messages and thus they don't need to be broken at 80 chars limit. In fact, they shouldn't. The reason is simple: easier grep as one doesn't have to try and guess how is the message broken into individual substrings. Of course, you will find plenty of "bad" examples throughout the code, but
On 7/11/23 08:47, K Shiva Kiran wrote: that's because it is an old code. Whenever we rewrite something or introduce new code this exception applies and the old code is, well, gradually fixed.
+ return -1; + } + + return 0; This code is basically the same as in networkUpdate(). The first part that sets _LIVE or _CONFIG is there verbatim, the second one is hidden under virNetworkObjConfigChangeSetup(). There's one extra step that the function does - it calls virNetworkObjSetDefTransient() but I don't think that's necessary. Either the network is active and virNetworkObjSetDefTransient() was already called, or is inactive and thus it's a NOP. IOW, the call to virNetworkObjSetDefTransient() can be removed.
After this, virNetworkObjUpdateModificationImpact() could be exported (just like corresponding virDomain* sibling function is) so that it can be called from both src/conf/virnetworkobj.c and src/network/bridge_driver.c. Because I think we can get away with the networkUpdate() doing the following:
networkUpdate() { ... virNetworkObjUpdateModificationImpact();
if (isActive && (flags & VIR_NETWORK_UPDATE_AFFECT_LIVE)) { /* This is the check that possibly calls networkRemoveFirewallRules() and sets needFirewallRefresh = true */ }
virNetworkObjUpdate(); ... }
BTW: when cooking the patch, don't forget the same pattern is copied to our test driver (src/test/test_driver.c).
Michal
Applied in v2: https://listman.redhat.com/archives/libvir-list/2023-July/240831.html I have forgotten to mention the diff w.r.t v1 within the v2 patch itself, thus have included it in a reply. Shiva
This commit implements the newly defined Network Metadata Get and Set APIs into the test driver. It also adds a new testcase "networkmetadatatest" to test the APIs. Signed-off-by: K Shiva Kiran <shiva_kr@riseup.net> --- src/conf/network_conf.c | 3 + src/test/test_driver.c | 67 ++++++++ tests/meson.build | 1 + tests/networkmetadatatest.c | 297 ++++++++++++++++++++++++++++++++++++ 4 files changed, 368 insertions(+) create mode 100644 tests/networkmetadatatest.c diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 427635250c..8f39b4ed67 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -2564,6 +2564,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; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index e7fce053b4..af65c53bfc 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -633,6 +633,25 @@ static int testStoragePoolObjSetDefaults(virStoragePoolObj *obj); static int testNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info); static virNetworkObj *testNetworkObjFindByName(testDriver *privconn, const char *name); +static virNetworkObj * +testNetworkObjFromNetwork(virNetworkPtr network) +{ + virNetworkObj *net; + testDriver *driver = network->conn->privateData; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + + net = virNetworkObjFindByUUID(driver->networks, network->uuid); + if (!net) { + virUUIDFormat(network->uuid, uuidstr); + virReportError(VIR_ERR_NO_NETWORK, + _("no network with matching uuid '%1$s' (%2$s)"), + uuidstr, network->name); + } + + return net; +} + + static virDomainObj * testDomObjFromDomain(virDomainPtr domain) { @@ -9948,6 +9967,52 @@ testConnectGetAllDomainStats(virConnectPtr conn, return ret; } +static char * +testNetworkGetMetadata(virNetworkPtr net, + int type, + const char *uri, + unsigned int flags) +{ + virNetworkObj *privnet; + char *ret; + + virCheckFlags(VIR_NETWORK_UPDATE_AFFECT_LIVE | + VIR_NETWORK_UPDATE_AFFECT_CONFIG, NULL); + + if (!(privnet = testNetworkObjFromNetwork(net))) + return NULL; + + ret = virNetworkObjGetMetadata(privnet, type, uri, flags); + + virNetworkObjEndAPI(&privnet); + return ret; +} + +static int +testNetworkSetMetadata(virNetworkPtr net, + int type, + const char *metadata, + const char *key, + const char *uri, + unsigned int flags) +{ + virNetworkObj *privnet; + int ret; + + virCheckFlags(VIR_NETWORK_UPDATE_AFFECT_LIVE | + VIR_NETWORK_UPDATE_AFFECT_CONFIG, -1); + + if (!(privnet = testNetworkObjFromNetwork(net))) + return -1; + + ret = virNetworkObjSetMetadata(privnet, type, metadata, + key, uri, NULL, + NULL, NULL, flags); + + virNetworkObjEndAPI(&privnet); + return ret; +} + /* * Test driver */ @@ -10141,6 +10206,8 @@ static virNetworkDriver testNetworkDriver = { .networkSetAutostart = testNetworkSetAutostart, /* 0.3.2 */ .networkIsActive = testNetworkIsActive, /* 0.7.3 */ .networkIsPersistent = testNetworkIsPersistent, /* 0.7.3 */ + .networkSetMetadata = testNetworkSetMetadata, /* 9.6.0 */ + .networkGetMetadata = testNetworkGetMetadata, /* 9.6.0 */ }; static virInterfaceDriver testInterfaceDriver = { diff --git a/tests/meson.build b/tests/meson.build index 0082446029..d083548c0a 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -258,6 +258,7 @@ tests += [ { 'name': 'genericxml2xmltest' }, { 'name': 'interfacexml2xmltest' }, { 'name': 'metadatatest' }, + { 'name': 'networkmetadatatest' }, { 'name': 'networkxml2xmlupdatetest' }, { 'name': 'nodedevxml2xmltest' }, { 'name': 'nwfilterxml2xmltest' }, diff --git a/tests/networkmetadatatest.c b/tests/networkmetadatatest.c new file mode 100644 index 0000000000..4448472776 --- /dev/null +++ b/tests/networkmetadatatest.c @@ -0,0 +1,297 @@ +/* + * Copyright (C) 2013 Red Hat, Inc. + * + * 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/>. + */ + +#include <config.h> + +#include "testutils.h" + +#include "virerror.h" +#include "virxml.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +static const char metadata1[] = +"<derp xmlns:foobar='http://foo.bar/'>\n" +" <bar>foobar</bar>\n" +" <foo fooish='blurb'>foofoo</foo>\n" +" <foobar:baz>zomg</foobar:baz>\n" +"</derp>"; + + +static const char metadata1_ns[] = +"<herp:derp xmlns:foobar='http://foo.bar/' xmlns:herp='http://herp.derp/'>\n" +" <herp:bar>foobar</herp:bar>\n" +" <herp:foo fooish='blurb'>foofoo</herp:foo>\n" +" <foobar:baz>zomg</foobar:baz>\n" +"</herp:derp>"; + + +static const char metadata2[] = +"<foo>\n" +" <bar>baz</bar>\n" +"</foo>"; + + +static const char metadata2_ns[] = +"<blurb:foo xmlns:blurb='http://herp.derp/'>\n" +" <blurb:bar>baz</blurb:bar>\n" +"</blurb:foo>"; + + +static char * +getMetadataFromXML(virNetworkPtr net) +{ + g_autoptr(xmlDoc) doc = NULL; + g_autoptr(xmlXPathContext) ctxt = NULL; + xmlNodePtr node; + + g_autofree char *xml = NULL; + + if (!(xml = virNetworkGetXMLDesc(net, 0))) + return NULL; + + if (!(doc = virXMLParseStringCtxt(xml, "(network_definition)", &ctxt))) + return NULL; + + if (!(node = virXPathNode("//metadata/*", ctxt))) + return NULL; + + return virXMLNodeToString(node->doc, node); +} + + +static void +metadataXMLConvertApostrophe(char *str) +{ + do { + if (*str == '\"') + *str = '\''; + } while ((*++str) != '\0'); +} + + +static bool +verifyMetadata(virNetworkPtr net, + const char *expectXML, + const char *expectAPI, + const char *uri) +{ + g_autofree char *metadataXML = NULL; + g_autofree char *metadataAPI = NULL; + + if (!expectAPI) { + if ((metadataAPI = virNetworkGetMetadata(net, + VIR_NETWORK_METADATA_ELEMENT, + uri, 0))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "expected no metadata in API, but got:\n[%s]", + metadataAPI); + return false; + } + } else { + if (!(metadataAPI = virNetworkGetMetadata(net, + VIR_NETWORK_METADATA_ELEMENT, + uri, 0))) + return false; + + metadataXMLConvertApostrophe(metadataAPI); + + if (STRNEQ(metadataAPI, expectAPI)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "XML metadata in API doesn't match expected metadata: " + "expected:\n[%s]\ngot:\n[%s]", + expectAPI, metadataAPI); + return false; + } + + } + + if (!expectXML) { + if ((metadataXML = getMetadataFromXML(net))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "expected no metadata in XML, but got:\n[%s]", + metadataXML); + return false; + } + } else { + if (!(metadataXML = getMetadataFromXML(net))) + return false; + + metadataXMLConvertApostrophe(metadataXML); + + if (STRNEQ(metadataXML, expectXML)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "XML in dump doesn't match expected metadata: " + "expected:\n[%s]\ngot:\n[%s]", + expectXML, metadataXML); + return false; + } + } + + return true; +} + + +struct metadataTest { + virConnectPtr conn; + virNetworkPtr net; + + const char *data; + const char *expect; + int type; + bool fail; +}; + + +static int +testAssignMetadata(const void *data) +{ + const struct metadataTest *test = data; + + if (virNetworkSetMetadata(test->net, VIR_NETWORK_METADATA_ELEMENT, + metadata1, "herp", "http://herp.derp/", 0) < 0) + return -1; + + if (!verifyMetadata(test->net, metadata1_ns, metadata1, "http://herp.derp/")) + return -1; + + return 0; +} + +static int +testRewriteMetadata(const void *data) +{ + const struct metadataTest *test = data; + + if (virNetworkSetMetadata(test->net, VIR_NETWORK_METADATA_ELEMENT, + metadata2, "blurb", "http://herp.derp/", 0) < 0) + return -1; + + if (!verifyMetadata(test->net, metadata2_ns, metadata2, "http://herp.derp/")) + return -1; + + return 0; +} + +static int +testEraseMetadata(const void *data) +{ + const struct metadataTest *test = data; + + if (virNetworkSetMetadata(test->net, VIR_NETWORK_METADATA_ELEMENT, + NULL, NULL, "http://herp.derp/", 0) < 0) + return -1; + + if (!verifyMetadata(test->net, NULL, NULL, "http://herp.derp/")) + return -1; + + return 0; +} + +static int +testTextMetadata(const void *data) +{ + const struct metadataTest *test = data; + g_autofree char *actual = NULL; + + if (virNetworkSetMetadata(test->net, test->type, test->data, NULL, NULL, 0) < 0) { + if (test->fail) + return 0; + return -1; + } + + actual = virNetworkGetMetadata(test->net, test->type, NULL, 0); + + if (STRNEQ_NULLABLE(test->expect, actual)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "expected metadata doesn't match actual: " + "expected:'%s'\ngot: '%s'", + NULLSTR(test->data), NULLSTR(actual)); + return -1; + } + + return 0; +} + +#define TEST_TEXT_METADATA(INDEX, TYPE, DATA, EXPECT, FAIL) \ + do { \ + test.type = VIR_NETWORK_METADATA_ ## TYPE; \ + test.data = DATA; \ + test.expect = EXPECT; \ + test.fail = FAIL; \ + \ + if (virTestRun("text metadata: " #TYPE " " INDEX " ", \ + testTextMetadata, &test) < 0) \ + ret = EXIT_FAILURE; \ + } while (0) + +#define TEST_TITLE(INDEX, DATA) \ + TEST_TEXT_METADATA(INDEX, TITLE, DATA, DATA, false) +#define TEST_TITLE_EXPECT(INDEX, DATA, EXPECT) \ + TEST_TEXT_METADATA(INDEX, TITLE, DATA, EXPECT, false) +#define TEST_TITLE_FAIL(INDEX, DATA) \ + TEST_TEXT_METADATA(INDEX, TITLE, DATA, DATA, true) +#define TEST_DESCR(INDEX, DATA) \ + TEST_TEXT_METADATA(INDEX, DESCRIPTION, DATA, DATA, false) +#define TEST_DESCR_EXPECT(INDEX, DATA, EXPECT) \ + TEST_TEXT_METADATA(INDEX, DESCRIPTION, DATA, EXPECT, false) + +static int +mymain(void) +{ + struct metadataTest test = { 0 }; + int ret = EXIT_SUCCESS; + + if (!(test.conn = virConnectOpen("test:///default"))) + return EXIT_FAILURE; + + if (!(test.net = virNetworkLookupByName(test.conn, "default"))) { + virConnectClose(test.conn); + return EXIT_FAILURE; + } + + virTestQuiesceLibvirtErrors(false); + + if (virTestRun("Assign metadata ", testAssignMetadata, &test) < 0) + ret = EXIT_FAILURE; + if (virTestRun("Rewrite Metadata ", testRewriteMetadata, &test) < 0) + ret = EXIT_FAILURE; + if (virTestRun("Erase metadata ", testEraseMetadata, &test) < 0) + ret = EXIT_FAILURE; + + TEST_TITLE("1", "qwert"); + TEST_TITLE("2", NULL); + TEST_TITLE("3", "blah"); + TEST_TITLE_FAIL("4", "qwe\nrt"); + TEST_TITLE_EXPECT("5", "", NULL); + TEST_TITLE_FAIL("6", "qwert\n"); + TEST_TITLE_FAIL("7", "\n"); + + TEST_DESCR("1", "qwert\nqwert"); + TEST_DESCR("2", NULL); + TEST_DESCR("3", "qwert"); + TEST_DESCR("4", "\n"); + TEST_DESCR_EXPECT("5", "", NULL); + + virNetworkFree(test.net); + virConnectClose(test.conn); + + return ret; +} + +VIR_TEST_MAIN(mymain) -- 2.41.0
On 7/11/23 08:47, K Shiva Kiran wrote:
This commit introduces <title> and <description> fields to the XML schema of the Network object. It also adds public Get/Set APIs for modifying all metadata fields of the same, along with a test program.
K Shiva Kiran (4): Add <title> and <description> for Network Objects Adding Public Get and Set APIs for Network Metadata Add virNetworkObj Get and Set Methods for Metadata Add Test driver and testcase for Network Metadata change APIs
docs/formatnetwork.rst | 11 + include/libvirt/libvirt-network.h | 29 +++ include/libvirt/virterror.h | 1 + src/conf/network_conf.c | 21 ++ src/conf/network_conf.h | 2 + src/conf/schemas/basictypes.rng | 15 ++ src/conf/schemas/domaincommon.rng | 15 -- src/conf/schemas/network.rng | 10 + src/conf/virnetworkobj.c | 325 ++++++++++++++++++++++++++++++ src/conf/virnetworkobj.h | 17 ++ src/driver-network.h | 16 ++ src/libvirt-network.c | 167 +++++++++++++++ src/libvirt_public.syms | 6 + src/remote/remote_driver.c | 2 + src/remote/remote_protocol.x | 36 +++- src/remote_protocol-structs | 19 ++ src/test/test_driver.c | 67 ++++++ src/util/virerror.c | 3 + tests/meson.build | 1 + tests/networkmetadatatest.c | 297 +++++++++++++++++++++++++++ 20 files changed, 1044 insertions(+), 16 deletions(-) create mode 100644 tests/networkmetadatatest.c
No virsh exposure of these new APIs? :-( Michal
On 19/07/23 14:48, Michal Prívozník wrote:
On 7/11/23 08:47, K Shiva Kiran wrote:
This commit introduces <title> and <description> fields to the XML schema of the Network object. It also adds public Get/Set APIs for modifying all metadata fields of the same, along with a test program.
K Shiva Kiran (4): Add <title> and <description> for Network Objects Adding Public Get and Set APIs for Network Metadata Add virNetworkObj Get and Set Methods for Metadata Add Test driver and testcase for Network Metadata change APIs
docs/formatnetwork.rst | 11 + include/libvirt/libvirt-network.h | 29 +++ include/libvirt/virterror.h | 1 + src/conf/network_conf.c | 21 ++ src/conf/network_conf.h | 2 + src/conf/schemas/basictypes.rng | 15 ++ src/conf/schemas/domaincommon.rng | 15 -- src/conf/schemas/network.rng | 10 + src/conf/virnetworkobj.c | 325 ++++++++++++++++++++++++++++++ src/conf/virnetworkobj.h | 17 ++ src/driver-network.h | 16 ++ src/libvirt-network.c | 167 +++++++++++++++ src/libvirt_public.syms | 6 + src/remote/remote_driver.c | 2 + src/remote/remote_protocol.x | 36 +++- src/remote_protocol-structs | 19 ++ src/test/test_driver.c | 67 ++++++ src/util/virerror.c | 3 + tests/meson.build | 1 + tests/networkmetadatatest.c | 297 +++++++++++++++++++++++++++ 20 files changed, 1044 insertions(+), 16 deletions(-) create mode 100644 tests/networkmetadatatest.c
No virsh exposure of these new APIs? :-(
Michal
I had planned to introduce virsh as well as metadata change callbacks soon after these patches got pushed, is that ok? Shiva
participants (2)
-
K Shiva Kiran -
Michal Prívozník