On Sun, Jun 15, 2014 at 12:58:47PM +0300, Michael S. Tsirkin wrote:
> On Sat, Jun 14, 2014 at 07:07:39PM +0200, Paolo Bonzini wrote:
> > Il 14/06/2014 06:48, Hu Tao ha scritto:
> > >return -1 instead.
> > >
> > >Now user can add objects memory-backend-ram on-the-fly, fail it if
> > >cannot allocate memory rather than quit qemu.
> > >
> > >Signed-off-by: Hu Tao <hu...@cn.fujitsu.com>
> >
> > This needs an audit of all callers or, alternatively, we need to add
> > memory_region_init_ram_nofail. Better leave it for after the merge.
Paolo, IIUC you suggested we fix it after merge.
> >
> > Paolo
>
> Specifically memory_region_init_ram_from_file does not seem to
> handle failures.
memory_region_init_ram_from_file has an errp. Also see my updated patch.
>
> qemu_ram_free chunk also looks weird. Can we not avoid calling
> free on invalid addresses?
but we still need to check the address somewhere if not in
qemu_ram_free.
>
> > >---
> > > backends/hostmem-ram.c | 3 +++
> > > exec.c | 6 +++++-
> > > 2 files changed, 8 insertions(+), 1 deletion(-)
> > >
> > >diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c
> > >index d9a8290..afb305d 100644
> > >--- a/backends/hostmem-ram.c
> > >+++ b/backends/hostmem-ram.c
> > >@@ -28,6 +28,9 @@ ram_backend_memory_alloc(HostMemoryBackend *backend,
> > >Error **errp)
> > > path = object_get_canonical_path_component(OBJECT(backend));
> > > memory_region_init_ram(&backend->mr, OBJECT(backend), path,
> > > backend->size);
> > >+ if (backend->mr.ram_addr == -1) {
> > >+ error_setg(errp, "can't allocate memory");
> > >+ }
> > > g_free(path);
> > > }
> > >
> > >diff --git a/exec.c b/exec.c
> > >index 8705cc5..74560e5 100644
> > >--- a/exec.c
> > >+++ b/exec.c
> > >@@ -1228,7 +1228,7 @@ static ram_addr_t ram_block_add(RAMBlock *new_block)
> > > if (!new_block->host) {
> > > fprintf(stderr, "Cannot set up guest memory '%s': %s\n",
> > > new_block->mr->name, strerror(errno));
> > >- exit(1);
> > >+ return -1;
> > > }
> > > memory_try_enable_merging(new_block->host, new_block->length);
> > > }
> > >@@ -1356,6 +1356,10 @@ void qemu_ram_free(ram_addr_t addr)
> > > {
> > > RAMBlock *block;
> > >
> > >+ if (addr == -1) {
> > >+ return;
> > >+ }
> > >+
> > > /* This assumes the iothread lock is taken here too. */
> > > qemu_mutex_lock_ramlist();
> > > QTAILQ_FOREACH(block, &ram_list.blocks, next) {
> > >
