Re: [Qemu-devel] [PATCH V2 for 2.1 0/3] bug fixs for memory backend

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, Jul 07, 2014 at 04:17:30PM +0800, Hu Tao wrote:
> On Mon, Jul 07, 2014 at 06:35:42AM +0300, Michael S. Tsirkin wrote:
> > On Mon, Jul 07, 2014 at 06:24:59AM +0300, Michael S. Tsirkin wrote:
> > > On Mon, Jul 07, 2014 at 10:58:05AM +0800, Hu Tao wrote:
> > > > This series includes three patches to fix bugs of memory backend. Patch
> > > > 1 prepares for next patches, patch 2 and patch 3 fix two bugs
> > > > respectively, see each patch for the bugs and how to reproduce them.
> > > > 
> > > > changes to v1:
> > > > 
> > > >   - split patch 1 in v1 into 2 patches
> > > >   - don't rely on ram_block_add to return -1
> > > >   - error message tweak in file_ram_alloc
> > > >   - add error messages reported by qemu to commit message of patch 3
> > > > 
> > > > Hu Tao (3):
> > > >   memory: rename memory_region_init_ram() and
> > > >     memory_region_init_ram_ptr()
> > > >   memory: add errp parameter to memory_region_init_ram() and    
> > > >     memory_region_init_ram_ptr()
> > > >   exec: improve error handling and reporting in file_ram_alloc() and
> > > >     gethugepagesize()
> > > 
> > > I fixed up some minor issues and applied this, thanks.
> > 
> > And reverted.
> > 
> > Build fails, and a simple check after applying patch 1 gives me:
> > git grep memory_region_init_ram |grep -v nofail|wc -l
> > 132
> 
> Thanks for catching this! I should have built all targets.
> 
> > 
> > Apparently you fixed up about 10% of the files using this function.
> > So forget about me merging patch 1.
> > 
> > Add a new
> > memory_region_init_ram_may_fail
> > and
> > memory_region_init_ram_ptr_may_fail
> > 
> > and use it specifically for the new stuff.
> 
> Thanks for the change!

To clarify: I didn't apply this change.  I simply reverted the whole
patchset.  If you want this patchset applied please do it, at this point
I'm making no promises on whether it will get into 2.1.


> > 
> > Do the rename on top in two steps:
> > memory_region_init_ram -> memory_region_init_ram_nofail
> > memory_region_init_ram_may_fail -> memory_region_init_ram
> > 
> > Paolo can then merge it when he prefers, though I'd say 2.2
> > is more reasonable.
> > 
> > 
> > > >  backends/hostmem-ram.c   |  2 +-
> > > >  exec.c                   | 51 ++++++++++++++++++++++++++++---------------
> > > >  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    | 43 +++++++++++++++++++++++++++++++++---
> > > >  include/exec/ram_addr.h  |  4 ++--
> > > >  memory.c                 | 57 +++++++++++++++++++++++++++++++++++++++++++-----
> > > >  numa.c                   |  4 ++--
> > > >  17 files changed, 158 insertions(+), 49 deletions(-)
> > > > 
> > > > -- 
> > > > 1.9.3