| Message ID | 20161027042300.5929-4-haozhong.zhang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Oct 27, 2016 at 12:23:00PM +0800, Haozhong Zhang wrote: > If 'size' option is not given, Qemu will use the file size of 'mem-path' > instead. If an empty file, a non-existing file or a directory is specified > by option 'mem-path', a non-zero option 'size' is still needed. > > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> > --- > backends/hostmem-file.c | 28 ++++++++++++++++++++-------- > exec.c | 33 ++++++++++++++++++++------------- > 2 files changed, 40 insertions(+), 21 deletions(-) > > diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c > index 42efb2f..6ee4352 100644 > --- a/backends/hostmem-file.c > +++ b/backends/hostmem-file.c > @@ -39,17 +39,14 @@ static void > file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) > { > HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(backend); > + Error *local_err = NULL; > > - if (!backend->size) { > - error_setg(errp, "can't create backend with size 0"); > - return; > - } > if (!fb->mem_path) { > - error_setg(errp, "mem-path property not set"); > - return; > + error_setg(&local_err, "mem-path property not set"); > + goto out; > } > #ifndef CONFIG_LINUX > - error_setg(errp, "-mem-path not supported on this host"); > + error_setg(&local_err, "-mem-path not supported on this host"); > #else > if (!memory_region_size(&backend->mr)) { > gchar *path; > @@ -58,10 +55,25 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) > memory_region_init_ram_from_file(&backend->mr, OBJECT(backend), > path, > backend->size, fb->share, > - fb->mem_path, errp); > + fb->mem_path, &local_err); > g_free(path); > + > + if (local_err) { > + goto out; > + } > + > + if (!backend->size) { > + backend->size = memory_region_size(&backend->mr); > + } > } > #endif > + > + if (!backend->size) { > + error_setg(&local_err, "can't create backend with size 0"); > + } You need to move this check before the #endif line, as an error is already unconditionally set in local_err in the !CONFIG_LINUX path, and a second error_setg() call would trigger an assert() inside error_setv(). > + > + out: > + error_propagate(errp, local_err); > } > > static char *get_mem_path(Object *o, Error **errp) > diff --git a/exec.c b/exec.c > index 264a25f..89065bd 100644 > --- a/exec.c > +++ b/exec.c > @@ -1234,7 +1234,7 @@ static int64_t get_file_size(int fd) > } > > static void *file_ram_alloc(RAMBlock *block, > - ram_addr_t memory, > + ram_addr_t *memory, > const char *path, > Error **errp) > { > @@ -1245,6 +1245,7 @@ static void *file_ram_alloc(RAMBlock *block, > void *area = MAP_FAILED; > int fd = -1; > int64_t file_size; > + ram_addr_t mem_size = *memory; > > if (kvm_enabled() && !kvm_has_sync_mmu()) { > error_setg(errp, > @@ -1309,21 +1310,27 @@ static void *file_ram_alloc(RAMBlock *block, > > file_size = get_file_size(fd); > > - if (memory < block->page_size) { > + if (!mem_size && file_size > 0) { > + mem_size = file_size; Maybe we should set *memory here and not below? > + memory_region_set_size(block->mr, mem_size); This could be simplified (and made safer) if the memory region got initialized by memory_region_init_ram_from_file() after we already mapped/allocated the file (so we avoid surprises in case other code does something weird because of the temporarily zero-sized MemoryRegion). But it would probably be an intrusive change, so I guess changing the memory size here is OK. Paolo, what do you think? > + } > + > + if (mem_size < block->page_size) { > error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be equal to " > "or larger than page size 0x%zx", > - memory, block->page_size); > + mem_size, block->page_size); > goto error; > } > > - if (file_size > 0 && file_size < memory) { > + if (file_size > 0 && file_size < mem_size) { > error_setg(errp, "backing store %s size %"PRId64 > " does not match 'size' option %"PRIu64, > - path, file_size, memory); > + path, file_size, mem_size); > goto error; > } > > - memory = ROUND_UP(memory, block->page_size); > + mem_size = ROUND_UP(mem_size, block->page_size); > + *memory = mem_size; Why exactly did you choose to set *memory to the rounded-up size and not file_size? Changing *memory to the rounded-up value would be additional behavior that is not described in the commit message. I believe we should change *memory only if *memory == 0, and avoid touching it otherwise. > > /* > * ftruncate is not supported by hugetlbfs in older > @@ -1339,11 +1346,11 @@ static void *file_ram_alloc(RAMBlock *block, > * those labels. Therefore, extending the non-empty backend file > * is disabled as well. > */ > - if (!file_size && ftruncate(fd, memory)) { > + if (!file_size && ftruncate(fd, mem_size)) { > perror("ftruncate"); > } > > - area = qemu_ram_mmap(fd, memory, block->mr->align, > + area = qemu_ram_mmap(fd, mem_size, block->mr->align, > block->flags & RAM_SHARED); > if (area == MAP_FAILED) { > error_setg_errno(errp, errno, > @@ -1352,7 +1359,7 @@ static void *file_ram_alloc(RAMBlock *block, > } > > if (mem_prealloc) { > - os_mem_prealloc(fd, area, memory, errp); > + os_mem_prealloc(fd, area, mem_size, errp); > if (errp && *errp) { > goto error; > } > @@ -1363,7 +1370,7 @@ static void *file_ram_alloc(RAMBlock *block, > > error: > if (area != MAP_FAILED) { > - qemu_ram_munmap(area, memory); > + qemu_ram_munmap(area, mem_size); > } > if (unlink_on_error) { > unlink(path); > @@ -1690,15 +1697,15 @@ RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr, > size = HOST_PAGE_ALIGN(size); > new_block = g_malloc0(sizeof(*new_block)); > new_block->mr = mr; > - new_block->used_length = size; > - new_block->max_length = size; > new_block->flags = share ? RAM_SHARED : 0; > - new_block->host = file_ram_alloc(new_block, size, > + new_block->host = file_ram_alloc(new_block, &size, > mem_path, errp); > if (!new_block->host) { > g_free(new_block); > return NULL; > } > + new_block->used_length = size; > + new_block->max_length = size; > > ram_block_add(new_block, &local_err); > if (local_err) { > -- > 2.10.1 >
On 10/27/16 12:55 -0200, Eduardo Habkost wrote: >On Thu, Oct 27, 2016 at 12:23:00PM +0800, Haozhong Zhang wrote: >> If 'size' option is not given, Qemu will use the file size of 'mem-path' >> instead. If an empty file, a non-existing file or a directory is specified >> by option 'mem-path', a non-zero option 'size' is still needed. >> >> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> >> --- >> backends/hostmem-file.c | 28 ++++++++++++++++++++-------- >> exec.c | 33 ++++++++++++++++++++------------- >> 2 files changed, 40 insertions(+), 21 deletions(-) >> >> diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c >> index 42efb2f..6ee4352 100644 >> --- a/backends/hostmem-file.c >> +++ b/backends/hostmem-file.c >> @@ -39,17 +39,14 @@ static void >> file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) >> { >> HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(backend); >> + Error *local_err = NULL; >> >> - if (!backend->size) { >> - error_setg(errp, "can't create backend with size 0"); >> - return; >> - } >> if (!fb->mem_path) { >> - error_setg(errp, "mem-path property not set"); >> - return; >> + error_setg(&local_err, "mem-path property not set"); >> + goto out; >> } >> #ifndef CONFIG_LINUX >> - error_setg(errp, "-mem-path not supported on this host"); >> + error_setg(&local_err, "-mem-path not supported on this host"); >> #else >> if (!memory_region_size(&backend->mr)) { >> gchar *path; >> @@ -58,10 +55,25 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) >> memory_region_init_ram_from_file(&backend->mr, OBJECT(backend), >> path, >> backend->size, fb->share, >> - fb->mem_path, errp); >> + fb->mem_path, &local_err); >> g_free(path); >> + >> + if (local_err) { >> + goto out; >> + } >> + >> + if (!backend->size) { >> + backend->size = memory_region_size(&backend->mr); >> + } >> } >> #endif >> + >> + if (!backend->size) { >> + error_setg(&local_err, "can't create backend with size 0"); >> + } > >You need to move this check before the #endif line, as an error >is already unconditionally set in local_err in the !CONFIG_LINUX >path, and a second error_setg() call would trigger an assert() >inside error_setv(). > will change >> + >> + out: >> + error_propagate(errp, local_err); >> } >> >> static char *get_mem_path(Object *o, Error **errp) >> diff --git a/exec.c b/exec.c >> index 264a25f..89065bd 100644 >> --- a/exec.c >> +++ b/exec.c >> @@ -1234,7 +1234,7 @@ static int64_t get_file_size(int fd) >> } >> >> static void *file_ram_alloc(RAMBlock *block, >> - ram_addr_t memory, >> + ram_addr_t *memory, >> const char *path, >> Error **errp) >> { >> @@ -1245,6 +1245,7 @@ static void *file_ram_alloc(RAMBlock *block, >> void *area = MAP_FAILED; >> int fd = -1; >> int64_t file_size; >> + ram_addr_t mem_size = *memory; >> >> if (kvm_enabled() && !kvm_has_sync_mmu()) { >> error_setg(errp, >> @@ -1309,21 +1310,27 @@ static void *file_ram_alloc(RAMBlock *block, >> >> file_size = get_file_size(fd); >> >> - if (memory < block->page_size) { >> + if (!mem_size && file_size > 0) { >> + mem_size = file_size; > >Maybe we should set *memory here and not below? > Qemu currently sets the memory region size to the file size, and block length to the aligned file size, so the code here can be changed as below: memory_region_set_size(block->mr, mem_size); mem_size = HOST_PAGE_ALIGN(mem_size); *memory = mem_size; The second line is necessary because Qemu currently passes the aligned file size to file_ram_alloc(). >> + memory_region_set_size(block->mr, mem_size); > >This could be simplified (and made safer) if the memory region >got initialized by memory_region_init_ram_from_file() after we >already mapped/allocated the file (so we avoid surprises in case >other code does something weird because of the temporarily >zero-sized MemoryRegion). But it would probably be an intrusive >change, so I guess changing the memory size here is OK. Paolo, >what do you think? > I will add a check to ensure that the size of block->mr is either 0 or mem_size, and only set the region size in the former case. The former corresponds to the case that 'size' option is not given or 0. The latter corresponds to the case that 'size' option is given. >> + } >> + >> + if (mem_size < block->page_size) { >> error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be equal to " >> "or larger than page size 0x%zx", >> - memory, block->page_size); >> + mem_size, block->page_size); >> goto error; >> } >> >> - if (file_size > 0 && file_size < memory) { >> + if (file_size > 0 && file_size < mem_size) { >> error_setg(errp, "backing store %s size %"PRId64 >> " does not match 'size' option %"PRIu64, >> - path, file_size, memory); >> + path, file_size, mem_size); >> goto error; >> } >> >> - memory = ROUND_UP(memory, block->page_size); >> + mem_size = ROUND_UP(mem_size, block->page_size); >> + *memory = mem_size; > >Why exactly did you choose to set *memory to the rounded-up size >and not file_size? Changing *memory to the rounded-up value would >be additional behavior that is not described in the commit >message. I believe we should change *memory only if *memory == 0, >and avoid touching it otherwise. > Setting *memory here is buggy. I'll move it as above. Thanks, Haozhong >> >> /* >> * ftruncate is not supported by hugetlbfs in older >> @@ -1339,11 +1346,11 @@ static void *file_ram_alloc(RAMBlock *block, >> * those labels. Therefore, extending the non-empty backend file >> * is disabled as well. >> */ >> - if (!file_size && ftruncate(fd, memory)) { >> + if (!file_size && ftruncate(fd, mem_size)) { >> perror("ftruncate"); >> } >> >> - area = qemu_ram_mmap(fd, memory, block->mr->align, >> + area = qemu_ram_mmap(fd, mem_size, block->mr->align, >> block->flags & RAM_SHARED); >> if (area == MAP_FAILED) { >> error_setg_errno(errp, errno, >> @@ -1352,7 +1359,7 @@ static void *file_ram_alloc(RAMBlock *block, >> } >> >> if (mem_prealloc) { >> - os_mem_prealloc(fd, area, memory, errp); >> + os_mem_prealloc(fd, area, mem_size, errp); >> if (errp && *errp) { >> goto error; >> } >> @@ -1363,7 +1370,7 @@ static void *file_ram_alloc(RAMBlock *block, >> >> error: >> if (area != MAP_FAILED) { >> - qemu_ram_munmap(area, memory); >> + qemu_ram_munmap(area, mem_size); >> } >> if (unlink_on_error) { >> unlink(path); >> @@ -1690,15 +1697,15 @@ RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr, >> size = HOST_PAGE_ALIGN(size); >> new_block = g_malloc0(sizeof(*new_block)); >> new_block->mr = mr; >> - new_block->used_length = size; >> - new_block->max_length = size; >> new_block->flags = share ? RAM_SHARED : 0; >> - new_block->host = file_ram_alloc(new_block, size, >> + new_block->host = file_ram_alloc(new_block, &size, >> mem_path, errp); >> if (!new_block->host) { >> g_free(new_block); >> return NULL; >> } >> + new_block->used_length = size; >> + new_block->max_length = size; >> >> ram_block_add(new_block, &local_err); >> if (local_err) { >> -- >> 2.10.1 >> > >-- >Eduardo
On 10/28/16 10:06 +0800, Haozhong Zhang wrote: >On 10/27/16 12:55 -0200, Eduardo Habkost wrote: >>On Thu, Oct 27, 2016 at 12:23:00PM +0800, Haozhong Zhang wrote: [..] >>> static char *get_mem_path(Object *o, Error **errp) >>>diff --git a/exec.c b/exec.c >>>index 264a25f..89065bd 100644 >>>--- a/exec.c >>>+++ b/exec.c >>>@@ -1234,7 +1234,7 @@ static int64_t get_file_size(int fd) >>> } >>> >>> static void *file_ram_alloc(RAMBlock *block, >>>- ram_addr_t memory, >>>+ ram_addr_t *memory, >>> const char *path, >>> Error **errp) >>> { >>>@@ -1245,6 +1245,7 @@ static void *file_ram_alloc(RAMBlock *block, >>> void *area = MAP_FAILED; >>> int fd = -1; >>> int64_t file_size; >>>+ ram_addr_t mem_size = *memory; >>> >>> if (kvm_enabled() && !kvm_has_sync_mmu()) { >>> error_setg(errp, >>>@@ -1309,21 +1310,27 @@ static void *file_ram_alloc(RAMBlock *block, >>> >>> file_size = get_file_size(fd); >>> >>>- if (memory < block->page_size) { >>>+ if (!mem_size && file_size > 0) { >>>+ mem_size = file_size; >> >>Maybe we should set *memory here and not below? >> > >Qemu currently sets the memory region size to the file size, and block >length to the aligned file size, so the code here can be changed as below: > > memory_region_set_size(block->mr, mem_size); > mem_size = HOST_PAGE_ALIGN(mem_size); > *memory = mem_size; > >The second line is necessary because Qemu currently passes the aligned >file size to file_ram_alloc(). > I meant that QEMU currently sets the memory region size of the 'size' option and the block length to the aligned 'size' option. If the file size is used when 'size' option is not specified, QEMU should do the same thing. Haozhong >>>+ memory_region_set_size(block->mr, mem_size); >> >>This could be simplified (and made safer) if the memory region >>got initialized by memory_region_init_ram_from_file() after we >>already mapped/allocated the file (so we avoid surprises in case >>other code does something weird because of the temporarily >>zero-sized MemoryRegion). But it would probably be an intrusive >>change, so I guess changing the memory size here is OK. Paolo, >>what do you think? >> > >I will add a check to ensure that the size of block->mr is either 0 or >mem_size, and only set the region size in the former case. The former >corresponds to the case that 'size' option is not given or 0. The >latter corresponds to the case that 'size' option is given. > >>>+ } >>>+ >>>+ if (mem_size < block->page_size) { >>> error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be equal to " >>> "or larger than page size 0x%zx", >>>- memory, block->page_size); >>>+ mem_size, block->page_size); >>> goto error; >>> } >>> >>>- if (file_size > 0 && file_size < memory) { >>>+ if (file_size > 0 && file_size < mem_size) { >>> error_setg(errp, "backing store %s size %"PRId64 >>> " does not match 'size' option %"PRIu64, >>>- path, file_size, memory); >>>+ path, file_size, mem_size); >>> goto error; >>> } >>> >>>- memory = ROUND_UP(memory, block->page_size); >>>+ mem_size = ROUND_UP(mem_size, block->page_size); >>>+ *memory = mem_size; >> >>Why exactly did you choose to set *memory to the rounded-up size >>and not file_size? Changing *memory to the rounded-up value would >>be additional behavior that is not described in the commit >>message. I believe we should change *memory only if *memory == 0, >>and avoid touching it otherwise. >> > >Setting *memory here is buggy. I'll move it as above. > >Thanks, >Haozhong > [..]
On Fri, Oct 28, 2016 at 10:06:40AM +0800, Haozhong Zhang wrote: [...] > > > diff --git a/exec.c b/exec.c > > > index 264a25f..89065bd 100644 > > > --- a/exec.c > > > +++ b/exec.c > > > @@ -1234,7 +1234,7 @@ static int64_t get_file_size(int fd) > > > } > > > > > > static void *file_ram_alloc(RAMBlock *block, > > > - ram_addr_t memory, > > > + ram_addr_t *memory, > > > const char *path, > > > Error **errp) > > > { > > > @@ -1245,6 +1245,7 @@ static void *file_ram_alloc(RAMBlock *block, > > > void *area = MAP_FAILED; > > > int fd = -1; > > > int64_t file_size; > > > + ram_addr_t mem_size = *memory; > > > > > > if (kvm_enabled() && !kvm_has_sync_mmu()) { > > > error_setg(errp, > > > @@ -1309,21 +1310,27 @@ static void *file_ram_alloc(RAMBlock *block, > > > > > > file_size = get_file_size(fd); > > > > > > - if (memory < block->page_size) { > > > + if (!mem_size && file_size > 0) { > > > + mem_size = file_size; > > > > Maybe we should set *memory here and not below? > > > > Qemu currently sets the memory region size to the file size, and block > length to the aligned file size, so the code here can be changed as below: > > memory_region_set_size(block->mr, mem_size); > mem_size = HOST_PAGE_ALIGN(mem_size); > *memory = mem_size; > > The second line is necessary because Qemu currently passes the aligned > file size to file_ram_alloc(). That would duplicate the existing HOST_PAGE_ALIGN logic from qemu_ram_alloc_from_file(), won't it? I believe that's yet another reason to check file size before initializing the memory region, instead of initializing it first, and fixing up its size later.
On 10/31/16 16:18 -0200, Eduardo Habkost wrote: >On Fri, Oct 28, 2016 at 10:06:40AM +0800, Haozhong Zhang wrote: >[...] >> > > diff --git a/exec.c b/exec.c >> > > index 264a25f..89065bd 100644 >> > > --- a/exec.c >> > > +++ b/exec.c >> > > @@ -1234,7 +1234,7 @@ static int64_t get_file_size(int fd) >> > > } >> > > >> > > static void *file_ram_alloc(RAMBlock *block, >> > > - ram_addr_t memory, >> > > + ram_addr_t *memory, >> > > const char *path, >> > > Error **errp) >> > > { >> > > @@ -1245,6 +1245,7 @@ static void *file_ram_alloc(RAMBlock *block, >> > > void *area = MAP_FAILED; >> > > int fd = -1; >> > > int64_t file_size; >> > > + ram_addr_t mem_size = *memory; >> > > >> > > if (kvm_enabled() && !kvm_has_sync_mmu()) { >> > > error_setg(errp, >> > > @@ -1309,21 +1310,27 @@ static void *file_ram_alloc(RAMBlock *block, >> > > >> > > file_size = get_file_size(fd); >> > > >> > > - if (memory < block->page_size) { >> > > + if (!mem_size && file_size > 0) { >> > > + mem_size = file_size; >> > >> > Maybe we should set *memory here and not below? >> > >> >> Qemu currently sets the memory region size to the file size, and block >> length to the aligned file size, so the code here can be changed as below: >> >> memory_region_set_size(block->mr, mem_size); >> mem_size = HOST_PAGE_ALIGN(mem_size); >> *memory = mem_size; >> >> The second line is necessary because Qemu currently passes the aligned >> file size to file_ram_alloc(). > >That would duplicate the existing HOST_PAGE_ALIGN logic from >qemu_ram_alloc_from_file(), won't it? > >I believe that's yet another reason to check file size before >initializing the memory region, instead of initializing it first, >and fixing up its size later. > Agree. In the next version of this patch 3, I'll move file operations (open/close/get size) from file_ram_alloc() to qemu_ram_alloc_from_file(). Thanks, Haozhong
diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c index 42efb2f..6ee4352 100644 --- a/backends/hostmem-file.c +++ b/backends/hostmem-file.c @@ -39,17 +39,14 @@ static void file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) { HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(backend); + Error *local_err = NULL; - if (!backend->size) { - error_setg(errp, "can't create backend with size 0"); - return; - } if (!fb->mem_path) { - error_setg(errp, "mem-path property not set"); - return; + error_setg(&local_err, "mem-path property not set"); + goto out; } #ifndef CONFIG_LINUX - error_setg(errp, "-mem-path not supported on this host"); + error_setg(&local_err, "-mem-path not supported on this host"); #else if (!memory_region_size(&backend->mr)) { gchar *path; @@ -58,10 +55,25 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) memory_region_init_ram_from_file(&backend->mr, OBJECT(backend), path, backend->size, fb->share, - fb->mem_path, errp); + fb->mem_path, &local_err); g_free(path); + + if (local_err) { + goto out; + } + + if (!backend->size) { + backend->size = memory_region_size(&backend->mr); + } } #endif + + if (!backend->size) { + error_setg(&local_err, "can't create backend with size 0"); + } + + out: + error_propagate(errp, local_err); } static char *get_mem_path(Object *o, Error **errp) diff --git a/exec.c b/exec.c index 264a25f..89065bd 100644 --- a/exec.c +++ b/exec.c @@ -1234,7 +1234,7 @@ static int64_t get_file_size(int fd) } static void *file_ram_alloc(RAMBlock *block, - ram_addr_t memory, + ram_addr_t *memory, const char *path, Error **errp) { @@ -1245,6 +1245,7 @@ static void *file_ram_alloc(RAMBlock *block, void *area = MAP_FAILED; int fd = -1; int64_t file_size; + ram_addr_t mem_size = *memory; if (kvm_enabled() && !kvm_has_sync_mmu()) { error_setg(errp, @@ -1309,21 +1310,27 @@ static void *file_ram_alloc(RAMBlock *block, file_size = get_file_size(fd); - if (memory < block->page_size) { + if (!mem_size && file_size > 0) { + mem_size = file_size; + memory_region_set_size(block->mr, mem_size); + } + + if (mem_size < block->page_size) { error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be equal to " "or larger than page size 0x%zx", - memory, block->page_size); + mem_size, block->page_size); goto error; } - if (file_size > 0 && file_size < memory) { + if (file_size > 0 && file_size < mem_size) { error_setg(errp, "backing store %s size %"PRId64 " does not match 'size' option %"PRIu64, - path, file_size, memory); + path, file_size, mem_size); goto error; } - memory = ROUND_UP(memory, block->page_size); + mem_size = ROUND_UP(mem_size, block->page_size); + *memory = mem_size; /* * ftruncate is not supported by hugetlbfs in older @@ -1339,11 +1346,11 @@ static void *file_ram_alloc(RAMBlock *block, * those labels. Therefore, extending the non-empty backend file * is disabled as well. */ - if (!file_size && ftruncate(fd, memory)) { + if (!file_size && ftruncate(fd, mem_size)) { perror("ftruncate"); } - area = qemu_ram_mmap(fd, memory, block->mr->align, + area = qemu_ram_mmap(fd, mem_size, block->mr->align, block->flags & RAM_SHARED); if (area == MAP_FAILED) { error_setg_errno(errp, errno, @@ -1352,7 +1359,7 @@ static void *file_ram_alloc(RAMBlock *block, } if (mem_prealloc) { - os_mem_prealloc(fd, area, memory, errp); + os_mem_prealloc(fd, area, mem_size, errp); if (errp && *errp) { goto error; } @@ -1363,7 +1370,7 @@ static void *file_ram_alloc(RAMBlock *block, error: if (area != MAP_FAILED) { - qemu_ram_munmap(area, memory); + qemu_ram_munmap(area, mem_size); } if (unlink_on_error) { unlink(path); @@ -1690,15 +1697,15 @@ RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr, size = HOST_PAGE_ALIGN(size); new_block = g_malloc0(sizeof(*new_block)); new_block->mr = mr; - new_block->used_length = size; - new_block->max_length = size; new_block->flags = share ? RAM_SHARED : 0; - new_block->host = file_ram_alloc(new_block, size, + new_block->host = file_ram_alloc(new_block, &size, mem_path, errp); if (!new_block->host) { g_free(new_block); return NULL; } + new_block->used_length = size; + new_block->max_length = size; ram_block_add(new_block, &local_err); if (local_err) {
If 'size' option is not given, Qemu will use the file size of 'mem-path' instead. If an empty file, a non-existing file or a directory is specified by option 'mem-path', a non-zero option 'size' is still needed. Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> --- backends/hostmem-file.c | 28 ++++++++++++++++++++-------- exec.c | 33 ++++++++++++++++++++------------- 2 files changed, 40 insertions(+), 21 deletions(-)