Re: [Qemu-devel] [PATCH for 2.1 1/2] memory: introduce memory_region_init_ram_nofail() and memory_region_init_ram_ptr_nofail()

[Hu Tao]

On Thu, Jul 03, 2014 at 09:51:13AM +0300, Michael S. Tsirkin wrote:
> On Thu, Jul 03, 2014 at 02:10:55PM +0800, Hu Tao wrote:
> > Introduce memory_region_init_ram_nofail() and
> > memory_region_init_ram_ptr_nofail(), which are the same as
> > memory_region_init_ram() and memory_region_init_ram_ptr()
> > respectively. They will exit qemu if there is an error, this is the
> > behaviour of old memory_region_init_ram() and
> > memory_region_init_ram_ptr().
> > 
> > All existing calls to memory_region_init_ram() and
> > memory_region_init_ram_ptr() are replaced with
> > memory_region_init_ram_nofail() and memory_region_init_ram_ptr_nofail().
> > 
> > memory_region_init_ram() and memory_region_init_ram_ptr() are added an
> > extra parameter errp to let callers handle the error.
> > 
> > This patch solves a problem that qemu just exits when using monitor
> > command object_add to add a memory backend whose size is way too large.
> > In the case we'd better give an error message and keep guest running.
> > 
> > How to reproduce:
> > 
> > 1. run qemu
> > 2. (monitor)object_add memory-backend-ram,size=100000G,id=ram0
> > 
> > 
> 
> Don't put two empty lines in a row please.

I'll fix it.

> 
> > Signed-off-by: Hu Tao <address@hidden>
> > ---
> >  backends/hostmem-ram.c   |  2 +-
> >  exec.c                   | 30 +++++++++++++++++--------
> >  hw/block/pflash_cfi01.c  |  5 ++++-
> >  hw/block/pflash_cfi02.c  |  5 ++++-
> >  hw/core/loader.c         |  2 +-
> >  hw/display/vga.c         |  2 +-
> >  hw/display/vmware_vga.c  |  3 ++-
> >  hw/i386/kvm/pci-assign.c |  9 ++++----
> >  hw/i386/pc.c             |  2 +-
> >  hw/i386/pc_sysfw.c       |  4 ++--
> >  hw/misc/ivshmem.c        |  9 ++++----
> >  hw/misc/vfio.c           |  3 ++-
> >  hw/pci/pci.c             |  2 +-
> >  include/exec/memory.h    | 32 ++++++++++++++++++++++++---
> >  include/exec/ram_addr.h  |  4 ++--
> >  memory.c                 | 57 
> > +++++++++++++++++++++++++++++++++++++++++++-----
> >  numa.c                   |  4 ++--
> >  17 files changed, 134 insertions(+), 41 deletions(-)
> > 
> > diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c
> > index d9a8290..a67a134 100644
> > --- a/backends/hostmem-ram.c
> > +++ b/backends/hostmem-ram.c
> > @@ -27,7 +27,7 @@ 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);
> > +                           backend->size, errp);
> >      g_free(path);
> >  }
> >  
> 
> Sigh.  So you are still mixing a huge mechanical rename with
> a bugfix.  I'm not merging this, please split up the patch:
> 1. rename existing functions and convert all users to _nofail
> 2. add parameter to qemu_ram_alloc variants,
>    add new function and use in hostmem-ram

Sure. Thanks for review!


Regards,
Hu