Re: [PATCH v2] hw/s390x: Attach the sclpconsole to /machine/sclp/s390-sclp-event-facility

[Cédric Le Goater]

On 4/30/24 21:08, Thomas Huth wrote:
> The sclpconsole currently does not have a proper parent in the QOM
> tree, so it shows up under /machine/unattached - which is somewhat
> ugly. We should rather attach it to /machine/sclp/s390-sclp-event-facility
> where the other devices of type TYPE_SCLP_EVENT already reside.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Cédric Le Goater <clg@redhat.com>

> ---
>   hw/s390x/s390-virtio-ccw.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 5c83d1ea17..41be8bf857 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -242,11 +242,13 @@ static void s390_create_virtio_net(BusState *bus, const 
> char *name)
>   
>   static void s390_create_sclpconsole(const char *type, Chardev *chardev)
>   {
> +    BusState *ev_fac_bus = sclp_get_event_facility_bus();

This routine sclp_get_event_facility_bus() which scans all the machine
could be avoided and even removed. For that, SCLPDevice should be an
attribute of the machine. This means reshuffling definitions and
object instanciations a little in the code. Would you be OK with that ?

Other machine units/models could undergo the same kind of changes.

Thanks,

C.

>       DeviceState *dev;
>   
>       dev = qdev_new(type);
> +    object_property_add_child(OBJECT(ev_fac_bus->parent), type, OBJECT(dev));
>       qdev_prop_set_chr(dev, "chardev", chardev);
> -    qdev_realize_and_unref(dev, sclp_get_event_facility_bus(), &error_fatal);
> +    qdev_realize_and_unref(dev, ev_fac_bus, &error_fatal);
>   }
>   
>   static void ccw_init(MachineState *machine)