From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35915) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ztcdx-0006ab-8v for qemu-devel@nongnu.org; Tue, 03 Nov 2015 09:33:02 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ztcds-0005rN-4I for qemu-devel@nongnu.org; Tue, 03 Nov 2015 09:33:01 -0500 Received: from mga09.intel.com ([134.134.136.24]:25555) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ztcdr-0005qu-P5 for qemu-devel@nongnu.org; Tue, 03 Nov 2015 09:32:56 -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> <56383076.5090400@linux.intel.com> <5638BCCD.2040801@redhat.com> From: Xiao Guangrong Message-ID: <5638C401.60600@linux.intel.com> Date: Tue, 3 Nov 2015 22:26:09 +0800 MIME-Version: 1.0 In-Reply-To: <5638BCCD.2040801@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 09:55 PM, Paolo Bonzini wrote: > > > On 03/11/2015 04:56, Xiao Guangrong wrote: >> >> >> 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 > > That makes sense, but I think it's better to be consistent with the > handling of block devices. Block devices do not use O_EXCL when QEMU > opens them; I guess in principle it would also be possible to share a > single pmem backend between multiple guests. Yup. Will make a separate patch to do this. :)