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@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.
>
Ah ok this works too. Thanks!
Claudio
