| Message ID | 20180517081527.14410-6-david@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | MemoryDevice: use multi stage hotplug handlers | expand |
On Thu, 17 May 2018 10:15:18 +0200 David Hildenbrand <david@redhat.com> wrote: > Necessary to hotplug them cleanly later. We could drop the PC_DIMM > check, as PC_DIMM are just memory devices, but this approach is cleaner. > > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > hw/i386/pc.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index 510076e156..8bc41ef24b 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -74,6 +74,7 @@ > #include "hw/nmi.h" > #include "hw/i386/intel_iommu.h" > #include "hw/net/ne2000-isa.h" > +#include "hw/mem/memory-device.h" > > /* debug PC/ISA interrupts */ > //#define DEBUG_IRQ > @@ -2075,6 +2076,7 @@ static HotplugHandler *pc_get_hotpug_handler(MachineState *machine, > DeviceState *dev) > { > if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) || > + object_dynamic_cast(OBJECT(dev), TYPE_MEMORY_DEVICE) || you probably could drop TYPE_PC_DIMM above, it's redundant since DIMM can be cast to TYPE_MEMORY_DEVICE ditto for spapr > object_dynamic_cast(OBJECT(dev), TYPE_CPU)) { > return HOTPLUG_HANDLER(machine); > }
On 30.05.2018 15:12, Igor Mammedov wrote: > On Thu, 17 May 2018 10:15:18 +0200 > David Hildenbrand <david@redhat.com> wrote: > >> Necessary to hotplug them cleanly later. We could drop the PC_DIMM >> check, as PC_DIMM are just memory devices, but this approach is cleaner. >> >> Signed-off-by: David Hildenbrand <david@redhat.com> >> --- >> hw/i386/pc.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/hw/i386/pc.c b/hw/i386/pc.c >> index 510076e156..8bc41ef24b 100644 >> --- a/hw/i386/pc.c >> +++ b/hw/i386/pc.c >> @@ -74,6 +74,7 @@ >> #include "hw/nmi.h" >> #include "hw/i386/intel_iommu.h" >> #include "hw/net/ne2000-isa.h" >> +#include "hw/mem/memory-device.h" >> >> /* debug PC/ISA interrupts */ >> //#define DEBUG_IRQ >> @@ -2075,6 +2076,7 @@ static HotplugHandler *pc_get_hotpug_handler(MachineState *machine, >> DeviceState *dev) >> { >> if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) || >> + object_dynamic_cast(OBJECT(dev), TYPE_MEMORY_DEVICE) || > you probably could drop TYPE_PC_DIMM above, it's redundant since DIMM > can be cast to TYPE_MEMORY_DEVICE > > ditto for spapr > Yes, had the same in mind but left it for now this way (basically because we do special handling for PC_DIMM, so anybody reading this code is not directly confused) >> object_dynamic_cast(OBJECT(dev), TYPE_CPU)) { >> return HOTPLUG_HANDLER(machine); >> } >
On 30/05/2018 16:08, David Hildenbrand wrote: >>> if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) || >>> + object_dynamic_cast(OBJECT(dev), TYPE_MEMORY_DEVICE) || >> you probably could drop TYPE_PC_DIMM above, it's redundant since DIMM >> can be cast to TYPE_MEMORY_DEVICE >> >> ditto for spapr >> > Yes, had the same in mind but left it for now this way (basically > because we do special handling for PC_DIMM, so anybody reading this code > is not directly confused) > Hmm, I think what Igor proposes is nicer. Can you send a v5 when he's done with review? Paolo
On 30.05.2018 16:27, Paolo Bonzini wrote: > On 30/05/2018 16:08, David Hildenbrand wrote: >>>> if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) || >>>> + object_dynamic_cast(OBJECT(dev), TYPE_MEMORY_DEVICE) || >>> you probably could drop TYPE_PC_DIMM above, it's redundant since DIMM >>> can be cast to TYPE_MEMORY_DEVICE >>> >>> ditto for spapr >>> >> Yes, had the same in mind but left it for now this way (basically >> because we do special handling for PC_DIMM, so anybody reading this code >> is not directly confused) >> > > Hmm, I think what Igor proposes is nicer. Can you send a v5 when he's > done with review? Sure! > > Paolo >
diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 510076e156..8bc41ef24b 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -74,6 +74,7 @@ #include "hw/nmi.h" #include "hw/i386/intel_iommu.h" #include "hw/net/ne2000-isa.h" +#include "hw/mem/memory-device.h" /* debug PC/ISA interrupts */ //#define DEBUG_IRQ @@ -2075,6 +2076,7 @@ static HotplugHandler *pc_get_hotpug_handler(MachineState *machine, DeviceState *dev) { if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) || + object_dynamic_cast(OBJECT(dev), TYPE_MEMORY_DEVICE) || object_dynamic_cast(OBJECT(dev), TYPE_CPU)) { return HOTPLUG_HANDLER(machine); }
Necessary to hotplug them cleanly later. We could drop the PC_DIMM check, as PC_DIMM are just memory devices, but this approach is cleaner. Signed-off-by: David Hildenbrand <david@redhat.com> --- hw/i386/pc.c | 2 ++ 1 file changed, 2 insertions(+)