11:58 a.m.
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
11:58 a.m.
New subject: [libvirt PATCH 1/4] util: virxml: introduce virXMLFormatElementDirect
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(a)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
11:58 a.m.
New subject: [libvirt PATCH 2/4] conf: use virXMLFormatElementDirect
Signed-off-by: Pavel Hrdina <phrdina(a)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
11:58 a.m.
New subject: [libvirt PATCH 3/4] domain_conf: refactor virDomainLoaderDefFormatNvram
Use the new virXMLFormatDirect in order to remove usage of
virXMLFormatInternal.
Signed-off-by: Pavel Hrdina <phrdina(a)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
6:37 a.m.
New subject: [libvirt PATCH 3/4] domain_conf: refactor
virDomainLoaderDefFormatNvram
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(a)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
6:46 a.m.
New subject: [libvirt PATCH 3/4] domain_conf: refactor
virDomainLoaderDefFormatNvram
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(a)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
>
11:58 a.m.
New subject: [libvirt PATCH 4/4] util: virxml: unexport virXMLFormatElementInternal
It is no longer used anywhere else.
Signed-off-by: Pavel Hrdina <phrdina(a)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
7:12 a.m.
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(a)redhat.com>
Jano
6
days inactive
7
days old
7 comments
2 participants
participants (2)
-
Ján Tomko -
Pavel Hrdina