From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52877) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fTlXb-0003UQ-LG for qemu-devel@nongnu.org; Fri, 15 Jun 2018 06:01:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fTlXa-0001aB-Lm for qemu-devel@nongnu.org; Fri, 15 Jun 2018 06:01:11 -0400 Date: Fri, 15 Jun 2018 12:01:06 +0200 From: Igor Mammedov Message-ID: <20180615120106.537970f0@redhat.com> In-Reply-To: References: <20180611121655.19616-1-david@redhat.com> <20180611121655.19616-11-david@redhat.com> <20180613151056.6ebbccc2@redhat.com> <569f9534-162c-29c1-a097-51427e028b4e@redhat.com> <20180615113440.56844675@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v1 10/11] pc-dimm: introduce and use pc_dimm_memory_pre_plug() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Hildenbrand Cc: qemu-devel@nongnu.org, Eduardo Habkost , "Michael S . Tsirkin" , Xiao Guangrong , Alexander Graf , qemu-ppc@nongnu.org, Paolo Bonzini , David Gibson , Richard Henderson On Fri, 15 Jun 2018 11:48:33 +0200 David Hildenbrand wrote: > On 15.06.2018 11:34, Igor Mammedov wrote: > > On Wed, 13 Jun 2018 16:15:48 +0200 > > David Hildenbrand wrote: > > > >> On 13.06.2018 15:10, Igor Mammedov wrote: > >>> On Mon, 11 Jun 2018 14:16:54 +0200 > >>> David Hildenbrand wrote: > >>> > >>>> We'll be factoring out some pc-dimm specific and some memory-device > >>>> checks next. > >>>> > >>>> Signed-off-by: David Hildenbrand > >>>> --- > >>>> 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