From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42523) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zvk5x-0006UH-62 for qemu-devel@nongnu.org; Mon, 09 Nov 2015 05:54:42 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zvk5t-0000nu-1x for qemu-devel@nongnu.org; Mon, 09 Nov 2015 05:54:41 -0500 Received: from mga09.intel.com ([134.134.136.24]:11812) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zvk5s-0000na-TS for qemu-devel@nongnu.org; Mon, 09 Nov 2015 05:54:37 -0500 References: <1446184587-142784-1-git-send-email-guangrong.xiao@linux.intel.com> <1446184587-142784-10-git-send-email-guangrong.xiao@linux.intel.com> <20151109120731-mutt-send-email-mst@redhat.com> From: Xiao Guangrong Message-ID: <564079EC.6010804@linux.intel.com> Date: Mon, 9 Nov 2015 18:48:12 +0800 MIME-Version: 1.0 In-Reply-To: <20151109120731-mutt-send-email-mst@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v6 09/33] exec: allow file_ram_alloc to work on file List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: ehabkost@redhat.com, kvm@vger.kernel.org, gleb@kernel.org, mtosatti@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, imammedo@redhat.com, pbonzini@redhat.com, dan.j.williams@intel.com, rth@twiddle.net On 11/09/2015 06:13 PM, Michael S. Tsirkin wrote: > On Fri, Oct 30, 2015 at 01:56:03PM +0800, 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 3ca7e50..f219010 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); > > This means file doesn't exist is treated as a file. > Can't figure out if that's intentional, should > be documented in any case. The path is dir only if it passes S_ISDIR test. Otherwise, it is treated as regular file. Any error on the file will be figured out by open() and mmap() later. > >> +} >> + >> +static int open_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; > > Why does this make sense? RAM_SHARED means its write is invisible to others also it does not actual change the content of the file. Readonly permission is enough for this case. It is also help us to pass a readonly file to the guess but discard its change after guest shutdown. > >> + >> + flags |= O_EXCL; > > And why does this makes sense? 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 BTW, the similar logic has already been in upsteam QEMU which is introduced by commit 8d31d6b6 (backends/hostmem-file: Allow to specify full pathname for backing file). I will drop these patches: util: introduce qemu_file_get_page_size() exec: allow memory to be allocated from any kind of path exec: allow file_ram_alloc to work on file