9:27 a.m.
Few cleanups that I've suggested in the review for adding remote store
of nvram.
Peter Krempa (3):
virXMLFormatElement: Introduce virXMLFormatElementInternal
virDomainLoaderDefFormat: Use modern XML formatting approach
vboxSetBootDeviceOrder: Remove whitespace alignment in VIR_DEBUG
statements
src/conf/domain_conf.c | 40 +++++++++++++--------------
src/libvirt_private.syms | 1 +
src/util/virxml.c | 58 +++++++++++++++++++++++++++++++---------
src/util/virxml.h | 7 +++++
src/vbox/vbox_common.c | 36 ++++++++++++-------------
5 files changed, 90 insertions(+), 52 deletions(-)
--
2.35.1
9:27 a.m.
New subject: [PATCH 1/3] virXMLFormatElement: Introduce virXMLFormatElementInternal
The new function aggregates the internal working of virXMLFormatElement
and virXMLFormatElementEmpty and also allows skipping the newline
after the opening tag to allow using this helper also in cases where we
don't format any child elements but directly a value.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/libvirt_private.syms | 1 +
src/util/virxml.c | 58 +++++++++++++++++++++++++++++++---------
src/util/virxml.h | 7 +++++
3 files changed, 54 insertions(+), 12 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 8a3e5f7f7c..3645a2f447 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -3634,6 +3634,7 @@ virXMLCheckIllegalChars;
virXMLExtractNamespaceXML;
virXMLFormatElement;
virXMLFormatElementEmpty;
+virXMLFormatElementInternal;
virXMLFormatMetadata;
virXMLNewNode;
virXMLNodeContentString;
diff --git a/src/util/virxml.c b/src/util/virxml.c
index 8ff59e7cda..0a447df676 100644
--- a/src/util/virxml.c
+++ b/src/util/virxml.c
@@ -1591,21 +1591,48 @@ virXMLValidatorFree(virXMLValidator *validator)
}
-/* same as virXMLFormatElement but outputs an empty element if @attrBuf and
- * @childBuf are both empty */
+/**
+ * virXMLFormatElementInternal
+ * @buf: the parent buffer where the element will be placed
+ * @name: the name of the element
+ * @attrBuf: buffer with attributes for element, may be NULL
+ * @childBuf: buffer with child elements, may be NULL
+ * @allowEmpty: Format empty element if @attrBuf and @childBuf are empty
+ * @childNewline: Add a newline after the opening element before formatting @childBuf
+ *
+ * Helper to format element where attributes or child elements
+ * are optional and may not be formatted. If both @attrBuf and
+ * @childBuf are NULL or are empty buffers the element is not
+ * formatted.
+ *
+ * Passing false for @childNewline allows to format elements where we directly
+ * output a value without subelements.
+ *
+ * Both passed buffers are always consumed and freed.
+ */
void
-virXMLFormatElementEmpty(virBuffer *buf,
- const char *name,
- virBuffer *attrBuf,
- virBuffer *childBuf)
+virXMLFormatElementInternal(virBuffer *buf,
+ const char *name,
+ virBuffer *attrBuf,
+ virBuffer *childBuf,
+ bool allowEmpty,
+ bool childNewline)
{
+ if (!allowEmpty) {
+ if ((!attrBuf || virBufferUse(attrBuf) == 0) &&
+ (!childBuf || virBufferUse(childBuf) == 0))
+ return;
+ }
+
virBufferAsprintf(buf, "<%s", name);
if (attrBuf && virBufferUse(attrBuf) > 0)
virBufferAddBuffer(buf, attrBuf);
if (childBuf && virBufferUse(childBuf) > 0) {
- virBufferAddLit(buf, ">\n");
+ virBufferAddLit(buf, ">");
+ if (childNewline)
+ virBufferAddLit(buf, "\n");
virBufferAddBuffer(buf, childBuf);
virBufferAsprintf(buf, "</%s>\n", name);
} else {
@@ -1616,6 +1643,17 @@ virXMLFormatElementEmpty(virBuffer *buf,
virBufferFreeAndReset(childBuf);
}
+/* same as virXMLFormatElement but outputs an empty element if @attrBuf and
+ * @childBuf are both empty */
+void
+virXMLFormatElementEmpty(virBuffer *buf,
+ const char *name,
+ virBuffer *attrBuf,
+ virBuffer *childBuf)
+{
+ virXMLFormatElementInternal(buf, name, attrBuf, childBuf, true, true);
+}
+
/**
* virXMLFormatElement
@@ -1637,11 +1675,7 @@ virXMLFormatElement(virBuffer *buf,
virBuffer *attrBuf,
virBuffer *childBuf)
{
- if ((!attrBuf || virBufferUse(attrBuf) == 0) &&
- (!childBuf || virBufferUse(childBuf) == 0))
- return;
-
- virXMLFormatElementEmpty(buf, name, attrBuf, childBuf);
+ virXMLFormatElementInternal(buf, name, attrBuf, childBuf, false, true);
}
diff --git a/src/util/virxml.h b/src/util/virxml.h
index c39eae6282..a13853983a 100644
--- a/src/util/virxml.h
+++ b/src/util/virxml.h
@@ -315,6 +315,13 @@ 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.35.1
9:27 a.m.
New subject: [PATCH 2/3] virDomainLoaderDefFormat: Use modern XML formatting approach
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/conf/domain_conf.c | 40 ++++++++++++++++++----------------------
1 file changed, 18 insertions(+), 22 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ba8dd7d7d0..98da3f2460 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -26911,34 +26911,30 @@ static void
virDomainLoaderDefFormat(virBuffer *buf,
virDomainLoaderDef *loader)
{
- const char *readonly = virTristateBoolTypeToString(loader->readonly);
- const char *secure = virTristateBoolTypeToString(loader->secure);
- const char *type = virDomainLoaderTypeToString(loader->type);
+ g_auto(virBuffer) loaderAttrBuf = VIR_BUFFER_INITIALIZER;
+ g_auto(virBuffer) loaderChildBuf = VIR_BUFFER_INITIALIZER;
+ g_auto(virBuffer) nvramAttrBuf = VIR_BUFFER_INITIALIZER;
+ g_auto(virBuffer) nvramChildBuf = VIR_BUFFER_INITIALIZER;
- virBufferAddLit(buf, "<loader");
+ if (loader->readonly != VIR_TRISTATE_BOOL_ABSENT)
+ virBufferAsprintf(&loaderAttrBuf, " readonly='%s'",
+ virTristateBoolTypeToString(loader->readonly));
- if (loader->readonly)
- virBufferAsprintf(buf, " readonly='%s'", readonly);
-
- if (loader->secure)
- virBufferAsprintf(buf, " secure='%s'", secure);
+ if (loader->secure != VIR_TRISTATE_BOOL_ABSENT)
+ virBufferAsprintf(&loaderAttrBuf, " secure='%s'",
+ virTristateBoolTypeToString(loader->secure));
if (loader->type != VIR_DOMAIN_LOADER_TYPE_NONE)
- virBufferAsprintf(buf, " type='%s'", type);
+ virBufferAsprintf(&loaderAttrBuf, " type='%s'",
+ virDomainLoaderTypeToString(loader->type));
- if (loader->path)
- virBufferEscapeString(buf, ">%s</loader>\n",
loader->path);
- else
- virBufferAddLit(buf, "/>\n");
+ virBufferEscapeString(&loaderChildBuf, "%s", loader->path);
- if (loader->nvram || loader->nvramTemplate) {
- virBufferAddLit(buf, "<nvram");
- virBufferEscapeString(buf, " template='%s'",
loader->nvramTemplate);
- if (loader->nvram)
- virBufferEscapeString(buf, ">%s</nvram>\n",
loader->nvram);
- else
- virBufferAddLit(buf, "/>\n");
- }
+ virXMLFormatElementInternal(buf, "loader", &loaderAttrBuf,
&loaderChildBuf, false, false);
+
+ virBufferEscapeString(&nvramAttrBuf, " template='%s'",
loader->nvramTemplate);
+ virBufferEscapeString(&nvramChildBuf, "%s", loader->nvram);
+ virXMLFormatElementInternal(buf, "nvram", &nvramAttrBuf,
&nvramChildBuf, false, false);
}
static void
--
2.35.1
9:27 a.m.
New subject: [PATCH 3/3] vboxSetBootDeviceOrder: Remove whitespace alignment in VIR_DEBUG statements
Don't try to align the output, it's not future-proof and it's for
debugging only.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/vbox/vbox_common.c | 36 ++++++++++++++++++------------------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index acd18494d3..734371a201 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -980,27 +980,27 @@ vboxSetBootDeviceOrder(virDomainDef *def, struct _vboxDriver *data,
PRUint32 maxBootPosition = 0;
size_t i = 0;
- VIR_DEBUG("def->os.type %s",
virDomainOSTypeToString(def->os.type));
- VIR_DEBUG("def->os.arch %s",
virArchToString(def->os.arch));
- VIR_DEBUG("def->os.machine %s", def->os.machine);
- VIR_DEBUG("def->os.nBootDevs %zu", def->os.nBootDevs);
- VIR_DEBUG("def->os.bootDevs[0] %d", def->os.bootDevs[0]);
- VIR_DEBUG("def->os.bootDevs[1] %d", def->os.bootDevs[1]);
- VIR_DEBUG("def->os.bootDevs[2] %d", def->os.bootDevs[2]);
- VIR_DEBUG("def->os.bootDevs[3] %d", def->os.bootDevs[3]);
- VIR_DEBUG("def->os.init %s", def->os.init);
- VIR_DEBUG("def->os.kernel %s", def->os.kernel);
- VIR_DEBUG("def->os.initrd %s", def->os.initrd);
- VIR_DEBUG("def->os.cmdline %s", def->os.cmdline);
- VIR_DEBUG("def->os.root %s", def->os.root);
+ VIR_DEBUG("def->os.type %s", virDomainOSTypeToString(def->os.type));
+ VIR_DEBUG("def->os.arch %s", virArchToString(def->os.arch));
+ VIR_DEBUG("def->os.machine %s", def->os.machine);
+ VIR_DEBUG("def->os.nBootDevs %zu", def->os.nBootDevs);
+ VIR_DEBUG("def->os.bootDevs[0] %d", def->os.bootDevs[0]);
+ VIR_DEBUG("def->os.bootDevs[1] %d", def->os.bootDevs[1]);
+ VIR_DEBUG("def->os.bootDevs[2] %d", def->os.bootDevs[2]);
+ VIR_DEBUG("def->os.bootDevs[3] %d", def->os.bootDevs[3]);
+ VIR_DEBUG("def->os.init %s", def->os.init);
+ VIR_DEBUG("def->os.kernel %s", def->os.kernel);
+ VIR_DEBUG("def->os.initrd %s", def->os.initrd);
+ VIR_DEBUG("def->os.cmdline %s", def->os.cmdline);
+ VIR_DEBUG("def->os.root %s", def->os.root);
if (def->os.loader) {
- VIR_DEBUG("def->os.loader->path %s",
def->os.loader->path);
+ VIR_DEBUG("def->os.loader->path %s",
def->os.loader->path);
VIR_DEBUG("def->os.loader->readonly %d",
def->os.loader->readonly);
- VIR_DEBUG("def->os.loader->type %d",
def->os.loader->type);
- VIR_DEBUG("def->os.loader->nvram %s",
def->os.loader->nvram);
+ VIR_DEBUG("def->os.loader->type %d",
def->os.loader->type);
+ VIR_DEBUG("def->os.loader->nvram %s",
def->os.loader->nvram);
}
- VIR_DEBUG("def->os.bootloader %s", def->os.bootloader);
- VIR_DEBUG("def->os.bootloaderArgs %s", def->os.bootloaderArgs);
+ VIR_DEBUG("def->os.bootloader %s", def->os.bootloader);
+ VIR_DEBUG("def->os.bootloaderArgs %s", def->os.bootloaderArgs);
gVBoxAPI.UIVirtualBox.GetSystemProperties(data->vboxObj, &systemProperties);
if (systemProperties) {
--
2.35.1
4:47 a.m.
On a Monday in 2022, Peter Krempa wrote:
Few cleanups that I've suggested in the review for adding remote
store
of nvram.
Peter Krempa (3):
virXMLFormatElement: Introduce virXMLFormatElementInternal
virDomainLoaderDefFormat: Use modern XML formatting approach
vboxSetBootDeviceOrder: Remove whitespace alignment in VIR_DEBUG
statements
src/conf/domain_conf.c | 40 +++++++++++++--------------
src/libvirt_private.syms | 1 +
src/util/virxml.c | 58 +++++++++++++++++++++++++++++++---------
src/util/virxml.h | 7 +++++
src/vbox/vbox_common.c | 36 ++++++++++++-------------
5 files changed, 90 insertions(+), 52 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
997
days inactive
999
days old
4 comments
2 participants
participants (2)
-
Ján Tomko -
Peter Krempa