On Fri, 15 Jun 2018 11:48:33 +0200 David Hildenbrand <da...@redhat.com> wrote:
> On 15.06.2018 11:34, Igor Mammedov wrote: > > On Wed, 13 Jun 2018 16:15:48 +0200 > > David Hildenbrand <da...@redhat.com> wrote: > > > >> On 13.06.2018 15:10, Igor Mammedov wrote: > >>> On Mon, 11 Jun 2018 14:16:54 +0200 > >>> David Hildenbrand <da...@redhat.com> wrote: > >>> > >>>> We'll be factoring out some pc-dimm specific and some memory-device > >>>> checks next. > >>>> > >>>> Signed-off-by: David Hildenbrand <da...@redhat.com> > >>>> --- > >>>> hw/i386/pc.c | 2 ++ > >>>> hw/mem/pc-dimm.c | 5 +++++ > >>>> hw/ppc/spapr.c | 1 + > >>>> include/hw/mem/pc-dimm.h | 2 ++ > >>>> 4 files changed, 10 insertions(+) > >>>> > >>>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c > >>>> index 017396fe84..dc8e7b033b 100644 > >>>> --- a/hw/i386/pc.c > >>>> +++ b/hw/i386/pc.c > >>>> @@ -1695,6 +1695,8 @@ static void pc_dimm_pre_plug(HotplugHandler > >>>> *hotplug_dev, DeviceState *dev, > >>> keeping ^^^^^ > >>> similar to newly introduced pc_dimm_memory_pre_plug() > >>> and I have no clue what to suggest as alternative > >> > >> Can you elaborate? > > > > It's just that pc_dimm_pre_plug and pc_dimm_memory_pre_plug > > are very similar so it become confusing and with name alone > > you can't figure if both do the same or different things. > > > > Looking at 11/11 maybe you could just drop this and the next > > patch for now as there isn't obvious (if any) demand for it > > within this series at all. > > And introduce similar patch when it's actually required. > > > > I might imagine following naming in future: > > > > pc_dimm_pre_plug() > > memory_device_pre_plug() > > ... some pc-dimm only stuff ... > > > > virtio_mem_pre_plug() > > memory_device_pre_plug() > > ... some virtio-mem only stuff ... > > We will always have the chain > > $machine_XXX_pre_plug() > ... some machine specific stuff > pc_dimm_memory_pre_plug() > ... some pc-dimm specific stuff > memory_device_pre_plug() > > I can rename > > pc_dimm_(plug|unplug ...) to > pc_memory_(plug|unplug ...) > > and pc_dimm_memory_(plug|unplug ...) to > pc_dimm_(plug|unplug ...) > > That way we match spapr naming scheme: > > spapr_memory_plug/spapr_memory_unplug sounds good to me
