s390x/event-facility: fix error propagation

[PATCH] s390x/event-facility: fix error propagation

Posted by Cornelia Huck 6 years ago

We currently check (by error) if the passed-in Error pointer errp
is non-null and return after realizing the first child of the
event facility in that case. Symptom is that 'virsh shutdown'
does not work, as the sclpquiesce device is not realized.

Fix this by (correctly) checking the local Error err.

Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
Fixes: 3d508334dd2c ("s390x/event-facility: Fix realize() error API violations")
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
 hw/s390x/event-facility.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
index 8a93b8a1da97..9d6972afa8b3 100644
--- a/hw/s390x/event-facility.c
+++ b/hw/s390x/event-facility.c
@@ -338,7 +338,7 @@ static void sclp_events_bus_realize(BusState *bus, Error **errp)
         DeviceState *dev = kid->child;
 
         object_property_set_bool(OBJECT(dev), true, "realized", &err);
-        if (errp) {
+        if (err) {
             error_propagate(errp, err);
             return;
         }
-- 
2.21.1

Re: [PATCH] s390x/event-facility: fix error propagation

Posted by David Hildenbrand 6 years ago

On 21.01.20 10:55, Cornelia Huck wrote:
> We currently check (by error) if the passed-in Error pointer errp
> is non-null and return after realizing the first child of the
> event facility in that case. Symptom is that 'virsh shutdown'
> does not work, as the sclpquiesce device is not realized.
> 
> Fix this by (correctly) checking the local Error err.
> 
> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Fixes: 3d508334dd2c ("s390x/event-facility: Fix realize() error API violations")
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> ---
>  hw/s390x/event-facility.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
> index 8a93b8a1da97..9d6972afa8b3 100644
> --- a/hw/s390x/event-facility.c
> +++ b/hw/s390x/event-facility.c
> @@ -338,7 +338,7 @@ static void sclp_events_bus_realize(BusState *bus, Error **errp)
>          DeviceState *dev = kid->child;
>  
>          object_property_set_bool(OBJECT(dev), true, "realized", &err);
> -        if (errp) {
> +        if (err) {
>              error_propagate(errp, err);
>              return;
>          }
> 

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb

Re: [PATCH] s390x/event-facility: fix error propagation

Posted by Christian Borntraeger 6 years ago


On 21.01.20 10:55, Cornelia Huck wrote:
> We currently check (by error) if the passed-in Error pointer errp
> is non-null and return after realizing the first child of the
> event facility in that case. Symptom is that 'virsh shutdown'
> does not work, as the sclpquiesce device is not realized.
> 
> Fix this by (correctly) checking the local Error err.
> 
> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Fixes: 3d508334dd2c ("s390x/event-facility: Fix realize() error API violations")
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>

Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>

> ---
>  hw/s390x/event-facility.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
> index 8a93b8a1da97..9d6972afa8b3 100644
> --- a/hw/s390x/event-facility.c
> +++ b/hw/s390x/event-facility.c
> @@ -338,7 +338,7 @@ static void sclp_events_bus_realize(BusState *bus, Error **errp)
>          DeviceState *dev = kid->child;
>  
>          object_property_set_bool(OBJECT(dev), true, "realized", &err);
> -        if (errp) {
> +        if (err) {
>              error_propagate(errp, err);
>              return;
>          }
>

Re: [PATCH] s390x/event-facility: fix error propagation

Posted by Thomas Huth 6 years ago

On 21/01/2020 10.55, Cornelia Huck wrote:
> We currently check (by error) if the passed-in Error pointer errp
> is non-null and return after realizing the first child of the
> event facility in that case. Symptom is that 'virsh shutdown'
> does not work, as the sclpquiesce device is not realized.
> 
> Fix this by (correctly) checking the local Error err.
> 
> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Fixes: 3d508334dd2c ("s390x/event-facility: Fix realize() error API violations")
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> ---
>  hw/s390x/event-facility.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
> index 8a93b8a1da97..9d6972afa8b3 100644
> --- a/hw/s390x/event-facility.c
> +++ b/hw/s390x/event-facility.c
> @@ -338,7 +338,7 @@ static void sclp_events_bus_realize(BusState *bus, Error **errp)
>          DeviceState *dev = kid->child;
>  
>          object_property_set_bool(OBJECT(dev), true, "realized", &err);
> -        if (errp) {
> +        if (err) {
>              error_propagate(errp, err);
>              return;
>          }
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH] s390x/event-facility: fix error propagation

Posted by Markus Armbruster 6 years ago

Cornelia Huck <cohuck@redhat.com> writes:

> We currently check (by error) if the passed-in Error pointer errp
> is non-null and return after realizing the first child of the
> event facility in that case. Symptom is that 'virsh shutdown'
> does not work, as the sclpquiesce device is not realized.
>
> Fix this by (correctly) checking the local Error err.
>
> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Fixes: 3d508334dd2c ("s390x/event-facility: Fix realize() error API violations")
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> ---
>  hw/s390x/event-facility.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
> index 8a93b8a1da97..9d6972afa8b3 100644
> --- a/hw/s390x/event-facility.c
> +++ b/hw/s390x/event-facility.c
> @@ -338,7 +338,7 @@ static void sclp_events_bus_realize(BusState *bus, Error **errp)
>          DeviceState *dev = kid->child;
>  
>          object_property_set_bool(OBJECT(dev), true, "realized", &err);
> -        if (errp) {
> +        if (err) {
>              error_propagate(errp, err);
>              return;
>          }

Thanks for cleaning the mess I made!

Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [PATCH] s390x/event-facility: fix error propagation

Posted by Cornelia Huck 6 years ago

On Tue, 21 Jan 2020 10:55:06 +0100
Cornelia Huck <cohuck@redhat.com> wrote:

> We currently check (by error) if the passed-in Error pointer errp
> is non-null and return after realizing the first child of the
> event facility in that case. Symptom is that 'virsh shutdown'
> does not work, as the sclpquiesce device is not realized.
> 
> Fix this by (correctly) checking the local Error err.
> 
> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Fixes: 3d508334dd2c ("s390x/event-facility: Fix realize() error API violations")
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> ---
>  hw/s390x/event-facility.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
> index 8a93b8a1da97..9d6972afa8b3 100644
> --- a/hw/s390x/event-facility.c
> +++ b/hw/s390x/event-facility.c
> @@ -338,7 +338,7 @@ static void sclp_events_bus_realize(BusState *bus, Error **errp)
>          DeviceState *dev = kid->child;
>  
>          object_property_set_bool(OBJECT(dev), true, "realized", &err);
> -        if (errp) {
> +        if (err) {
>              error_propagate(errp, err);
>              return;
>          }

Queued to s390-next.