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()).
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().
> + }
> +
> 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
