File pacemaker-libcrmcommon-tools-improve-XML-write-... of Package pacemaker

Overview Repositories Revisions Requests Users Attributes Meta

File pacemaker-libcrmcommon-tools-improve-XML-write-error-handling.patch of Package pacemaker (Revision 0e051afa0cc7159c73d2a3fcc79577e4)

Currently displaying revision 0e051afa0cc7159c73d2a3fcc79577e4 , Show latest

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;
         }
     }

Places

File pacemaker-libcrmcommon-tools-improve-XML-write-error-handling.patch of Package pacemaker (Revision 0e051afa0cc7159c73d2a3fcc79577e4)

Places