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
--
Thanks,
David / dhildenb
