On 5/6/22 1:03 PM, Daniel P. Berrangé wrote: > On Thu, May 05, 2022 at 02:52:19PM +0200, Claudio Fontana wrote: >> Signed-off-by: Claudio Fontana <cfont...@suse.de> >> --- >> 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. >
Ah ok this works too. Thanks! Claudio