File pacemaker-libcrmcommon-tools-improve-XML-write-error-handling.patch of Package pacemaker.14737
commit bdaf392a077586b55961f57ef649a2841c51b297
Author: Ken Gaillot <kgaillot@redhat.com>
Date: Fri Jan 5 12:19:22 2018 -0600
Fix: libcrmcommon,tools: improve XML write error handling
The write_xml_*() functions previously would return -1 on error
(only sometimes setting errno for more information) and sometimes
0 (when compressed) or the number of bytes written (when uncompressed) on
success.
Some callers were relying on the return value being -errno, others relied on
errno being set if the return value was negative, and others relied on positive
values indicating success.
Now, the functions always return -errno on error, and the number of bytes
written (whether compressed or uncompressed) on success, and callers treat it
as such.
Also, error log messages have been improved.
Index: pacemaker/lib/common/xml.c
===================================================================
--- pacemaker.orig/lib/common/xml.c
+++ pacemaker/lib/common/xml.c
@@ -3320,6 +3320,17 @@ crm_xml_add_last_written(xmlNode *xml_no
return crm_xml_add(xml_node, XML_CIB_ATTR_WRITTEN, now_str);
}
+/*!
+ * \internal
+ * \brief Write XML to a file stream
+ *
+ * \param[in] xml_node XML to write
+ * \param[in] filename Name of file being written (for logging only)
+ * \param[in] stream Open file stream corresponding to filename
+ * \param[in] compress Whether to compress XML before writing
+ *
+ * \return Number of bytes written on success, -errno otherwise
+ */
static int
write_xml_stream(xmlNode * xml_node, const char *filename, FILE * stream, gboolean compress)
{
@@ -3327,20 +3338,12 @@ write_xml_stream(xmlNode * xml_node, con
char *buffer = NULL;
unsigned int out = 0;
- CRM_CHECK(stream != NULL, return -1);
-
- crm_trace("Writing XML out to %s", filename);
- if (xml_node == NULL) {
- crm_err("Cannot write NULL to %s", filename);
- fclose(stream);
- return -1;
- }
-
-
- crm_log_xml_trace(xml_node, "Writing out");
+ crm_log_xml_trace(xml_node, "writing");
buffer = dump_xml_formatted(xml_node);
- CRM_CHECK(buffer != NULL && strlen(buffer) > 0, crm_log_xml_warn(xml_node, "dump:failed");
+ CRM_CHECK(buffer && strlen(buffer),
+ crm_log_xml_warn(xml_node, "formatting failed");
+ res = -pcmk_err_generic;
goto bail);
if (compress) {
@@ -3351,32 +3354,38 @@ write_xml_stream(xmlNode * xml_node, con
bz_file = BZ2_bzWriteOpen(&rc, stream, 5, 0, 30);
if (rc != BZ_OK) {
- crm_err("bzWriteOpen failed: %d", rc);
+ crm_warn("Not compressing %s: could not prepare file stream "
+ CRM_XS " bzerror=%d", filename, rc);
} else {
BZ2_bzWrite(&rc, bz_file, buffer, strlen(buffer));
if (rc != BZ_OK) {
- crm_err("bzWrite() failed: %d", rc);
+ crm_warn("Not compressing %s: could not compress data "
+ CRM_XS " bzerror=%d errno=%d", filename, rc, errno);
}
}
if (rc == BZ_OK) {
BZ2_bzWriteClose(&rc, bz_file, 0, &in, &out);
if (rc != BZ_OK) {
- crm_err("bzWriteClose() failed: %d", rc);
- out = -1;
+ crm_warn("Not compressing %s: could not write compressed data "
+ CRM_XS " bzerror=%d errno=%d", filename, rc, errno);
+ out = -1; // retry without compression
} else {
- crm_trace("%s: In: %d, out: %d", filename, in, out);
+ res = (int) out;
+ crm_trace("Compressed XML for %s from %d bytes to %d",
+ filename, in, out);
}
}
#else
- crm_err("Cannot write compressed files:" " bzlib was not available at compile time");
+ crm_warn("Not compressing %s: not built with bzlib support", filename);
#endif
}
if (out <= 0) {
res = fprintf(stream, "%s", buffer);
if (res < 0) {
- crm_perror(LOG_ERR, "Cannot write output to %s", filename);
+ res = -errno;
+ crm_perror(LOG_ERR, "writing %s", filename);
goto bail;
}
}
@@ -3384,41 +3393,67 @@ write_xml_stream(xmlNode * xml_node, con
bail:
if (fflush(stream) != 0) {
- crm_perror(LOG_ERR, "fflush for %s failed", filename);
- res = -1;
+ res = -errno;
+ crm_perror(LOG_ERR, "flushing %s", filename);
}
/* Don't report error if the file does not support synchronization */
if (fsync(fileno(stream)) < 0 && errno != EROFS && errno != EINVAL) {
- crm_perror(LOG_ERR, "fsync for %s failed", filename);
- res = -1;
+ res = -errno;
+ crm_perror(LOG_ERR, "synchronizing %s", filename);
}
fclose(stream);
- crm_trace("Saved %d bytes to the Cib as XML", res);
+ crm_trace("Saved %d bytes%s to %s as XML",
+ res, ((out > 0)? " (compressed)" : ""), filename);
free(buffer);
return res;
}
+/*!
+ * \brief Write XML to a file descriptor
+ *
+ * \param[in] xml_node XML to write
+ * \param[in] filename Name of file being written (for logging only)
+ * \param[in] fd Open file descriptor corresponding to filename
+ * \param[in] compress Whether to compress XML before writing
+ *
+ * \return Number of bytes written on success, -errno otherwise
+ */
int
write_xml_fd(xmlNode * xml_node, const char *filename, int fd, gboolean compress)
{
FILE *stream = NULL;
- CRM_CHECK(fd > 0, return -1);
+ CRM_CHECK(xml_node && (fd > 0), return -EINVAL);
stream = fdopen(fd, "w");
+ if (stream == NULL) {
+ return -errno;
+ }
return write_xml_stream(xml_node, filename, stream, compress);
}
+/*!
+ * \brief Write XML to a file
+ *
+ * \param[in] xml_node XML to write
+ * \param[in] filename Name of file to write
+ * \param[in] compress Whether to compress XML before writing
+ *
+ * \return Number of bytes written on success, -errno otherwise
+ */
int
write_xml_file(xmlNode * xml_node, const char *filename, gboolean compress)
{
FILE *stream = NULL;
+ CRM_CHECK(xml_node && filename, return -EINVAL);
stream = fopen(filename, "w");
-
+ if (stream == NULL) {
+ return -errno;
+ }
return write_xml_stream(xml_node, filename, stream, compress);
}
Index: pacemaker/tools/cib_shadow.c
===================================================================
--- pacemaker.orig/tools/cib_shadow.c
+++ pacemaker/tools/cib_shadow.c
@@ -382,7 +382,7 @@ main(int argc, char **argv)
if (rc < 0) {
fprintf(stderr, "Could not %s the shadow instance '%s': %s\n",
command == 'r' ? "reset" : "create",
- shadow, strerror(errno));
+ shadow, pcmk_strerror(rc));
goto done;
}
shadow_setup(shadow, FALSE);
Index: pacemaker/tools/crm_simulate.c
===================================================================
--- pacemaker.orig/tools/crm_simulate.c
+++ pacemaker/tools/crm_simulate.c
@@ -445,7 +445,8 @@ setup_input(const char *input, const cha
cib_object = NULL;
if (rc < 0) {
- fprintf(stderr, "Could not create '%s': %s\n", output, strerror(errno));
+ fprintf(stderr, "Could not create '%s': %s\n",
+ output, pcmk_strerror(rc));
crm_exit(rc);
}
setenv("CIB_file", output, 1);
@@ -857,7 +858,8 @@ main(int argc, char **argv)
if (input_file != NULL) {
rc = write_xml_file(input, input_file, FALSE);
if (rc < 0) {
- fprintf(stderr, "Could not create '%s': %s\n", input_file, strerror(errno));
+ fprintf(stderr, "Could not create '%s': %s\n",
+ input_file, pcmk_strerror(rc));
goto done;
}
}
