Re: [Qemu-devel] [PATCH v2 0/3] Improve truncation behavior of memory-backend-file

[Paolo Bonzini]

On 27/10/2016 06:22, Haozhong Zhang wrote:
> For a mmeory backend file, e.g.
>     -object memory-backend-file,mem-path=foo,size=SZ,...
> the size of the backend file 'foo' is specified by the 'size'
> option. If the specified size 'SZ' does not match the actual size of
> file 'foo', QEMU will truncate the backend file 'foo'. In certain
> usage scenarios (e.g. vNVDIMM), the truncation may corrupt the
> existing data in the file.
> 
> Patch 1 in this series avoids such data corruption by disabling
> truncating non-empty backend files. If a non-existing file, an empty
> file or a directory is specified by 'mem-path' option, QEMU will
> truncate the backend file to the size specified by 'size' option.
> 
> Patch 2 adds an additional check to avoid creating a memory backend
> that can not be hold in the backend file. For a non-empty backend
> file, if its size is smaller than 'size' option, QEMU will report
> error.
> 
> Patch 3 makes the option 'size' optional. It's to avoid the misusing
> of 'size' option. If the user is uncertain about the backend file
> size, they can skip the 'size' option and let QEMU use the actual file
> size.  If a non-existing file, an empty file or a directory is
> specified by 'mem-path' option, the 'size' option is still needed.
> 
> Changes since v1:
>  * Fix errors in v1 patches.
>  * Split truncation skip and size check into separate patches.
>  * Do not error out for backend file whose size is unknown.
>  * Only error out when file size is smaller than 'size' option.
>  * Change the error handling path of file_backend_memory_alloc().
>  * Do not duplicate the setting of block->used_length/max_length in
>    file_ram_alloc().

Nice.  I'm squashing this in for slightly better error messages.

Thanks,

Paolo

diff --git a/exec.c b/exec.c
index d9034b1..eea9c10 100644
--- a/exec.c
+++ b/exec.c
@@ -1267,6 +1267,13 @@ static void *file_ram_alloc(RAMBlock *block,
                 break;
             }
         } else if (errno == EISDIR) {
+            if (!mem_size) {
+                error_setg_errno(errp, errno,
+                                 "%s is a directory but no 'size' option was 
specified",
+                                 path);
+                goto error;
+            }
+
             /* @path names a directory, create a file there */
             /* Make name safe to use with mkstemp by replacing '/' with '_'. */
             sanitized_name = g_strdup(memory_region_name(block->mr));

> 
> Haozhong Zhang (3):
>   exec.c: do not truncate non-empty memory backend file
>   exec.c: check memory backend file size with 'size' option
>   hostmem-file: make option 'size' optional
> 
>  backends/hostmem-file.c | 28 ++++++++++++++++++-------
>  exec.c                  | 56 
> +++++++++++++++++++++++++++++++++++++++----------
>  2 files changed, 65 insertions(+), 19 deletions(-)
>