[libvirt PATCH 0/4] introduce and use virXMLFormatElementDirect
Pavel Hrdina (4): util: virxml: introduce virXMLFormatElementDirect conf: use virXMLFormatElementDirect domain_conf: refactor virDomainLoaderDefFormatNvram util: virxml: unexport virXMLFormatElementInternal src/conf/domain_conf.c | 22 ++++++++++------------ src/conf/node_device_conf.c | 2 +- src/libvirt_private.syms | 2 +- src/util/virxml.c | 16 +++++++++++++++- src/util/virxml.h | 13 ++++++------- 5 files changed, 33 insertions(+), 22 deletions(-) -- 2.48.1
This can be used to format XML where the element has direct value instead of any subelement. For example: <maxMemory slots='16' unit='KiB'>1524288</maxMemory> Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virxml.c | 14 ++++++++++++++ src/util/virxml.h | 6 ++++++ 3 files changed, 21 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 30a9f806f0..e9ddef6453 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3779,6 +3779,7 @@ virXMLBufferCreate; virXMLCheckIllegalChars; virXMLExtractNamespaceXML; virXMLFormatElement; +virXMLFormatElementDirect; virXMLFormatElementEmpty; virXMLFormatElementInternal; virXMLFormatMetadata; diff --git a/src/util/virxml.c b/src/util/virxml.c index 670cace4ab..1295945472 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -1767,6 +1767,20 @@ virXMLFormatElementEmpty(virBuffer *buf, } +/** + * Same as virXMLFormatElement but the child is direct value without + * subelements. + */ +void +virXMLFormatElementDirect(virBuffer *buf, + const char *name, + virBuffer *attrBuf, + virBuffer *childBuf) +{ + virXMLFormatElementInternal(buf, name, attrBuf, childBuf, false, false); +} + + /** * virXMLFormatElement * @buf: the parent buffer where the element will be placed diff --git a/src/util/virxml.h b/src/util/virxml.h index 06ba324df0..4284a8ffce 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -351,6 +351,12 @@ virXMLFormatElementEmpty(virBuffer *buf, virBuffer *attrBuf, virBuffer *childBuf); +void +virXMLFormatElementDirect(virBuffer *buf, + const char *name, + virBuffer *attrBuf, + virBuffer *childBuf); + int virXMLFormatMetadata(virBuffer *buf, xmlNodePtr metadata); -- 2.48.1
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 4 ++-- src/conf/node_device_conf.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f42b7075ad..94f456a362 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -27053,7 +27053,7 @@ virDomainLoaderDefFormat(virBuffer *buf, virBufferEscapeString(&loaderChildBuf, "%s", loader->path); - virXMLFormatElementInternal(buf, "loader", &loaderAttrBuf, &loaderChildBuf, false, false); + virXMLFormatElementDirect(buf, "loader", &loaderAttrBuf, &loaderChildBuf); if (virDomainLoaderDefFormatNvram(buf, loader, xmlopt, flags) < 0) return -1; @@ -28304,7 +28304,7 @@ virDomainDefFormatInternalSetRootName(virDomainDef *def, virBufferAddLit(&attrBuf, " unit='KiB'"); virBufferAsprintf(&contentBuf, "%llu", def->mem.max_memory); - virXMLFormatElementInternal(buf, "maxMemory", &attrBuf, &contentBuf, false, false); + virXMLFormatElementDirect(buf, "maxMemory", &attrBuf, &contentBuf); } virBufferAddLit(buf, "<memory"); diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 88486a5f2d..9c7982c680 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -262,7 +262,7 @@ virNodeDeviceCapVPDFormatCustomField(virBuffer *buf, virBufferAsprintf(&attrBuf, " index='%c'", field->idx); virBufferEscapeString(&content, "%s", field->value); - virXMLFormatElementInternal(buf, fieldtype, &attrBuf, &content, false, false); + virXMLFormatElementDirect(buf, fieldtype, &attrBuf, &content); } static void -- 2.48.1
Use the new virXMLFormatDirect in order to remove usage of virXMLFormatInternal. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 94f456a362..0bcbf32bb4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -26980,10 +26980,8 @@ virDomainLoaderDefFormatNvram(virBuffer *buf, unsigned int flags) { g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; - g_auto(virBuffer) childBufDirect = VIR_BUFFER_INITIALIZER; - g_auto(virBuffer) childBufChild = VIR_BUFFER_INIT_CHILD(buf); - virBuffer *childBuf = &childBufDirect; - bool childNewline = false; + g_auto(virBuffer) directBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); virBufferEscapeString(&attrBuf, " template='%s'", loader->nvramTemplate); @@ -26996,14 +26994,11 @@ virDomainLoaderDefFormatNvram(virBuffer *buf, virStorageSource *src = loader->nvram; if (!loader->newStyleNVRAM) { - virBufferEscapeString(&childBufDirect, "%s", src->path); + virBufferEscapeString(&directBuf, "%s", src->path); } else { - childNewline = true; - childBuf = &childBufChild; - virBufferAsprintf(&attrBuf, " type='%s'", virStorageTypeToString(src->type)); - if (virDomainDiskSourceFormat(&childBufChild, src, "source", 0, + if (virDomainDiskSourceFormat(&childBuf, src, "source", 0, false, flags, false, false, xmlopt) < 0) return -1; } @@ -27014,7 +27009,10 @@ virDomainLoaderDefFormatNvram(virBuffer *buf, } } - virXMLFormatElementInternal(buf, "nvram", &attrBuf, childBuf, false, childNewline); + if (virBufferUse(&directBuf) > 0) + virXMLFormatElementDirect(buf, "nvram", &attrBuf, &directBuf); + else + virXMLFormatElement(buf, "nvram", &attrBuf, &childBuf); return 0; } -- 2.48.1
On a Thursday in 2025, Pavel Hrdina wrote:
Use the new virXMLFormatDirect in order to remove usage of virXMLFormatInternal.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 94f456a362..0bcbf32bb4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -27014,7 +27009,10 @@ virDomainLoaderDefFormatNvram(virBuffer *buf, } }
- virXMLFormatElementInternal(buf, "nvram", &attrBuf, childBuf, false, childNewline); + if (virBufferUse(&directBuf) > 0) + virXMLFormatElementDirect(buf, "nvram", &attrBuf, &directBuf); + else + virXMLFormatElement(buf, "nvram", &attrBuf, &childBuf);
Not a fan of these extra lines. '<' is not a valid character in the element's "direct" value. Instead of the "childNewline" boolean, maybe virXMLFormatElement can check the first character for this and decide based on that? Jano
return 0; } -- 2.48.1
On Fri, Mar 07, 2025 at 01:37:49PM +0100, Ján Tomko wrote:
On a Thursday in 2025, Pavel Hrdina wrote:
Use the new virXMLFormatDirect in order to remove usage of virXMLFormatInternal.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 94f456a362..0bcbf32bb4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -27014,7 +27009,10 @@ virDomainLoaderDefFormatNvram(virBuffer *buf, } }
- virXMLFormatElementInternal(buf, "nvram", &attrBuf, childBuf, false, childNewline); + if (virBufferUse(&directBuf) > 0) + virXMLFormatElementDirect(buf, "nvram", &attrBuf, &directBuf); + else + virXMLFormatElement(buf, "nvram", &attrBuf, &childBuf);
Not a fan of these extra lines.
'<' is not a valid character in the element's "direct" value. Instead of the "childNewline" boolean, maybe virXMLFormatElement can check the first character for this and decide based on that?
It would remove few lines but hide the logic inside virXMLFormatElement and make the code less readable IMHO. This makes it explicit what is happening and I don't know if there is any other part of our XML that would need this special case where the content can be both value or subelement. Pavel
Jano
return 0; } -- 2.48.1
It is no longer used anywhere else. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/libvirt_private.syms | 1 - src/util/virxml.c | 2 +- src/util/virxml.h | 7 ------- 3 files changed, 1 insertion(+), 9 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e9ddef6453..82b0974b94 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3781,7 +3781,6 @@ virXMLExtractNamespaceXML; virXMLFormatElement; virXMLFormatElementDirect; virXMLFormatElementEmpty; -virXMLFormatElementInternal; virXMLFormatMetadata; virXMLNewNode; virXMLNodeContentString; diff --git a/src/util/virxml.c b/src/util/virxml.c index 1295945472..06e698cdf8 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -1721,7 +1721,7 @@ virXMLValidatorFree(virXMLValidator *validator) * * Both passed buffers are always consumed and freed. */ -void +static void virXMLFormatElementInternal(virBuffer *buf, const char *name, virBuffer *attrBuf, diff --git a/src/util/virxml.h b/src/util/virxml.h index 4284a8ffce..7cc3f47913 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -332,13 +332,6 @@ void virXMLValidatorFree(virXMLValidator *validator); G_DEFINE_AUTOPTR_CLEANUP_FUNC(virXMLValidator, virXMLValidatorFree); -void -virXMLFormatElementInternal(virBuffer *buf, - const char *name, - virBuffer *attrBuf, - virBuffer *childBuf, - bool allowEmpty, - bool childNewline); void virXMLFormatElement(virBuffer *buf, const char *name, -- 2.48.1
On a Thursday in 2025, Pavel Hrdina wrote:
Pavel Hrdina (4): util: virxml: introduce virXMLFormatElementDirect conf: use virXMLFormatElementDirect domain_conf: refactor virDomainLoaderDefFormatNvram util: virxml: unexport virXMLFormatElementInternal
src/conf/domain_conf.c | 22 ++++++++++------------ src/conf/node_device_conf.c | 2 +- src/libvirt_private.syms | 2 +- src/util/virxml.c | 16 +++++++++++++++- src/util/virxml.h | 13 ++++++------- 5 files changed, 33 insertions(+), 22 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Pavel Hrdina