From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51959) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W31yr-0000AW-9I for qemu-devel@nongnu.org; Tue, 14 Jan 2014 06:16:30 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W31ym-0002nN-Br for qemu-devel@nongnu.org; Tue, 14 Jan 2014 06:16:25 -0500 Received: from mx1.redhat.com ([209.132.183.28]:43497) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W31ym-0002nI-2B for qemu-devel@nongnu.org; Tue, 14 Jan 2014 06:16:20 -0500 Date: Tue, 14 Jan 2014 13:16:08 +0200 From: "Michael S. Tsirkin" Message-ID: <20140114111608.GC27922@redhat.com> References: <1389623119-15863-1-git-send-email-a.motakis@virtualopensystems.com> <1389623119-15863-3-git-send-email-a.motakis@virtualopensystems.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1389623119-15863-3-git-send-email-a.motakis@virtualopensystems.com> Subject: Re: [Qemu-devel] [PATCH v6 2/8] New -mem-path option - unlink. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Antonios Motakis Cc: Peter Maydell , snabb-devel@googlegroups.com, Anthony Liguori , Jan Kiszka , Michael Tokarev , qemu-devel@nongnu.org, n.nikolaev@virtualopensystems.com, Markus Armbruster , Stefan Hajnoczi , lukego@gmail.com, Paolo Bonzini , tech@virtualopensystems.com, Andreas =?iso-8859-1?Q?F=E4rber?= , Richard Henderson On Mon, Jan 13, 2014 at 03:25:13PM +0100, Antonios Motakis wrote: > The unlink option allows the created file to be externally deleted > after QEMU is terminated. > > - unlink=on|off - default on, unlink the file after opening it > > Signed-off-by: Antonios Motakis > Signed-off-by: Nikolay Nikolaev I have doubts about this patch. In particular we seem to commit to a specific file naming scheme without ever documenting its users or adding any tests. Please document who uses this in the commit log, document the scheme in docs/ and add a test so we don't break this without noticing. > --- > exec.c | 18 +++++++++++++----- > qemu-options.hx | 7 ++++--- > vl.c | 4 ++++ > 3 files changed, 21 insertions(+), 8 deletions(-) > > diff --git a/exec.c b/exec.c > index 1c40a0d..30f4019 100644 > --- a/exec.c > +++ b/exec.c > @@ -999,7 +999,7 @@ static void *file_ram_alloc(RAMBlock *block, > int flags; > unsigned long hpagesize; > QemuOpts *opts; > - unsigned int mem_prealloc = 0, mem_share = 0; > + unsigned int mem_prealloc = 0, mem_share = 0, mem_unlink = 1; > > hpagesize = gethugepagesize(path); > if (!hpagesize) { > @@ -1020,6 +1020,7 @@ static void *file_ram_alloc(RAMBlock *block, > if (opts) { > mem_prealloc = qemu_opt_get_bool(opts, "prealloc", 0); > mem_share = qemu_opt_get_bool(opts, "share", 0); > + mem_unlink = qemu_opt_get_bool(opts, "unlink", 1); > } > > /* Make name safe to use with mkstemp by replacing '/' with '_'. */ > @@ -1029,18 +1030,25 @@ static void *file_ram_alloc(RAMBlock *block, > *c = '_'; > } > > - filename = g_strdup_printf("%s/qemu_back_mem.%s.XXXXXX", path, > - sanitized_name); > + filename = g_strdup_printf("%s/qemu_back_mem.%s%s", path, sanitized_name, > + (mem_unlink) ? ".XXXXXX" : ""); > g_free(sanitized_name); > > - fd = mkstemp(filename); > + if (mem_unlink) { > + fd = mkstemp(filename); > + } else { > + fd = open(filename, O_CREAT | O_RDWR | O_EXCL, > + S_IRWXU | S_IRWXG | S_IRWXO); > + } > if (fd < 0) { > perror("unable to create guest RAM backing store"); > g_free(filename); > return NULL; > } > > - unlink(filename); > + if (mem_unlink) { > + unlink(filename); > + } > g_free(filename); > > memory = (memory + hpagesize - 1) & ~(hpagesize - 1); > diff --git a/qemu-options.hx b/qemu-options.hx > index 60ecc95..a12af97 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -221,14 +221,15 @@ gigabytes respectively. > ETEXI > > DEF("mem-path", HAS_ARG, QEMU_OPTION_mempath, > - "-mem-path [path=]path[,prealloc=on|off][,share=on|off]\n" > + "-mem-path [path=]path[,prealloc=on|off][,share=on|off][,unlink=on|off]\n" > " provide backing storage for guest RAM\n" > " path= a directory path for the backing store\n" > " prealloc= preallocate guest memory [default disabled]\n" > - " share= enable mmap share flag [default disabled]\n", > + " share= enable mmap share flag [default disabled]\n" > + " unlink= enable unlinking the guest RAM files [default enabled]\n", > QEMU_ARCH_ALL) > STEXI > -@item -mem-path [path=]@var{path}[,prealloc=on|off][,share=on|off] > +@item -mem-path [path=]@var{path}[,prealloc=on|off][,share=on|off][,unlink=on|off] > @findex -mem-path > Allocate guest RAM from a temporarily created file in @var{path}. > ETEXI > diff --git a/vl.c b/vl.c > index e98abc8..5034bb6 100644 > --- a/vl.c > +++ b/vl.c > @@ -546,6 +546,10 @@ static QemuOptsList qemu_mem_path_opts = { > .name = "share", > .type = QEMU_OPT_BOOL, > }, > + { > + .name = "unlink", > + .type = QEMU_OPT_BOOL, > + }, > { /* end of list */ } > }, > }; > -- > 1.8.3.2 >