Re: [Qemu-devel] [PATCH v2 1/3] exec.c: do not truncate non-empty memory backend file

[Eduardo Habkost]

On Thu, Oct 27, 2016 at 12:22:58PM +0800, Haozhong Zhang wrote:
> For '-object memory-backend-file,mem-path=foo,size=xyz', if the size of
> file 'foo' does not match the given size 'xyz', the current QEMU will
> truncate the file to the given size, which may corrupt the existing data
> in that file. To avoid such data corruption, this patch disables
> truncating non-empty backend files.
> 
> Signed-off-by: Haozhong Zhang <address@hidden>

Reviewed-by: Eduardo Habkost <address@hidden>

But I would add comment near the get_file_size() call to indicate
that not stopping on get_file_size() errors is on purpose and not
a mistake.

> ---
>  exec.c | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/exec.c b/exec.c
> index 587b489..a2b371a 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1224,6 +1224,15 @@ void qemu_mutex_unlock_ramlist(void)
>  }
>  
>  #ifdef __linux__
> +static int64_t get_file_size(int fd)
> +{
> +    int64_t size = lseek(fd, 0, SEEK_END);
> +    if (size < 0) {
> +        return -errno;
> +    }
> +    return size;
> +}
> +
>  static void *file_ram_alloc(RAMBlock *block,
>                              ram_addr_t memory,
>                              const char *path,
> @@ -1235,6 +1244,7 @@ static void *file_ram_alloc(RAMBlock *block,
>      char *c;
>      void *area = MAP_FAILED;
>      int fd = -1;
> +    int64_t file_size;
>  
>      if (kvm_enabled() && !kvm_has_sync_mmu()) {
>          error_setg(errp,
> @@ -1297,6 +1307,8 @@ static void *file_ram_alloc(RAMBlock *block,
>      }
>  #endif
>  
> +    file_size = get_file_size(fd);
> +
>      if (memory < block->page_size) {
>          error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be equal to "
>                     "or larger than page size 0x%zx",
> @@ -1311,8 +1323,16 @@ static void *file_ram_alloc(RAMBlock *block,
>       * hosts, so don't bother bailing out on errors.
>       * If anything goes wrong with it under other filesystems,
>       * mmap will fail.
> +     *
> +     * Do not truncate the non-empty backend file to avoid corrupting
> +     * the existing data in the file. Disabling shrinking is not
> +     * enough. For example, the current vNVDIMM implementation stores
> +     * the guest NVDIMM labels at the end of the backend file. If the
> +     * backend file is later extended, QEMU will not be able to find
> +     * those labels. Therefore, extending the non-empty backend file
> +     * is disabled as well.
>       */
> -    if (ftruncate(fd, memory)) {
> +    if (!file_size && ftruncate(fd, memory)) {
>          perror("ftruncate");
>      }
>  
> -- 
> 2.10.1
> 

-- 
Eduardo