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 <a.mota...@virtualopensystems.com>
> Signed-off-by: Nikolay Nikolaev <n.nikol...@virtualopensystems.com>
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
>
