On 12/7/22 12:38, Janis Schoetterl-Glausch wrote:
* On Wed, 2022-12-07 at 11:00 +0100, Pierre Morel wrote:
On 12/6/22 22:06, Janis Schoetterl-Glausch wrote:
On Tue, 2022-12-06 at 15:35 +0100, Pierre Morel wrote:
On 12/6/22 14:35, Janis Schoetterl-Glausch wrote:
On Tue, 2022-12-06 at 11:32 +0100, Pierre Morel wrote:
On 12/6/22 10:31, Janis Schoetterl-Glausch wrote:
On Tue, 2022-11-29 at 18:42 +0100, Pierre Morel wrote:
We will need a Topology device to transfer the topology
during migration and to implement machine reset.
The device creation is fenced by s390_has_topology().
Signed-off-by: Pierre Morel <pmorel@xxxxxxxxxxxxx>
---
include/hw/s390x/cpu-topology.h | 44 +++++++++++++++
include/hw/s390x/s390-virtio-ccw.h | 1 +
hw/s390x/cpu-topology.c | 87 ++++++++++++++++++++++++++++++
hw/s390x/s390-virtio-ccw.c | 25 +++++++++
hw/s390x/meson.build | 1 +
5 files changed, 158 insertions(+)
create mode 100644 include/hw/s390x/cpu-topology.h
create mode 100644 hw/s390x/cpu-topology.c
[...]
+ object_property_set_int(OBJECT(dev), "num-cores",
+ machine->smp.cores * machine->smp.threads, errp);
+ object_property_set_int(OBJECT(dev), "num-sockets",
+ machine->smp.sockets, errp);
+
+ sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), errp);
I must admit that I haven't fully grokked qemu's memory management yet.
Is the topology devices now owned by the sysbus?
Yes it is so we see it on the qtree with its properties.
If so, is it fine to have a pointer to it S390CcwMachineState?
Why not?
If it's owned by the sysbus and the object is not explicitly referenced
for the pointer, it might be deallocated and then you'd have a dangling pointer.
Why would it be deallocated ?
That's beside the point, if you transfer ownership, you have no control over when
the deallocation happens.
It's going to be fine in practice, but I don't think you should rely on it.
I think you could just do sysbus_realize instead of ..._and_unref,
but like I said, I haven't fully understood qemu memory management.
(It would also leak in a sense, but since the machine exists forever that should be fine)
If I understand correctly:
- qdev_new adds a reference count to the new created object, dev.
- object_property_add_child adds a reference count to the child also
here the new created device dev so the ref count of dev is 2 .
after the unref on dev, the ref count of dev get down to 1
then it seems OK. Did I miss something?
The properties ref belongs to the property, if the property were removed,
it would be unref'ed. There is no extra ref for the pointer in S390CcwMachineState.
I'm coming from a clean code perspective, I don't think we'd run into this problem in practice.
OK, I understand, you are right.
My original code used object_resolve_path() to retrieve the object what
made things cleaner I think.
For performance reason, Cedric proposed during the review of V10 to add
the pointer to the machine state instead.
I must say that I am not very comfortable to argument on this.
@Cedric what do you think?
Regards,
Pierre
--
Pierre Morel
IBM Lab Boeblingen
