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

[Haozhong Zhang <haozhong.zhang@intel.com>]

On 10/27/16 12:31 -0200, Eduardo Habkost wrote:
>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 <haozhong.zhang@intel.com>
>
>Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
>
>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.
>

I'll add comments in the next version.

Thanks,
Haozhong

>> ---
>>  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