We're plugging/unplugging a PCDIMMDevice, so directly pass this type
instead of a more generic DeviceState.
Signed-off-by: David Hildenbrand <david@redhat.com>
---
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);
Error *local_err = NULL;
MemoryRegion *mr;
uint64_t addr, align;
@@ -69,32 +69,30 @@ out:
error_propagate(errp, local_err);
}
-void pc_dimm_plug(DeviceState *dev, MachineState *machine, Error **errp)
+void pc_dimm_plug(PCDIMMDevice *dimm, MachineState *machine, Error **errp)
{
- PCDIMMDevice *dimm = PC_DIMM(dev);
PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
MemoryRegion *vmstate_mr = ddc->get_vmstate_memory_region(dimm,
&error_abort);
MemoryRegion *mr = ddc->get_memory_region(dimm, &error_abort);
uint64_t addr;
- addr = object_property_get_uint(OBJECT(dev), PC_DIMM_ADDR_PROP,
+ addr = object_property_get_uint(OBJECT(dimm), PC_DIMM_ADDR_PROP,
&error_abort);
memory_device_plug_region(machine, mr, addr);
- vmstate_register_ram(vmstate_mr, dev);
+ vmstate_register_ram(vmstate_mr, DEVICE(dimm));
}
-void pc_dimm_unplug(DeviceState *dev, MachineState *machine)
+void pc_dimm_unplug(PCDIMMDevice *dimm, MachineState *machine)
{
- PCDIMMDevice *dimm = PC_DIMM(dev);
PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
MemoryRegion *vmstate_mr = ddc->get_vmstate_memory_region(dimm,
&error_abort);
MemoryRegion *mr = ddc->get_memory_region(dimm, &error_abort);
memory_device_unplug_region(machine, mr);
- vmstate_unregister_ram(vmstate_mr, dev);
+ vmstate_unregister_ram(vmstate_mr, DEVICE(dimm));
}
static int pc_dimm_slot2bitmap(Object *obj, void *opaque)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 98868d893a..c078347b66 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3135,7 +3135,7 @@ static void spapr_memory_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
size = memory_region_size(mr);
- pc_dimm_plug(dev, MACHINE(ms), &local_err);
+ pc_dimm_plug(dimm, MACHINE(ms), &local_err);
if (local_err) {
goto out;
}
@@ -3158,7 +3158,7 @@ static void spapr_memory_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
return;
out_unplug:
- pc_dimm_unplug(dev, MACHINE(ms));
+ pc_dimm_unplug(dimm, MACHINE(ms));
out:
error_propagate(errp, local_err);
}
@@ -3202,7 +3202,7 @@ static void spapr_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
return;
}
- pc_dimm_pre_plug(dev, MACHINE(hotplug_dev), NULL, errp);
+ pc_dimm_pre_plug(dimm, MACHINE(hotplug_dev), NULL, errp);
}
struct sPAPRDIMMState {
@@ -3314,7 +3314,7 @@ static void spapr_memory_unplug(HotplugHandler *hotplug_dev, DeviceState *dev)
sPAPRMachineState *spapr = SPAPR_MACHINE(hotplug_dev);
sPAPRDIMMState *ds = spapr_pending_dimm_unplugs_find(spapr, PC_DIMM(dev));
- pc_dimm_unplug(dev, MACHINE(hotplug_dev));
+ pc_dimm_unplug(PC_DIMM(dev), MACHINE(hotplug_dev));
object_unparent(OBJECT(dev));
spapr_pending_dimm_unplugs_remove(spapr, ds);
}
diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
index b382eb4303..99cbd54de7 100644
--- a/include/hw/mem/pc-dimm.h
+++ b/include/hw/mem/pc-dimm.h
@@ -79,8 +79,8 @@ typedef struct PCDIMMDeviceClass {
Error **errp);
} PCDIMMDeviceClass;
-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);
-void pc_dimm_plug(DeviceState *dev, MachineState *machine, Error **errp);
-void pc_dimm_unplug(DeviceState *dev, MachineState *machine);
+void pc_dimm_plug(PCDIMMDevice *dimm, MachineState *machine, Error **errp);
+void pc_dimm_unplug(PCDIMMDevice *dimm, MachineState *machine);
#endif
--
2.17.1
On Wed, Sep 26, 2018 at 11:41:58AM +0200, David Hildenbrand wrote:
> We're plugging/unplugging a PCDIMMDevice, so directly pass this type
> instead of a more generic DeviceState.
>
> Signed-off-by: David Hildenbrand <david@redhat.com>
Seems type-safer than the previous version.
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
and ppc parts:
Acked-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> 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);
> Error *local_err = NULL;
> MemoryRegion *mr;
> uint64_t addr, align;
> @@ -69,32 +69,30 @@ out:
> error_propagate(errp, local_err);
> }
>
> -void pc_dimm_plug(DeviceState *dev, MachineState *machine, Error **errp)
> +void pc_dimm_plug(PCDIMMDevice *dimm, MachineState *machine, Error **errp)
> {
> - PCDIMMDevice *dimm = PC_DIMM(dev);
> PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
> MemoryRegion *vmstate_mr = ddc->get_vmstate_memory_region(dimm,
> &error_abort);
> MemoryRegion *mr = ddc->get_memory_region(dimm, &error_abort);
> uint64_t addr;
>
> - addr = object_property_get_uint(OBJECT(dev), PC_DIMM_ADDR_PROP,
> + addr = object_property_get_uint(OBJECT(dimm), PC_DIMM_ADDR_PROP,
> &error_abort);
>
> memory_device_plug_region(machine, mr, addr);
> - vmstate_register_ram(vmstate_mr, dev);
> + vmstate_register_ram(vmstate_mr, DEVICE(dimm));
> }
>
> -void pc_dimm_unplug(DeviceState *dev, MachineState *machine)
> +void pc_dimm_unplug(PCDIMMDevice *dimm, MachineState *machine)
> {
> - PCDIMMDevice *dimm = PC_DIMM(dev);
> PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
> MemoryRegion *vmstate_mr = ddc->get_vmstate_memory_region(dimm,
> &error_abort);
> MemoryRegion *mr = ddc->get_memory_region(dimm, &error_abort);
>
> memory_device_unplug_region(machine, mr);
> - vmstate_unregister_ram(vmstate_mr, dev);
> + vmstate_unregister_ram(vmstate_mr, DEVICE(dimm));
> }
>
> static int pc_dimm_slot2bitmap(Object *obj, void *opaque)
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 98868d893a..c078347b66 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3135,7 +3135,7 @@ static void spapr_memory_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
>
> size = memory_region_size(mr);
>
> - pc_dimm_plug(dev, MACHINE(ms), &local_err);
> + pc_dimm_plug(dimm, MACHINE(ms), &local_err);
> if (local_err) {
> goto out;
> }
> @@ -3158,7 +3158,7 @@ static void spapr_memory_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
> return;
>
> out_unplug:
> - pc_dimm_unplug(dev, MACHINE(ms));
> + pc_dimm_unplug(dimm, MACHINE(ms));
> out:
> error_propagate(errp, local_err);
> }
> @@ -3202,7 +3202,7 @@ static void spapr_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
> return;
> }
>
> - pc_dimm_pre_plug(dev, MACHINE(hotplug_dev), NULL, errp);
> + pc_dimm_pre_plug(dimm, MACHINE(hotplug_dev), NULL, errp);
> }
>
> struct sPAPRDIMMState {
> @@ -3314,7 +3314,7 @@ static void spapr_memory_unplug(HotplugHandler *hotplug_dev, DeviceState *dev)
> sPAPRMachineState *spapr = SPAPR_MACHINE(hotplug_dev);
> sPAPRDIMMState *ds = spapr_pending_dimm_unplugs_find(spapr, PC_DIMM(dev));
>
> - pc_dimm_unplug(dev, MACHINE(hotplug_dev));
> + pc_dimm_unplug(PC_DIMM(dev), MACHINE(hotplug_dev));
> object_unparent(OBJECT(dev));
> spapr_pending_dimm_unplugs_remove(spapr, ds);
> }
> diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
> index b382eb4303..99cbd54de7 100644
> --- a/include/hw/mem/pc-dimm.h
> +++ b/include/hw/mem/pc-dimm.h
> @@ -79,8 +79,8 @@ typedef struct PCDIMMDeviceClass {
> Error **errp);
> } PCDIMMDeviceClass;
>
> -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);
> -void pc_dimm_plug(DeviceState *dev, MachineState *machine, Error **errp);
> -void pc_dimm_unplug(DeviceState *dev, MachineState *machine);
> +void pc_dimm_plug(PCDIMMDevice *dimm, MachineState *machine, Error **errp);
> +void pc_dimm_unplug(PCDIMMDevice *dimm, MachineState *machine);
> #endif
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
On Wed, 26 Sep 2018 11:41:58 +0200
David Hildenbrand <david@redhat.com> wrote:
> We're plugging/unplugging a PCDIMMDevice, so directly pass this type
> instead of a more generic DeviceState.
>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
> 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.
With this fixed:
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> Error *local_err = NULL;
> MemoryRegion *mr;
> uint64_t addr, align;
> @@ -69,32 +69,30 @@ out:
> error_propagate(errp, local_err);
> }
>
> -void pc_dimm_plug(DeviceState *dev, MachineState *machine, Error **errp)
> +void pc_dimm_plug(PCDIMMDevice *dimm, MachineState *machine, Error **errp)
> {
> - PCDIMMDevice *dimm = PC_DIMM(dev);
> PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
> MemoryRegion *vmstate_mr = ddc->get_vmstate_memory_region(dimm,
> &error_abort);
> MemoryRegion *mr = ddc->get_memory_region(dimm, &error_abort);
> uint64_t addr;
>
> - addr = object_property_get_uint(OBJECT(dev), PC_DIMM_ADDR_PROP,
> + addr = object_property_get_uint(OBJECT(dimm), PC_DIMM_ADDR_PROP,
> &error_abort);
>
> memory_device_plug_region(machine, mr, addr);
> - vmstate_register_ram(vmstate_mr, dev);
> + vmstate_register_ram(vmstate_mr, DEVICE(dimm));
> }
>
> -void pc_dimm_unplug(DeviceState *dev, MachineState *machine)
> +void pc_dimm_unplug(PCDIMMDevice *dimm, MachineState *machine)
> {
> - PCDIMMDevice *dimm = PC_DIMM(dev);
> PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
> MemoryRegion *vmstate_mr = ddc->get_vmstate_memory_region(dimm,
> &error_abort);
> MemoryRegion *mr = ddc->get_memory_region(dimm, &error_abort);
>
> memory_device_unplug_region(machine, mr);
> - vmstate_unregister_ram(vmstate_mr, dev);
> + vmstate_unregister_ram(vmstate_mr, DEVICE(dimm));
> }
>
> static int pc_dimm_slot2bitmap(Object *obj, void *opaque)
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 98868d893a..c078347b66 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3135,7 +3135,7 @@ static void spapr_memory_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
>
> size = memory_region_size(mr);
>
> - pc_dimm_plug(dev, MACHINE(ms), &local_err);
> + pc_dimm_plug(dimm, MACHINE(ms), &local_err);
> if (local_err) {
> goto out;
> }
> @@ -3158,7 +3158,7 @@ static void spapr_memory_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
> return;
>
> out_unplug:
> - pc_dimm_unplug(dev, MACHINE(ms));
> + pc_dimm_unplug(dimm, MACHINE(ms));
> out:
> error_propagate(errp, local_err);
> }
> @@ -3202,7 +3202,7 @@ static void spapr_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
> return;
> }
>
> - pc_dimm_pre_plug(dev, MACHINE(hotplug_dev), NULL, errp);
> + pc_dimm_pre_plug(dimm, MACHINE(hotplug_dev), NULL, errp);
> }
>
> struct sPAPRDIMMState {
> @@ -3314,7 +3314,7 @@ static void spapr_memory_unplug(HotplugHandler *hotplug_dev, DeviceState *dev)
> sPAPRMachineState *spapr = SPAPR_MACHINE(hotplug_dev);
> sPAPRDIMMState *ds = spapr_pending_dimm_unplugs_find(spapr, PC_DIMM(dev));
>
> - pc_dimm_unplug(dev, MACHINE(hotplug_dev));
> + pc_dimm_unplug(PC_DIMM(dev), MACHINE(hotplug_dev));
> object_unparent(OBJECT(dev));
> spapr_pending_dimm_unplugs_remove(spapr, ds);
> }
> diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
> index b382eb4303..99cbd54de7 100644
> --- a/include/hw/mem/pc-dimm.h
> +++ b/include/hw/mem/pc-dimm.h
> @@ -79,8 +79,8 @@ typedef struct PCDIMMDeviceClass {
> Error **errp);
> } PCDIMMDeviceClass;
>
> -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);
> -void pc_dimm_plug(DeviceState *dev, MachineState *machine, Error **errp);
> -void pc_dimm_unplug(DeviceState *dev, MachineState *machine);
> +void pc_dimm_plug(PCDIMMDevice *dimm, MachineState *machine, Error **errp);
> +void pc_dimm_unplug(PCDIMMDevice *dimm, MachineState *machine);
> #endif
On 27/09/2018 09:40, Igor Mammedov wrote:
> On Wed, 26 Sep 2018 11:41:58 +0200
> David Hildenbrand <david@redhat.com> wrote:
>
>> We're plugging/unplugging a PCDIMMDevice, so directly pass this type
>> instead of a more generic DeviceState.
>>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>> 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 <imammedo@redhat.com>
--
Thanks,
David / dhildenb
Hi David,
On 9/26/18 11:41 AM, David Hildenbrand wrote:
> We're plugging/unplugging a PCDIMMDevice, so directly pass this type
> instead of a more generic DeviceState.
>
> Signed-off-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Thanks
Eric
> ---
> 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);
> Error *local_err = NULL;
> MemoryRegion *mr;
> uint64_t addr, align;
> @@ -69,32 +69,30 @@ out:
> error_propagate(errp, local_err);
> }
>
> -void pc_dimm_plug(DeviceState *dev, MachineState *machine, Error **errp)
> +void pc_dimm_plug(PCDIMMDevice *dimm, MachineState *machine, Error **errp)
> {
> - PCDIMMDevice *dimm = PC_DIMM(dev);
> PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
> MemoryRegion *vmstate_mr = ddc->get_vmstate_memory_region(dimm,
> &error_abort);
> MemoryRegion *mr = ddc->get_memory_region(dimm, &error_abort);
> uint64_t addr;
>
> - addr = object_property_get_uint(OBJECT(dev), PC_DIMM_ADDR_PROP,
> + addr = object_property_get_uint(OBJECT(dimm), PC_DIMM_ADDR_PROP,
> &error_abort);
>
> memory_device_plug_region(machine, mr, addr);
> - vmstate_register_ram(vmstate_mr, dev);
> + vmstate_register_ram(vmstate_mr, DEVICE(dimm));
> }
>
> -void pc_dimm_unplug(DeviceState *dev, MachineState *machine)
> +void pc_dimm_unplug(PCDIMMDevice *dimm, MachineState *machine)
> {
> - PCDIMMDevice *dimm = PC_DIMM(dev);
> PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
> MemoryRegion *vmstate_mr = ddc->get_vmstate_memory_region(dimm,
> &error_abort);
> MemoryRegion *mr = ddc->get_memory_region(dimm, &error_abort);
>
> memory_device_unplug_region(machine, mr);
> - vmstate_unregister_ram(vmstate_mr, dev);
> + vmstate_unregister_ram(vmstate_mr, DEVICE(dimm));
> }
>
> static int pc_dimm_slot2bitmap(Object *obj, void *opaque)
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 98868d893a..c078347b66 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3135,7 +3135,7 @@ static void spapr_memory_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
>
> size = memory_region_size(mr);
>
> - pc_dimm_plug(dev, MACHINE(ms), &local_err);
> + pc_dimm_plug(dimm, MACHINE(ms), &local_err);
> if (local_err) {
> goto out;
> }
> @@ -3158,7 +3158,7 @@ static void spapr_memory_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
> return;
>
> out_unplug:
> - pc_dimm_unplug(dev, MACHINE(ms));
> + pc_dimm_unplug(dimm, MACHINE(ms));
> out:
> error_propagate(errp, local_err);
> }
> @@ -3202,7 +3202,7 @@ static void spapr_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
> return;
> }
>
> - pc_dimm_pre_plug(dev, MACHINE(hotplug_dev), NULL, errp);
> + pc_dimm_pre_plug(dimm, MACHINE(hotplug_dev), NULL, errp);
> }
>
> struct sPAPRDIMMState {
> @@ -3314,7 +3314,7 @@ static void spapr_memory_unplug(HotplugHandler *hotplug_dev, DeviceState *dev)
> sPAPRMachineState *spapr = SPAPR_MACHINE(hotplug_dev);
> sPAPRDIMMState *ds = spapr_pending_dimm_unplugs_find(spapr, PC_DIMM(dev));
>
> - pc_dimm_unplug(dev, MACHINE(hotplug_dev));
> + pc_dimm_unplug(PC_DIMM(dev), MACHINE(hotplug_dev));
> object_unparent(OBJECT(dev));
> spapr_pending_dimm_unplugs_remove(spapr, ds);
> }
> diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
> index b382eb4303..99cbd54de7 100644
> --- a/include/hw/mem/pc-dimm.h
> +++ b/include/hw/mem/pc-dimm.h
> @@ -79,8 +79,8 @@ typedef struct PCDIMMDeviceClass {
> Error **errp);
> } PCDIMMDeviceClass;
>
> -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);
> -void pc_dimm_plug(DeviceState *dev, MachineState *machine, Error **errp);
> -void pc_dimm_unplug(DeviceState *dev, MachineState *machine);
> +void pc_dimm_plug(PCDIMMDevice *dimm, MachineState *machine, Error **errp);
> +void pc_dimm_unplug(PCDIMMDevice *dimm, MachineState *machine);
> #endif
>
© 2016 - 2026 Red Hat, Inc.