From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37191) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZcUvb-0002Yh-2K for qemu-devel@nongnu.org; Thu, 17 Sep 2015 04:52:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZcUvX-00066k-Og for qemu-devel@nongnu.org; Thu, 17 Sep 2015 04:52:26 -0400 Received: from relay.parallels.com ([195.214.232.42]:46612) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZcUvX-00066X-Bu for qemu-devel@nongnu.org; Thu, 17 Sep 2015 04:52:23 -0400 Message-ID: <55FA7F39.5080809@virtuozzo.com> Date: Thu, 17 Sep 2015 11:52:09 +0300 From: Vladimir Sementsov-Ogievskiy MIME-Version: 1.0 References: <1442333283-13119-1-git-send-email-marcandre.lureau@redhat.com> <1442333283-13119-35-git-send-email-marcandre.lureau@redhat.com> <55F993D8.60400@virtuozzo.com> <1344358616.12792838.1442420071714.JavaMail.zimbra@redhat.com> In-Reply-To: <1344358616.12792838.1442420071714.JavaMail.zimbra@redhat.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH v3 34/46] ivshmem-server: fix hugetlbfs support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?TWFyYy1BbmRyw6kgTHVyZWF1?= Cc: marcandre lureau , drjones@redhat.com, cam@cs.ualberta.ca, qemu-devel@nongnu.org, stefanha@redhat.com On 16.09.2015 19:14, Marc-André Lureau wrote: > > ----- Original Message ----- >> On 15.09.2015 19:07, marcandre.lureau@redhat.com wrote: >>> From: Marc-André Lureau >>> >>> 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 >>> --- >>> contrib/ivshmem-server/ivshmem-server.c | 40 >>> ++++++++++++++++++++++++++++++++- >>> contrib/ivshmem-server/ivshmem-server.h | 3 +-- >>> contrib/ivshmem-server/main.c | 5 ++--- >>> 3 files changed, 42 insertions(+), 6 deletions(-) >>> >>> diff --git a/contrib/ivshmem-server/ivshmem-server.c >>> b/contrib/ivshmem-server/ivshmem-server.c >>> index 972fda2..51264b4 100644 >>> --- a/contrib/ivshmem-server/ivshmem-server.c >>> +++ b/contrib/ivshmem-server/ivshmem-server.c >>> @@ -11,6 +11,7 @@ >>> #include >>> #include >>> #include >>> +#include >>> >>> #include "qemu-common.h" >>> #include "qemu/queue.h" >>> @@ -271,15 +272,52 @@ 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) { >> should it be silent? > Well, the same given path may be shm name or a hugefs. I guess we could use a different argument instead and make sure that it is hugefs. At the minimum, I understand we may want a warning here. > >>> + return -1; >>> + } >>> + >>> + return fs.f_bsize; >>> +} >>> + >>> /* 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) { >> filename is used only in this block, it may be defined here. > ok > >>> + filename = g_strdup_printf("%s/ivshmem.XXXXXX", server->shm_path); >>> + shm_fd = mkstemp(filename); >>> + 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 : 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 of shared memory in bytes. The >>> suffix\n" >>> " K, M and G can be used (ex: 1K means 1024).\n" >> >> -- >> Best regards, >> Vladimir >> * now, @virtuozzo.com instead of @parallels.com. Sorry for this >> inconvenience. >> >> with these tiny fixes: Reviewed-by: Vladimir Sementsov-Ogievskiy -- Best regards, Vladimir * now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.