[libvirt PATCHv1 0/4] do not format empty metadata element
https://issues.redhat.com/browse/RHEL-27172 Ján Tomko (4): conf: networkobj: fix indentation conf: obj: remove extra empty line conf: metadata: ignore empty metadata element conf: metadata: remove metadata node if all metadata is removed src/conf/domain_conf.c | 8 +++++-- src/conf/network_conf.c | 3 ++- src/conf/virnetworkobj.c | 48 +++++++++++++++++++++------------------- 3 files changed, 33 insertions(+), 26 deletions(-) -- 2.48.1
'Network' has one more letter than 'Domain' where these helpers were copied from. Shift the unaligned lines by one. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/virnetworkobj.c | 44 ++++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c index 5abc295506..ded4c58b47 100644 --- a/src/conf/virnetworkobj.c +++ b/src/conf/virnetworkobj.c @@ -1911,9 +1911,9 @@ virNetworkObjUpdateModificationImpact(virNetworkObj *obj, */ static int virNetworkObjGetDefs(virNetworkObj *net, - unsigned int flags, - virNetworkDef **liveDef, - virNetworkDef **persDef) + unsigned int flags, + virNetworkDef **liveDef, + virNetworkDef **persDef) { if (liveDef) *liveDef = NULL; @@ -1957,8 +1957,8 @@ virNetworkObjGetDefs(virNetworkObj *net, */ static virNetworkDef * virNetworkObjGetOneDefState(virNetworkObj *net, - unsigned int flags, - bool *live) + unsigned int flags, + bool *live) { if (flags & VIR_NETWORK_UPDATE_AFFECT_LIVE && flags & VIR_NETWORK_UPDATE_AFFECT_CONFIG) { @@ -1996,7 +1996,7 @@ virNetworkObjGetOneDefState(virNetworkObj *net, */ static virNetworkDef * virNetworkObjGetOneDef(virNetworkObj *net, - unsigned int flags) + unsigned int flags) { return virNetworkObjGetOneDefState(net, flags, NULL); } @@ -2004,9 +2004,9 @@ virNetworkObjGetOneDef(virNetworkObj *net, char * virNetworkObjGetMetadata(virNetworkObj *net, - int type, - const char *uri, - unsigned int flags) + int type, + const char *uri, + unsigned int flags) { virNetworkDef *def; char *ret = NULL; @@ -2054,10 +2054,10 @@ virNetworkObjGetMetadata(virNetworkObj *net, static int virNetworkDefSetMetadata(virNetworkDef *def, - int type, - const char *metadata, - const char *key, - const char *uri) + int type, + const char *metadata, + const char *key, + const char *uri) { g_autoptr(xmlDoc) doc = NULL; xmlNodePtr old; @@ -2131,14 +2131,14 @@ virNetworkDefSetMetadata(virNetworkDef *def, 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) + 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; @@ -2159,7 +2159,7 @@ virNetworkObjSetMetadata(virNetworkObj *net, if (persistentDef) { if (virNetworkDefSetMetadata(persistentDef, type, metadata, key, - uri) < 0) + uri) < 0) return -1; if (virNetworkSaveConfig(configDir, persistentDef, xmlopt) < 0) -- 2.48.1
Originally present in virDomainDefSetMetadata it got copied to virNetworkDefSetMetadata too. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/domain_conf.c | 1 - src/conf/virnetworkobj.c | 1 - 2 files changed, 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 49555efc56..878d1f68c1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -30231,7 +30231,6 @@ virDomainDefSetMetadata(virDomainDef *def, case VIR_DOMAIN_METADATA_ELEMENT: if (metadata) { - /* parse and modify the xml from the user */ if (!(doc = virXMLParseStringCtxt(metadata, _("(metadata_xml)"), NULL))) return -1; diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c index ded4c58b47..7d4566bdf1 100644 --- a/src/conf/virnetworkobj.c +++ b/src/conf/virnetworkobj.c @@ -2086,7 +2086,6 @@ virNetworkDefSetMetadata(virNetworkDef *def, case VIR_NETWORK_METADATA_ELEMENT: if (metadata) { - /* parse and modify the xml from the user */ if (!(doc = virXMLParseStringCtxt(metadata, _("(metadata_xml)"), NULL))) return -1; -- 2.48.1
Do not copy the <metadata> node to domain/network definition if its empty. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/domain_conf.c | 4 +++- src/conf/network_conf.c | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 878d1f68c1..6ca604a60b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -19853,8 +19853,10 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, return NULL; /* Extract custom metadata */ - if ((node = virXPathNode("./metadata[1]", ctxt)) != NULL) + if ((node = virXPathNode("./metadata[1]", ctxt)) != NULL && + xmlFirstElementChild(node)) { def->metadata = xmlCopyNode(node, 1); + } /* we have to make a copy of all of the callback pointers here since * we won't have the virCaps structure available during free diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 316a84502d..8cd26de72f 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -1898,7 +1898,8 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt, } /* Extract custom metadata */ - if ((metadataNode = virXPathNode("./metadata[1]", ctxt)) != NULL) { + if ((metadataNode = virXPathNode("./metadata[1]", ctxt)) != NULL && + xmlFirstElementChild(metadataNode)) { def->metadata = xmlCopyNode(metadataNode, 1); virXMLNodeSanitizeNamespaces(def->metadata); } -- 2.48.1
When removing the last child element from a network or domain metadata, free the metadata node itself as well, to prevent displaying an empty metadata element. https://issues.redhat.com/browse/RHEL-27172 Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/domain_conf.c | 3 +++ src/conf/virnetworkobj.c | 3 +++ 2 files changed, 6 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6ca604a60b..50763d9514 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -30264,6 +30264,9 @@ virDomainDefSetMetadata(virDomainDef *def, return -1; } new = NULL; + } else if (!xmlFirstElementChild(def->metadata)) { + xmlFreeNode(def->metadata); + def->metadata = NULL; } break; diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c index 7d4566bdf1..c908c61e67 100644 --- a/src/conf/virnetworkobj.c +++ b/src/conf/virnetworkobj.c @@ -2117,6 +2117,9 @@ virNetworkDefSetMetadata(virNetworkDef *def, return -1; } new = NULL; + } else if (!xmlFirstElementChild(def->metadata)) { + xmlFreeNode(def->metadata); + def->metadata = NULL; } break; -- 2.48.1
On 2/20/25 22:59, Ján Tomko wrote:
When removing the last child element from a network or domain metadata, free the metadata node itself as well, to prevent displaying an empty metadata element.
https://issues.redhat.com/browse/RHEL-27172
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/domain_conf.c | 3 +++ src/conf/virnetworkobj.c | 3 +++ 2 files changed, 6 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6ca604a60b..50763d9514 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -30264,6 +30264,9 @@ virDomainDefSetMetadata(virDomainDef *def, return -1; } new = NULL; + } else if (!xmlFirstElementChild(def->metadata)) { + xmlFreeNode(def->metadata); + def->metadata = NULL; } break;
diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c index 7d4566bdf1..c908c61e67 100644 --- a/src/conf/virnetworkobj.c +++ b/src/conf/virnetworkobj.c @@ -2117,6 +2117,9 @@ virNetworkDefSetMetadata(virNetworkDef *def, return -1; } new = NULL; + } else if (!xmlFirstElementChild(def->metadata)) { + xmlFreeNode(def->metadata); + def->metadata = NULL; } break;
Alternatively: g_clear_pointer(&def->metadata, xmlFreeNode); Michal
On 2/20/25 22:59, Ján Tomko wrote:
https://issues.redhat.com/browse/RHEL-27172
Ján Tomko (4): conf: networkobj: fix indentation conf: obj: remove extra empty line conf: metadata: ignore empty metadata element conf: metadata: remove metadata node if all metadata is removed
src/conf/domain_conf.c | 8 +++++-- src/conf/network_conf.c | 3 ++- src/conf/virnetworkobj.c | 48 +++++++++++++++++++++------------------- 3 files changed, 33 insertions(+), 26 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Ján Tomko -
Michal Prívozník