Richard W.M. Jones
2018-Oct-04 12:03 UTC
[Libguestfs] [PATCH v2 0/4] common/utils: Move libxml2 writer macros to a common header file.
v1 was here: https://www.redhat.com/archives/libguestfs/2018-October/msg00047.html However it was broken in a few ways. First of all the documentation was broken because "/**" enhanced comments were not permitted on macros. This is fixed in the new 1/4 patch. Secondly we didn't use single_element() everywhere possible, which is fixed in the new 4/4 patch. Lastly I've enhanced the documentation so it's clear what precise XML is generated. Rich.
Richard W.M. Jones
2018-Oct-04 12:03 UTC
[Libguestfs] [PATCH v2 1/4] docs: Allow enhanced comments for macros (ie. #define).
---
docs/make-internal-documentation.pl | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/docs/make-internal-documentation.pl
b/docs/make-internal-documentation.pl
index a6673c48f..e08adad70 100755
--- a/docs/make-internal-documentation.pl
+++ b/docs/make-internal-documentation.pl
@@ -245,13 +245,19 @@ foreach $dir (@dirs) {
}
}
else {
- # First line tells us if this is a struct
- # or function.
+ # First line tells us if this is a struct,
+ # define or function.
if (/^struct ([\w_]+) \{$/) {
$thing = "Structure";
$name = $1;
$end = "};";
}
+ elsif (/^#define ([\w_]+)/) {
+ $thing = "Definition";
+ $name = $1;
+ $found_end = 1;
+ last;
+ }
else {
$thing = "Function";
$end = "{";
@@ -276,6 +282,10 @@ foreach $dir (@dirs) {
push @defn, "};"
}
+ if ($thing eq "Definition") {
+ @defn = ( "#define $name" )
+ }
+
# Print the definition, followed by the comment.
print OUTPUT "=head4 $thing
C<$input:$name>\n\n";
print OUTPUT " ", join ("\n ", @defn),
"\n";
--
2.19.0.rc0
Richard W.M. Jones
2018-Oct-04 12:03 UTC
[Libguestfs] [PATCH v2 2/4] common/utils: Move libxml2 writer macros to a common header file.
In some places when generating XML output in C code we use some clever
macros:
start_element ("memory") {
attribute ("unit", "MiB");
string ("%d", g->memsize);
} end_element ();
This commit which is mostly refactoring moves the repeated definitions
of these macros into a common header file.
I also took this opportunity to change / clean up the macros:
- The macros are now documented properly.
- comment() and empty_element() macros are now available everywhere.
- Error handling has been made generic.
- Added do..while(0) around some of the macros to make them safe to
use in all contexts.
- Both string() and attribute() take a format string (previously
there were string_format() and attribute_format() variants).
The last point causes the most churn since we must change the callers
to avoid format string security bugs.
---
common/utils/Makefile.am | 1 +
common/utils/libxml2-writer-macros.h | 143 +++++++++++++++++++++++
docs/C_SOURCE_FILES | 1 +
lib/launch-libvirt.c | 169 +++++++++------------------
p2v/physical-xml.c | 99 +++++-----------
5 files changed, 225 insertions(+), 188 deletions(-)
diff --git a/common/utils/Makefile.am b/common/utils/Makefile.am
index 1fa98f992..4ff9efcbd 100644
--- a/common/utils/Makefile.am
+++ b/common/utils/Makefile.am
@@ -28,6 +28,7 @@ libutils_la_SOURCES = \
libxml2-cleanups.c \
libxml2-utils.c \
libxml2-utils.h \
+ libxml2-writer-macros.h \
utils.c
libutils_la_CPPFLAGS = \
-DGUESTFS_WARN_DEPRECATED=1 \
diff --git a/common/utils/libxml2-writer-macros.h
b/common/utils/libxml2-writer-macros.h
new file mode 100644
index 000000000..b5705645a
--- /dev/null
+++ b/common/utils/libxml2-writer-macros.h
@@ -0,0 +1,143 @@
+/* libguestfs
+ * Copyright (C) 2009-2018 Red Hat Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/**
+ * These macros make it easier to write XML. To use them correctly
+ * you must be aware of these assumptions:
+ *
+ * =over 4
+ *
+ * =item *
+ *
+ * The C<xmlTextWriterPtr> is called C<xo>. It is used implicitly
+ * by all the macros.
+ *
+ * =item *
+ *
+ * On failure, a function called C<xml_error> is called which you must
+ * define (usually as a macro). You must use C<CLEANUP_*> macros in
+ * your functions if you want correct cleanup of local variables along
+ * the error path.
+ *
+ * =item *
+ *
+ * All the "bad" casting is hidden inside the macros.
+ *
+ * =back
+ */
+
+#ifndef GUESTFS_LIBXML2_WRITER_MACROS_H_
+#define GUESTFS_LIBXML2_WRITER_MACROS_H_
+
+#include <stdarg.h>
+
+/**
+ * To define an XML element use:
+ *
+ * start_element ("name") {
+ * ...
+ * } end_element ();
+ *
+ * which produces C<<< <name>...</name> >>>
+ */
+#define start_element(element) \
+ if (xmlTextWriterStartElement (xo, BAD_CAST (element)) == -1) { \
+ xml_error ("xmlTextWriterStartElement"); \
+ } \
+ do
+
+#define end_element() \
+ while (0); \
+ do { \
+ if (xmlTextWriterEndElement (xo) == -1) { \
+ xml_error ("xmlTextWriterEndElement"); \
+ } \
+ } while (0)
+
+/**
+ * To define an empty element:
+ *
+ * empty_element ("name");
+ *
+ * which produces C<<< <name/> >>>
+ */
+#define empty_element(element) \
+ do { start_element ((element)) {} end_element (); } while (0)
+
+/**
+ * To define an XML element with attributes, use:
+ *
+ * start_element ("name") {
+ * attribute ("foo", "bar");
+ * attribute ("count", "%d", count);
+ * ...
+ * } end_element ();
+ *
+ * which produces C<<< <name foo="bar"
count="123">...</name> >>>
+ */
+#define attribute(key,fs,...) \
+ do { \
+ if (xmlTextWriterWriteFormatAttribute (xo, BAD_CAST (key), \
+ fs, ##__VA_ARGS__) == -1) { \
+ xml_error ("xmlTextWriterWriteFormatAttribute");
\
+ } \
+ } while (0)
+
+/**
+ * C<attribute_ns (prefix, key, namespace_uri, value)> defines a
+ * namespaced attribute.
+ */
+#define attribute_ns(prefix,key,namespace_uri,value) \
+ do { \
+ if (xmlTextWriterWriteAttributeNS (xo, BAD_CAST (prefix), \
+ BAD_CAST (key), \
+ BAD_CAST (namespace_uri), \
+ BAD_CAST (value)) == -1) { \
+ xml_error ("xmlTextWriterWriteAttribute");
\
+ } \
+ } while (0)
+
+/**
+ * To define a verbatim string, use:
+ *
+ * string ("hello");
+ *
+ * or:
+ *
+ * string ("%s, world", greeting);
+ */
+#define string(fs,...) \
+ do { \
+ if (xmlTextWriterWriteFormatString (xo, fs, ##__VA_ARGS__) == -1) { \
+ xml_error ("xmlTextWriterWriteFormatString");
\
+ } \
+ } while (0)
+
+/**
+ * To define a comment in the XML, use:
+ *
+ * comment ("number of items = %d", nr_items);
+ */
+#define comment(fs,...) \
+ do { \
+ if (xmlTextWriterWriteFormatComment (xo, fs, ##__VA_ARGS__) == -1) { \
+ xml_error ("xmlTextWriterWriteFormatComment");
\
+ } \
+ } while (0)
+
+#endif /* GUESTFS_LIBXML2_WRITER_MACROS_H_ */
diff --git a/docs/C_SOURCE_FILES b/docs/C_SOURCE_FILES
index 1c262973c..7f1c60b30 100644
--- a/docs/C_SOURCE_FILES
+++ b/docs/C_SOURCE_FILES
@@ -62,6 +62,7 @@ common/utils/cleanups.h
common/utils/gnulib-cleanups.c
common/utils/guestfs-utils.h
common/utils/libxml2-cleanups.c
+common/utils/libxml2-writer-macros.h
common/utils/utils.c
common/visit/visit.c
common/visit/visit.h
diff --git a/lib/launch-libvirt.c b/lib/launch-libvirt.c
index 1a074fd6c..7b4b9f752 100644
--- a/lib/launch-libvirt.c
+++ b/lib/launch-libvirt.c
@@ -52,6 +52,16 @@
#include "guestfs-internal.h"
#include "guestfs_protocol.h"
+#include "libxml2-writer-macros.h"
+
+/* This macro is used by the macros in "libxml2-writer-macros.h"
+ * when an error occurs.
+ */
+#define xml_error(fn) \
+ perrorf (g, _("%s:%d: error constructing libvirt XML near call to
\"%s\""), \
+ __FILE__, __LINE__, (fn)); \
+ return -1;
+
/* Fixes for Mac OS X */
#ifndef SOCK_CLOEXEC
# define SOCK_CLOEXEC O_CLOEXEC
@@ -909,79 +919,6 @@ static int construct_libvirt_xml_disk_source_hosts
(guestfs_h *g, xmlTextWriterP
static int construct_libvirt_xml_disk_source_seclabel (guestfs_h *g, const
struct backend_libvirt_data *data, xmlTextWriterPtr xo);
static int construct_libvirt_xml_appliance (guestfs_h *g, const struct
libvirt_xml_params *params, xmlTextWriterPtr xo);
-/* These macros make it easier to write XML, but they also make a lot
- * of assumptions:
- *
- * - The xmlTextWriterPtr is called 'xo'. It is used implicitly.
- *
- * - The guestfs handle is called 'g'. It is used implicitly for
errors.
- *
- * - It is safe to 'return -1' on failure. This is OK provided you
- * always use CLEANUP_* macros.
- *
- * - All the "bad" casting is hidden inside the macros.
- */
-
-/* <element */
-#define start_element(element) \
- if (xmlTextWriterStartElement (xo, BAD_CAST (element)) == -1) { \
- xml_error ("xmlTextWriterStartElement"); \
- return -1; \
- } \
- do
-
-/* finish current </element> */
-#define end_element() \
- while (0); \
- do { \
- if (xmlTextWriterEndElement (xo) == -1) { \
- xml_error ("xmlTextWriterEndElement"); \
- return -1; \
- } \
- } while (0)
-
-/* key=value attribute of the current element. */
-#define attribute(key,value) \
- if (xmlTextWriterWriteAttribute (xo, BAD_CAST (key), BAD_CAST (value)) ==
-1){ \
- xml_error ("xmlTextWriterWriteAttribute");
\
- return -1; \
- }
-
-/* key=value, but value is a printf-style format string. */
-#define attribute_format(key,fs,...) \
- if (xmlTextWriterWriteFormatAttribute (xo, BAD_CAST (key), \
- fs, ##__VA_ARGS__) == -1) { \
- xml_error ("xmlTextWriterWriteFormatAttribute");
\
- return -1; \
- }
-
-/* attribute with namespace. */
-#define attribute_ns(prefix,key,namespace_uri,value) \
- if (xmlTextWriterWriteAttributeNS (xo, BAD_CAST (prefix), \
- BAD_CAST (key), BAD_CAST (namespace_uri),
\
- BAD_CAST (value)) == -1) { \
- xml_error ("xmlTextWriterWriteAttribute");
\
- return -1; \
- }
-
-/* A string, eg. within an element. */
-#define string(str) \
- if (xmlTextWriterWriteString (xo, BAD_CAST (str)) == -1) { \
- xml_error ("xmlTextWriterWriteString"); \
- return -1; \
- }
-
-/* A string, using printf-style formatting. */
-#define string_format(fs,...) \
- if (xmlTextWriterWriteFormatString (xo, fs, ##__VA_ARGS__) == -1) { \
- xml_error ("xmlTextWriterWriteFormatString");
\
- return -1; \
- }
-
-#define xml_error(fn) \
- perrorf (g, _("%s:%d: error constructing libvirt XML near call to
\"%s\""), \
- __FILE__, __LINE__, (fn));
-
static xmlChar *
construct_libvirt_xml (guestfs_h *g, const struct libvirt_xml_params *params)
{
@@ -1040,7 +977,7 @@ construct_libvirt_xml_domain (guestfs_h *g,
xmlTextWriterPtr xo)
{
start_element ("domain") {
- attribute ("type", params->data->is_kvm ? "kvm" :
"qemu");
+ attribute ("type", "%s", params->data->is_kvm ?
"kvm" : "qemu");
attribute_ns ("xmlns", "qemu", NULL,
"http://libvirt.org/schemas/domain/qemu/1.0");
@@ -1070,7 +1007,7 @@ construct_libvirt_xml_name (guestfs_h *g,
xmlTextWriterPtr xo)
{
start_element ("name") {
- string (params->data->name);
+ string ("%s", params->data->name);
} end_element ();
return 0;
@@ -1086,12 +1023,12 @@ construct_libvirt_xml_cpu (guestfs_h *g,
start_element ("memory") {
attribute ("unit", "MiB");
- string_format ("%d", g->memsize);
+ string ("%d", g->memsize);
} end_element ();
start_element ("currentMemory") {
attribute ("unit", "MiB");
- string_format ("%d", g->memsize);
+ string ("%d", g->memsize);
} end_element ();
cpu_model = guestfs_int_get_cpu_model (params->data->is_kvm);
@@ -1105,14 +1042,14 @@ construct_libvirt_xml_cpu (guestfs_h *g,
}
else {
start_element ("model") {
- string (cpu_model);
+ string ("%s", cpu_model);
} end_element ();
}
} end_element ();
}
start_element ("vcpu") {
- string_format ("%d", g->smp);
+ string ("%d", g->smp);
} end_element ();
start_element ("clock") {
@@ -1162,7 +1099,7 @@ construct_libvirt_xml_boot (guestfs_h *g,
start_element ("os") {
start_element ("type") {
#ifdef MACHINE_TYPE
- attribute ("machine", MACHINE_TYPE);
+ attribute ("machine", "%s", MACHINE_TYPE);
#endif
string ("hvm");
} end_element ();
@@ -1171,12 +1108,12 @@ construct_libvirt_xml_boot (guestfs_h *g,
start_element ("loader") {
attribute ("readonly", "yes");
attribute ("type", "pflash");
- string (params->data->uefi_code);
+ string ("%s", params->data->uefi_code);
} end_element ();
if (params->data->uefi_vars) {
start_element ("nvram") {
- string (params->data->uefi_vars);
+ string ("%s", params->data->uefi_vars);
} end_element ();
}
}
@@ -1192,15 +1129,15 @@ construct_libvirt_xml_boot (guestfs_h *g,
#endif
start_element ("kernel") {
- string (params->kernel);
+ string ("%s", params->kernel);
} end_element ();
start_element ("initrd") {
- string (params->initrd);
+ string ("%s", params->initrd);
} end_element ();
start_element ("cmdline") {
- string (cmdline);
+ string ("%s", cmdline);
} end_element ();
#if defined(__i386__) || defined(__x86_64__)
@@ -1252,10 +1189,10 @@ construct_libvirt_xml_seclabel (guestfs_h *g,
attribute ("model", "selinux");
attribute ("relabel", "yes");
start_element ("label") {
- string (params->data->selinux_label);
+ string ("%s", params->data->selinux_label);
} end_element ();
start_element ("imagelabel") {
- string (params->data->selinux_imagelabel);
+ string ("%s", params->data->selinux_imagelabel);
} end_element ();
} end_element ();
}
@@ -1292,7 +1229,7 @@ construct_libvirt_xml_devices (guestfs_h *g,
*/
if (is_custom_hv (g)) {
start_element ("emulator") {
- string (g->hv);
+ string ("%s", g->hv);
} end_element ();
}
#if defined(__arm__)
@@ -1301,7 +1238,7 @@ construct_libvirt_xml_devices (guestfs_h *g,
*/
else {
start_element ("emulator") {
- string (QEMU);
+ string ("%s", QEMU);
} end_element ();
}
#endif
@@ -1345,7 +1282,7 @@ construct_libvirt_xml_devices (guestfs_h *g,
attribute ("type", "unix");
start_element ("source") {
attribute ("mode", "connect");
- attribute ("path", params->data->console_path);
+ attribute ("path", "%s",
params->data->console_path);
} end_element ();
start_element ("target") {
attribute ("port", "0");
@@ -1359,7 +1296,7 @@ construct_libvirt_xml_devices (guestfs_h *g,
attribute ("type", "unix");
start_element ("source") {
attribute ("mode", "connect");
- attribute ("path", params->data->console_path);
+ attribute ("path", "%s",
params->data->console_path);
} end_element ();
start_element ("target") {
attribute ("type", "sclp");
@@ -1373,7 +1310,7 @@ construct_libvirt_xml_devices (guestfs_h *g,
attribute ("type", "unix");
start_element ("source") {
attribute ("mode", "connect");
- attribute ("path", params->data->guestfsd_path);
+ attribute ("path", "%s",
params->data->guestfsd_path);
} end_element ();
start_element ("target") {
attribute ("type", "virtio");
@@ -1386,7 +1323,7 @@ construct_libvirt_xml_devices (guestfs_h *g,
start_element ("interface") {
attribute ("type", "bridge");
start_element ("source") {
- attribute ("bridge", params->data->network_bridge);
+ attribute ("bridge", "%s",
params->data->network_bridge);
} end_element ();
start_element ("model") {
attribute ("type", "virtio");
@@ -1440,7 +1377,7 @@ construct_libvirt_xml_disk (guestfs_h *g,
attribute ("type", "file");
start_element ("source") {
- attribute ("file", drv->overlay);
+ attribute ("file", "%s", drv->overlay);
if (construct_libvirt_xml_disk_source_seclabel (g, data, xo) == -1)
return -1;
} end_element ();
@@ -1480,7 +1417,7 @@ construct_libvirt_xml_disk (guestfs_h *g,
attribute ("type", "file");
start_element ("source") {
- attribute ("file", path);
+ attribute ("file", "%s", path);
if (construct_libvirt_xml_disk_source_seclabel (g, data, xo) == -1)
return -1;
} end_element ();
@@ -1489,7 +1426,7 @@ construct_libvirt_xml_disk (guestfs_h *g,
attribute ("type", "block");
start_element ("source") {
- attribute ("dev", drv->src.u.path);
+ attribute ("dev", "%s", drv->src.u.path);
if (construct_libvirt_xml_disk_source_seclabel (g, data, xo) == -1)
return -1;
} end_element ();
@@ -1521,9 +1458,9 @@ construct_libvirt_xml_disk (guestfs_h *g,
attribute ("type", "network");
start_element ("source") {
- attribute ("protocol", protocol_str);
+ attribute ("protocol", "%s", protocol_str);
if (STRNEQ (drv->src.u.exportname, ""))
- attribute ("name", drv->src.u.exportname);
+ attribute ("name", "%s",
drv->src.u.exportname);
if (construct_libvirt_xml_disk_source_hosts (g, xo,
&drv->src) == -1)
return -1;
@@ -1533,14 +1470,14 @@ construct_libvirt_xml_disk (guestfs_h *g,
if (drv->src.username != NULL) {
start_element ("auth") {
- attribute ("username", drv->src.username);
+ attribute ("username", "%s",
drv->src.username);
r = find_secret (g, data, drv, &type, &uuid);
if (r == -1)
return -1;
if (r == 1) {
start_element ("secret") {
- attribute ("type", type);
- attribute ("uuid", uuid);
+ attribute ("type", "%s", type);
+ attribute ("uuid", "%s", uuid);
} end_element ();
}
} end_element ();
@@ -1576,7 +1513,7 @@ construct_libvirt_xml_disk (guestfs_h *g,
if (drv->disk_label) {
start_element ("serial") {
- string (drv->disk_label);
+ string ("%s", drv->disk_label);
} end_element ();
}
@@ -1597,7 +1534,7 @@ construct_libvirt_xml_disk_target (guestfs_h *g,
xmlTextWriterPtr xo,
guestfs_int_drive_name (drv_index, &drive_name[2]);
start_element ("target") {
- attribute ("dev", drive_name);
+ attribute ("dev", "%s", drive_name);
attribute ("bus", "scsi");
} end_element ();
@@ -1642,8 +1579,8 @@ construct_libvirt_xml_disk_driver_qemu (guestfs_h *g,
start_element ("driver") {
attribute ("name", "qemu");
- attribute ("type", format);
- attribute ("cache", cachemode);
+ attribute ("type", "%s", format);
+ attribute ("cache", "%s", cachemode);
if (discard_unmap)
attribute ("discard", "unmap");
if (copyonread)
@@ -1679,7 +1616,7 @@ construct_libvirt_xml_disk_address (guestfs_h *g,
xmlTextWriterPtr xo,
* virtio-scsi driver, this is the ".id" field). This is a
number
* in the range 0-255.
*/
- attribute_format ("target", "%zu", drv_index);
+ attribute ("target", "%zu", drv_index);
/* libvirt "unit" == qemu "lun". This is the SCSI
logical unit
* number, which is a number in the range 0..16383.
@@ -1702,9 +1639,9 @@ construct_libvirt_xml_disk_source_hosts (guestfs_h *g,
switch (src->servers[i].transport) {
case drive_transport_none:
case drive_transport_tcp: {
- attribute ("name", src->servers[i].u.hostname);
+ attribute ("name", "%s",
src->servers[i].u.hostname);
if (src->servers[i].port > 0)
- attribute_format ("port", "%d",
src->servers[i].port);
+ attribute ("port", "%d",
src->servers[i].port);
break;
}
@@ -1722,7 +1659,7 @@ construct_libvirt_xml_disk_source_hosts (guestfs_h *g,
}
attribute ("transport", "unix");
- attribute ("socket", abs_socket);
+ attribute ("socket", "%s", abs_socket);
break;
}
}
@@ -1758,11 +1695,11 @@ construct_libvirt_xml_appliance (guestfs_h *g,
attribute ("device", "disk");
start_element ("source") {
- attribute ("file", params->appliance_overlay);
+ attribute ("file", "%s",
params->appliance_overlay);
} end_element ();
start_element ("target") {
- attribute ("dev", ¶ms->appliance_dev[5]);
+ attribute ("dev", "%s",
¶ms->appliance_dev[5]);
attribute ("bus", "scsi");
} end_element ();
@@ -1798,18 +1735,18 @@ construct_libvirt_xml_qemu_cmdline (guestfs_h *g,
start_element ("qemu:env") {
attribute ("name", "TMPDIR");
- attribute ("value", tmpdir);
+ attribute ("value", "%s", tmpdir);
} end_element ();
/* The qemu command line arguments requested by the caller. */
for (hp = g->hv_params; hp; hp = hp->next) {
start_element ("qemu:arg") {
- attribute ("value", hp->hv_param);
+ attribute ("value", "%s", hp->hv_param);
} end_element ();
if (hp->hv_value) {
start_element ("qemu:arg") {
- attribute ("value", hp->hv_value);
+ attribute ("value", "%s", hp->hv_value);
} end_element ();
}
}
@@ -1828,8 +1765,8 @@ construct_libvirt_xml_secret (guestfs_h *g,
attribute ("ephemeral", "yes");
attribute ("private", "yes");
start_element ("description") {
- string_format ("guestfs secret associated with %s %s",
- data->name, drv->src.u.path);
+ string ("guestfs secret associated with %s %s",
+ data->name, drv->src.u.path);
} end_element ();
} end_element ();
diff --git a/p2v/physical-xml.c b/p2v/physical-xml.c
index f65a514d5..6ac8cc991 100644
--- a/p2v/physical-xml.c
+++ b/p2v/physical-xml.c
@@ -39,65 +39,20 @@
#include "getprogname.h"
+#include "libxml2-writer-macros.h"
+
#include "p2v.h"
+/* This macro is used by the macros in "libxml2-writer-macros.h"
+ * when an error occurs.
+ */
+#define xml_error(fn) \
+ error (EXIT_FAILURE, errno, \
+ "%s:%d: error constructing XML near call to
\"%s\"", \
+ __FILE__, __LINE__, (fn));
+
static const char *map_interface_to_network (struct config *, const char
*interface);
-/* Macros "inspired" by lib/launch-libvirt.c */
-/* <element */
-#define start_element(element) \
- if (xmlTextWriterStartElement (xo, BAD_CAST (element)) == -1) \
- error (EXIT_FAILURE, errno, "xmlTextWriterStartElement"); \
- do
-
-/* finish current </element> */
-#define end_element() \
- while (0); \
- do { \
- if (xmlTextWriterEndElement (xo) == -1) \
- error (EXIT_FAILURE, errno, "xmlTextWriterEndElement"); \
- } while (0)
-
-/* <element/> */
-#define empty_element(element) \
- do { start_element(element) {} end_element (); } while (0)
-
-/* key=value attribute of the current element. */
-#define attribute(key,value) \
- do { \
- if (xmlTextWriterWriteAttribute (xo, BAD_CAST (key), BAD_CAST (value)) ==
-1) \
- error (EXIT_FAILURE, errno, "xmlTextWriterWriteAttribute");
\
- } while (0)
-
-/* key=value, but value is a printf-style format string. */
-#define attribute_format(key,fs,...) \
- do { \
- if (xmlTextWriterWriteFormatAttribute (xo, BAD_CAST (key), \
- fs, ##__VA_ARGS__) == -1) \
- error (EXIT_FAILURE, errno,
"xmlTextWriterWriteFormatAttribute"); \
- } while (0)
-
-/* A string, eg. within an element. */
-#define string(str) \
- do { \
- if (xmlTextWriterWriteString (xo, BAD_CAST (str)) == -1) \
- error (EXIT_FAILURE, errno, "xmlTextWriterWriteString"); \
- } while (0)
-
-/* A string, using printf-style formatting. */
-#define string_format(fs,...) \
- do { \
- if (xmlTextWriterWriteFormatString (xo, fs, ##__VA_ARGS__) == -1) \
- error (EXIT_FAILURE, errno, "xmlTextWriterWriteFormatString");
\
- } while (0)
-
-/* An XML comment. */
-#define comment(fs,...) \
- do { \
- if (xmlTextWriterWriteFormatComment (xo, fs, ##__VA_ARGS__) == -1) \
- error (EXIT_FAILURE, errno, "xmlTextWriterWriteFormatComment");
\
- } while (0)
-
/**
* Write the libvirt XML for this physical machine.
*
@@ -143,21 +98,21 @@ generate_physical_xml (struct config *config, struct
data_conn *data_conns,
attribute ("type", "physical");
start_element ("name") {
- string (config->guestname);
+ string ("%s", config->guestname);
} end_element ();
start_element ("memory") {
attribute ("unit", "KiB");
- string_format ("%" PRIu64, memkb);
+ string ("%" PRIu64, memkb);
} end_element ();
start_element ("currentMemory") {
attribute ("unit", "KiB");
- string_format ("%" PRIu64, memkb);
+ string ("%" PRIu64, memkb);
} end_element ();
start_element ("vcpu") {
- string_format ("%d", config->vcpus);
+ string ("%d", config->vcpus);
} end_element ();
if (config->cpu.vendor || config->cpu.model ||
@@ -167,23 +122,23 @@ generate_physical_xml (struct config *config, struct
data_conn *data_conns,
attribute ("match", "minimum");
if (config->cpu.vendor) {
start_element ("vendor") {
- string (config->cpu.vendor);
+ string ("%s", config->cpu.vendor);
} end_element ();
}
if (config->cpu.model) {
start_element ("model") {
attribute ("fallback", "allow");
- string (config->cpu.model);
+ string ("%s", config->cpu.model);
} end_element ();
}
if (config->cpu.sockets || config->cpu.cores ||
config->cpu.threads) {
start_element ("topology") {
if (config->cpu.sockets)
- attribute_format ("sockets", "%u",
config->cpu.sockets);
+ attribute ("sockets", "%u",
config->cpu.sockets);
if (config->cpu.cores)
- attribute_format ("cores", "%u",
config->cpu.cores);
+ attribute ("cores", "%u",
config->cpu.cores);
if (config->cpu.threads)
- attribute_format ("threads", "%u",
config->cpu.threads);
+ attribute ("threads", "%u",
config->cpu.threads);
} end_element ();
}
} end_element ();
@@ -200,7 +155,7 @@ generate_physical_xml (struct config *config, struct
data_conn *data_conns,
else {
attribute ("offset", "variable");
attribute ("basis", "utc");
- attribute_format ("adjustment", "%d",
config->rtc.offset);
+ attribute ("adjustment", "%d",
config->rtc.offset);
}
} end_element ();
break;
@@ -214,7 +169,7 @@ generate_physical_xml (struct config *config, struct
data_conn *data_conns,
start_element ("os") {
start_element ("type") {
- attribute ("arch", host_cpu);
+ attribute ("arch", "%s", host_cpu);
string ("hvm");
} end_element ();
} end_element ();
@@ -252,11 +207,11 @@ generate_physical_xml (struct config *config, struct
data_conn *data_conns,
attribute ("protocol", "nbd");
start_element ("host") {
attribute ("name", "localhost");
- attribute_format ("port", "%d",
data_conns[i].nbd_remote_port);
+ attribute ("port", "%d",
data_conns[i].nbd_remote_port);
} end_element ();
} end_element ();
start_element ("target") {
- attribute ("dev", target_dev);
+ attribute ("dev", "%s", target_dev);
/* XXX Need to set bus to "ide" or "scsi" here.
*/
} end_element ();
} end_element ();
@@ -272,7 +227,7 @@ generate_physical_xml (struct config *config, struct
data_conn *data_conns,
attribute ("type", "raw");
} end_element ();
start_element ("target") {
- attribute ("dev", config->removable[i]);
+ attribute ("dev", "%s",
config->removable[i]);
} end_element ();
} end_element ();
}
@@ -300,14 +255,14 @@ generate_physical_xml (struct config *config, struct
data_conn *data_conns,
start_element ("interface") {
attribute ("type", "network");
start_element ("source") {
- attribute ("network", target_network);
+ attribute ("network", "%s", target_network);
} end_element ();
start_element ("target") {
- attribute ("dev", config->interfaces[i]);
+ attribute ("dev", "%s",
config->interfaces[i]);
} end_element ();
if (mac) {
start_element ("mac") {
- attribute ("address", mac);
+ attribute ("address", "%s", mac);
} end_element ();
}
} end_element ();
--
2.19.0.rc0
Richard W.M. Jones
2018-Oct-04 12:03 UTC
[Libguestfs] [PATCH v2 3/4] inspector: Use libxml writer macros.
Change virt-inspector so it uses the common set of macros. I also
added:
- single_element():
creates <foo>bar</foo> which is used extensively by
virt-inspector
- base64():
used by virt-inspector for the icon
---
common/utils/libxml2-writer-macros.h | 26 ++
inspector/inspector.c | 524 ++++++++++++---------------
2 files changed, 258 insertions(+), 292 deletions(-)
diff --git a/common/utils/libxml2-writer-macros.h
b/common/utils/libxml2-writer-macros.h
index b5705645a..13cd22ecd 100644
--- a/common/utils/libxml2-writer-macros.h
+++ b/common/utils/libxml2-writer-macros.h
@@ -79,6 +79,20 @@
#define empty_element(element) \
do { start_element ((element)) {} end_element (); } while (0)
+/**
+ * To define a single element with no attributes containing some text:
+ *
+ * single_element ("name", "%s", text);
+ *
+ * which produces C<<< <name>text</name> >>>
+ */
+#define single_element(element,fs,...) \
+ do { \
+ start_element ((element)) { \
+ string ((fs), ##__VA_ARGS__); \
+ } end_element (); \
+ } while (0)
+
/**
* To define an XML element with attributes, use:
*
@@ -128,6 +142,18 @@
} \
} while (0)
+/**
+ * To write a string encoded as base64:
+ *
+ * base64 (data, size);
+ */
+#define base64(data, size) \
+ do { \
+ if (xmlTextWriterWriteBase64 (xo, (data), 0, (size)) == -1) { \
+ xml_error ("xmlTextWriterWriteBase64");
\
+ } \
+ } while (0)
+
/**
* To define a comment in the XML, use:
*
diff --git a/inspector/inspector.c b/inspector/inspector.c
index 9be8f5110..cc5b49545 100644
--- a/inspector/inspector.c
+++ b/inspector/inspector.c
@@ -43,6 +43,7 @@
#include "structs-cleanups.h"
#include "options.h"
#include "display-options.h"
+#include "libxml2-writer-macros.h"
/* Currently open libguestfs handle. */
guestfs_h *g;
@@ -69,6 +70,14 @@ static void output_drive_mappings (xmlTextWriterPtr xo, char
*root);
static void output_applications (xmlTextWriterPtr xo, char *root);
static void do_xpath (const char *query);
+/* This macro is used by the macros in "libxml2-writer-macros.h"
+ * when an error occurs.
+ */
+#define xml_error(fn) \
+ error (EXIT_FAILURE, errno, \
+ "%s:%d: error constructing XML near call to
\"%s\"", \
+ __FILE__, __LINE__, (fn));
+
static void __attribute__((noreturn))
usage (int status)
{
@@ -307,11 +316,6 @@ main (int argc, char *argv[])
exit (EXIT_SUCCESS);
}
-#define XMLERROR(code,e) do { \
- if ((e) == (code)) \
- error (EXIT_FAILURE, errno, _("XML write error at
\"%s\""), #e); \
- } while (0)
-
static void
output (char **roots)
{
@@ -327,12 +331,17 @@ output (char **roots)
_("xmlNewTextWriter: failed to create libxml2 writer"));
/* Pretty-print the output. */
- XMLERROR (-1, xmlTextWriterSetIndent (xo, 1));
- XMLERROR (-1, xmlTextWriterSetIndentString (xo, BAD_CAST " "));
+ if (xmlTextWriterSetIndent (xo, 1) == -1 ||
+ xmlTextWriterSetIndentString (xo, BAD_CAST " ") == -1)
+ error (EXIT_FAILURE, errno, "could not set XML indent");
+
+ if (xmlTextWriterStartDocument (xo, NULL, NULL, NULL) == -1)
+ error (EXIT_FAILURE, errno, "xmlTextWriterStartDocument");
- XMLERROR (-1, xmlTextWriterStartDocument (xo, NULL, NULL, NULL));
output_roots (xo, roots);
- XMLERROR (-1, xmlTextWriterEndDocument (xo));
+
+ if (xmlTextWriterEndDocument (xo) == -1)
+ error (EXIT_FAILURE, errno, "xmlTextWriterEndDocument");
}
static void
@@ -340,10 +349,10 @@ output_roots (xmlTextWriterPtr xo, char **roots)
{
size_t i;
- XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST
"operatingsystems"));
- for (i = 0; roots[i] != NULL; ++i)
- output_root (xo, roots[i]);
- XMLERROR (-1, xmlTextWriterEndElement (xo));
+ start_element ("operatingsystems") {
+ for (i = 0; roots[i] != NULL; ++i)
+ output_root (xo, roots[i]);
+ } end_element ();
}
static void
@@ -351,152 +360,129 @@ output_root (xmlTextWriterPtr xo, char *root)
{
char *str;
int i;
- char buf[32];
char *canonical_root;
size_t size;
- XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST
"operatingsystem"));
-
- canonical_root = guestfs_canonical_device_name (g, root);
- if (canonical_root == NULL)
- exit (EXIT_FAILURE);
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "root", BAD_CAST
canonical_root));
- free (canonical_root);
-
- str = guestfs_inspect_get_type (g, root);
- if (!str) exit (EXIT_FAILURE);
- if (STRNEQ (str, "unknown"))
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "name", BAD_CAST
str));
- free (str);
-
- str = guestfs_inspect_get_arch (g, root);
- if (!str) exit (EXIT_FAILURE);
- if (STRNEQ (str, "unknown"))
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "arch", BAD_CAST
str));
- free (str);
-
- str = guestfs_inspect_get_distro (g, root);
- if (!str) exit (EXIT_FAILURE);
- if (STRNEQ (str, "unknown"))
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "distro", BAD_CAST
str));
- free (str);
-
- str = guestfs_inspect_get_product_name (g, root);
- if (!str) exit (EXIT_FAILURE);
- if (STRNEQ (str, "unknown"))
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "product_name",
BAD_CAST str));
- free (str);
-
- str = guestfs_inspect_get_product_variant (g, root);
- if (!str) exit (EXIT_FAILURE);
- if (STRNEQ (str, "unknown"))
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "product_variant",
BAD_CAST str));
- free (str);
-
- i = guestfs_inspect_get_major_version (g, root);
- snprintf (buf, sizeof buf, "%d", i);
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "major_version",
BAD_CAST buf));
- i = guestfs_inspect_get_minor_version (g, root);
- snprintf (buf, sizeof buf, "%d", i);
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "minor_version",
BAD_CAST buf));
-
- str = guestfs_inspect_get_package_format (g, root);
- if (!str) exit (EXIT_FAILURE);
- if (STRNEQ (str, "unknown"))
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "package_format",
BAD_CAST str));
- free (str);
-
- str = guestfs_inspect_get_package_management (g, root);
- if (!str) exit (EXIT_FAILURE);
- if (STRNEQ (str, "unknown"))
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "package_management",
- BAD_CAST str));
- free (str);
-
- /* inspect-get-windows-systemroot will fail with non-windows guests,
- * or if the systemroot could not be determined for a windows guest.
- * Disable error output around this call.
- */
- guestfs_push_error_handler (g, NULL, NULL);
- str = guestfs_inspect_get_windows_systemroot (g, root);
- if (str)
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST
"windows_systemroot",
- BAD_CAST str));
- free (str);
- str = guestfs_inspect_get_windows_current_control_set (g, root);
- if (str)
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST
"windows_current_control_set",
- BAD_CAST str));
- free (str);
- guestfs_pop_error_handler (g);
-
- str = guestfs_inspect_get_hostname (g, root);
- if (!str) exit (EXIT_FAILURE);
- if (STRNEQ (str, "unknown"))
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "hostname",
- BAD_CAST str));
- free (str);
-
- str = guestfs_inspect_get_osinfo (g, root);
- if (!str) exit (EXIT_FAILURE);
- if (STRNEQ (str, "unknown"))
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "osinfo",
BAD_CAST str));
- free (str);
-
- output_mountpoints (xo, root);
-
- output_filesystems (xo, root);
-
- output_drive_mappings (xo, root);
-
- /* We need to mount everything up in order to read out the list of
- * applications and the icon, ie. everything below this point.
- */
- if (inspect_apps || inspect_icon) {
- inspect_mount_root (g, root);
-
- if (inspect_apps)
- output_applications (xo, root);
-
- if (inspect_icon) {
- /* Don't return favicon. RHEL 7 and Fedora have crappy 16x16
- * favicons in the base distro.
- */
- str = guestfs_inspect_get_icon (g, root, &size,
- GUESTFS_INSPECT_GET_ICON_FAVICON, 0,
- -1);
- if (!str) exit (EXIT_FAILURE);
- if (size > 0) {
- XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST
"icon"));
- XMLERROR (-1, xmlTextWriterWriteBase64 (xo, str, 0, size));
- XMLERROR (-1, xmlTextWriterEndElement (xo));
+ start_element ("operatingsystem") {
+ canonical_root = guestfs_canonical_device_name (g, root);
+ if (canonical_root == NULL)
+ exit (EXIT_FAILURE);
+ single_element ("root", "%s", canonical_root);
+ free (canonical_root);
+
+ str = guestfs_inspect_get_type (g, root);
+ if (!str) exit (EXIT_FAILURE);
+ if (STRNEQ (str, "unknown"))
+ single_element ("name", "%s", str);
+ free (str);
+
+ str = guestfs_inspect_get_arch (g, root);
+ if (!str) exit (EXIT_FAILURE);
+ if (STRNEQ (str, "unknown"))
+ single_element ("arch", "%s", str);
+ free (str);
+
+ str = guestfs_inspect_get_distro (g, root);
+ if (!str) exit (EXIT_FAILURE);
+ if (STRNEQ (str, "unknown"))
+ single_element ("distro", "%s", str);
+ free (str);
+
+ str = guestfs_inspect_get_product_name (g, root);
+ if (!str) exit (EXIT_FAILURE);
+ if (STRNEQ (str, "unknown"))
+ single_element ("product_name", "%s", str);
+ free (str);
+
+ str = guestfs_inspect_get_product_variant (g, root);
+ if (!str) exit (EXIT_FAILURE);
+ if (STRNEQ (str, "unknown"))
+ single_element ("product_variant", "%s", str);
+ free (str);
+
+ i = guestfs_inspect_get_major_version (g, root);
+ single_element ("major_version", "%d", i);
+ i = guestfs_inspect_get_minor_version (g, root);
+ single_element ("minor_version", "%d", i);
+
+ str = guestfs_inspect_get_package_format (g, root);
+ if (!str) exit (EXIT_FAILURE);
+ if (STRNEQ (str, "unknown"))
+ single_element ("package_format", "%s", str);
+ free (str);
+
+ str = guestfs_inspect_get_package_management (g, root);
+ if (!str) exit (EXIT_FAILURE);
+ if (STRNEQ (str, "unknown"))
+ single_element ("package_management", "%s", str);
+ free (str);
+
+ /* inspect-get-windows-systemroot will fail with non-windows guests,
+ * or if the systemroot could not be determined for a windows guest.
+ * Disable error output around this call.
+ */
+ guestfs_push_error_handler (g, NULL, NULL);
+ str = guestfs_inspect_get_windows_systemroot (g, root);
+ if (str)
+ single_element ("windows_systemroot", "%s", str);
+ free (str);
+ str = guestfs_inspect_get_windows_current_control_set (g, root);
+ if (str)
+ single_element ("windows_current_control_set", "%s",
str);
+ free (str);
+ guestfs_pop_error_handler (g);
+
+ str = guestfs_inspect_get_hostname (g, root);
+ if (!str) exit (EXIT_FAILURE);
+ if (STRNEQ (str, "unknown"))
+ single_element ("hostname", "%s", str);
+ free (str);
+
+ str = guestfs_inspect_get_osinfo (g, root);
+ if (!str) exit (EXIT_FAILURE);
+ if (STRNEQ (str, "unknown"))
+ single_element ("osinfo", "%s", str);
+ free (str);
+
+ output_mountpoints (xo, root);
+
+ output_filesystems (xo, root);
+
+ output_drive_mappings (xo, root);
+
+ /* We need to mount everything up in order to read out the list of
+ * applications and the icon, ie. everything below this point.
+ */
+ if (inspect_apps || inspect_icon) {
+ inspect_mount_root (g, root);
+
+ if (inspect_apps)
+ output_applications (xo, root);
+
+ if (inspect_icon) {
+ /* Don't return favicon. RHEL 7 and Fedora have crappy 16x16
+ * favicons in the base distro.
+ */
+ str = guestfs_inspect_get_icon (g, root, &size,
+ GUESTFS_INSPECT_GET_ICON_FAVICON, 0,
+ -1);
+ if (!str) exit (EXIT_FAILURE);
+ if (size > 0) {
+ start_element ("icon") {
+ base64 (str, size);
+ } end_element ();
+ }
+ /* Note we must free (str) even if size == 0, because that indicates
+ * there was no icon.
+ */
+ free (str);
}
- /* Note we must free (str) even if size == 0, because that indicates
- * there was no icon.
- */
- free (str);
+
+ /* Unmount (see inspect_mount_root above). */
+ if (guestfs_umount_all (g) == -1)
+ exit (EXIT_FAILURE);
}
-
- /* Unmount (see inspect_mount_root above). */
- if (guestfs_umount_all (g) == -1)
- exit (EXIT_FAILURE);
- }
-
- XMLERROR (-1, xmlTextWriterEndElement (xo));
+ } end_element (); /* operatingsystem */
}
static int
@@ -548,23 +534,19 @@ output_mountpoints (xmlTextWriterPtr xo, char *root)
2 * sizeof (char *),
compare_keys_len);
- XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST
"mountpoints"));
+ start_element ("mountpoints") {
+ for (i = 0; mountpoints[i] != NULL; i += 2) {
+ CLEANUP_FREE char *p + guestfs_canonical_device_name (g,
mountpoints[i+1]);
+ if (!p)
+ exit (EXIT_FAILURE);
- for (i = 0; mountpoints[i] != NULL; i += 2) {
- CLEANUP_FREE char *p = guestfs_canonical_device_name (g, mountpoints[i+1]);
- if (!p)
- exit (EXIT_FAILURE);
-
- XMLERROR (-1,
- xmlTextWriterStartElement (xo, BAD_CAST "mountpoint"));
- XMLERROR (-1,
- xmlTextWriterWriteAttribute (xo, BAD_CAST "dev",
BAD_CAST p));
- XMLERROR (-1,
- xmlTextWriterWriteString (xo, BAD_CAST mountpoints[i]));
- XMLERROR (-1, xmlTextWriterEndElement (xo));
- }
-
- XMLERROR (-1, xmlTextWriterEndElement (xo));
+ start_element ("mountpoint") {
+ attribute ("dev", "%s", p);
+ string ("%s", mountpoints[i]);
+ } end_element ();
+ }
+ } end_element ();
}
static void
@@ -582,47 +564,37 @@ output_filesystems (xmlTextWriterPtr xo, char *root)
qsort (filesystems, guestfs_int_count_strings (filesystems), sizeof (char *),
compare_keys);
- XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST
"filesystems"));
-
- for (i = 0; filesystems[i] != NULL; ++i) {
- str = guestfs_canonical_device_name (g, filesystems[i]);
- if (!str)
- exit (EXIT_FAILURE);
-
- XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST
"filesystem"));
- XMLERROR (-1,
- xmlTextWriterWriteAttribute (xo, BAD_CAST "dev",
BAD_CAST str));
- free (str);
-
- guestfs_push_error_handler (g, NULL, NULL);
-
- str = guestfs_vfs_type (g, filesystems[i]);
- if (str && str[0])
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "type",
- BAD_CAST str));
- free (str);
-
- str = guestfs_vfs_label (g, filesystems[i]);
- if (str && str[0])
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "label",
- BAD_CAST str));
- free (str);
-
- str = guestfs_vfs_uuid (g, filesystems[i]);
- if (str && str[0])
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "uuid",
- BAD_CAST str));
- free (str);
-
- guestfs_pop_error_handler (g);
-
- XMLERROR (-1, xmlTextWriterEndElement (xo));
- }
-
- XMLERROR (-1, xmlTextWriterEndElement (xo));
+ start_element ("filesystems") {
+ for (i = 0; filesystems[i] != NULL; ++i) {
+ str = guestfs_canonical_device_name (g, filesystems[i]);
+ if (!str)
+ exit (EXIT_FAILURE);
+
+ start_element ("filesystem") {
+ attribute ("dev", "%s", str);
+ free (str);
+
+ guestfs_push_error_handler (g, NULL, NULL);
+
+ str = guestfs_vfs_type (g, filesystems[i]);
+ if (str && str[0])
+ single_element ("type", "%s", str);
+ free (str);
+
+ str = guestfs_vfs_label (g, filesystems[i]);
+ if (str && str[0])
+ single_element ("label", "%s", str);
+ free (str);
+
+ str = guestfs_vfs_uuid (g, filesystems[i]);
+ if (str && str[0])
+ single_element ("uuid", "%s", str);
+ free (str);
+
+ guestfs_pop_error_handler (g);
+ } end_element ();
+ }
+ } end_element ();
}
static void
@@ -646,26 +618,20 @@ output_drive_mappings (xmlTextWriterPtr xo, char *root)
guestfs_int_count_strings (drive_mappings) / 2, 2 * sizeof (char *),
compare_keys_nocase);
- XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST
"drive_mappings"));
-
- for (i = 0; drive_mappings[i] != NULL; i += 2) {
- str = guestfs_canonical_device_name (g, drive_mappings[i+1]);
- if (!str)
- exit (EXIT_FAILURE);
-
- XMLERROR (-1,
- xmlTextWriterStartElement (xo, BAD_CAST
"drive_mapping"));
- XMLERROR (-1,
- xmlTextWriterWriteAttribute (xo, BAD_CAST "name",
- BAD_CAST drive_mappings[i]));
- XMLERROR (-1,
- xmlTextWriterWriteString (xo, BAD_CAST str));
- XMLERROR (-1, xmlTextWriterEndElement (xo));
-
- free (str);
- }
-
- XMLERROR (-1, xmlTextWriterEndElement (xo));
+ start_element ("drive_mappings") {
+ for (i = 0; drive_mappings[i] != NULL; i += 2) {
+ str = guestfs_canonical_device_name (g, drive_mappings[i+1]);
+ if (!str)
+ exit (EXIT_FAILURE);
+
+ start_element ("drive_mapping") {
+ attribute ("name", "%s", drive_mappings[i]);
+ string ("%s", str);
+ } end_element ();
+
+ free (str);
+ }
+ } end_element ();
}
static void
@@ -681,71 +647,45 @@ output_applications (xmlTextWriterPtr xo, char *root)
if (apps == NULL)
exit (EXIT_FAILURE);
- XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST
"applications"));
+ start_element ("applications") {
+ for (i = 0; i < apps->len; ++i) {
+ start_element ("application") {
+ assert (apps->val[i].app2_name &&
apps->val[i].app2_name[0]);
+ single_element ("name", "%s",
apps->val[i].app2_name);
- for (i = 0; i < apps->len; ++i) {
- XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST
"application"));
+ if (apps->val[i].app2_display_name &&
+ apps->val[i].app2_display_name[0])
+ single_element ("display_name", "%s",
+ apps->val[i].app2_display_name);
- assert (apps->val[i].app2_name && apps->val[i].app2_name[0]);
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "name",
- BAD_CAST apps->val[i].app2_name));
+ if (apps->val[i].app2_epoch != 0)
+ single_element ("epoch", "%d",
apps->val[i].app2_epoch);
- if (apps->val[i].app2_display_name &&
apps->val[i].app2_display_name[0])
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "display_name",
- BAD_CAST apps->val[i].app2_display_name));
-
- if (apps->val[i].app2_epoch != 0) {
- char buf[32];
-
- snprintf (buf, sizeof buf, "%d", apps->val[i].app2_epoch);
-
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "epoch", BAD_CAST buf));
+ if (apps->val[i].app2_version &&
apps->val[i].app2_version[0])
+ single_element ("version", "%s",
apps->val[i].app2_version);
+ if (apps->val[i].app2_release &&
apps->val[i].app2_release[0])
+ single_element ("release", "%s",
apps->val[i].app2_release);
+ if (apps->val[i].app2_arch && apps->val[i].app2_arch[0])
+ single_element ("arch", "%s",
apps->val[i].app2_arch);
+ if (apps->val[i].app2_install_path &&
+ apps->val[i].app2_install_path[0])
+ single_element ("install_path", "%s",
+ apps->val[i].app2_install_path);
+ if (apps->val[i].app2_publisher &&
apps->val[i].app2_publisher[0])
+ single_element ("publisher", "%s",
apps->val[i].app2_publisher);
+ if (apps->val[i].app2_url && apps->val[i].app2_url[0])
+ single_element ("url", "%s",
apps->val[i].app2_url);
+ if (apps->val[i].app2_source_package &&
+ apps->val[i].app2_source_package[0])
+ single_element ("source_package", "%s",
+ apps->val[i].app2_source_package);
+ if (apps->val[i].app2_summary &&
apps->val[i].app2_summary[0])
+ single_element ("summary", "%s",
apps->val[i].app2_summary);
+ if (apps->val[i].app2_description &&
apps->val[i].app2_description[0])
+ single_element ("description", "%s",
apps->val[i].app2_description);
+ } end_element ();
}
-
- if (apps->val[i].app2_version &&
apps->val[i].app2_version[0])
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "version",
- BAD_CAST apps->val[i].app2_version));
- if (apps->val[i].app2_release &&
apps->val[i].app2_release[0])
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "release",
- BAD_CAST apps->val[i].app2_release));
- if (apps->val[i].app2_arch && apps->val[i].app2_arch[0])
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "arch",
- BAD_CAST apps->val[i].app2_arch));
- if (apps->val[i].app2_install_path &&
apps->val[i].app2_install_path[0])
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "install_path",
- BAD_CAST apps->val[i].app2_install_path));
- if (apps->val[i].app2_publisher &&
apps->val[i].app2_publisher[0])
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "publisher",
- BAD_CAST apps->val[i].app2_publisher));
- if (apps->val[i].app2_url && apps->val[i].app2_url[0])
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "url",
- BAD_CAST apps->val[i].app2_url));
- if (apps->val[i].app2_source_package &&
apps->val[i].app2_source_package[0])
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "source_package",
- BAD_CAST apps->val[i].app2_source_package));
- if (apps->val[i].app2_summary &&
apps->val[i].app2_summary[0])
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "summary",
- BAD_CAST apps->val[i].app2_summary));
- if (apps->val[i].app2_description &&
apps->val[i].app2_description[0])
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "description",
- BAD_CAST apps->val[i].app2_description));
-
- XMLERROR (-1, xmlTextWriterEndElement (xo));
- }
-
- XMLERROR (-1, xmlTextWriterEndElement (xo));
+ } end_element ();
}
/* Run an XPath query on XML on stdin, print results to stdout. */
--
2.19.0.rc0
Richard W.M. Jones
2018-Oct-04 12:03 UTC
[Libguestfs] [PATCH v2 4/4] lib, p2v: Use single_element() macro where possible.
After the previous commit, wherever we had:
start_element ("foo") {
string ("bar");
} end_element ();
this can now be replaced by:
single_element ("foo", "bar");
---
lib/launch-libvirt.c | 81 ++++++++++++--------------------------------
p2v/physical-xml.c | 15 +++-----
2 files changed, 25 insertions(+), 71 deletions(-)
diff --git a/lib/launch-libvirt.c b/lib/launch-libvirt.c
index 7b4b9f752..f6ef6bcad 100644
--- a/lib/launch-libvirt.c
+++ b/lib/launch-libvirt.c
@@ -1006,10 +1006,7 @@ construct_libvirt_xml_name (guestfs_h *g,
const struct libvirt_xml_params *params,
xmlTextWriterPtr xo)
{
- start_element ("name") {
- string ("%s", params->data->name);
- } end_element ();
-
+ single_element ("name", "%s", params->data->name);
return 0;
}
@@ -1040,17 +1037,12 @@ construct_libvirt_xml_cpu (guestfs_h *g,
attribute ("fallback", "allow");
} end_element ();
}
- else {
- start_element ("model") {
- string ("%s", cpu_model);
- } end_element ();
- }
+ else
+ single_element ("model", "%s", cpu_model);
} end_element ();
}
- start_element ("vcpu") {
- string ("%d", g->smp);
- } end_element ();
+ single_element ("vcpu", "%d", g->smp);
start_element ("clock") {
attribute ("offset", "utc");
@@ -1111,11 +1103,8 @@ construct_libvirt_xml_boot (guestfs_h *g,
string ("%s", params->data->uefi_code);
} end_element ();
- if (params->data->uefi_vars) {
- start_element ("nvram") {
- string ("%s", params->data->uefi_vars);
- } end_element ();
- }
+ if (params->data->uefi_vars)
+ single_element ("nvram", "%s",
params->data->uefi_vars);
}
#ifdef __powerpc64__
@@ -1128,17 +1117,9 @@ construct_libvirt_xml_boot (guestfs_h *g,
}
#endif
- start_element ("kernel") {
- string ("%s", params->kernel);
- } end_element ();
-
- start_element ("initrd") {
- string ("%s", params->initrd);
- } end_element ();
-
- start_element ("cmdline") {
- string ("%s", cmdline);
- } end_element ();
+ single_element ("kernel", "%s", params->kernel);
+ single_element ("initrd", "%s", params->initrd);
+ single_element ("cmdline", "%s", cmdline);
#if defined(__i386__) || defined(__x86_64__)
if (g->verbose) {
@@ -1167,9 +1148,7 @@ construct_libvirt_xml_seclabel (guestfs_h *g,
attribute ("type", "static");
attribute ("model", "dac");
attribute ("relabel", "no");
- start_element ("label") {
- string ("0:0");
- } end_element ();
+ single_element ("label", "0:0");
} end_element ();
}
@@ -1188,12 +1167,8 @@ construct_libvirt_xml_seclabel (guestfs_h *g,
attribute ("type", "static");
attribute ("model", "selinux");
attribute ("relabel", "yes");
- start_element ("label") {
- string ("%s", params->data->selinux_label);
- } end_element ();
- start_element ("imagelabel") {
- string ("%s", params->data->selinux_imagelabel);
- } end_element ();
+ single_element ("label", "%s",
params->data->selinux_label);
+ single_element ("imagelabel", "%s",
params->data->selinux_imagelabel);
} end_element ();
}
@@ -1206,10 +1181,7 @@ construct_libvirt_xml_lifecycle (guestfs_h *g,
const struct libvirt_xml_params *params,
xmlTextWriterPtr xo)
{
- start_element ("on_reboot") {
- string ("destroy");
- } end_element ();
-
+ single_element ("on_reboot", "destroy");
return 0;
}
@@ -1227,20 +1199,14 @@ construct_libvirt_xml_devices (guestfs_h *g,
/* Path to hypervisor. Only write this if the user has changed the
* default, otherwise allow libvirt to choose the best one.
*/
- if (is_custom_hv (g)) {
- start_element ("emulator") {
- string ("%s", g->hv);
- } end_element ();
- }
+ if (is_custom_hv (g))
+ single_element ("emulator", "%s", g->hv);
#if defined(__arm__)
/* Hopefully temporary hack to make ARM work (otherwise libvirt
* chooses to run /usr/bin/qemu-kvm).
*/
- else {
- start_element ("emulator") {
- string ("%s", QEMU);
- } end_element ();
- }
+ else
+ single_element ("emulator", "%s", QEMU);
#endif
/* Add a random number generator (backend for virtio-rng). This
@@ -1511,11 +1477,8 @@ construct_libvirt_xml_disk (guestfs_h *g,
return -1;
}
- if (drv->disk_label) {
- start_element ("serial") {
- string ("%s", drv->disk_label);
- } end_element ();
- }
+ if (drv->disk_label)
+ single_element ("serial", "%s", drv->disk_label);
if (construct_libvirt_xml_disk_address (g, xo, drv_index) == -1)
return -1;
@@ -1764,10 +1727,8 @@ construct_libvirt_xml_secret (guestfs_h *g,
start_element ("secret") {
attribute ("ephemeral", "yes");
attribute ("private", "yes");
- start_element ("description") {
- string ("guestfs secret associated with %s %s",
- data->name, drv->src.u.path);
- } end_element ();
+ single_element ("description", "guestfs secret associated
with %s %s",
+ data->name, drv->src.u.path);
} end_element ();
return 0;
diff --git a/p2v/physical-xml.c b/p2v/physical-xml.c
index 6ac8cc991..ffcb9e24e 100644
--- a/p2v/physical-xml.c
+++ b/p2v/physical-xml.c
@@ -97,9 +97,7 @@ generate_physical_xml (struct config *config, struct data_conn
*data_conns,
start_element ("domain") {
attribute ("type", "physical");
- start_element ("name") {
- string ("%s", config->guestname);
- } end_element ();
+ single_element ("name", "%s", config->guestname);
start_element ("memory") {
attribute ("unit", "KiB");
@@ -111,20 +109,15 @@ generate_physical_xml (struct config *config, struct
data_conn *data_conns,
string ("%" PRIu64, memkb);
} end_element ();
- start_element ("vcpu") {
- string ("%d", config->vcpus);
- } end_element ();
+ single_element ("vcpu", "%d", config->vcpus);
if (config->cpu.vendor || config->cpu.model ||
config->cpu.sockets || config->cpu.cores ||
config->cpu.threads) {
/* https://libvirt.org/formatdomain.html#elementsCPU */
start_element ("cpu") {
attribute ("match", "minimum");
- if (config->cpu.vendor) {
- start_element ("vendor") {
- string ("%s", config->cpu.vendor);
- } end_element ();
- }
+ if (config->cpu.vendor)
+ single_element ("vendor", "%s",
config->cpu.vendor);
if (config->cpu.model) {
start_element ("model") {
attribute ("fallback", "allow");
--
2.19.0.rc0
Pino Toscano
2018-Nov-02 14:15 UTC
Re: [Libguestfs] [PATCH v2 2/4] common/utils: Move libxml2 writer macros to a common header file.
On Thursday, 4 October 2018 14:03:48 CET Richard W.M. Jones wrote:> In some places when generating XML output in C code we use some clever > macros: > > start_element ("memory") { > attribute ("unit", "MiB"); > string ("%d", g->memsize); > } end_element (); > > This commit which is mostly refactoring moves the repeated definitions > of these macros into a common header file. > > I also took this opportunity to change / clean up the macros: > > - The macros are now documented properly. > > - comment() and empty_element() macros are now available everywhere. > > - Error handling has been made generic. > > - Added do..while(0) around some of the macros to make them safe to > use in all contexts. > > - Both string() and attribute() take a format string (previously > there were string_format() and attribute_format() variants).I'd still keep these variants, as there is no need for a format string in case there is already the final string to be used. -- Pino Toscano
Pino Toscano
2018-Nov-02 14:24 UTC
Re: [Libguestfs] [PATCH v2 3/4] inspector: Use libxml writer macros.
On Thursday, 4 October 2018 14:03:49 CET Richard W.M. Jones wrote:> Change virt-inspector so it uses the common set of macros. I also > added: > > - single_element(): > > creates <foo>bar</foo> which is used extensively by virt-inspectorAs mentioned in patch #2, having two variants single_elements (for string) and single_element_format (for format string) is better IMHO. -- Pino Toscano
Apparently Analagous Threads
- [PATCH 0/2] Use common macros to help with libxml2 writer.
- [PATCH v3 0/4] common/utils: Move libxml2 writer macros to a common header file.
- [PATCH 1/2] Change 'fprintf (stdout,...)' -> printf.
- [PATCH v2 0/3] New inspect_list_applications2 API
- [PATCH libguestfs 0/4] Add a libvirt backend to libguestfs.