storage: avoid short reads while chasing backing chain

Our backing file chain code was not very robust to an ill-timed
EINTR, which could lead to a short read causing us to randomly
treat metadata differently than usual.  But the existing
virFileReadLimFD forces an error if we don't read the entire
file, even though we only care about the header of the file.
So add a new virFile function that does what we want.

* src/util/virfile.h (virFileReadHeaderFD): New prototype.
* src/util/virfile.c (virFileReadHeaderFD): New function.
* src/libvirt_private.syms (virfile.h): Export it.
* src/util/virstoragefile.c (virStorageFileGetMetadataInternal)
(virStorageFileProbeFormatFromFD): Use it.

Signed-off-by: Eric Blake <eblake@redhat.com>

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ad8a79d7dc23d3cbe05bfdf00acea185e8ee106a..398b0c08d1b3dc9ae5f10057c0ab14e36af76d7e 100644 (file)
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1208,6 +1208,7 @@ virFileOpenAs;
 virFileOpenTty;
 virFilePrintf;
 virFileReadAll;
+virFileReadHeaderFD;
 virFileReadLimFD;
 virFileResolveAllLinks;
 virFileResolveLink;

diff --git a/src/util/virfile.c b/src/util/virfile.c
index 4882006d98a092271ef1fdbc3c2ea9dc7f0b0121..0e8d52d308d54fedd5219e9227f82f0ea0d776bf 100644 (file)
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -1225,6 +1225,27 @@ saferead_lim(int fd, size_t max_len, size_t *length)
     return NULL;
 }
 
+
+/* A wrapper around saferead_lim that merely stops reading at the
+ * specified maximum size.  */
+int
+virFileReadHeaderFD(int fd, int maxlen, char **buf)
+{
+    size_t len;
+    char *s;
+
+    if (maxlen <= 0) {
+        errno = EINVAL;
+        return -1;
+    }
+    s = saferead_lim(fd, maxlen, &len);
+    if (s == NULL)
+        return -1;
+    *buf = s;
+    return len;
+}
+
+
 /* A wrapper around saferead_lim that maps a failure due to
    exceeding the maximum size limitation to EOVERFLOW.  */
 int

diff --git a/src/util/virfile.h b/src/util/virfile.h
index ff84719ccfcba57e0bc7ea14d44b5bf4e1fe8748..10cf8bd9dc5d0bbeaf3f610bddfd400a7b1c55ec 100644 (file)
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -122,9 +122,12 @@ int virFileNBDDeviceAssociate(const char *file,
 
 int virFileDeleteTree(const char *dir);
 
-int virFileReadLimFD(int fd, int maxlen, char **buf) ATTRIBUTE_RETURN_CHECK;
-
-int virFileReadAll(const char *path, int maxlen, char **buf) ATTRIBUTE_RETURN_CHECK;
+int virFileReadHeaderFD(int fd, int maxlen, char **buf)
+    ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(3);
+int virFileReadLimFD(int fd, int maxlen, char **buf)
+    ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(3);
+int virFileReadAll(const char *path, int maxlen, char **buf)
+    ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3);
 
 int virFileWriteStr(const char *path, const char *str, mode_t mode)
     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 589778c0900680f88b1d2793ada66fccec7012e6..bbd5a7e2344471f7d13890b1228d2be78c9dcdfe 100644 (file)
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -793,10 +793,7 @@ virStorageFileGetMetadataInternal(const char *path,
         goto cleanup;
     }
 
-    if (VIR_ALLOC_N(buf, len) < 0)
-        goto cleanup;
-
-    if ((len = read(fd, buf, len)) < 0) {
+    if ((len = virFileReadHeaderFD(fd, len, &buf)) < 0) {
         virReportSystemError(errno, _("cannot read header '%s'"), path);
         goto cleanup;
     }
@@ -939,15 +936,12 @@ virStorageFileProbeFormatFromFD(const char *path, int fd)
         return VIR_STORAGE_FILE_DIR;
     }
 
-    if (VIR_ALLOC_N(head, len) < 0)
-        return -1;
-
     if (lseek(fd, 0, SEEK_SET) == (off_t)-1) {
         virReportSystemError(errno, _("cannot set to start of '%s'"), path);
         goto cleanup;
     }
 
-    if ((len = read(fd, head, len)) < 0) {
+    if ((len = virFileReadHeaderFD(fd, len, &head)) < 0) {
         virReportSystemError(errno, _("cannot read header '%s'"), path);
         goto cleanup;
     }