[PATCH 00/16] nwfilter: Add support for user defined metadata

This patchset adds support for the following user defined metadata fields for network filters. - title: A short description of the filter. - description: Any documentation that the user wants to store. - metadata: Other metadata in XML form. K Shiva Kiran (16): xml: Add <title>, <description> and <metadata> to nwfilter xml schema conf: Add parser logic for nwfilter metadata fields nwfilter: Add enum to operate on user defined metadata nwfilter: Add error code and message for missing metadata nwfilter: Introduce public API to modify user metadata nwfilter: Introduce public API to retrieve user-defined metadata nwfilter: Implement RPC virsh: Add helper method to retrieve xml from NWFilter def virsh: Add new command `nwfilter-desc` virsh: Add new command `nwfilter-metadata` virsh: Add option --title for nwfilter-list docs: Document nwfilter metadata related commands virnwfilterobj: Add virNWFilterObjGetMetadata() virnwfilterobj: Add virNWFilterObjSetMetadata() nwfilter_driver: Add Driver implementation for metadata NEWS: Introduce user-defined metadata fields for NWFilter object NEWS.rst | 18 ++ docs/formatnwfilter.rst | 31 +++ docs/manpages/virsh.rst | 98 +++++++- include/libvirt/libvirt-nwfilter.h | 27 ++ include/libvirt/virterror.h | 1 + src/conf/nwfilter_conf.c | 30 +++ src/conf/nwfilter_conf.h | 5 + src/conf/schemas/nwfilter.rng | 9 + src/conf/virnwfilterobj.c | 148 +++++++++++ src/conf/virnwfilterobj.h | 13 + src/driver-nwfilter.h | 15 ++ src/libvirt-nwfilter.c | 154 ++++++++++++ src/libvirt_private.syms | 2 + src/libvirt_public.syms | 6 + src/nwfilter/nwfilter_driver.c | 61 +++++ src/remote/remote_driver.c | 2 + src/remote/remote_protocol.x | 34 ++- src/remote_protocol-structs | 19 ++ src/util/virerror.c | 3 + tools/virsh-nwfilter.c | 387 ++++++++++++++++++++++++++++- tools/virsh-util.c | 25 ++ tools/virsh-util.h | 9 + 22 files changed, 1089 insertions(+), 8 deletions(-) -- 2.42.0

