Re: [PATCH] block-backend: Fix argument order when calling 'qapi_event_send_block_io_error()'

[Daniel P . Berrangé]

On Mon, Jan 27, 2025 at 11:29:24AM +0100, Peter Krempa wrote:
> Commit 7452162adec25c10 introduced 'qom-path' argument to BLOCK_IO_ERROR
> event but when the event is instantiated in 'send_qmp_error_event()' the
> arguments for 'device' and 'qom_path' in
> qapi_event_send_block_io_error() were reversed :
> 
> Generated code for sending event:
> 
>   void qapi_event_send_block_io_error(const char *qom_path,
>                                       const char *device,
>                                       const char *node_name,
>                                       IoOperationType operation,
>                                       [...]
> 
> Call inside send_qmp_error_event():
> 
>      qapi_event_send_block_io_error(blk_name(blk),
>                                     blk_get_attached_dev_path(blk),
>                                     bs ? bdrv_get_node_name(bs) : NULL, 
> optype,
>                                     [...]
> 
> This results into reporting the QOM path as the device alias and vice
> versa which in turn breaks libvirt, which expects the device alias being
> either a valid alias or empty (which would make libvirt do the lookup by
> node-name instead).
> 
> Fixes: 7452162adec25c1003d5bf0079aca52913a80e0c
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>

Hmm, luckily that was only introduced in 9.2.0, so we can probably just
fix it in stable, and not worry about libvirt needs to add back compat
to workaround the initially broken 9.2.0 release events.

> ---
>  block/block-backend.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

> 
> diff --git a/block/block-backend.c b/block/block-backend.c
> index c93a7525ad..981be67e5d 100644
> --- a/block/block-backend.c
> +++ b/block/block-backend.c
> @@ -2136,8 +2136,8 @@ static void send_qmp_error_event(BlockBackend *blk,
>      BlockDriverState *bs = blk_bs(blk);
> 
>      optype = is_read ? IO_OPERATION_TYPE_READ : IO_OPERATION_TYPE_WRITE;
> -    qapi_event_send_block_io_error(blk_name(blk),
> -                                   blk_get_attached_dev_path(blk),
> +    qapi_event_send_block_io_error(blk_get_attached_dev_path(blk),
> +                                   blk_name(blk),
>                                     bs ? bdrv_get_node_name(bs) : NULL, 
> optype,
>                                     action, blk_iostatus_is_enabled(blk),
>                                     error == ENOSPC, strerror(error));
> -- 
> 2.48.1
> 
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|