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>
(cherry picked from commit 5327fad4f292e4f3f84884ffe158c492bf00519c)

Conflicts:
	src/util/virstoragefile.c: OOM error reporting & buffer signedness

Conflicts:
	src/libvirt_private.syms,
	src/util/virfile.c,
	src/util/virfile.h: Moved code to virutil.{c,h}

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 4c95334a1787d3f27e6051eb17b0b98317d68b2f..a728fed3838479381bbbdd984dafe829ba2b906d 100644 (file)
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1880,6 +1880,7 @@ virFileMatchesNameSuffix;
 virFileOpenAs;
 virFileOpenTty;
 virFileReadAll;
+virFileReadHeaderFD;
 virFileReadLimFD;
 virFileResolveAllLinks;
 virFileResolveLink;

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 60fdcf3ab20515bede269e545ff1cb6c9170feae..12f842025b3a23d4b8ab54cb305b88d24bd0c630 100644 (file)
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -707,12 +707,7 @@ virStorageFileGetMetadataInternal(const char *path,
         goto cleanup;
     }
 
-    if (VIR_ALLOC_N(buf, len) < 0) {
-        virReportOOMError();
-        goto cleanup;
-    }
-
-    if ((len = read(fd, buf, len)) < 0) {
+    if ((len = virFileReadHeaderFD(fd, len, (char **)&buf)) < 0) {
         virReportSystemError(errno, _("cannot read header '%s'"), path);
         goto cleanup;
     }
@@ -849,17 +844,12 @@ virStorageFileProbeFormatFromFD(const char *path, int fd)
         return VIR_STORAGE_FILE_DIR;
     }
 
-    if (VIR_ALLOC_N(head, len) < 0) {
-        virReportOOMError();
-        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, (char **)&head)) < 0) {
         virReportSystemError(errno, _("cannot read header '%s'"), path);
         goto cleanup;
     }

diff --git a/src/util/virutil.c b/src/util/virutil.c
index e93885ace074b74fd303340f26ecfaaed927e791..1dcc5dfc1288d71e89866f0f0ff6f3645a1f6991 100644 (file)
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -439,6 +439,25 @@ 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/virutil.h b/src/util/virutil.h
index e498d95059b7342250e3b3e2ee2830b7df83c419..750006ac49c04adada1c9172cb311c74b64d82a2 100644 (file)
--- a/src/util/virutil.h
+++ b/src/util/virutil.h
@@ -58,6 +58,9 @@ int virSetUIDGIDWithCaps(uid_t uid, gid_t gid, gid_t *groups, int ngroups,
                          unsigned long long capBits,
                          bool clearExistingCaps);
 
+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;
 
 int virFileReadAll(const char *path, int maxlen, char **buf) ATTRIBUTE_RETURN_CHECK;