diff mbox

[v2] exec: make -mem-path filenames deterministic

Message ID 1362158493-11907-1-git-send-email-peter@gridcentric.ca
State New
Headers show

Commit Message

Peter Feiner March 1, 2013, 5:21 p.m. UTC
From: Peter Feiner <peter@gridcentric.ca>

Adds ramblocks' names to their backing files when using -mem-path.  Eases
introspection and debugging.

Signed-off-by: Peter Feiner <peter@gridcentric.ca>
---

On Tue, Jan 8, 2013 at 2:04 PM, Anthony Liguori <aliguori@us.ibm.com> wrote:
>
> Yes, please submit the oneliner.

Here it is :)

The commit should probably be called "exec: add ramblocks' names to -mem-path
files" since the paths aren't deterministic.

v1 -> v2: Just add ramblock name to mkstemp template. 

Thanks,
Peter

 exec.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Andreas Färber March 1, 2013, 6:47 p.m. UTC | #1
Am 01.03.2013 18:21, schrieb peter@gridcentric.ca:
> From: Peter Feiner <peter@gridcentric.ca>
> 
> Adds ramblocks' names to their backing files when using -mem-path.  Eases
> introspection and debugging.
> 
> Signed-off-by: Peter Feiner <peter@gridcentric.ca>
> ---
> 
> On Tue, Jan 8, 2013 at 2:04 PM, Anthony Liguori <aliguori@us.ibm.com> wrote:
>>
>> Yes, please submit the oneliner.
> 
> Here it is :)
> 
> The commit should probably be called "exec: add ramblocks' names to -mem-path
> files" since the paths aren't deterministic.
> 
> v1 -> v2: Just add ramblock name to mkstemp template. 
> 
> Thanks,
> Peter
> 
>  exec.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/exec.c b/exec.c
> index a41bcb8..16a5452 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -865,7 +865,8 @@ static void *file_ram_alloc(RAMBlock *block,
>          return NULL;
>      }
>  
> -    filename = g_strdup_printf("%s/qemu_back_mem.XXXXXX", path);
> +    filename = g_strdup_printf("%s/qemu_back_mem.%s.XXXXXX", path,
> +                               block->mr->name);

Is it safe to use a MemoryRegion name as part of mkstemp()? "/../" seems
rather unlikely but a "/" less so.

Also note that v2 is expected to be posted as a top-level patch since
the patches tool does not cope well with "hidden" reposts.

Andreas

>  
>      fd = mkstemp(filename);
>      if (fd < 0) {
>
Peter Feiner March 1, 2013, 7:20 p.m. UTC | #2
On Fri, Mar 1, 2013 at 1:47 PM, Andreas Färber <afaerber@suse.de> wrote:
> Am 01.03.2013 18:21, schrieb peter@gridcentric.ca:
>
> Is it safe to use a MemoryRegion name as part of mkstemp()? "/../" seems
> rather unlikely but a "/" less so.

Since mkstemp essentially calls open, "/" does what you'd expect. So a
MemoryRegion name with "/" would cause problems. I can sanitize the
name by replacing "/" with "_". I'll post this as v3.

> Also note that v2 is expected to be posted as a top-level patch since
> the patches tool does not cope well with "hidden" reposts.

Thanks for the tip. I'll post v3 without "In-Reply-To".

Peter
diff mbox

Patch

diff --git a/exec.c b/exec.c
index a41bcb8..16a5452 100644
--- a/exec.c
+++ b/exec.c
@@ -865,7 +865,8 @@  static void *file_ram_alloc(RAMBlock *block,
         return NULL;
     }
 
-    filename = g_strdup_printf("%s/qemu_back_mem.XXXXXX", path);
+    filename = g_strdup_printf("%s/qemu_back_mem.%s.XXXXXX", path,
+                               block->mr->name);
 
     fd = mkstemp(filename);
     if (fd < 0) {