Re: [PATCHv2 1/2] iohelper: skip lseek() and ftruncate() on block devices
On 8/23/21 5:40 PM, Simon Rowe wrote:
Signed-off-by: Simon Rowe <simon.rowe(a)nutanix.com>
---
src/util/iohelper.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/util/iohelper.c b/src/util/iohelper.c
index b8810d16d3..e6eb178fde 100644
--- a/src/util/iohelper.c
+++ b/src/util/iohelper.c
@@ -28,6 +28,8 @@
#include <unistd.h>
#include <fcntl.h>
#include <stdlib.h>
+#include <sys/types.h>
+#include <sys/stat.h>
#include "virthread.h"
#include "virfile.h"
@@ -56,6 +58,8 @@ runIO(const char *path, int fd, int oflags)
unsigned long long total = 0;
bool direct = O_DIRECT && ((oflags & O_DIRECT) != 0);
off_t end = 0;
+ struct stat sb;
+ bool isBlockDev = false;
#if WITH_POSIX_MEMALIGN
if (posix_memalign(&base, alignMask + 1, buflen))
@@ -86,9 +90,11 @@ runIO(const char *path, int fd, int oflags)
fdinname = "stdin";
fdout = fd;
fdoutname = path;
+ if (fstat(fd, &sb) == 0)
+ isBlockDev = S_ISBLK(sb.st_mode);
I think we can check for this before this switch() and report error if
fstat() fails. That way we can ..
/* To make the implementation simpler, we give up on any
* attempt to use O_DIRECT in a non-trivial manner. */
- if (direct && (end = lseek(fd, 0, SEEK_END)) != 0) {
+ if (!isBlockDev && direct && (end = lseek(fd, 0, SEEK_END)) !=
0) {
.. do this change for O_RDONLY case too. IOW, I suggest squashing this
in:
diff --git i/src/util/iohelper.c w/src/util/iohelper.c
index e6eb178fde..2c91bf4f93 100644
--- i/src/util/iohelper.c
+++ w/src/util/iohelper.c
@@ -71,6 +71,14 @@ runIO(const char *path, int fd, int oflags)
buf = (char *) (((intptr_t) base + alignMask) & ~alignMask);
#endif
+ if (fstat(fd, &sb) < 0) {
+ virReportSystemError(errno,
+ _("Unable to access file descriptor %d path %s"),
+ fd, path);
+ goto cleanup;
+ }
+ isBlockDev = S_ISBLK(sb.st_mode);
+
switch (oflags & O_ACCMODE) {
case O_RDONLY:
fdin = fd;
@@ -79,7 +87,7 @@ runIO(const char *path, int fd, int oflags)
fdoutname = "stdout";
/* To make the implementation simpler, we give up on any
* attempt to use O_DIRECT in a non-trivial manner. */
- if (direct && ((end = lseek(fd, 0, SEEK_CUR)) != 0)) {
+ if (!isBlockDev && direct && ((end = lseek(fd, 0, SEEK_CUR)) !=
0)) {
virReportSystemError(end < 0 ? errno : EINVAL, "%s",
_("O_DIRECT read needs entire seekable
file"));
goto cleanup;
@@ -90,8 +98,6 @@ runIO(const char *path, int fd, int oflags)
fdinname = "stdin";
fdout = fd;
fdoutname = path;
- if (fstat(fd, &sb) == 0)
- isBlockDev = S_ISBLK(sb.st_mode);
/* To make the implementation simpler, we give up on any
* attempt to use O_DIRECT in a non-trivial manner. */
if (!isBlockDev && direct && (end = lseek(fd, 0, SEEK_END)) != 0)
{
No need to resend, just let me know if you're okay with the suggested
change and I'll squash it in before pushing.
Michal