Signed-off-by: K Shiva Kiran <shiva_kr@riseup.net> --- docs/formatnwfilter.rst | 31 +++++++++++++++++++++++++++++++ src/conf/schemas/nwfilter.rng | 9 +++++++++ 2 files changed, 40 insertions(+) diff --git a/docs/formatnwfilter.rst b/docs/formatnwfilter.rst index 434da5b1fd..766d7e85f5 100644 --- a/docs/formatnwfilter.rst +++ b/docs/formatnwfilter.rst @@ -419,6 +419,37 @@ better organized for more efficient processing by the firewall subsystem of the underlying host. Currently the system only supports the chains ``root, ipv4, ipv6, arp and rarp``. +General Metadata +~~~~~~~~~~~~~~~~ + +:: + + <filter name='clean-traffic' filter='arp'> + <uuid>6ef53069-ba34-94a0-d33d-17751b9b8cb1</uuid> + <title>A short description - title - of the filter</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> + </metadata> + ... + </filter> + +``title`` + The optional element ``title`` provides space for a short description of the + filter. The title should not contain any newlines. :since:`Since 9.8.0` . +``description`` + The content of the ``description`` element provides a human readable + description of the filter. This data is not used by libvirt in any + way, it can contain any information the user wants. :since:`Since 9.8.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 + application needs structure, they should have sub-elements to their namespace + element). :since:`Since 9.8.0` + + References to other filters ~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/src/conf/schemas/nwfilter.rng b/src/conf/schemas/nwfilter.rng index 262bd551e3..c56bbac732 100644 --- a/src/conf/schemas/nwfilter.rng +++ b/src/conf/schemas/nwfilter.rng @@ -14,6 +14,15 @@ <ref name="UUID"/> </element> </optional> + <optional> + <ref name="title"/> + </optional> + <optional> + <ref name="description"/> + </optional> + <optional> + <ref name="metadata"/> + </optional> <zeroOrMore> <choice> <element name="filterref"> -- 2.42.0

On 9/3/23 17:49, K Shiva Kiran wrote:
Signed-off-by: K Shiva Kiran <shiva_kr@riseup.net> --- docs/formatnwfilter.rst | 31 +++++++++++++++++++++++++++++++ src/conf/schemas/nwfilter.rng | 9 +++++++++ 2 files changed, 40 insertions(+)
This patch alone does not make any sense. Merge it into 02/16. Michal

Signed-off-by: K Shiva Kiran <shiva_kr@riseup.net> --- src/conf/nwfilter_conf.c | 30 ++++++++++++++++++++++++++++++ src/conf/nwfilter_conf.h | 5 +++++ 2 files changed, 35 insertions(+) diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index 35f6efbbe2..d03f78af4d 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -327,6 +327,10 @@ virNWFilterDefFree(virNWFilterDef *def) g_free(def->filterEntries); g_free(def->chainsuffix); + g_free(def->title); + g_free(def->description); + xmlFreeNode(def->metadata); + g_free(def); } @@ -2516,6 +2520,7 @@ virNWFilterDefParseXML(xmlXPathContextPtr ctxt) virNWFilterEntry *entry; int chain_priority; const char *name_prefix; + xmlNodePtr metadataNode = NULL; ret = g_new0(virNWFilterDef, 1); @@ -2582,6 +2587,23 @@ virNWFilterDefParseXML(xmlXPathContextPtr ctxt) } } + /* Extract short description of filter (title) */ + ret->title = virXPathString("string(./title[1])", ctxt); + if (ret->title && strchr(ret->title, '\n')) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Network filter title can't contain newlines")); + return NULL; + } + + /* Extract documentation if present */ + ret->description = virXPathString("string(./description[1])", ctxt); + + /* Extract custom metadata */ + if ((metadataNode = virXPathNode("./metadata[1]", ctxt)) != NULL) { + ret->metadata = xmlCopyNode(metadataNode, 1); + virXMLNodeSanitizeNamespaces(ret->metadata); + } + curr = curr->children; while (curr != NULL) { @@ -2873,6 +2895,14 @@ virNWFilterDefFormat(const virNWFilterDef *def) virUUIDFormat(def->uuid, uuid); virBufferAsprintf(&buf, "<uuid>%s</uuid>\n", uuid); + virBufferEscapeString(&buf, "<title>%s</title>\n", def->title); + + virBufferEscapeString(&buf, "<description>%s</description>\n", + def->description); + + if (virXMLFormatMetadata(&buf, def->metadata) < 0) + return NULL; + for (i = 0; i < def->nentries; i++) { if (virNWFilterEntryFormat(&buf, def->filterEntries[i]) < 0) return NULL; diff --git a/src/conf/nwfilter_conf.h b/src/conf/nwfilter_conf.h index 22c2fb51f0..34de6eab3d 100644 --- a/src/conf/nwfilter_conf.h +++ b/src/conf/nwfilter_conf.h @@ -517,6 +517,11 @@ struct _virNWFilterDef { size_t nentries; virNWFilterEntry **filterEntries; + + /* User-defined metadata */ + char* title; + char* description; + xmlNodePtr metadata; }; -- 2.42.0

On 9/3/23 17:49, K Shiva Kiran wrote:
Signed-off-by: K Shiva Kiran <shiva_kr@riseup.net> --- src/conf/nwfilter_conf.c | 30 ++++++++++++++++++++++++++++++ src/conf/nwfilter_conf.h | 5 +++++ 2 files changed, 35 insertions(+)
Here, I'd also expect a test case. Either introduce a new test case to tests/nwfilterxml2xmltest.c (preferred), or modify an existing one. This not only verifies the parsing/formatting works, but also that our schema is valid. Michal

Adds enum `virNWFilterMetadataType` to choose between `<title>`, `<description>` or `<metadata>`. Signed-off-by: K Shiva Kiran <shiva_kr@riseup.net> --- include/libvirt/libvirt-nwfilter.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/include/libvirt/libvirt-nwfilter.h b/include/libvirt/libvirt-nwfilter.h index 33b842b464..a9930a1e66 100644 --- a/include/libvirt/libvirt-nwfilter.h +++ b/include/libvirt/libvirt-nwfilter.h @@ -159,4 +159,18 @@ int virNWFilterBindingDelete(virNWFilterBindingPtr binding); int virNWFilterBindingRef(virNWFilterBindingPtr binding); int virNWFilterBindingFree(virNWFilterBindingPtr binding); +/** + * virNWFilterMetadataType: + * + * Since: 9.8.0 + */ +typedef enum { + VIR_NWFILTER_METADATA_DESCRIPTION = 0, /* Operate on <description> (Since: 9.8.0) */ + VIR_NWFILTER_METADATA_TITLE = 1, /* Operate on <title> (Since: 9.8.0) */ + VIR_NWFILTER_METADATA_ELEMENT = 2, /* Operate on <metadata> (Since: 9.8.0) */ + +# ifdef VIR_ENUM_SENTINELS + VIR_NWFILTER_METADATA_LAST /* (Since: 9.8.0) */ +# endif +} virNWFilterMetadataType; #endif /* LIBVIRT_NWFILTER_H */ -- 2.42.0

On 9/3/23 17:49, K Shiva Kiran wrote:
Adds enum `virNWFilterMetadataType` to choose between `<title>`, `<description>` or `<metadata>`.
Signed-off-by: K Shiva Kiran <shiva_kr@riseup.net> --- include/libvirt/libvirt-nwfilter.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/include/libvirt/libvirt-nwfilter.h b/include/libvirt/libvirt-nwfilter.h index 33b842b464..a9930a1e66 100644 --- a/include/libvirt/libvirt-nwfilter.h +++ b/include/libvirt/libvirt-nwfilter.h @@ -159,4 +159,18 @@ int virNWFilterBindingDelete(virNWFilterBindingPtr binding); int virNWFilterBindingRef(virNWFilterBindingPtr binding); int virNWFilterBindingFree(virNWFilterBindingPtr binding);
+/** + * virNWFilterMetadataType: + * + * Since: 9.8.0 + */ +typedef enum { + VIR_NWFILTER_METADATA_DESCRIPTION = 0, /* Operate on <description> (Since: 9.8.0) */ + VIR_NWFILTER_METADATA_TITLE = 1, /* Operate on <title> (Since: 9.8.0) */ + VIR_NWFILTER_METADATA_ELEMENT = 2, /* Operate on <metadata> (Since: 9.8.0) */ + +# ifdef VIR_ENUM_SENTINELS + VIR_NWFILTER_METADATA_LAST /* (Since: 9.8.0) */ +# endif +} virNWFilterMetadataType; #endif /* LIBVIRT_NWFILTER_H */
This patch alone makes no sense. Nobody will ever want to backport just this typedef. Merge it into 05/16. Michal

Signed-off-by: K Shiva Kiran <shiva_kr@riseup.net> --- include/libvirt/virterror.h | 1 + src/util/virerror.c | 3 +++ 2 files changed, 4 insertions(+) diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index 224eddc9e4..ffb47a3242 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -349,6 +349,7 @@ typedef enum { 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.7.0) */ + VIR_ERR_NO_NWFILTER_METADATA = 112, /* NWFilter metadata is not present (Since: 9.8.0) */ # ifdef VIR_ENUM_SENTINELS VIR_ERR_NUMBER_LAST /* (Since: 5.0.0) */ diff --git a/src/util/virerror.c b/src/util/virerror.c index 227a182417..a1d0d73e5d 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -1290,6 +1290,9 @@ static const virErrorMsgTuple virErrorMsgStrings[] = { [VIR_ERR_NO_NETWORK_METADATA] = { N_("metadata not found"), N_("metadata not found: %1$s") }, + [VIR_ERR_NO_NWFILTER_METADATA] = { + N_("metadata not found"), + N_("metadata not found: %1$s") }, }; G_STATIC_ASSERT(G_N_ELEMENTS(virErrorMsgStrings) == VIR_ERR_NUMBER_LAST); -- 2.42.0

On 9/3/23 17:49, K Shiva Kiran wrote:
Signed-off-by: K Shiva Kiran <shiva_kr@riseup.net> --- include/libvirt/virterror.h | 1 + src/util/virerror.c | 3 +++ 2 files changed, 4 insertions(+)
diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index 224eddc9e4..ffb47a3242 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -349,6 +349,7 @@ typedef enum { 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.7.0) */ + VIR_ERR_NO_NWFILTER_METADATA = 112, /* NWFilter metadata is not present (Since: 9.8.0) */
# ifdef VIR_ENUM_SENTINELS VIR_ERR_NUMBER_LAST /* (Since: 5.0.0) */ diff --git a/src/util/virerror.c b/src/util/virerror.c index 227a182417..a1d0d73e5d 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -1290,6 +1290,9 @@ static const virErrorMsgTuple virErrorMsgStrings[] = { [VIR_ERR_NO_NETWORK_METADATA] = { N_("metadata not found"), N_("metadata not found: %1$s") }, + [VIR_ERR_NO_NWFILTER_METADATA] = { + N_("metadata not found"), + N_("metadata not found: %1$s") }, };
G_STATIC_ASSERT(G_N_ELEMENTS(virErrorMsgStrings) == VIR_ERR_NUMBER_LAST);
Again, this patch alone makes no sense. Merge it into 05/16. Michal

Introduces `virNWFilterSetMetadata()` to modify user defined metadata fields, i.e `<title>`, `<description>` and `<metadata>` Signed-off-by: K Shiva Kiran <shiva_kr@riseup.net> --- include/libvirt/libvirt-nwfilter.h | 8 +++ src/driver-nwfilter.h | 9 ++++ src/libvirt-nwfilter.c | 87 ++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 ++ 4 files changed, 109 insertions(+) diff --git a/include/libvirt/libvirt-nwfilter.h b/include/libvirt/libvirt-nwfilter.h index a9930a1e66..ecab779665 100644 --- a/include/libvirt/libvirt-nwfilter.h +++ b/include/libvirt/libvirt-nwfilter.h @@ -173,4 +173,12 @@ typedef enum { VIR_NWFILTER_METADATA_LAST /* (Since: 9.8.0) */ # endif } virNWFilterMetadataType; + +int virNWFilterSetMetadata(virNWFilterPtr nwfilter, + int type, + const char *metadata, + const char *key, + const char *uri, + unsigned int flags); + #endif /* LIBVIRT_NWFILTER_H */ diff --git a/src/driver-nwfilter.h b/src/driver-nwfilter.h index 1ec591ece9..3426781fa0 100644 --- a/src/driver-nwfilter.h +++ b/src/driver-nwfilter.h @@ -86,6 +86,14 @@ typedef int typedef int (*virDrvNWFilterBindingFree)(virNWFilterBindingPtr binding); +typedef int +(*virDrvNWFilterSetMetadata)(virNWFilterPtr nwfilter, + int type, + const char *metadata, + const char *key, + const char *uri, + unsigned int flags); + typedef struct _virNWFilterDriver virNWFilterDriver; @@ -111,4 +119,5 @@ struct _virNWFilterDriver { virDrvNWFilterBindingCreateXML nwfilterBindingCreateXML; virDrvNWFilterBindingDelete nwfilterBindingDelete; virDrvNWFilterBindingGetXMLDesc nwfilterBindingGetXMLDesc; + virDrvNWFilterSetMetadata nwfilterSetMetadata; }; diff --git a/src/libvirt-nwfilter.c b/src/libvirt-nwfilter.c index ff82cc3b7f..0a0fbdd195 100644 --- a/src/libvirt-nwfilter.c +++ b/src/libvirt-nwfilter.c @@ -920,3 +920,90 @@ virNWFilterBindingRef(virNWFilterBindingPtr binding) virObjectRef(binding); return 0; } + + +/** + * virNWFilterSetMetadata: + * @nwfilter: a network filter object + * @type: type of metadata, from virNWFilterMetadataType + * @metadata: new metadata text + * @key: XML namespace key, or NULL + * @uri: XML namespace URI, or NULL + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Sets the appropriate nwfilter element given by @type to the + * value of @metadata. + * For options VIR_NWFILTER_METADATA_TITLE and VIR_NWFILTER_METADATA_DESCRIPTION + * @key and @uri are irrelevant and must be set to NULL. + * + * For type VIR_NWFILTER_METADATA_ELEMENT @metadata must be a 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 filter XML (passing the empty string leaves the element present). + * + * The resulting metadata will be present in virNWFilterGetXMLDesc(), + * as well as quick access through virNWFilterGetMetadata(). + * + * Returns 0 on success, -1 in case of failure. + * + * Since: 9.8.0 + */ +int +virNWFilterSetMetadata(virNWFilterPtr nwfilter, + int type, + const char *metadata, + const char *key, + const char *uri, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DEBUG("nwfilter=%p, type=%d, metadata='%s', key='%s', uri='%s', flags=0x%x", + nwfilter, type, NULLSTR(metadata), NULLSTR(key), NULLSTR(uri), + flags); + + virResetLastError(); + + virCheckNWFilterReturn(nwfilter, -1); + conn = nwfilter->conn; + + virCheckReadOnlyGoto(conn->flags, error); + + switch (type) { + case VIR_NWFILTER_METADATA_TITLE: + if (metadata && strchr(metadata, '\n')) { + virReportInvalidArg(metadata, "%s", + _("metadata title can't contain newlines")); + goto error; + } + G_GNUC_FALLTHROUGH; + case VIR_NWFILTER_METADATA_DESCRIPTION: + virCheckNullArgGoto(uri, error); + virCheckNullArgGoto(key, error); + break; + case VIR_NWFILTER_METADATA_ELEMENT: + virCheckNonNullArgGoto(uri, error); + if (metadata) + virCheckNonNullArgGoto(key, error); + break; + default: + /* For future expansion */ + break; + } + + if (conn->nwfilterDriver->nwfilterSetMetadata) { + int ret; + ret = conn->nwfilterDriver->nwfilterSetMetadata(nwfilter, type, metadata, key, uri, + flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(nwfilter->conn); + return -1; +} diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index bd1e916d2a..4e3b17b475 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -938,4 +938,9 @@ LIBVIRT_9.7.0 { virNetworkSetMetadata; } LIBVIRT_9.0.0; +LIBVIRT_9.8.0 { + global: + virNWFilterSetMetadata; +} LIBVIRT_9.7.0; + # .... define new API here using predicted next version number .... -- 2.42.0

Introduces `virNWFilterGetMetadata()` to retrieve user defined metadata fields, i.e `<title>`, `<description>` and `<metadata>` Signed-off-by: K Shiva Kiran <shiva_kr@riseup.net> --- include/libvirt/libvirt-nwfilter.h | 5 +++ src/driver-nwfilter.h | 6 +++ src/libvirt-nwfilter.c | 67 ++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + 4 files changed, 79 insertions(+) diff --git a/include/libvirt/libvirt-nwfilter.h b/include/libvirt/libvirt-nwfilter.h index ecab779665..2eed685300 100644 --- a/include/libvirt/libvirt-nwfilter.h +++ b/include/libvirt/libvirt-nwfilter.h @@ -181,4 +181,9 @@ int virNWFilterSetMetadata(virNWFilterPtr nwfilter, const char *uri, unsigned int flags); +char * virNWFilterGetMetadata(virNWFilterPtr nwfilter, + int type, + const char *uri, + unsigned int flags); + #endif /* LIBVIRT_NWFILTER_H */ diff --git a/src/driver-nwfilter.h b/src/driver-nwfilter.h index 3426781fa0..8d38dbd23c 100644 --- a/src/driver-nwfilter.h +++ b/src/driver-nwfilter.h @@ -94,6 +94,11 @@ typedef int const char *uri, unsigned int flags); +typedef char * +(*virDrvNWFilterGetMetadata)(virNWFilterPtr nwfilter, + int type, + const char *uri, + unsigned int flags); typedef struct _virNWFilterDriver virNWFilterDriver; @@ -120,4 +125,5 @@ struct _virNWFilterDriver { virDrvNWFilterBindingDelete nwfilterBindingDelete; virDrvNWFilterBindingGetXMLDesc nwfilterBindingGetXMLDesc; virDrvNWFilterSetMetadata nwfilterSetMetadata; + virDrvNWFilterGetMetadata nwfilterGetMetadata; }; diff --git a/src/libvirt-nwfilter.c b/src/libvirt-nwfilter.c index 0a0fbdd195..2340d87c4b 100644 --- a/src/libvirt-nwfilter.c +++ b/src/libvirt-nwfilter.c @@ -1007,3 +1007,70 @@ virNWFilterSetMetadata(virNWFilterPtr nwfilter, virDispatchError(nwfilter->conn); return -1; } + + +/** + * virNWFilterGetMetadata: + * @nwfilter: a network filter object + * @type: type of metadata, from virNWFilterMetadataType + * @uri: XML namespace identifier + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Retrieves the appropriate network filter element given by @type. + * If VIR_NWFILTER_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 filter XML is not present, the resulting + * error will be VIR_ERR_NO_NWFILTER_METADATA. This method forms + * a shortcut for seeing information from virNWFilterSetMetadata() + * without having to go through virNWFilterGetXMLDesc(). + * + * Returns the metadata string on success (caller must free), + * or NULL in case of failure. + * + * Since: 9.8.0 + */ +char * +virNWFilterGetMetadata(virNWFilterPtr nwfilter, + int type, + const char *uri, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DEBUG("nwfilter=%p, type=%d, uri='%s', flags=0x%x", + nwfilter, type, NULLSTR(uri), flags); + + virResetLastError(); + + virCheckNWFilterReturn(nwfilter, NULL); + + switch (type) { + case VIR_NWFILTER_METADATA_TITLE: + case VIR_NWFILTER_METADATA_DESCRIPTION: + virCheckNullArgGoto(uri, error); + break; + case VIR_NWFILTER_METADATA_ELEMENT: + virCheckNonNullArgGoto(uri, error); + break; + default: + /* For future expansion */ + break; + } + + conn = nwfilter->conn; + + if (conn->nwfilterDriver->nwfilterGetMetadata) { + char *ret; + if (!(ret = conn->nwfilterDriver->nwfilterGetMetadata(nwfilter, type, uri, flags))) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(nwfilter->conn); + return NULL; +} diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 4e3b17b475..81cd9afc93 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -940,6 +940,7 @@ LIBVIRT_9.7.0 { LIBVIRT_9.8.0 { global: + virNWFilterGetMetadata; virNWFilterSetMetadata; } LIBVIRT_9.7.0; -- 2.42.0

On 9/3/23 17:49, K Shiva Kiran wrote:
Introduces `virNWFilterGetMetadata()` to retrieve user defined metadata fields, i.e `<title>`, `<description>` and `<metadata>`
Signed-off-by: K Shiva Kiran <shiva_kr@riseup.net> --- include/libvirt/libvirt-nwfilter.h | 5 +++ src/driver-nwfilter.h | 6 +++ src/libvirt-nwfilter.c | 67 ++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + 4 files changed, 79 insertions(+)
Alright, introducing setter and getter in two different APIs is a bit of a stretch but I can live with it. Somebody might want just the setter and use nwfilter-dumpxml as getter. But then you implement RPC for both of these APIs in a single patch (07/16). Therefore, please merge also 05 and 06. Michal

Signed-off-by: K Shiva Kiran <shiva_kr@riseup.net> --- src/remote/remote_driver.c | 2 ++ src/remote/remote_protocol.x | 34 +++++++++++++++++++++++++++++++++- src/remote_protocol-structs | 19 +++++++++++++++++++ 3 files changed, 54 insertions(+), 1 deletion(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index b81c9bc611..8f140ef864 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8278,6 +8278,8 @@ static virNWFilterDriver nwfilter_driver = { .nwfilterBindingCreateXML = remoteNWFilterBindingCreateXML, /* 4.5.0 */ .nwfilterBindingDelete = remoteNWFilterBindingDelete, /* 4.5.0 */ .nwfilterBindingGetXMLDesc = remoteNWFilterBindingGetXMLDesc, /* 4.5.0 */ + .nwfilterSetMetadata = remoteNWFilterSetMetadata, /* 9.8.0 */ + .nwfilterGetMetadata = remoteNWFilterGetMetadata, /* 9.8.0 */ }; static virConnectDriver connect_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 7ff059e393..1a4a77a35f 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -1684,6 +1684,25 @@ struct remote_nwfilter_get_xml_desc_ret { remote_nonnull_string xml; }; +struct remote_nwfilter_set_metadata_args { + remote_nonnull_nwfilter nwfilter; + int type; + remote_string metadata; + remote_string key; + remote_string uri; + unsigned int flags; +}; + +struct remote_nwfilter_get_metadata_args { + remote_nonnull_nwfilter nwfilter; + int type; + remote_string uri; + unsigned int flags; +}; + +struct remote_nwfilter_get_metadata_ret { + remote_nonnull_string metadata; +}; /* Interface calls: */ @@ -7008,5 +7027,18 @@ enum remote_procedure { * @generate: both * @acl: network:read */ - REMOTE_PROC_NETWORK_GET_METADATA = 445 + REMOTE_PROC_NETWORK_GET_METADATA = 445, + + /** + * @generate: both + * @acl: nwfilter:write + * @acl: nwfilter:save + */ + REMOTE_PROC_NWFILTER_SET_METADATA = 446, + + /** + * @generate: both + * @acl: nwfilter:read + */ + REMOTE_PROC_NWFILTER_GET_METADATA = 447 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index c07e0af1e6..c3172e3f06 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -1220,6 +1220,23 @@ struct remote_nwfilter_get_xml_desc_args { struct remote_nwfilter_get_xml_desc_ret { remote_nonnull_string xml; }; +struct remote_nwfilter_set_metadata_args { + remote_nonnull_nwfilter nwfilter; + int type; + remote_string metadata; + remote_string key; + remote_string uri; + u_int flags; +}; +struct remote_nwfilter_get_metadata_args { + remote_nonnull_nwfilter nwfilter; + int type; + remote_string uri; + u_int flags; +}; +struct remote_nwfilter_get_metadata_ret { + remote_nonnull_string metadata; +}; struct remote_connect_num_of_interfaces_ret { int num; }; @@ -3736,4 +3753,6 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_FD_ASSOCIATE = 443, REMOTE_PROC_NETWORK_SET_METADATA = 444, REMOTE_PROC_NETWORK_GET_METADATA = 445, + REMOTE_PROC_NWFILTER_SET_METADATA = 446, + REMOTE_PROC_NWFILTER_GET_METADATA = 447, }; -- 2.42.0

On 9/3/23 17:49, K Shiva Kiran wrote:
Signed-off-by: K Shiva Kiran <shiva_kr@riseup.net> --- src/remote/remote_driver.c | 2 ++ src/remote/remote_protocol.x | 34 +++++++++++++++++++++++++++++++++- src/remote_protocol-structs | 19 +++++++++++++++++++ 3 files changed, 54 insertions(+), 1 deletion(-)
You'll need to rebase this as the master has moved. Michal

Signed-off-by: K Shiva Kiran <shiva_kr@riseup.net> --- tools/virsh-util.c | 25 +++++++++++++++++++++++++ tools/virsh-util.h | 9 +++++++++ 2 files changed, 34 insertions(+) diff --git a/tools/virsh-util.c b/tools/virsh-util.c index fb6327613a..c3af770c29 100644 --- a/tools/virsh-util.c +++ b/tools/virsh-util.c @@ -423,6 +423,31 @@ virshNetworkGetXMLFromNet(vshControl *ctl, } +int +virshNWFilterGetXMLFromNWFilter(vshControl *ctl, + virNWFilterPtr nwfilter, + unsigned int flags, + xmlDocPtr *xml, + xmlXPathContextPtr *ctxt) +{ + g_autofree char *desc = NULL; + + if (!(desc = virNWFilterGetXMLDesc(nwfilter, flags))) { + vshError(ctl, _("Failed to get nwfilter description xml")); + return -1; + } + + *xml = virXMLParseStringCtxt(desc, _("(nwfilter_definition)"), ctxt); + + if (!(*xml)) { + vshError(ctl, _("Failed to parse nwfilter description xml")); + return -1; + } + + return 0; +} + + int virshDomainGetXML(vshControl *ctl, const vshCmd *cmd, diff --git a/tools/virsh-util.h b/tools/virsh-util.h index 2386847072..4cad3d7eb9 100644 --- a/tools/virsh-util.h +++ b/tools/virsh-util.h @@ -152,6 +152,15 @@ virshNetworkGetXMLFromNet(vshControl *ctl, ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5) G_GNUC_WARN_UNUSED_RESULT; +int +virshNWFilterGetXMLFromNWFilter(vshControl *ctl, + virNWFilterPtr nwfilter, + unsigned int flags, + xmlDocPtr *xml, + xmlXPathContextPtr *ctxt) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4) + ATTRIBUTE_NONNULL(5) G_GNUC_WARN_UNUSED_RESULT; + int virshDomainGetXML(vshControl *ctl, const vshCmd *cmd, -- 2.42.0

On 9/3/23 17:49, K Shiva Kiran wrote:
Signed-off-by: K Shiva Kiran <shiva_kr@riseup.net> --- tools/virsh-util.c | 25 +++++++++++++++++++++++++ tools/virsh-util.h | 9 +++++++++ 2 files changed, 34 insertions(+)
diff --git a/tools/virsh-util.h b/tools/virsh-util.h index 2386847072..4cad3d7eb9 100644 --- a/tools/virsh-util.h +++ b/tools/virsh-util.h @@ -152,6 +152,15 @@ virshNetworkGetXMLFromNet(vshControl *ctl, ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5) G_GNUC_WARN_UNUSED_RESULT;
+int +virshNWFilterGetXMLFromNWFilter(vshControl *ctl, + virNWFilterPtr nwfilter, + unsigned int flags, + xmlDocPtr *xml, + xmlXPathContextPtr *ctxt) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4) + ATTRIBUTE_NONNULL(5) G_GNUC_WARN_UNUSED_RESULT; +
Looking into the future, there's just one user of this function. Please consider merging this patch into the next one. Michal

This command can be used to view/modify the `<title>` and `<description>` fields of the NWFilter object. Signed-off-by: K Shiva Kiran <shiva_kr@riseup.net> --- tools/virsh-nwfilter.c | 209 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 209 insertions(+) diff --git a/tools/virsh-nwfilter.c b/tools/virsh-nwfilter.c index 92b2b7b3bc..615d126def 100644 --- a/tools/virsh-nwfilter.c +++ b/tools/virsh-nwfilter.c @@ -26,6 +26,7 @@ #include "viralloc.h" #include "virfile.h" #include "vsh-table.h" +#include "virxml.h" virNWFilterPtr virshCommandOptNWFilterBy(vshControl *ctl, const vshCmd *cmd, @@ -345,6 +346,53 @@ virshNWFilterListCollect(vshControl *ctl, return list; } +/* extract description or title from nwfilter xml */ +static char * +virshGetNWFilterDescription(vshControl *ctl, virNWFilterPtr nwfilter, + bool title, unsigned int flags, + unsigned int queryflags) +{ + char *desc = NULL; + g_autoptr(xmlDoc) doc = NULL; + g_autoptr(xmlXPathContext) ctxt = NULL; + int type; + + if (title) + type = VIR_NWFILTER_METADATA_TITLE; + else + type = VIR_NWFILTER_METADATA_DESCRIPTION; + + if ((desc = virNWFilterGetMetadata(nwfilter, type, NULL, flags))) { + return desc; + } else { + int errCode = virGetLastErrorCode(); + + if (errCode == VIR_ERR_NO_NWFILTER_METADATA) { + desc = g_strdup(""); + vshResetLibvirtError(); + return desc; + } + + if (errCode != VIR_ERR_NO_SUPPORT) + return desc; + } + + /* fall back to xml */ + if (virshNWFilterGetXMLFromNWFilter(ctl, nwfilter, queryflags, &doc, &ctxt) < 0) + return NULL; + + if (title) + desc = virXPathString("string(./title[1])", ctxt); + else + desc = virXPathString("string(./description[1])", ctxt); + + if (!desc) + desc = g_strdup(""); + + return desc; +} + + /* * "nwfilter-list" command */ @@ -768,6 +816,161 @@ cmdNWFilterBindingList(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED) } +/* + * "nwfilter-desc" command + */ +static const vshCmdInfo info_nwfilter_desc[] = { + {.name = "help", + .data = N_("show or set network filter's description or title") + }, + {.name = "desc", + .data = N_("Allows setting or modifying the description or title of a network filter.") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_nwfilter_desc[] = { + {.name = "nwfilter", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("network filter name or uuid"), + .completer = virshNWFilterNameCompleter, + }, + {.name = "title", + .type = VSH_OT_BOOL, + .help = N_("modify/get the title instead of description") + }, + {.name = "edit", + .type = VSH_OT_BOOL, + .help = N_("open an editor to modify the description") + }, + {.name = "remove", + .type = VSH_OT_BOOL, + .help = N_("remove the element") + }, + {.name = "new-desc", + .type = VSH_OT_ARGV, + .help = N_("message") + }, + {.name = NULL} +}; + +static bool +cmdNWFilterDesc(vshControl *ctl, const vshCmd *cmd) +{ + g_autoptr(virshNWFilter) nwfilter = NULL; + bool title = vshCommandOptBool(cmd, "title"); + bool edit = vshCommandOptBool(cmd, "edit"); + bool remove = vshCommandOptBool(cmd, "remove"); + int type; + g_autofree char *descArg = NULL; + const vshCmdOpt *opt = NULL; + g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; + unsigned int flags = 0; + unsigned int queryflags = 0; + + VSH_EXCLUSIVE_OPTIONS("remove", "edit"); + + if (!(nwfilter = virshCommandOptNWFilter(ctl, cmd, NULL))) + return false; + + if (title) + type = VIR_NWFILTER_METADATA_TITLE; + else + type = VIR_NWFILTER_METADATA_DESCRIPTION; + + + while ((opt = vshCommandOptArgv(ctl, cmd, opt))) + virBufferAsprintf(&buf, "%s ", opt->data); + + virBufferTrim(&buf, " "); + + descArg = virBufferContentAndReset(&buf); + + if (remove) { + + if (descArg) { + vshPrintExtra(ctl, "unexpected data: \'%s\'", descArg); + return false; + } + + if (virNWFilterSetMetadata(nwfilter, type, "", NULL, NULL, flags) < 0) + goto error; + + vshPrintExtra(ctl, "%s removed successfully", title ? "Title" : "Description"); + + } else if (edit || descArg) { + + g_autofree char *descNWFilter = NULL; + g_autofree char *descNew = NULL; + + if (!(descNWFilter = virshGetNWFilterDescription(ctl, nwfilter, + title, flags, queryflags))) + return false; + + if (!descArg) + descArg = g_strdup(descNWFilter); + + if (edit) { + g_autoptr(vshTempFile) tmp = NULL; + g_autofree char *desc_edited = NULL; + char *tmpstr; + + /* Create and open a temporary file. */ + if (!(tmp = vshEditWriteToTempFile(ctl, descArg))) + return false; + + /* Start the editor. */ + if (vshEditFile(ctl, tmp) == -1) + return false; + + /* Read back the edited file. */ + if (!(desc_edited = vshEditReadBackFile(ctl, tmp))) + return false; + + /* strip a possible newline at the end */ + if (title && + (tmpstr = strrchr(desc_edited, '\n')) && + *(tmpstr+1) == '\0') + *tmpstr = '\0'; + + /* Check whether XML has changed */ + if (STREQ(descNWFilter, desc_edited)) { + vshPrintExtra(ctl, "Network filter %s has not changed", title ? "title" : "description"); + return true; + } + + descNew = g_steal_pointer(&desc_edited); + + } else { + descNew = g_steal_pointer(&descArg); + } + + if (virNWFilterSetMetadata(nwfilter, type, descNew, NULL, NULL, flags) < 0) + goto error; + + vshPrintExtra(ctl, "Network filter %s updated successfully", title ? "title" : "description"); + + } else { + g_autofree char *desc = virshGetNWFilterDescription(ctl, nwfilter, title, flags, queryflags); + if (!desc) + return false; + + if (strlen(desc) > 0) { + vshPrint(ctl, "%s", desc); + } else { + vshPrintExtra(ctl, _("No %1$s for network filter: %2$s"), title ? "title" : "description", virNWFilterGetName(nwfilter)); + } + } + + return true; + + error: + vshError(ctl, "Failed to set %s for network filter", title ? "title" : "description"); + return false; +} + + const vshCmdDef nwfilterCmds[] = { {.name = "nwfilter-define", .handler = cmdNWFilterDefine, @@ -823,5 +1026,11 @@ const vshCmdDef nwfilterCmds[] = { .info = info_nwfilter_binding_list, .flags = 0 }, + {.name = "nwfilter-desc", + .handler = cmdNWFilterDesc, + .opts = opts_nwfilter_desc, + .info = info_nwfilter_desc, + .flags = 0 + }, {.name = NULL} }; -- 2.42.0

On 9/3/23 17:49, K Shiva Kiran wrote:
This command can be used to view/modify the `<title>` and `<description>` fields of the NWFilter object.
Signed-off-by: K Shiva Kiran <shiva_kr@riseup.net> --- tools/virsh-nwfilter.c | 209 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 209 insertions(+)
The documentation you're introducing in 12/16 should be introduced here. Michal

With the new command `nwfilter-metadata`, users will be able to view and modify the `<metadata>` field of the Network Filter XML. Signed-off-by: K Shiva Kiran <shiva_kr@riseup.net> --- tools/virsh-nwfilter.c | 142 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 142 insertions(+) diff --git a/tools/virsh-nwfilter.c b/tools/virsh-nwfilter.c index 615d126def..801a52746e 100644 --- a/tools/virsh-nwfilter.c +++ b/tools/virsh-nwfilter.c @@ -971,6 +971,142 @@ cmdNWFilterDesc(vshControl *ctl, const vshCmd *cmd) } +/* + * "nwfilter-metadata" command + */ +static const vshCmdInfo info_nwfilter_metadata[] = { + {.name = "help", + .data = N_("show or set network filter's custom XML metadata") + }, + {.name = "desc", + .data = N_("Shows or modifies the XML metadata of a network filter.") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_nwfilter_metadata[] = { + {.name = "nwfilter", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("network filter name or uuid"), + .completer = virshNWFilterNameCompleter, + }, + {.name = "uri", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("URI of the namespace") + }, + {.name = "edit", + .type = VSH_OT_BOOL, + .help = N_("use an editor to change the metadata") + }, + {.name = "key", + .type = VSH_OT_STRING, + .help = N_("key to be used as a namespace identifier"), + }, + {.name = "set", + .type = VSH_OT_STRING, + .completer = virshCompleteEmpty, + .help = N_("new metadata to set"), + }, + {.name = "remove", + .type = VSH_OT_BOOL, + .help = N_("remove the metadata corresponding to an uri") + }, + {.name = NULL} +}; + +/* helper to add new metadata using the --edit option */ +static char * +virshNWFilterGetEditMetadata(vshControl *ctl G_GNUC_UNUSED, + virNWFilterPtr nwfilter, + const char *uri, + unsigned int flags) +{ + char *ret; + + if (!(ret = virNWFilterGetMetadata(nwfilter, VIR_NWFILTER_METADATA_ELEMENT, + uri, flags))) { + vshResetLibvirtError(); + ret = g_strdup("\n"); + } + + return ret; +} + +static bool +cmdNWFilterMetadata(vshControl *ctl, const vshCmd *cmd) +{ + g_autoptr(virshNWFilter) nwfilter = NULL; + bool edit = vshCommandOptBool(cmd, "edit"); + bool rem = vshCommandOptBool(cmd, "remove"); + const char *set = NULL; + const char *uri = NULL; + const char *key = NULL; + unsigned int flags = 0; + bool ret = false; + + VSH_EXCLUSIVE_OPTIONS("edit", "set"); + VSH_EXCLUSIVE_OPTIONS("remove", "set"); + VSH_EXCLUSIVE_OPTIONS("remove", "edit"); + + if (!(nwfilter = virshCommandOptNWFilter(ctl, cmd, NULL))) + return false; + + if (vshCommandOptStringReq(ctl, cmd, "uri", &uri) < 0 || + vshCommandOptStringReq(ctl, cmd, "key", &key) < 0 || + vshCommandOptStringReq(ctl, cmd, "set", &set) < 0) + return false; + + if ((set || edit) && !key) { + vshError(ctl, "%s", + _("namespace key is required when modifying metadata")); + return false; + } + + if (set || rem) { + if (virNWFilterSetMetadata(nwfilter, VIR_NWFILTER_METADATA_ELEMENT, + set, key, uri, flags)) + return false; + + if (rem) + vshPrintExtra(ctl, "%s\n", _("Metadata removed")); + else + vshPrintExtra(ctl, "%s\n", _("Metadata modified")); + } else if (edit) { +#define EDIT_GET_XML \ + virshNWFilterGetEditMetadata(ctl, nwfilter, uri, flags) +#define EDIT_NOT_CHANGED \ + do { \ + vshPrintExtra(ctl, "%s", _("Metadata not changed")); \ + ret = true; \ + goto edit_cleanup; \ + } while (0) + +#define EDIT_DEFINE \ + (virNWFilterSetMetadata(nwfilter, VIR_NWFILTER_METADATA_ELEMENT, doc_edited, \ + key, uri, flags) == 0) +#include "virsh-edit.c" + + vshPrintExtra(ctl, "%s\n", _("Metadata modified")); + } else { + g_autofree char *data = NULL; + + /* get */ + if (!(data = virNWFilterGetMetadata(nwfilter, VIR_NWFILTER_METADATA_ELEMENT, + uri, flags))) + return false; + + vshPrint(ctl, "%s\n", data); + } + + ret = true; + + cleanup: + return ret; +} + + const vshCmdDef nwfilterCmds[] = { {.name = "nwfilter-define", .handler = cmdNWFilterDefine, @@ -1032,5 +1168,11 @@ const vshCmdDef nwfilterCmds[] = { .info = info_nwfilter_desc, .flags = 0 }, + {.name = "nwfilter-metadata", + .handler = cmdNWFilterMetadata, + .opts = opts_nwfilter_metadata, + .info = info_nwfilter_metadata, + .flags = 0 + }, {.name = NULL} }; -- 2.42.0

Signed-off-by: K Shiva Kiran <shiva_kr@riseup.net> --- tools/virsh-nwfilter.c | 36 ++++++++++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/tools/virsh-nwfilter.c b/tools/virsh-nwfilter.c index 801a52746e..ca803865bf 100644 --- a/tools/virsh-nwfilter.c +++ b/tools/virsh-nwfilter.c @@ -407,6 +407,10 @@ static const vshCmdInfo info_nwfilter_list[] = { }; static const vshCmdOptDef opts_nwfilter_list[] = { + {.name = "title", + .type = VSH_OT_BOOL, + .help = N_("show network filter title") + }, {.name = NULL} }; @@ -416,13 +420,18 @@ cmdNWFilterList(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED) size_t i; char uuid[VIR_UUID_STRING_BUFLEN]; bool ret = false; + bool optTitle = vshCommandOptBool(cmd, "title"); struct virshNWFilterList *list = NULL; g_autoptr(vshTable) table = NULL; if (!(list = virshNWFilterListCollect(ctl, 0))) return false; - table = vshTableNew(_("UUID"), _("Name"), NULL); + if (optTitle) + table = vshTableNew(_("UUID"), _("Name"), _("Title"), NULL); + else + table = vshTableNew(_("UUID"), _("Name"), NULL); + if (!table) goto cleanup; @@ -430,11 +439,26 @@ cmdNWFilterList(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED) virNWFilterPtr nwfilter = list->filters[i]; virNWFilterGetUUIDString(nwfilter, uuid); - if (vshTableRowAppend(table, - uuid, - virNWFilterGetName(nwfilter), - NULL) < 0) - goto cleanup; + + if (optTitle) { + g_autofree char *title = NULL; + if (!(title = virshGetNWFilterDescription(ctl, nwfilter, true, 0, 0))) + goto cleanup; + + if (vshTableRowAppend(table, + uuid, + virNWFilterGetName(nwfilter), + title, + NULL) < 0) + goto cleanup; + + } else { + if (vshTableRowAppend(table, + uuid, + virNWFilterGetName(nwfilter), + NULL) < 0) + goto cleanup; + } } vshTablePrintToStdout(table, ctl); -- 2.42.0

Documents the commands `nwfilter-desc`, `nwfilter-metadata` and the new `--title` option of `nwfilter-list`. Signed-off-by: K Shiva Kiran <shiva_kr@riseup.net> --- docs/manpages/virsh.rst | 98 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 97 insertions(+), 1 deletion(-) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index 91e1d5de37..c81927d290 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -8082,10 +8082,53 @@ nwfilter-list :: - nwfilter-list + nwfilter-list [--title] List all of the available network filters. +If *--title* is specified, then the short network filter description +(title) is printed in an extra column. + + +nwfilter-desc +------------- + +**Syntax:** + +:: + + nwfilter-desc [--nwfilter] nwfilter-name + [--title] [--edit] [--remove] + [--new-desc new-value] + +Show or modify description and title of a network filter. + +These values are user fields that allow storing arbitrary textual data to +allow easy identification of network filters. +Title should be short, although it's not enforced. +(See also ``nwfilter-metadata`` that works with XML based network filter metadata.) + +- *--title* + + Specifies to operate on the title field instead of description. + +- *--edit* + + Opens an editor with the current title or description. + Modifications to the contents will be saved back. + Alternatively, the new contents can be provided via the *--new-desc* option. + +- *--remove* + + Removes the title or description field. + +- *--new-desc* + + Stores the provided title/description string. + +If neither of *--edit* or *--new-desc* are specified, the title or description +is displayed instead of being modified. + nwfilter-dumpxml ---------------- @@ -8133,6 +8176,59 @@ The editor used can be supplied by the ``$VISUAL`` or ``$EDITOR`` environment variables, and defaults to ``vi``. +nwfilter-metadata +----------------- + +**Syntax:** + +:: + + nwfilter-metadata [--nwfilter] nwfilter-name [--uri] uri + [--edit] [[--key] nskey] [--set new-metadata-xml] + [--remove] + +Show or modify custom XML metadata of a network filter. + +The metadata is a user defined XML that allows storing arbitrary XML data +in the network filter definition. +Multiple separate custom metadata pieces can be stored in the +network filter XML. The pieces are identified by a private XML namespace +provided via the *uri* argument. +(See also ``nwfilter-desc`` that works with textual metadata of +a network filter, such as title and description.) + +- *--uri* + + Specifies the URI for the private namespace. + +- *--edit* + + Opens an editor with the metadata identified by the *uri* argument. + Modifications to the contents will be saved back. + Alternatively, the new contents can be provided via the *--set* argument. + +- *--key* + + Specifies the namespace key to be used. + +- *--set* + + Validates and stores the provided metadata string. + + **Note:** When setting metadata via *--edit* or *--set* the *--key* argument + must be specified and is used to prefix the custom elements to bind them + to the private namespace. + +If neither of *--edit* and *--set* are specified the XML metadata corresponding +to the *uri* namespace is displayed instead of being modified. + +- *--remove* + + Specifies that the metadata element specified by the *--uri* argument should + be removed rather than updated. + + + NWFILTER BINDING COMMANDS ========================= -- 2.42.0

Signed-off-by: K Shiva Kiran <shiva_kr@riseup.net> --- src/conf/virnwfilterobj.c | 46 +++++++++++++++++++++++++++++++++++++++ src/conf/virnwfilterobj.h | 5 +++++ src/libvirt_private.syms | 1 + 3 files changed, 52 insertions(+) diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c index 6456add593..eab864fe2e 100644 --- a/src/conf/virnwfilterobj.c +++ b/src/conf/virnwfilterobj.c @@ -635,3 +635,49 @@ virNWFilterObjUnlock(virNWFilterObj *obj) { virMutexUnlock(&obj->lock); } + + +char * +virNWFilterObjGetMetadata(virNWFilterObj *obj, + int type, + const char *uri) +{ + virNWFilterDef *def; + char *ret = NULL; + + if (type >= VIR_NWFILTER_METADATA_LAST) { + virReportError(VIR_ERR_INVALID_ARG, + _("unknown metadata type '%1$d'"), type); + return NULL; + } + + if (!(def = virNWFilterObjGetDef(obj))) + return NULL; + + switch ((virNWFilterMetadataType) type) { + case VIR_NWFILTER_METADATA_DESCRIPTION: + ret = g_strdup(def->description); + break; + + case VIR_NWFILTER_METADATA_TITLE: + ret = g_strdup(def->title); + break; + + case VIR_NWFILTER_METADATA_ELEMENT: + if (!def->metadata) + break; + + if (virXMLExtractNamespaceXML(def->metadata, uri, &ret) < 0) + return NULL; + break; + + case VIR_NWFILTER_METADATA_LAST: + break; + } + + if (!ret) + virReportError(VIR_ERR_NO_NWFILTER_METADATA, "%s", + _("Requested metadata element is not present")); + + return ret; +} diff --git a/src/conf/virnwfilterobj.h b/src/conf/virnwfilterobj.h index b67dc017c5..dd4dd63cee 100644 --- a/src/conf/virnwfilterobj.h +++ b/src/conf/virnwfilterobj.h @@ -117,3 +117,8 @@ virNWFilterObjLock(virNWFilterObj *obj); void virNWFilterObjUnlock(virNWFilterObj *obj); + +char * +virNWFilterObjGetMetadata(virNWFilterObj *obj, + int type, + const char *uri); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1e3e407097..8c4983ae17 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1408,6 +1408,7 @@ virNWFilterBindingObjListRemove; # conf/virnwfilterobj.h virNWFilterObjGetDef; +virNWFilterObjGetMetadata; virNWFilterObjGetNewDef; virNWFilterObjListAssignDef; virNWFilterObjListExport; -- 2.42.0

Signed-off-by: K Shiva Kiran <shiva_kr@riseup.net> --- src/conf/virnwfilterobj.c | 102 ++++++++++++++++++++++++++++++++++++++ src/conf/virnwfilterobj.h | 8 +++ src/libvirt_private.syms | 1 + 3 files changed, 111 insertions(+) diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c index eab864fe2e..437df9631b 100644 --- a/src/conf/virnwfilterobj.c +++ b/src/conf/virnwfilterobj.c @@ -681,3 +681,105 @@ virNWFilterObjGetMetadata(virNWFilterObj *obj, return ret; } + + +static int +virNWFilterDefSetMetadata(virNWFilterDef *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_NWFILTER_METADATA_LAST) { + virReportError(VIR_ERR_INVALID_ARG, + _("unknown metadata type '%1$d'"), type); + return -1; + } + + switch ((virNWFilterMetadataType) type) { + case VIR_NWFILTER_METADATA_DESCRIPTION: + g_clear_pointer(&def->description, g_free); + + if (STRNEQ_NULLABLE(metadata, "")) + def->description = g_strdup(metadata); + break; + + case VIR_NWFILTER_METADATA_TITLE: + g_clear_pointer(&def->title, g_free); + + if (STRNEQ_NULLABLE(metadata, "")) + def->title = g_strdup(metadata); + break; + + case VIR_NWFILTER_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_NWFILTER_METADATA_LAST: + break; + } + + return 0; +} + + +int +virNWFilterObjSetMetadata(virNWFilterObj *obj, + int type, + const char *metadata, + const char *key, + const char *uri, + const char *configDir) +{ + virNWFilterDef *def; + + if (!(def = virNWFilterObjGetDef(obj))) + return -1; + + if (def) { + if (virNWFilterDefSetMetadata(def, type, metadata, key, uri) < 0) + return -1; + + if (virNWFilterSaveConfig(configDir, def) < 0) + return -1; + } + + return 0; +} diff --git a/src/conf/virnwfilterobj.h b/src/conf/virnwfilterobj.h index dd4dd63cee..4079c0ab3d 100644 --- a/src/conf/virnwfilterobj.h +++ b/src/conf/virnwfilterobj.h @@ -122,3 +122,11 @@ char * virNWFilterObjGetMetadata(virNWFilterObj *obj, int type, const char *uri); + +int +virNWFilterObjSetMetadata(virNWFilterObj *obj, + int type, + const char *metadata, + const char *key, + const char *uri, + const char *configDir); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8c4983ae17..628723f06d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1422,6 +1422,7 @@ virNWFilterObjListNew; virNWFilterObjListNumOfNWFilters; virNWFilterObjListRemove; virNWFilterObjLock; +virNWFilterObjSetMetadata; virNWFilterObjTestUnassignDef; virNWFilterObjUnlock; virNWFilterObjWantRemoved; -- 2.42.0

Signed-off-by: K Shiva Kiran <shiva_kr@riseup.net> --- src/nwfilter/nwfilter_driver.c | 61 ++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index 09719edd75..3a28532ca2 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -812,6 +812,65 @@ nwfilterBindingDelete(virNWFilterBindingPtr binding) } +static int +nwfilterSetMetadata(virNWFilterPtr nwfilter, + int type, + const char *metadata, + const char *key, + const char *uri, + unsigned int flags) +{ + virNWFilterObj *obj = NULL; + virNWFilterDef *def = NULL; + int ret = -1; + + virCheckFlags(0, -1); + + if (!(obj = nwfilterObjFromNWFilter(nwfilter->uuid))) + return -1; + + def = virNWFilterObjGetDef(obj); + + if (virNWFilterSetMetadataEnsureACL(nwfilter->conn, def) < 0) + goto cleanup; + + ret = virNWFilterObjSetMetadata(obj, type, metadata, key, uri, + driver->configDir); + + cleanup: + virNWFilterObjUnlock(obj); + return ret; +} + + +static char * +nwfilterGetMetadata(virNWFilterPtr nwfilter, + int type, + const char *uri, + unsigned int flags) +{ + virNWFilterObj *obj = NULL; + virNWFilterDef *def = NULL; + char *ret = NULL; + + virCheckFlags(0, NULL); + + if (!(obj = nwfilterObjFromNWFilter(nwfilter->uuid))) + return NULL; + + def = virNWFilterObjGetDef(obj); + + if (virNWFilterGetMetadataEnsureACL(nwfilter->conn, def) < 0) + goto cleanup; + + ret = virNWFilterObjGetMetadata(obj, type, uri); + + cleanup: + virNWFilterObjUnlock(obj); + return ret; +} + + static virNWFilterDriver nwfilterDriver = { .name = "nwfilter", .connectNumOfNWFilters = nwfilterConnectNumOfNWFilters, /* 0.8.0 */ @@ -828,6 +887,8 @@ static virNWFilterDriver nwfilterDriver = { .nwfilterBindingGetXMLDesc = nwfilterBindingGetXMLDesc, /* 4.5.0 */ .nwfilterBindingCreateXML = nwfilterBindingCreateXML, /* 4.5.0 */ .nwfilterBindingDelete = nwfilterBindingDelete, /* 4.5.0 */ + .nwfilterGetMetadata = nwfilterGetMetadata, /* 9.8.0 */ + .nwfilterSetMetadata = nwfilterSetMetadata, /* 9.8.0 */ }; -- 2.42.0

Signed-off-by: K Shiva Kiran <shiva_kr@riseup.net> --- NEWS.rst | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index a03ef20bc2..bd38efb43c 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -17,6 +17,24 @@ v9.8.0 (unreleased) * **New features** + * nwfilter: Support for user-defined metadata + + The network filter object adds three user-defined metadata fields: + 1) ``<title>``: Stores a short description (title) and can't contain newlines. + 2) ``<description>``: Can store any documentation the user wants. + 3) ``<metadata>``: Can store custom metadata in form of XML nodes. + + Two new APIs have been added to manage the above fields: + 1) ``virNWFilterGetMetadata()`` + 2) ``virNWFilterSetMetadata()`` + + virsh adds two new commands: + 1) ``nwfilter-desc``: To view or modify the title or description. + 2) ``nwfilter-metadata``: To view or modify ``<metadata>``. + + ``nwfilter-list`` adds a new option ``--title`` that prints the content of ``<title>`` + in an extra column within the default ``--table`` output. + * **Improvements** * **Bug fixes** -- 2.42.0

On 9/3/23 17:49, K Shiva Kiran wrote:
This patchset adds support for the following user defined metadata fields for network filters.
- title: A short description of the filter. - description: Any documentation that the user wants to store. - metadata: Other metadata in XML form.
K Shiva Kiran (16): xml: Add <title>, <description> and <metadata> to nwfilter xml schema conf: Add parser logic for nwfilter metadata fields nwfilter: Add enum to operate on user defined metadata nwfilter: Add error code and message for missing metadata nwfilter: Introduce public API to modify user metadata nwfilter: Introduce public API to retrieve user-defined metadata nwfilter: Implement RPC virsh: Add helper method to retrieve xml from NWFilter def virsh: Add new command `nwfilter-desc` virsh: Add new command `nwfilter-metadata` virsh: Add option --title for nwfilter-list docs: Document nwfilter metadata related commands virnwfilterobj: Add virNWFilterObjGetMetadata() virnwfilterobj: Add virNWFilterObjSetMetadata() nwfilter_driver: Add Driver implementation for metadata NEWS: Introduce user-defined metadata fields for NWFilter object
NEWS.rst | 18 ++ docs/formatnwfilter.rst | 31 +++ docs/manpages/virsh.rst | 98 +++++++- include/libvirt/libvirt-nwfilter.h | 27 ++ include/libvirt/virterror.h | 1 + src/conf/nwfilter_conf.c | 30 +++ src/conf/nwfilter_conf.h | 5 + src/conf/schemas/nwfilter.rng | 9 + src/conf/virnwfilterobj.c | 148 +++++++++++ src/conf/virnwfilterobj.h | 13 + src/driver-nwfilter.h | 15 ++ src/libvirt-nwfilter.c | 154 ++++++++++++ src/libvirt_private.syms | 2 + src/libvirt_public.syms | 6 + src/nwfilter/nwfilter_driver.c | 61 +++++ src/remote/remote_driver.c | 2 + src/remote/remote_protocol.x | 34 ++- src/remote_protocol-structs | 19 ++ src/util/virerror.c | 3 + tools/virsh-nwfilter.c | 387 ++++++++++++++++++++++++++++- tools/virsh-util.c | 25 ++ tools/virsh-util.h | 9 + 22 files changed, 1089 insertions(+), 8 deletions(-)
The code looks okay. But the split into patches is a bit awkward. Also, since I've merged your other series, there's a conflict in RPC definition file. Looking forward to v2. Michal
participants (2)
-
K Shiva Kiran
-
Michal Prívozník