Hi
On Wed, Apr 20, 2022 at 7:33 PM Daniel P. Berrangé <berra...@redhat.com> wrote:
>
> On Wed, Apr 20, 2022 at 05:26:03PM +0400, marcandre.lur...@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lur...@redhat.com>
> >
> > The implementation depends on the OS. (and longer-term goal is to move
> > cutils to a common subproject)
>
> Common with what other thing(s) ?
This is down the line, but my initial focus was to make qemu-ga a
subproject() (I have a wip/poc branch, so it is possible!)
For that I moved all qapi/qmp-related stuff in another subproject().
We could further split the qapi/qmp subproject (qemu-common?). This
"common" subproject could be used by libvhost-user for example.
Various helper binaries could also become subprojects at some point. I
haven't looked at all possibilities, there is already a lot of
preliminary cleanup to take care of :)
thanks
>
> IMHO alot of cutils should just go away in favour of using more glib
> APIs, and/or be split up, since its a random bag of largely unrelated
> bits.
>
>
>
> > Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> > ---
> > include/qemu/cutils.h | 1 -
> > include/qemu/osdep.h | 13 +++++++++++++
> > util/cutils.c | 38 --------------------------------------
> > util/oslib-posix.c | 18 ++++++++++++++++++
> > util/oslib-win32.c | 10 ++++++++++
> > 5 files changed, 41 insertions(+), 39 deletions(-)
>
> Reviewed-by: Daniel P. Berrangé <berra...@redhat.com>
>
> >
> > diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h
> > index e873bad36674..fb47ec931876 100644
> > --- a/include/qemu/cutils.h
> > +++ b/include/qemu/cutils.h
> > @@ -130,7 +130,6 @@ const char *qemu_strchrnul(const char *s, int c);
> > #endif
> > time_t mktimegm(struct tm *tm);
> > int qemu_fdatasync(int fd);
> > -int qemu_msync(void *addr, size_t length, int fd);
> > int qemu_parse_fd(const char *param);
> > int qemu_strtoi(const char *nptr, const char **endptr, int base,
> > int *result);
> > diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> > index 14b6b65a5fa9..bf4f75dcde8f 100644
> > --- a/include/qemu/osdep.h
> > +++ b/include/qemu/osdep.h
> > @@ -641,6 +641,19 @@ static inline void qemu_reset_optind(void)
> > #endif
> > }
> >
> > +/**
> > + * Sync changes made to the memory mapped file back to the backing
> > + * storage. For POSIX compliant systems this will fallback
> > + * to regular msync call. Otherwise it will trigger whole file sync
> > + * (including the metadata case there is no support to skip that otherwise)
> > + *
> > + * @addr - start of the memory area to be synced
> > + * @length - length of the are to be synced
> > + * @fd - file descriptor for the file to be synced
> > + * (mandatory only for POSIX non-compliant systems)
> > + */
> > +int qemu_msync(void *addr, size_t length, int fd);
> > +
> > /**
> > * qemu_get_host_name:
> > * @errp: Error object
> > diff --git a/util/cutils.c b/util/cutils.c
> > index a01a3a754049..c0775bb53c29 100644
> > --- a/util/cutils.c
> > +++ b/util/cutils.c
> > @@ -160,44 +160,6 @@ int qemu_fdatasync(int fd)
> > #endif
> > }
> >
> > -/**
> > - * Sync changes made to the memory mapped file back to the backing
> > - * storage. For POSIX compliant systems this will fallback
> > - * to regular msync call. Otherwise it will trigger whole file sync
> > - * (including the metadata case there is no support to skip that otherwise)
> > - *
> > - * @addr - start of the memory area to be synced
> > - * @length - length of the are to be synced
> > - * @fd - file descriptor for the file to be synced
> > - * (mandatory only for POSIX non-compliant systems)
> > - */
> > -int qemu_msync(void *addr, size_t length, int fd)
> > -{
> > -#ifdef CONFIG_POSIX
> > - size_t align_mask = ~(qemu_real_host_page_size() - 1);
> > -
> > - /**
> > - * There are no strict reqs as per the length of mapping
> > - * to be synced. Still the length needs to follow the address
> > - * alignment changes. Additionally - round the size to the multiple
> > - * of PAGE_SIZE
> > - */
> > - length += ((uintptr_t)addr & (qemu_real_host_page_size() - 1));
> > - length = (length + ~align_mask) & align_mask;
> > -
> > - addr = (void *)((uintptr_t)addr & align_mask);
> > -
> > - return msync(addr, length, MS_SYNC);
> > -#else /* CONFIG_POSIX */
> > - /**
> > - * Perform the sync based on the file descriptor
> > - * The sync range will most probably be wider than the one
> > - * requested - but it will still get the job done
> > - */
> > - return qemu_fdatasync(fd);
> > -#endif /* CONFIG_POSIX */
> > -}
> > -
> > static int64_t suffix_mul(char suffix, int64_t unit)
> > {
> > switch (qemu_toupper(suffix)) {
> > diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> > index c471c5bc9f8d..161f1123259f 100644
> > --- a/util/oslib-posix.c
> > +++ b/util/oslib-posix.c
> > @@ -950,3 +950,21 @@ int fcntl_setfl(int fd, int flag)
> > }
> > return 0;
> > }
> > +
> > +int qemu_msync(void *addr, size_t length, int fd)
> > +{
> > + size_t align_mask = ~(qemu_real_host_page_size() - 1);
> > +
> > + /**
> > + * There are no strict reqs as per the length of mapping
> > + * to be synced. Still the length needs to follow the address
> > + * alignment changes. Additionally - round the size to the multiple
> > + * of PAGE_SIZE
> > + */
> > + length += ((uintptr_t)addr & (qemu_real_host_page_size() - 1));
> > + length = (length + ~align_mask) & align_mask;
> > +
> > + addr = (void *)((uintptr_t)addr & align_mask);
> > +
> > + return msync(addr, length, MS_SYNC);
> > +}
> > diff --git a/util/oslib-win32.c b/util/oslib-win32.c
> > index f38b06914e12..1e05c316b311 100644
> > --- a/util/oslib-win32.c
> > +++ b/util/oslib-win32.c
> > @@ -596,3 +596,13 @@ size_t qemu_get_host_physmem(void)
> > }
> > return 0;
> > }
> > +
> > +int qemu_msync(void *addr, size_t length, int fd)
> > +{
> > + /**
> > + * Perform the sync based on the file descriptor
> > + * The sync range will most probably be wider than the one
> > + * requested - but it will still get the job done
> > + */
> > + return qemu_fdatasync(fd);
> > +}
> > --
> > 2.35.1.693.g805e0a68082a
> >
> >
>
> 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 :|
>
