[libvirt RFCv6 03/27] iohelper: move runIO function to virfile.c

[Claudio Fontana]

On 5/6/22 1:02 PM, Daniel P. Berrangé wrote:
> On Thu, May 05, 2022 at 02:52:17PM +0200, Claudio Fontana wrote:
>> where it can be reused by other helpers.
>> No changes other than the move.
>>
>> Note that this makes iohelper now dependent on -lutil, because unused
>> (for iohelper) parts of virfile.c contain calls to openpty(3).
> 
> Needs -lacl too on F35 at least.

do we end up needing the whole of the virt_util_lib dependencies for this?

 dependencies: [
    acl_dep,
    audit_dep,
    capng_dep,
    devmapper_dep,
    gnutls_dep,
    intl_dep,
    libm_dep,
    libnl_dep,
    libutil_dep,
    numactl_dep,
    secdriver_dep,
    src_dep,
    thread_dep,
    win32_dep,
    yajl_dep,
]

iohelper did not need -lutil or anything else before, just a file.
going back to just a separate helper_runio.c/h would make sense to me, but let me know.

Thanks,

Claudio




> 
>> Signed-off-by: Claudio Fontana <cfontana at suse.de>
>> ---
>>  src/util/iohelper.c  | 175 -------------------------------------------
>>  src/util/meson.build |   4 +
>>  src/util/virfile.c   | 172 ++++++++++++++++++++++++++++++++++++++++++
>>  src/util/virfile.h   |   2 +
>>  4 files changed, 178 insertions(+), 175 deletions(-)
>>
> 
>> diff --git a/src/util/meson.build b/src/util/meson.build
>> index 24350a3e67..84ef13ba32 100644
>> --- a/src/util/meson.build
>> +++ b/src/util/meson.build
>> @@ -175,6 +175,7 @@ keycode_dep = declare_dependency(
>>  
>>  io_helper_sources = [
>>    'iohelper.c',
>> +  'virfile.c',
>>  ]
>>  
>>  virt_util_lib = static_library(
>> @@ -213,6 +214,9 @@ if conf.has('WITH_LIBVIRTD')
>>        files(io_helper_sources),
>>        dtrace_gen_headers,
>>      ],
>> +    'deps': [
> 
> Adding   'acl_dep' here is needed
> 
>> +      libutil_dep,
>> +    ],
>>    }
>>  endif
>>  
>> diff --git a/src/util/virfile.c b/src/util/virfile.c
>> index 130b0fbace..b033a27264 100644
>> --- a/src/util/virfile.c
>> +++ b/src/util/virfile.c
>> @@ -4577,3 +4577,175 @@ virFileSetCOW(const char *path,
>>      return 0;
>>  #endif /* ! __linux__ */
>>  }
>> +
>> +struct runIOParams {
>> +    bool isBlockDev;
>> +    bool isDirect;
>> +    bool isWrite;
>> +    int fdin;
>> +    const char *fdinname;
>> +    int fdout;
>> +    const char *fdoutname;
>> +};
>> +
>> +/**
>> + * runIOCopy: execute the IO copy based on the passed parameters
>> + * @p: the IO parameters
>> + *
>> + * Execute the copy based on the passed parameters.
>> + *
>> + * Returns: size transfered, or < 0 on error.
>> + */
>> +
>> +static off_t
>> +runIOCopy(const struct runIOParams p)
>> +{
>> +    g_autofree void *base = NULL; /* Location to be freed */
>> +    char *buf = NULL; /* Aligned location within base */
>> +    size_t buflen = 1024*1024;
>> +    intptr_t alignMask = 64*1024 - 1;
>> +    off_t total = 0;
>> +
>> +#if WITH_POSIX_MEMALIGN
>> +    if (posix_memalign(&base, alignMask + 1, buflen))
>> +        abort();
>> +    buf = base;
>> +#else
>> +    buf = g_new0(char, buflen + alignMask);
>> +    base = buf;
>> +    buf = (char *) (((intptr_t) base + alignMask) & ~alignMask);
>> +#endif
> 
> For reasons I don't understand 'WITH_POSIX_MEMALIGN' is defined
> on mingw, but posix_memalign doesn't exist in header files and
> thus we fail to compile.
> 
> THe original iohelper.c file is not compiled at all unless
> WITH_LIBVIRTD is set, which is not on mingw, so we didn't
> see this problem before WITH_POSIX_MEMALIGN.
> 
> 
> With regards,
> Daniel
>