Re: [Qemu-devel] [PATCH v4 14/24] memory-device: complete factoring out plug handling

[Igor Mammedov <imammedo@redhat.com>]

On Mon, 1 Oct 2018 10:18:20 +0200
David Hildenbrand <david@redhat.com> wrote:

> On 01/10/2018 10:15, David Hildenbrand wrote:
> > On 30/09/2018 17:55, Auger Eric wrote:  
> >> Hi David,
> >>
> >> On 9/26/18 11:42 AM, David Hildenbrand wrote:  
> >>> With the new memory device functions in place, we can factor out
> >>> plugging of memory devices completely.
> >>>
> >>> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> >>> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> >>> Signed-off-by: David Hildenbrand <david@redhat.com>
> >>> ---
> >>>  hw/mem/memory-device.c         | 13 ++++++++++---
> >>>  hw/mem/pc-dimm.c               |  9 +--------
> >>>  include/hw/mem/memory-device.h |  3 +--
> >>>  3 files changed, 12 insertions(+), 13 deletions(-)
> >>>
> >>> diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c
> >>> index 54e3f23b15..3914e2fe6f 100644
> >>> --- a/hw/mem/memory-device.c
> >>> +++ b/hw/mem/memory-device.c
> >>> @@ -275,10 +275,17 @@ out:
> >>>      error_propagate(errp, local_err);
> >>>  }
> >>>  
> >>> -void memory_device_plug_region(MachineState *ms, MemoryRegion *mr,
> >>> -                               uint64_t addr)
> >>> +void memory_device_plug(MemoryDeviceState *md, MachineState *ms)
> >>>  {
> >>> -    /* we expect a previous call to memory_device_get_free_addr() */
> >>> +    const MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(md);
> >>> +    const uint64_t addr = mdc->get_addr(md);
> >>> +    MemoryRegion *mr;
> >>> +
> >>> +    /*
> >>> +     * We expect a previous call to memory_device_pre_plug() succeeded so
> >>> +     * it and can't fail at this point.  
> >> comment to be reworded
> >>  
> > 
> > Igor requested that rewording. I can turn that into "We expect that a
> > previous call ... succeeded, so " ..
> >   
> 
> "We expect that a previous call to memory_device_pre_plug() succeeded,
> so it can't fail at this point."
looks good to me

> 
> to be precise :)
> 
> >> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> >>
> >> Thanks
> >>
> >> Eric  
> > 
> >   
> 
>