1. Patchew
  2. QEMU
  3. [PATCH 00/41] Misc cleanups
  4. View patch

[PATCH 20/41] include: move qemu_msync() to osdep

marcandre.lureau@redhat.com posted 41 patches 3 years, 9 months ago
Maintainers: Richard Henderson <richard.henderson@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, Warner Losh <imp@bsdimp.com>, Kyle Evans <kevans@freebsd.org>, Stefan Hajnoczi <stefanha@redhat.com>, Beniamino Galvani <b.galvani@gmail.com>, Peter Maydell <peter.maydell@linaro.org>, Antony Pavlov <antonynpavlov@gmail.com>, Igor Mitsyanko <i.mitsyanko@gmail.com>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Yanan Wang <wangyanan55@huawei.com>, Markus Armbruster <armbru@redhat.com>, Laurent Vivier <laurent@vivier.eu>, Juan Quintela <quintela@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Michael Roth <michael.roth@amd.com>, Eric Blake <eblake@redhat.com>, Vladimir Sementsov-Ogievskiy <v.sementsov-og@mail.ru>, Konstantin Kostiuk <kkostiuk@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Fam Zheng <fam@euphon.net>, Taylor Simpson <tsimpson@quicinc.com>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Aurelien Jarno <aurelien@aurel32.net>, Jiaxun Yang <jiaxun.yang@flygoat.com>, Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>, Chris Wulff <crwulff@gmail.com>, Marek Vasut <marex@denx.de>, Stafford Horne <shorne@gmail.com>, "Cédric Le Goater" <clg@kaod.org>, Daniel Henrique Barboza <danielhb413@gmail.com>, David Gibson <david@gibson.dropbear.id.au>, Greg Kurz <groug@kaod.org>, Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Bin Meng <bin.meng@windriver.com>, Yoshinori Sato <ysato@users.sourceforge.jp>, David Hildenbrand <david@redhat.com>, Cornelia Huck <cohuck@redhat.com>, Thomas Huth <thuth@redhat.com>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Artyom Tarasenko <atar4qemu@gmail.com>, Bastian Koppelmann <kbastian@mail.uni-paderborn.de>, Max Filippov <jcmvbkbc@gmail.com>, "Alex Bennée" <alex.bennee@linaro.org>, John Snow <jsnow@redhat.com>, Alexander Bulekov <alxndr@bu.edu>, Bandan Das <bsd@redhat.com>, Darren Kenny <darren.kenny@oracle.com>, Qiuhao Li <Qiuhao.Li@outlook.com>, Akihiko Odaki <akihiko.odaki@gmail.com>, Gerd Hoffmann <kraxel@redhat.com>, Stefan Weil <sw@weilnetz.de>
There is a newer version of this series
[PATCH 20/41] include: move qemu_msync() to osdep
Posted by marcandre.lureau@redhat.com 3 years, 9 months ago 
From: Marc-André Lureau <marcandre.lureau@redhat.com>

The implementation depends on the OS. (and longer-term goal is to move
cutils to a common subproject)

Signed-off-by: Marc-André Lureau <marcandre.lureau@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(-)

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
Re: [PATCH 20/41] include: move qemu_msync() to osdep
Posted by Daniel P. Berrangé 3 years, 9 months ago 
On Wed, Apr 20, 2022 at 05:26:03PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@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) ?

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.lureau@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é <berrange@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 :|
Re: [PATCH 20/41] include: move qemu_msync() to osdep
Posted by Marc-André Lureau 3 years, 9 months ago 
Hi

On Wed, Apr 20, 2022 at 7:33 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Wed, Apr 20, 2022 at 05:26:03PM +0400, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@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.lureau@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é <berrange@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 :|
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.