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

2024-05-02 Thread Thomas Huth 

On 02/05/2024 09.57, Cédric Le Goater wrote:

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 


Reviewed-by: Cédric Le Goater 


---
  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 ?


Sounds like a good clean-up, yes!

 Thomas

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

2024-05-02 Thread 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 


Reviewed-by: Cédric Le Goater 


---
  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)

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

2024-05-01 Thread Eric Farman 
On Tue, 2024-04-30 at 21:08 +0200, 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 
> ---
>  hw/s390x/s390-virtio-ccw.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Eric Farman

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

2024-04-30 Thread Philippe Mathieu-Daudé 

On 30/4/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 
---
  hw/s390x/s390-virtio-ccw.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)


Reviewed-by: Philippe Mathieu-Daudé

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

2024-04-30 Thread David Hildenbrand 

On 30.04.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 
---
  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();
  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)


Reviewed-by: David Hildenbrand 

--
Cheers,

David / dhildenb

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

2024-04-30 Thread Thomas Huth 
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 
---
 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();
 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)
-- 
2.44.0