Re: [Qemu-devel] [PATCH] hostmem-memfd: add checks before adding hostmem-memfd & properties

[Paolo Bonzini]

On 06/09/2018 18:14, Marc-André Lureau wrote:
> Run some memfd-related checks before registering hostmem-memfd &
> various properties. This will help libvirt to figure out what the host
> is supposed to be capable of.
> 
> qemu_memfd_check() is changed to a less optimized version, since it is
> used with various flags, it no longer caches the result.
> 
> Signed-off-by: Marc-André Lureau <address@hidden>
> ---
>  include/qemu/memfd.h     | 18 +++++++++++++++++-
>  backends/hostmem-memfd.c | 32 +++++++++++++++++++-------------
>  tests/vhost-user-test.c  |  6 +++---
>  util/memfd.c             | 35 ++++++-----------------------------
>  4 files changed, 45 insertions(+), 46 deletions(-)
> 
> diff --git a/include/qemu/memfd.h b/include/qemu/memfd.h
> index 49e79634da..d551c28b68 100644
> --- a/include/qemu/memfd.h
> +++ b/include/qemu/memfd.h
> @@ -16,12 +16,28 @@
>  #define F_SEAL_WRITE    0x0008  /* prevent writes */
>  #endif
>  
> +#ifndef MFD_CLOEXEC
> +#define MFD_CLOEXEC 0x0001U
> +#endif
> +
> +#ifndef MFD_ALLOW_SEALING
> +#define MFD_ALLOW_SEALING 0x0002U
> +#endif
> +
> +#ifndef MFD_HUGETLB
> +#define MFD_HUGETLB 0x0004U
> +#endif
> +
> +#ifndef MFD_HUGE_SHIFT
> +#define MFD_HUGE_SHIFT 26
> +#endif
> +
>  int qemu_memfd_create(const char *name, size_t size, bool hugetlb,
>                        uint64_t hugetlbsize, unsigned int seals, Error 
> **errp);
>  bool qemu_memfd_alloc_check(void);
>  void *qemu_memfd_alloc(const char *name, size_t size, unsigned int seals,
>                         int *fd, Error **errp);
>  void qemu_memfd_free(void *ptr, size_t size, int fd);
> -bool qemu_memfd_check(void);
> +bool qemu_memfd_check(unsigned int flags);
>  
>  #endif /* QEMU_MEMFD_H */
> diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c
> index 7184918112..ccf70a0694 100644
> --- a/backends/hostmem-memfd.c
> +++ b/backends/hostmem-memfd.c
> @@ -138,18 +138,22 @@ memfd_backend_class_init(ObjectClass *oc, void *data)
>  
>      bc->alloc = memfd_backend_memory_alloc;
>  
> -    object_class_property_add_bool(oc, "hugetlb",
> -                                   memfd_backend_get_hugetlb,
> -                                   memfd_backend_set_hugetlb,
> -                                   &error_abort);
> -    object_class_property_add(oc, "hugetlbsize", "int",
> -                              memfd_backend_get_hugetlbsize,
> -                              memfd_backend_set_hugetlbsize,
> -                              NULL, NULL, &error_abort);
> -    object_class_property_add_bool(oc, "seal",
> -                                   memfd_backend_get_seal,
> -                                   memfd_backend_set_seal,
> -                                   &error_abort);
> +    if (qemu_memfd_check(MFD_HUGETLB)) {
> +        object_class_property_add_bool(oc, "hugetlb",
> +                                       memfd_backend_get_hugetlb,
> +                                       memfd_backend_set_hugetlb,
> +                                       &error_abort);
> +        object_class_property_add(oc, "hugetlbsize", "int",
> +                                  memfd_backend_get_hugetlbsize,
> +                                  memfd_backend_set_hugetlbsize,
> +                                  NULL, NULL, &error_abort);
> +    }
> +    if (qemu_memfd_check(MFD_ALLOW_SEALING)) {
> +        object_class_property_add_bool(oc, "seal",
> +                                       memfd_backend_get_seal,
> +                                       memfd_backend_set_seal,
> +                                       &error_abort);
> +    }
>  }
>  
>  static const TypeInfo memfd_backend_info = {
> @@ -162,7 +166,9 @@ static const TypeInfo memfd_backend_info = {
>  
>  static void register_types(void)
>  {
> -    type_register_static(&memfd_backend_info);
> +    if (qemu_memfd_check(0)) {
> +        type_register_static(&memfd_backend_info);
> +    }
>  }
>  
>  type_init(register_types);
> diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
> index 716aff7153..45d58d8ea2 100644
> --- a/tests/vhost-user-test.c
> +++ b/tests/vhost-user-test.c
> @@ -169,7 +169,7 @@ static char *get_qemu_cmd(TestServer *s,
>                            int mem, enum test_memfd memfd, const char 
> *mem_path,
>                            const char *chr_opts, const char *extra)
>  {
> -    if (memfd == TEST_MEMFD_AUTO && qemu_memfd_check()) {
> +    if (memfd == TEST_MEMFD_AUTO && qemu_memfd_check(0)) {
>          memfd = TEST_MEMFD_YES;
>      }
>  
> @@ -903,7 +903,7 @@ static void test_multiqueue(void)
>      s->queues = 2;
>      test_server_listen(s);
>  
> -    if (qemu_memfd_check()) {
> +    if (qemu_memfd_check(0)) {
>          cmd = g_strdup_printf(
>              QEMU_CMD_MEMFD QEMU_CMD_CHR QEMU_CMD_NETDEV ",queues=%d "
>              "-device virtio-net-pci,netdev=net0,mq=on,vectors=%d",
> @@ -963,7 +963,7 @@ int main(int argc, char **argv)
>      /* run the main loop thread so the chardev may operate */
>      thread = g_thread_new(NULL, thread_function, loop);
>  
> -    if (qemu_memfd_check()) {
> +    if (qemu_memfd_check(0)) {
>          qtest_add_data_func("/vhost-user/read-guest-mem/memfd",
>                              GINT_TO_POINTER(TEST_MEMFD_YES),
>                              test_read_guest_mem);
> diff --git a/util/memfd.c b/util/memfd.c
> index d248a53c3c..c4bd076182 100644
> --- a/util/memfd.c
> +++ b/util/memfd.c
> @@ -45,22 +45,6 @@ static int memfd_create(const char *name, unsigned int 
> flags)
>  }
>  #endif
>  
> -#ifndef MFD_CLOEXEC
> -#define MFD_CLOEXEC 0x0001U
> -#endif
> -
> -#ifndef MFD_ALLOW_SEALING
> -#define MFD_ALLOW_SEALING 0x0002U
> -#endif
> -
> -#ifndef MFD_HUGETLB
> -#define MFD_HUGETLB 0x0004U
> -#endif
> -
> -#ifndef MFD_HUGE_SHIFT
> -#define MFD_HUGE_SHIFT 26
> -#endif
> -
>  int qemu_memfd_create(const char *name, size_t size, bool hugetlb,
>                        uint64_t hugetlbsize, unsigned int seals, Error **errp)
>  {
> @@ -200,23 +184,16 @@ bool qemu_memfd_alloc_check(void)
>   *
>   * Check if host supports memfd.
>   */
> -bool qemu_memfd_check(void)
> +bool qemu_memfd_check(unsigned int flags)
>  {
>  #ifdef CONFIG_LINUX
> -    static int memfd_check = MEMFD_TODO;
> +    int mfd = memfd_create("test", flags);
>  
> -    if (memfd_check == MEMFD_TODO) {
> -        int mfd = memfd_create("test", 0);
> -        if (mfd >= 0) {
> -            memfd_check = MEMFD_OK;
> -            close(mfd);
> -        } else {
> -            memfd_check = MEMFD_KO;
> -        }
> +    if (mfd >= 0) {
> +        close(mfd);
> +        return true;
>      }
> +#endif
>  
> -    return memfd_check == MEMFD_OK;
> -#else
>      return false;
> -#endif
>  }
> 

Queued, thanks.

Paolo