[libvirt RFCv6 05/27] virfile: change virFileDiskCopy arguments to extend beyond stdin, stdout

[Claudio Fontana]

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 at 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