[PATCH] event_notifier: Set ->initialized earlier in event_notifier_init()

Greg Kurz posted 1 patch 4 years, 9 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210216120247.1293569-1-groug@kaod.org
Maintainers: Paolo Bonzini <pbonzini@redhat.com>
util/event_notifier-posix.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] event_notifier: Set ->initialized earlier in event_notifier_init()
Posted by Greg Kurz 4 years, 9 months ago
Otherwise the call to event_notifier_set() is a nop, which causes
the SLOF firmware on POWER to hang when booting from a virtio-scsi
device:

virtio_scsi_dataplane_start()
 virtio_scsi_vring_init()
  virtio_bus_set_host_notifier() <- assign == true
   event_notifier_init() <- active == 1
    event_notifier_set() <- fails right away if !e->initialized

Fixes: e34e47eb28c0 ("event_notifier: handle initialization failure better")
Cc: mlevitsk@redhat.com
Signed-off-by: Greg Kurz <groug@kaod.org>
---
 util/event_notifier-posix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util/event_notifier-posix.c b/util/event_notifier-posix.c
index 5b2110e86198..8307013c5df0 100644
--- a/util/event_notifier-posix.c
+++ b/util/event_notifier-posix.c
@@ -66,10 +66,10 @@ int event_notifier_init(EventNotifier *e, int active)
         e->rfd = fds[0];
         e->wfd = fds[1];
     }
+    e->initialized = true;
     if (active) {
         event_notifier_set(e);
     }
-    e->initialized = true;
     return 0;
 
 fail:
-- 
2.26.2


Re: [PATCH] event_notifier: Set ->initialized earlier in event_notifier_init()
Posted by Paolo Bonzini 4 years, 9 months ago
On 16/02/21 13:02, Greg Kurz wrote:
> Otherwise the call to event_notifier_set() is a nop, which causes
> the SLOF firmware on POWER to hang when booting from a virtio-scsi
> device:
> 
> virtio_scsi_dataplane_start()
>   virtio_scsi_vring_init()
>    virtio_bus_set_host_notifier() <- assign == true
>     event_notifier_init() <- active == 1
>      event_notifier_set() <- fails right away if !e->initialized
> 
> Fixes: e34e47eb28c0 ("event_notifier: handle initialization failure better")
> Cc: mlevitsk@redhat.com
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
>   util/event_notifier-posix.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/util/event_notifier-posix.c b/util/event_notifier-posix.c
> index 5b2110e86198..8307013c5df0 100644
> --- a/util/event_notifier-posix.c
> +++ b/util/event_notifier-posix.c
> @@ -66,10 +66,10 @@ int event_notifier_init(EventNotifier *e, int active)
>           e->rfd = fds[0];
>           e->wfd = fds[1];
>       }
> +    e->initialized = true;
>       if (active) {
>           event_notifier_set(e);
>       }
> -    e->initialized = true;
>       return 0;
>   
>   fail:
> 

Queued, thanks.

Paolo


Re: [PATCH] event_notifier: Set ->initialized earlier in event_notifier_init()
Posted by Maxim Levitsky 4 years, 9 months ago
On Tue, 2021-02-16 at 13:02 +0100, Greg Kurz wrote:
> Otherwise the call to event_notifier_set() is a nop, which causes
> the SLOF firmware on POWER to hang when booting from a virtio-scsi
> device:
> 
> virtio_scsi_dataplane_start()
>  virtio_scsi_vring_init()
>   virtio_bus_set_host_notifier() <- assign == true
>    event_notifier_init() <- active == 1
>     event_notifier_set() <- fails right away if !e->initialized
> 
> Fixes: e34e47eb28c0 ("event_notifier: handle initialization failure better")
> Cc: mlevitsk@redhat.com
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
>  util/event_notifier-posix.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/util/event_notifier-posix.c b/util/event_notifier-posix.c
> index 5b2110e86198..8307013c5df0 100644
> --- a/util/event_notifier-posix.c
> +++ b/util/event_notifier-posix.c
> @@ -66,10 +66,10 @@ int event_notifier_init(EventNotifier *e, int active)
>          e->rfd = fds[0];
>          e->wfd = fds[1];
>      }
> +    e->initialized = true;
>      if (active) {
>          event_notifier_set(e);
>      }
> -    e->initialized = true;
>      return 0;
>  
>  fail:
Sorry about that!

Best regards,
	Maxim levitsky