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

[Haozhong Zhang]

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

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