Re: [PATCH 1/2] iohelper: skip lseek() and ftruncate() on block devices

[Simon Rowe <simon.rowe@xxxxxxxxxxx>]

I can change to doing this if it preferred,

Simon

---

From: Michal Prívozník <mprivozn@xxxxxxxxxx>
Sent: 20 August 2021 16:09
To: Simon Rowe <simon.rowe@xxxxxxxxxxx>; libvir-list@xxxxxxxxxx <libvir-list@xxxxxxxxxx>
Subject: Re: [PATCH 1/2] iohelper: skip lseek() and ftruncate() on block devices

 

On 8/20/21 10:39 AM, Simon Rowe wrote:
> Signed-off-by: Simon Rowe <simon.rowe@xxxxxxxxxxx>
> ---
>  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);
>          /* 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) {
>              virReportSystemError(end < 0 ? errno : EINVAL, "%s",
>                                   _("O_DIRECT write needs empty seekable file"));
>              goto cleanup;
> @@ -140,7 +146,7 @@ runIO(const char *path, int fd, int oflags)
>                  goto cleanup;
>              }
> 
> -            if (ftruncate(fd, total) < 0) {
> +            if (!isBlockDev && ftruncate(fd, total) < 0) {
>                  virReportSystemError(errno, _("Unable to truncate %s"), fdoutname);
>                  goto cleanup;
>              }
>

IIUC, O_DIRECT is no good for block devices. I wonder whether the caller
(doCoreDump()) should learn whether the path is a block device and don't
set O_DIRECT flag instead of changing iohelper.

Michal