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

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

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