On Tue, Jul 28, 2015 at 02:32:45AM +0200, Marc-André Lureau wrote:
> From: Marc-André Lureau <marcandre.lur...@gmail.com>
>
> As pointed out on the ML by Andrew Jones, glibc no longer permits
> creating POSIX shm on hugetlbfs directly. When given a hugetlbfs path,
> create a shareable file there.
>
> Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> ---
> contrib/ivshmem-server/ivshmem-server.c | 47
> ++++++++++++++++++++++++++++++++-
> contrib/ivshmem-server/ivshmem-server.h | 3 +--
> contrib/ivshmem-server/main.c | 5 ++--
> 3 files changed, 49 insertions(+), 6 deletions(-)
>
> diff --git a/contrib/ivshmem-server/ivshmem-server.c
> b/contrib/ivshmem-server/ivshmem-server.c
> index 972fda2..4bf774b 100644
> --- a/contrib/ivshmem-server/ivshmem-server.c
> +++ b/contrib/ivshmem-server/ivshmem-server.c
> @@ -11,6 +11,7 @@
> #include <sys/socket.h>
> #include <sys/un.h>
> #include <sys/eventfd.h>
> +#include <sys/vfs.h>
>
> #include "qemu-common.h"
> #include "qemu/queue.h"
> @@ -271,15 +272,59 @@ ivshmem_server_init(IvshmemServer *server, const char
> *unix_sock_path,
> return 0;
> }
>
> +#define HUGETLBFS_MAGIC 0x958458f6
> +
> +static long gethugepagesize(const char *path)
> +{
> + struct statfs fs;
> + int ret;
> +
> + do {
> + ret = statfs(path, &fs);
> + } while (ret != 0 && errno == EINTR);
> +
> + if (ret != 0) {
> + if (errno != ENOENT) {
> + fprintf(stderr, "cannot stat shm file %s: %s\n", path,
> + strerror(errno));
> + }
> + return -1;
> + }
> +
> + if (fs.f_type != HUGETLBFS_MAGIC) {
> + return -1;
> + }
> +
> + return fs.f_bsize;
> +}
> +
> +
> +
few extra lines here
> /* open shm, create and bind to the unix socket */
> int
> ivshmem_server_start(IvshmemServer *server)
> {
> struct sockaddr_un sun;
> int shm_fd, sock_fd, ret;
> + long hpagesize;
> + gchar *filename;
>
> /* open shm file */
> - shm_fd = shm_open(server->shm_path, O_CREAT|O_RDWR, S_IRWXU);
> + hpagesize = gethugepagesize(server->shm_path);
> + if (hpagesize > 0) {
> + if (server->shm_size < hpagesize) {
should be >, but isn't this forcing the shared memory to be less than
equal to the size of a single hugepage? I think we should allow up to
nr-pages * page-size.
Also, I'm not sure we want the dependency, but what about libhugetlbfs?
It has hugetlbfs_test_path
> + fprintf(stderr, "hugepage must be at least of size: %ld\n",
> + hpagesize);
> + return -1;
> + }
> + filename = g_strdup_printf("%s/ivshmem.XXXXXX", server->shm_path);
> + shm_fd = mkstemp(filename);
Shouldn't we change the perms for shm_fd to match the non-hugetlbfs
case? Or change the non-hugetlbfs case to match mkstemp? Actually,
probably the later, because I don't think we want the region to have
execute perms, or do we? Also, I guess the plan is to pass the hugetlbfs
file descriptor around if other host processes need to know where the
memory is, as we never allow a full path. Should we do the same for shm?
i.e. keep them anonymous too and always pass file descriptors?
> + unlink(filename);
> + g_free(filename);
> + } else {
> + shm_fd = shm_open(server->shm_path, O_CREAT|O_RDWR, S_IRWXU);
> + }
> +
> if (shm_fd < 0) {
> fprintf(stderr, "cannot open shm file %s: %s\n", server->shm_path,
> strerror(errno));
> diff --git a/contrib/ivshmem-server/ivshmem-server.h
> b/contrib/ivshmem-server/ivshmem-server.h
> index 2176d5e..e9b0e7a 100644
> --- a/contrib/ivshmem-server/ivshmem-server.h
> +++ b/contrib/ivshmem-server/ivshmem-server.h
> @@ -81,8 +81,7 @@ typedef struct IvshmemServer {
> * @server: A pointer to an uninitialized IvshmemServer structure
> * @unix_sock_path: The pointer to the unix socket file name
> * @shm_path: Path to the shared memory. The path corresponds to a
> POSIX
> - * shm name. To use a real file, for instance in a
> hugetlbfs,
> - * it is possible to use /../../abspath/to/file.
> + * shm name or a hugetlbfs mount point.
> * @shm_size: Size of shared memory
> * @n_vectors: Number of interrupt vectors per client
> * @verbose: True to enable verbose mode
> diff --git a/contrib/ivshmem-server/main.c b/contrib/ivshmem-server/main.c
> index 84ffc4d..cd8d9ed 100644
> --- a/contrib/ivshmem-server/main.c
> +++ b/contrib/ivshmem-server/main.c
> @@ -47,9 +47,8 @@ ivshmem_server_usage(const char *name, int code)
> " to listen to.\n"
> " Default=%s\n",
> IVSHMEM_SERVER_DEFAULT_UNIX_SOCK_PATH);
> fprintf(stderr, " -m <shm_path>: path to the shared memory.\n"
> - " The path corresponds to a POSIX shm name. To use
> a\n"
> - " real file, for instance in a hugetlbfs, use\n"
> - " /../../abspath/to/file.\n"
> + " The path corresponds to a POSIX shm name or a\n"
> + " hugetlbfs mount point.\n"
> " default=%s\n", IVSHMEM_SERVER_DEFAULT_SHM_PATH);
> fprintf(stderr, " -l <size>: size of shared memory in bytes. The
> suffix\n"
> " K, M and G can be used (ex: 1K means 1024).\n"
> --
> 2.4.3
>
>
Thanks,
drew
