On Thu, May 05, 2022 at 02:52:19PM +0200, Claudio Fontana wrote:
> Signed-off-by: Claudio Fontana <cfontana@xxxxxxx>
> ---
> src/util/iohelper.c | 17 +---------
> src/util/virfile.c | 82 ++++++++++++++++++++++++++++++++++-----------
> src/util/virfile.h | 2 +-
> 3 files changed, 64 insertions(+), 37 deletions(-)
>
> diff --git a/src/util/iohelper.c b/src/util/iohelper.c
> index ce10ccb905..055540c8c4 100644
> --- a/src/util/iohelper.c
> +++ b/src/util/iohelper.c
> @@ -58,7 +58,6 @@ int
> main(int argc, char **argv)
> {
> const char *path;
> - int oflags = -1;
> int fd = -1;
>
> program_name = argv[0];
> @@ -79,25 +78,11 @@ main(int argc, char **argv)
> program_name, argv[3]);
> exit(EXIT_FAILURE);
> }
> -#ifdef F_GETFL
> - oflags = fcntl(fd, F_GETFL);
> -#else
> - /* Stupid mingw. */
> - if (fd == STDIN_FILENO)
> - oflags = O_RDONLY;
> - else if (fd == STDOUT_FILENO)
> - oflags = O_WRONLY;
> -#endif
> - if (oflags < 0) {
> - fprintf(stderr, _("%s: unable to determine access mode of fd %d"),
> - program_name, fd);
> - exit(EXIT_FAILURE);
> - }
> } else { /* unknown argc pattern */
> usage(EXIT_FAILURE);
> }
>
> - if (fd < 0 || virFileDiskCopy(path, fd, oflags) < 0)
> + if (fd < 0 || virFileDiskCopy(fd, path, -1, "stdio") < 0)
> goto error;
>
> return 0;
> diff --git a/src/util/virfile.c b/src/util/virfile.c
> index 30b0eee074..87d3e53819 100644
> --- a/src/util/virfile.c
> +++ b/src/util/virfile.c
> @@ -4669,19 +4669,64 @@ runIOCopy(const struct runIOParams p)
> return total;
> }
>
> +/**
> + * virFileDiskCopy: run IO to copy data between storage and a pipe or socket.
> + *
> + * @disk_fd: the already open regular file or block device
> + * @disk_path: the pathname corresponding to disk_fd (for error reporting)
> + * @remote_fd: the pipe or socket
> + * Use -1 to auto-choose between STDIN or STDOUT.
> + * @remote_path: the pathname corresponding to remote_fd (for error reporting)
> + *
> + * Note that the direction of the transfer is detected based on the @disk_fd
> + * file access mode (man 2 open). Therefore @disk_fd must be opened with
> + * O_RDONLY or O_WRONLY. O_RDWR is not supported.
> + *
> + * virFileDiskCopy always closes the file descriptor disk_fd,
> + * and any error during close(2) is reported and considered a failure.
> + *
> + * Returns: bytes transferred or < 0 on failure.
> + */
>
> off_t
> -virFileDiskCopy(const char *path, int fd, int oflags)
> +virFileDiskCopy(int disk_fd, const char *disk_path, int remote_fd, const char *remote_path)
> {
> int ret = -1;
> off_t total = 0;
> struct stat sb;
> struct runIOParams p;
> + int oflags = -1;
> +
> +#ifdef F_GETFL
> + oflags = fcntl(disk_fd, F_GETFL);
> +#else /* !F_GETFL */
> + /*
> + * mingw still does not support F_GETFL, and does not seem willing to
> + * support it in the future. So we need a hack to get iohelper working,
> + * specifically possible only when using stdio.
> + */
> + if (disk_fd == STDIN_FILENO) {
> + oflags = O_RDONLY;
> + } else if (disk_fd == STDOUT_FILENO) {
> + oflags = O_WRONLY;
> + } else {
> + virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
> + _("%s: disk_fd must be stdin or stdout, due to lack of F_GETFL (mingw)"),
> + __FUNCTION__);
> + goto cleanup;
> + }
> +#endif /* !F_GETFL */
With this issue, and the posix_memalign issue, I'm inclined to say we
should just take the easy way out and hide virFileDiskCopy behind
#ifndef WIN32, since we don't even need it on WIN32.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
