From mboxrd@z Thu Jan 1 00:00:00 1970 From: Xiao Guangrong Subject: Re: [PATCH v7 09/35] exec: allow file_ram_alloc to work on file Date: Tue, 3 Nov 2015 11:56:38 +0800 Message-ID: <56383076.5090400@linux.intel.com> References: <1446455617-129562-1-git-send-email-guangrong.xiao@linux.intel.com> <1446455617-129562-10-git-send-email-guangrong.xiao@linux.intel.com> <5637D1C0.5090006@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: gleb@kernel.org, mtosatti@redhat.com, stefanha@redhat.com, mst@redhat.com, rth@twiddle.net, ehabkost@redhat.com, dan.j.williams@intel.com, kvm@vger.kernel.org, qemu-devel@nongnu.org, vsementsov@virtuozzo.com, eblake@redhat.com To: Paolo Bonzini , imammedo@redhat.com Return-path: Received: from mga03.intel.com ([134.134.136.65]:43077 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751336AbbKCEDY (ORCPT ); Mon, 2 Nov 2015 23:03:24 -0500 In-Reply-To: <5637D1C0.5090006@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 11/03/2015 05:12 AM, Paolo Bonzini wrote: > > > On 02/11/2015 10:13, Xiao Guangrong wrote: >> Currently, file_ram_alloc() only works on directory - it creates a file >> under @path and do mmap on it >> >> This patch tries to allow it to work on file directly, if @path is a >> directory it works as before, otherwise it treats @path as the target >> file then directly allocate memory from it >> >> Signed-off-by: Xiao Guangrong >> --- >> exec.c | 80 ++++++++++++++++++++++++++++++++++++++++++------------------------ >> 1 file changed, 51 insertions(+), 29 deletions(-) >> >> diff --git a/exec.c b/exec.c >> index 9075f4d..db0fdaf 100644 >> --- a/exec.c >> +++ b/exec.c >> @@ -1174,14 +1174,60 @@ void qemu_mutex_unlock_ramlist(void) >> } >> >> #ifdef __linux__ >> +static bool path_is_dir(const char *path) >> +{ >> + struct stat fs; >> + >> + return stat(path, &fs) == 0 && S_ISDIR(fs.st_mode); >> +} >> + >> +static int open_ram_file_path(RAMBlock *block, const char *path, size_t size) >> +{ >> + char *filename; >> + char *sanitized_name; >> + char *c; >> + int fd; >> + >> + if (!path_is_dir(path)) { >> + int flags = (block->flags & RAM_SHARED) ? O_RDWR : O_RDONLY; >> + >> + flags |= O_EXCL; >> + return open(path, flags); >> + } >> + >> + /* Make name safe to use with mkstemp by replacing '/' with '_'. */ >> + sanitized_name = g_strdup(memory_region_name(block->mr)); >> + for (c = sanitized_name; *c != '\0'; c++) { >> + if (*c == '/') { >> + *c = '_'; >> + } >> + } >> + filename = g_strdup_printf("%s/qemu_back_mem.%s.XXXXXX", path, >> + sanitized_name); >> + g_free(sanitized_name); >> + fd = mkstemp(filename); >> + if (fd >= 0) { >> + unlink(filename); >> + /* >> + * ftruncate is not supported by hugetlbfs in older >> + * hosts, so don't bother bailing out on errors. >> + * If anything goes wrong with it under other filesystems, >> + * mmap will fail. >> + */ >> + if (ftruncate(fd, size)) { >> + perror("ftruncate"); >> + } >> + } >> + g_free(filename); >> + >> + return fd; >> +} >> + >> static void *file_ram_alloc(RAMBlock *block, >> ram_addr_t memory, >> const char *path, >> Error **errp) >> { >> - char *filename; >> - char *sanitized_name; >> - char *c; >> void *area; >> int fd; >> uint64_t pagesize; >> @@ -1212,38 +1258,14 @@ static void *file_ram_alloc(RAMBlock *block, >> goto error; >> } >> >> - /* Make name safe to use with mkstemp by replacing '/' with '_'. */ >> - sanitized_name = g_strdup(memory_region_name(block->mr)); >> - for (c = sanitized_name; *c != '\0'; c++) { >> - if (*c == '/') >> - *c = '_'; >> - } >> - >> - filename = g_strdup_printf("%s/qemu_back_mem.%s.XXXXXX", path, >> - sanitized_name); >> - g_free(sanitized_name); >> + memory = ROUND_UP(memory, pagesize); >> >> - fd = mkstemp(filename); >> + fd = open_ram_file_path(block, path, memory); >> if (fd < 0) { >> error_setg_errno(errp, errno, >> "unable to create backing store for path %s", path); >> - g_free(filename); >> goto error; >> } >> - unlink(filename); >> - g_free(filename); >> - >> - memory = ROUND_UP(memory, pagesize); >> - >> - /* >> - * ftruncate is not supported by hugetlbfs in older >> - * hosts, so don't bother bailing out on errors. >> - * If anything goes wrong with it under other filesystems, >> - * mmap will fail. >> - */ >> - if (ftruncate(fd, memory)) { >> - perror("ftruncate"); >> - } >> >> area = qemu_ram_mmap(fd, memory, pagesize, block->flags & RAM_SHARED); >> if (area == MAP_FAILED) { >> > > I was going to send tomorrow a pull request for a similar patch, > "backends/hostmem-file: Allow to specify full pathname for backing file". > > The main difference seems to be your usage of O_EXCL. Can you explain > why you added it? It' used if we pass a block device as a NVDIMM backend memory: O_EXCL can be used without O_CREAT if pathname refers to a block device. If the block device is in use by the system (e.g., mounted), open() fails with the error EBUSY From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50839) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZtSoi-0005TU-7P for qemu-devel@nongnu.org; Mon, 02 Nov 2015 23:03:29 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZtSof-0002nq-0I for qemu-devel@nongnu.org; Mon, 02 Nov 2015 23:03:28 -0500 Received: from mga01.intel.com ([192.55.52.88]:13474) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZtSoe-0002nj-LL for qemu-devel@nongnu.org; Mon, 02 Nov 2015 23:03:24 -0500 References: <1446455617-129562-1-git-send-email-guangrong.xiao@linux.intel.com> <1446455617-129562-10-git-send-email-guangrong.xiao@linux.intel.com> <5637D1C0.5090006@redhat.com> From: Xiao Guangrong Message-ID: <56383076.5090400@linux.intel.com> Date: Tue, 3 Nov 2015 11:56:38 +0800 MIME-Version: 1.0 In-Reply-To: <5637D1C0.5090006@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v7 09/35] exec: allow file_ram_alloc to work on file List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , imammedo@redhat.com Cc: vsementsov@virtuozzo.com, ehabkost@redhat.com, kvm@vger.kernel.org, mst@redhat.com, gleb@kernel.org, mtosatti@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, dan.j.williams@intel.com, rth@twiddle.net On 11/03/2015 05:12 AM, Paolo Bonzini wrote: > > > On 02/11/2015 10:13, Xiao Guangrong wrote: >> Currently, file_ram_alloc() only works on directory - it creates a file >> under @path and do mmap on it >> >> This patch tries to allow it to work on file directly, if @path is a >> directory it works as before, otherwise it treats @path as the target >> file then directly allocate memory from it >> >> Signed-off-by: Xiao Guangrong >> --- >> exec.c | 80 ++++++++++++++++++++++++++++++++++++++++++------------------------ >> 1 file changed, 51 insertions(+), 29 deletions(-) >> >> diff --git a/exec.c b/exec.c >> index 9075f4d..db0fdaf 100644 >> --- a/exec.c >> +++ b/exec.c >> @@ -1174,14 +1174,60 @@ void qemu_mutex_unlock_ramlist(void) >> } >> >> #ifdef __linux__ >> +static bool path_is_dir(const char *path) >> +{ >> + struct stat fs; >> + >> + return stat(path, &fs) == 0 && S_ISDIR(fs.st_mode); >> +} >> + >> +static int open_ram_file_path(RAMBlock *block, const char *path, size_t size) >> +{ >> + char *filename; >> + char *sanitized_name; >> + char *c; >> + int fd; >> + >> + if (!path_is_dir(path)) { >> + int flags = (block->flags & RAM_SHARED) ? O_RDWR : O_RDONLY; >> + >> + flags |= O_EXCL; >> + return open(path, flags); >> + } >> + >> + /* Make name safe to use with mkstemp by replacing '/' with '_'. */ >> + sanitized_name = g_strdup(memory_region_name(block->mr)); >> + for (c = sanitized_name; *c != '\0'; c++) { >> + if (*c == '/') { >> + *c = '_'; >> + } >> + } >> + filename = g_strdup_printf("%s/qemu_back_mem.%s.XXXXXX", path, >> + sanitized_name); >> + g_free(sanitized_name); >> + fd = mkstemp(filename); >> + if (fd >= 0) { >> + unlink(filename); >> + /* >> + * ftruncate is not supported by hugetlbfs in older >> + * hosts, so don't bother bailing out on errors. >> + * If anything goes wrong with it under other filesystems, >> + * mmap will fail. >> + */ >> + if (ftruncate(fd, size)) { >> + perror("ftruncate"); >> + } >> + } >> + g_free(filename); >> + >> + return fd; >> +} >> + >> static void *file_ram_alloc(RAMBlock *block, >> ram_addr_t memory, >> const char *path, >> Error **errp) >> { >> - char *filename; >> - char *sanitized_name; >> - char *c; >> void *area; >> int fd; >> uint64_t pagesize; >> @@ -1212,38 +1258,14 @@ static void *file_ram_alloc(RAMBlock *block, >> goto error; >> } >> >> - /* Make name safe to use with mkstemp by replacing '/' with '_'. */ >> - sanitized_name = g_strdup(memory_region_name(block->mr)); >> - for (c = sanitized_name; *c != '\0'; c++) { >> - if (*c == '/') >> - *c = '_'; >> - } >> - >> - filename = g_strdup_printf("%s/qemu_back_mem.%s.XXXXXX", path, >> - sanitized_name); >> - g_free(sanitized_name); >> + memory = ROUND_UP(memory, pagesize); >> >> - fd = mkstemp(filename); >> + fd = open_ram_file_path(block, path, memory); >> if (fd < 0) { >> error_setg_errno(errp, errno, >> "unable to create backing store for path %s", path); >> - g_free(filename); >> goto error; >> } >> - unlink(filename); >> - g_free(filename); >> - >> - memory = ROUND_UP(memory, pagesize); >> - >> - /* >> - * ftruncate is not supported by hugetlbfs in older >> - * hosts, so don't bother bailing out on errors. >> - * If anything goes wrong with it under other filesystems, >> - * mmap will fail. >> - */ >> - if (ftruncate(fd, memory)) { >> - perror("ftruncate"); >> - } >> >> area = qemu_ram_mmap(fd, memory, pagesize, block->flags & RAM_SHARED); >> if (area == MAP_FAILED) { >> > > I was going to send tomorrow a pull request for a similar patch, > "backends/hostmem-file: Allow to specify full pathname for backing file". > > The main difference seems to be your usage of O_EXCL. Can you explain > why you added it? It' used if we pass a block device as a NVDIMM backend memory: O_EXCL can be used without O_CREAT if pathname refers to a block device. If the block device is in use by the system (e.g., mounted), open() fails with the error EBUSY