Re: [Qemu-devel] [PATCH v4 03/24] pc-dimm: pass PCDIMMDevice to pc_dimm_.*plug

[David Hildenbrand]

On 27/09/2018 09:40, Igor Mammedov wrote:
> On Wed, 26 Sep 2018 11:41:58 +0200
> David Hildenbrand <address@hidden> wrote:
> 
>> We're plugging/unplugging a PCDIMMDevice, so directly pass this type
>> instead of a more generic DeviceState.
>>
>> Signed-off-by: David Hildenbrand <address@hidden>
>> ---
>>  hw/i386/pc.c             |  6 +++---
>>  hw/mem/pc-dimm.c         | 16 +++++++---------
>>  hw/ppc/spapr.c           |  8 ++++----
>>  include/hw/mem/pc-dimm.h |  6 +++---
>>  4 files changed, 17 insertions(+), 19 deletions(-)
>>
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index 03148450c8..86c16f9aaf 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -1699,7 +1699,7 @@ static void pc_memory_pre_plug(HotplugHandler 
>> *hotplug_dev, DeviceState *dev,
>>          return;
>>      }
>>  
>> -    pc_dimm_pre_plug(dev, MACHINE(hotplug_dev),
>> +    pc_dimm_pre_plug(PC_DIMM(dev), MACHINE(hotplug_dev),
>>                       pcmc->enforce_aligned_dimm ? NULL : &legacy_align, 
>> errp);
>>  }
>>  
>> @@ -1711,7 +1711,7 @@ static void pc_memory_plug(HotplugHandler *hotplug_dev,
>>      PCMachineState *pcms = PC_MACHINE(hotplug_dev);
>>      bool is_nvdimm = object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM);
>>  
>> -    pc_dimm_plug(dev, MACHINE(pcms), &local_err);
>> +    pc_dimm_plug(PC_DIMM(dev), MACHINE(pcms), &local_err);
>>      if (local_err) {
>>          goto out;
>>      }
>> @@ -1771,7 +1771,7 @@ static void pc_memory_unplug(HotplugHandler 
>> *hotplug_dev,
>>          goto out;
>>      }
>>  
>> -    pc_dimm_unplug(dev, MACHINE(pcms));
>> +    pc_dimm_unplug(PC_DIMM(dev), MACHINE(pcms));
>>      object_unparent(OBJECT(dev));
>>  
>>   out:
>> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
>> index fb6bcaedc4..2375eb2731 100644
>> --- a/hw/mem/pc-dimm.c
>> +++ b/hw/mem/pc-dimm.c
>> @@ -29,11 +29,11 @@
>>  
>>  static int pc_dimm_get_free_slot(const int *hint, int max_slots, Error 
>> **errp);
>>  
>> -void pc_dimm_pre_plug(DeviceState *dev, MachineState *machine,
>> +void pc_dimm_pre_plug(PCDIMMDevice *dimm, MachineState *machine,
>>                        const uint64_t *legacy_align, Error **errp)
>>  {
>> -    PCDIMMDevice *dimm = PC_DIMM(dev);
>>      PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
>> +    DeviceState *dev = DEVICE(dimm);
> There is no reason to keep 'dev'
> since you are removing it from signature, remove it altogether
> even if touches all OBJECT(dev) sites in the function.
> 

I can do that, thanks.

> With this fixed:
> Reviewed-by: Igor Mammedov <address@hidden>


-- 

Thanks,

David / dhildenb