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).
Signed-off-by: Claudio Fontana <cfontana@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/iohelper.c b/src/util/iohelper.c
index 1584321839..b3aaf82b9f 100644
--- a/src/util/iohelper.c
+++ b/src/util/iohelper.c
@@ -41,181 +41,6 @@
#define VIR_FROM_THIS VIR_FROM_STORAGE
-#ifndef O_DIRECT
-# define O_DIRECT 0
-#endif
-
-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
-
- while (1) {
- ssize_t got;
-
- /* If we read with O_DIRECT from file we can't use saferead as
- * it can lead to unaligned read after reading last bytes.
- * If we write with O_DIRECT use should use saferead so that
- * writes will be aligned.
- * In other cases using saferead reduces number of syscalls.
- */
- if (!p.isWrite && p.isDirect) {
- if ((got = read(p.fdin, buf, buflen)) < 0 &&
- errno == EINTR)
- continue;
- } else {
- got = saferead(p.fdin, buf, buflen);
- }
-
- if (got < 0) {
- virReportSystemError(errno, _("Unable to read %s"), p.fdinname);
- return -2;
- }
- if (got == 0)
- break;
-
- total += got;
-
- /* handle last write size align in direct case */
- if (got < buflen && p.isDirect && p.isWrite) {
- ssize_t aligned_got = (got + alignMask) & ~alignMask;
-
- memset(buf + got, 0, aligned_got - got);
-
- if (safewrite(p.fdout, buf, aligned_got) < 0) {
- virReportSystemError(errno, _("Unable to write %s"), p.fdoutname);
- return -3;
- }
-
- if (!p.isBlockDev && ftruncate(p.fdout, total) < 0) {
- virReportSystemError(errno, _("Unable to truncate %s"), p.fdoutname);
- return -4;
- }
-
- break;
- }
-
- if (safewrite(p.fdout, buf, got) < 0) {
- virReportSystemError(errno, _("Unable to write %s"), p.fdoutname);
- return -3;
- }
- }
- return total;
-}
-
-static int
-runIO(const char *path, int fd, int oflags)
-{
- int ret = -1;
- off_t total = 0;
- struct stat sb;
- struct runIOParams p;
-
- if (fstat(fd, &sb) < 0) {
- virReportSystemError(errno,
- _("Unable to access file descriptor %d path %s"),
- fd, path);
- goto cleanup;
- }
- p.isBlockDev = S_ISBLK(sb.st_mode);
- p.isDirect = O_DIRECT && (oflags & O_DIRECT);
-
- switch (oflags & O_ACCMODE) {
- case O_RDONLY:
- p.isWrite = false;
- p.fdin = fd;
- p.fdinname = path;
- p.fdout = STDOUT_FILENO;
- p.fdoutname = "stdout";
- break;
- case O_WRONLY:
- p.isWrite = true;
- p.fdin = STDIN_FILENO;
- p.fdinname = "stdin";
- p.fdout = fd;
- p.fdoutname = path;
- break;
-
- case O_RDWR:
- default:
- virReportSystemError(EINVAL,
- _("Unable to process file with flags %d"),
- (oflags & O_ACCMODE));
- goto cleanup;
- }
- /* To make the implementation simpler, we give up on any
- * attempt to use O_DIRECT in a non-trivial manner. */
- if (!p.isBlockDev && p.isDirect) {
- off_t off;
- if (p.isWrite) {
- if ((off = lseek(fd, 0, SEEK_END)) != 0) {
- virReportSystemError(off < 0 ? errno : EINVAL, "%s",
- _("O_DIRECT write needs empty seekable file"));
- goto cleanup;
- }
- } else if ((off = lseek(fd, 0, SEEK_CUR)) != 0) {
- virReportSystemError(off < 0 ? errno : EINVAL, "%s",
- _("O_DIRECT read needs entire seekable file"));
- goto cleanup;
- }
- }
- total = runIOCopy(p);
- if (total < 0)
- goto cleanup;
-
- /* Ensure all data is written */
- if (virFileDataSync(p.fdout) < 0) {
- if (errno != EINVAL && errno != EROFS) {
- /* fdatasync() may fail on some special FDs, e.g. pipes */
- virReportSystemError(errno, _("unable to fsync %s"), p.fdoutname);
- goto cleanup;
- }
- }
-
- ret = 0;
-
- cleanup:
- if (VIR_CLOSE(fd) < 0 &&
- ret == 0) {
- virReportSystemError(errno, _("Unable to close %s"), path);
- ret = -1;
- }
- return ret;
-}
-
static const char *program_name;
G_GNUC_NORETURN static void
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': [
+ 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
+
+ while (1) {
+ ssize_t got;
+
+ /* If we read with O_DIRECT from file we can't use saferead as
+ * it can lead to unaligned read after reading last bytes.
+ * If we write with O_DIRECT use should use saferead so that
+ * writes will be aligned.
+ * In other cases using saferead reduces number of syscalls.
+ */
+ if (!p.isWrite && p.isDirect) {
+ if ((got = read(p.fdin, buf, buflen)) < 0 &&
+ errno == EINTR)
+ continue;
+ } else {
+ got = saferead(p.fdin, buf, buflen);
+ }
+
+ if (got < 0) {
+ virReportSystemError(errno, _("Unable to read %s"), p.fdinname);
+ return -2;
+ }
+ if (got == 0)
+ break;
+
+ total += got;
+
+ /* handle last write size align in direct case */
+ if (got < buflen && p.isDirect && p.isWrite) {
+ ssize_t aligned_got = (got + alignMask) & ~alignMask;
+
+ memset(buf + got, 0, aligned_got - got);
+
+ if (safewrite(p.fdout, buf, aligned_got) < 0) {
+ virReportSystemError(errno, _("Unable to write %s"), p.fdoutname);
+ return -3;
+ }
+
+ if (!p.isBlockDev && ftruncate(p.fdout, total) < 0) {
+ virReportSystemError(errno, _("Unable to truncate %s"), p.fdoutname);
+ return -4;
+ }
+
+ break;
+ }
+
+ if (safewrite(p.fdout, buf, got) < 0) {
+ virReportSystemError(errno, _("Unable to write %s"), p.fdoutname);
+ return -3;
+ }
+ }
+ return total;
+}
+
+
+off_t
+runIO(const char *path, int fd, int oflags)
+{
+ int ret = -1;
+ off_t total = 0;
+ struct stat sb;
+ struct runIOParams p;
+
+ if (fstat(fd, &sb) < 0) {
+ virReportSystemError(errno,
+ _("Unable to access file descriptor %d path %s"),
+ fd, path);
+ goto cleanup;
+ }
+ p.isBlockDev = S_ISBLK(sb.st_mode);
+ p.isDirect = O_DIRECT && (oflags & O_DIRECT);
+
+ switch (oflags & O_ACCMODE) {
+ case O_RDONLY:
+ p.isWrite = false;
+ p.fdin = fd;
+ p.fdinname = path;
+ p.fdout = STDOUT_FILENO;
+ p.fdoutname = "stdout";
+ break;
+ case O_WRONLY:
+ p.isWrite = true;
+ p.fdin = STDIN_FILENO;
+ p.fdinname = "stdin";
+ p.fdout = fd;
+ p.fdoutname = path;
+ break;
+
+ case O_RDWR:
+ default:
+ virReportSystemError(EINVAL,
+ _("Unable to process file with flags %d"),
+ (oflags & O_ACCMODE));
+ goto cleanup;
+ }
+ /* To make the implementation simpler, we give up on any
+ * attempt to use O_DIRECT in a non-trivial manner. */
+ if (!p.isBlockDev && p.isDirect) {
+ off_t off;
+ if (p.isWrite) {
+ if ((off = lseek(fd, 0, SEEK_END)) != 0) {
+ virReportSystemError(off < 0 ? errno : EINVAL, "%s",
+ _("O_DIRECT write needs empty seekable file"));
+ goto cleanup;
+ }
+ } else if ((off = lseek(fd, 0, SEEK_CUR)) != 0) {
+ virReportSystemError(off < 0 ? errno : EINVAL, "%s",
+ _("O_DIRECT read needs entire seekable file"));
+ goto cleanup;
+ }
+ }
+ total = runIOCopy(p);
+ if (total < 0)
+ goto cleanup;
+
+ /* Ensure all data is written */
+ if (virFileDataSync(p.fdout) < 0) {
+ if (errno != EINVAL && errno != EROFS) {
+ /* fdatasync() may fail on some special FDs, e.g. pipes */
+ virReportSystemError(errno, _("unable to fsync %s"), p.fdoutname);
+ goto cleanup;
+ }
+ }
+
+ ret = 0;
+
+ cleanup:
+ if (VIR_CLOSE(fd) < 0 &&
+ ret == 0) {
+ virReportSystemError(errno, _("Unable to close %s"), path);
+ ret = -1;
+ }
+ return ret;
+}
diff --git a/src/util/virfile.h b/src/util/virfile.h
index b04386f6e6..0dc336e339 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -383,3 +383,5 @@ int virFileDataSync(int fd);
int virFileSetCOW(const char *path,
virTristateBool state);
+
+off_t runIO(const char *path, int fd, int oflags);
--
2.35.3On 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@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
--
|: 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 :|
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@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
>
On Fri, May 06, 2022 at 01:51:53PM +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. > > 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. The linker will discard all the .o files that don't have any functions referenced, so we'll not need most of what virt_util_lib depends on, just the virfile.o deps. 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 :|
On 5/6/22 1:54 PM, Daniel P. Berrangé wrote: > On Fri, May 06, 2022 at 01:51:53PM +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. >> >> 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. > > The linker will discard all the .o files that don't have any functions > referenced, so we'll not need most of what virt_util_lib depends on, > just the virfile.o deps. Aye aye, Thanks, Claudio
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.
Can you help me figure out where this need comes from, to ensure we are not missing anything else?
On my SUSE systems it's not needed, maybe some other OS needs more -l parameters?
Thanks,
Claudio
>
>> Signed-off-by: Claudio Fontana <cfontana@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
>
On Fri, May 06, 2022 at 01:48:33PM +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. > > Can you help me figure out where this need comes from, to ensure we are not missing anything else? > On my SUSE systems it's not needed, maybe some other OS needs more -l parameters? Just let the gitlab CI run, and if it passes all jobs, we're fine. 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 :|
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@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?
Thanks,
Claudio
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@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 :|
© 2016 - 2026 Red Hat, Inc.