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

[Kevin Wolf]

Am 27.01.2025 um 12:07 hat Peter Krempa geschrieben:
> On Mon, Jan 27, 2025 at 10:53:16 +0000, Daniel P. Berrangé wrote:
> > 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.
> 
> Indeed, there's no need for a specific libvirt workaround. Although I
> still plan to modify the libvirt code to prefer node-name if present due
> to other reasons (to have the correct path of the image if the error
> happened in a backing image), which will also workaround this issue in
> the end.

Currently, QEMU always puts the node name of the root node there, so for
now you won't see backing file nodes in these events. But it's probably
a good idea to prepare libvirt for it anyway, maybe we can find a way to
have better error events later. The challenge in doing that is that the
error events are sent by the devices, not by the block node that
actually first encountered the error.

Kevin