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 <cfontana@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