On 10/25/16 17:50 -0200, Eduardo Habkost wrote:
On Mon, Oct 24, 2016 at 05:21:51PM +0800, Haozhong Zhang wrote:
If 'size' option of hostmem-file is not given, QEMU will use the file
size of 'mem-path' instead. For an empty file, a non-zero size must be
specified by the option 'size'.
Signed-off-by: Haozhong Zhang <haozhong.zh...@intel.com>
---
backends/hostmem-file.c | 10 ++++++----
exec.c | 8 ++++++++
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index 42efb2f..f94d2f7 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -40,10 +40,6 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error
**errp)
{
HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(backend);
- 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;
@@ -62,6 +58,12 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error
**errp)
g_free(path);
}
#endif
+ if (!errp && !backend->size) {
This condition is always false because non-NULL errp is always
provided by the only caller (host_memory_backend_memory_complete()).
Oops, I meant !*errp. Anyway, I'll change to the way you suggested below.
To simplify error checking, I suggest moving the error path to a
label at the end of the function, e.g.:
static void file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
{
Error *local_err = NULL;
/* ... */
memory_region_init_ram_from_file(..., &local_err);
if (local_err) {
goto out;
}
/* ... */
if (!backend->size) {
backend->size = memory_region_size(&backend->mr);
}
/* ... */
out:
error_propagate(errp, local_err);
}
+ backend->size = memory_region_size(&backend->mr);
+ if (!backend->size) {
+ error_setg(errp, "can't create backend with size 0");
+ }
+ }
}
static char *get_mem_path(Object *o, Error **errp)
diff --git a/exec.c b/exec.c
index 95983c9..91adc62 100644
--- a/exec.c
+++ b/exec.c
@@ -1274,6 +1274,14 @@ static void *file_ram_alloc(RAMBlock *block,
goto error;
}
+ if (memory) {
+ memory = memory ?: file_size;
This doesn't make sense to me. You already checked if memory is
zero above, and now you are checking if it's zero again.
file_size is never going to be used here.
+ memory_region_set_size(block->mr, memory);
+ memory = HOST_PAGE_ALIGN(memory);
+ block->used_length = memory;
+ block->max_length = memory;
This is fragile: it duplicates the logic that initializes
used_length and max_length in qemu_ram_alloc_*().
Maybe it's better to keep the file-size-probing magic inside
hostmem-file.c, and always give a non-zero size to
memory_region_init_ram_from_file().
Yes, I can move the logic in above if-statement to qemu_ram_alloc_from_file().
Thanks,
Haozhong
+ }
+
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",
--
2.10.1
--
Eduardo
