[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/2] libvhost-user: replace qemu/memfd.h usage
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [PATCH 2/2] libvhost-user: replace qemu/memfd.h usage |
Date: |
Tue, 24 Nov 2020 11:54:05 +0000 |
User-agent: |
Mutt/1.14.6 (2020-07-11) |
* marcandre.lureau@redhat.com (marcandre.lureau@redhat.com) wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Undo the damage from commit 5f9ff1eff3 ("libvhost-user: Support tracking
> inflight I/O in shared memory") which introduced glib dependency through
> osdep.h inclusion.
>
> libvhost-user.c tries to stay free from glib usage.
>
> Use glibc memfd_create directly when available (assumed so when
> MFD_ALLOW_SEALING is defined).
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> contrib/libvhost-user/libvhost-user.c | 50 +++++++++++++++++++++++----
> 1 file changed, 43 insertions(+), 7 deletions(-)
>
> diff --git a/contrib/libvhost-user/libvhost-user.c
> b/contrib/libvhost-user/libvhost-user.c
> index 1c1cfbf1e7..805521859d 100644
> --- a/contrib/libvhost-user/libvhost-user.c
> +++ b/contrib/libvhost-user/libvhost-user.c
> @@ -42,8 +42,6 @@
> #endif
>
> #include "qemu/atomic.h"
> -#include "qemu/osdep.h"
> -#include "qemu/memfd.h"
>
> #include "libvhost-user.h"
>
> @@ -1615,11 +1613,45 @@ vu_inflight_queue_size(uint16_t queue_size)
> sizeof(uint16_t), INFLIGHT_ALIGNMENT);
> }
>
> +#ifdef MFD_ALLOW_SEALING
> +static void *
> +memfd_alloc(const char *name, size_t size, unsigned int flags, int *fd)
> +{
> + void *ptr;
> + int ret;
> +
> + *fd = memfd_create(name, MFD_ALLOW_SEALING);
> + if (*fd < 0) {
> + return NULL;
> + }
> +
> + ret = ftruncate(*fd, size);
Do you need to do any of the page alignment?
> + if (ret < 0) {
> + close(*fd);
> + return NULL;
> + }
> +
> + ret = fcntl(*fd, F_ADD_SEALS, F_SEAL_GROW | F_SEAL_SHRINK | F_SEAL_SEAL);
I think you'd intended to use the 'flags' parameter there.
> + if (ret < 0) {
> + close(*fd);
> + return NULL;
> + }
> +
> + ptr = mmap(0, size, PROT_READ | PROT_WRITE, MAP_SHARED, *fd, 0);
> + if (ptr == MAP_FAILED) {
> + close(*fd);
> + return NULL;
> + }
> +
> + return ptr;
> +}
> +#endif
> +
> static bool
> vu_get_inflight_fd(VuDev *dev, VhostUserMsg *vmsg)
> {
> - int fd;
> - void *addr;
> + int fd = -1;
> + void *addr = NULL;
> uint64_t mmap_size;
> uint16_t num_queues, queue_size;
>
> @@ -1637,9 +1669,13 @@ vu_get_inflight_fd(VuDev *dev, VhostUserMsg *vmsg)
>
> mmap_size = vu_inflight_queue_size(queue_size) * num_queues;
>
> - addr = qemu_memfd_alloc("vhost-inflight", mmap_size,
> - F_SEAL_GROW | F_SEAL_SHRINK | F_SEAL_SEAL,
> - &fd, NULL);
> +#ifdef MFD_ALLOW_SEALING
> + addr = memfd_alloc("vhost-inflight", mmap_size,
> + F_SEAL_GROW | F_SEAL_SHRINK | F_SEAL_SEAL,
> + &fd);
> +#else
> + vu_panic(dev, "Not implemented: memfd support is missing");
Should there be an ifdef somewhere on the declared features, so it
doesn't get this far because it wont negotiate the feature?
Dave
> +#endif
>
> if (!addr) {
> vu_panic(dev, "Failed to alloc vhost inflight area");
> --
> 2.29.0
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK