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

[Daniel P. Berrangé]

On Fri, May 06, 2022 at 01:29:57PM +0200, Claudio Fontana wrote:
> 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.
> > 
> >> 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
> > 
> 
> Should I go back and put this stuff outside virFile.c then, would simplify things in my view,
> this is a very specialized use anyway no?

It is fine in virfile, I'd just ifdef the header + source together
using !WIN32.

I've found the bug wrt posix_memalign mis-detection

https://github.com/mesonbuild/meson/issues/1083

which i'll fix separately

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|