Re: [Qemu-devel] [PATCH 0/3] Add bdrv_get_device_or_node_name()

[Max Reitz]

On 2015-03-19 at 11:43, Alberto Garcia wrote:
> We have several cases of error messages related to BlockDriverState that expect 
> it to have a device name.
>
> However many of those can happen in nodes that don't have a device name 
> associated so the generated error messages are not very meaningful.
>
> This series adds a new function bdrv_get_device_or_node_name(), that returns 
> the node name if there is no device name for that node. Since both the device 
> and the node name live in the same namespace, there's no ambiguity. And since 
> it gives preference to the device name, it maintains backward compatibility.

Not necessarily. Imagine a program interpreting the 
BLOCK_IMAGE_CORRUPTED event, for example (after this series): It sees 
the @device value and expects a device name there; then it tries to do 
something with that, assuming it is a device, but that may fail (like 
looking it up in its own list of devices, or launching a block job, or 
whatever; I can very well imagine someone wanting to automatically start 
a blockdev_backup() on that event, for instance).

Of course, this is a very weak argument. Before this series, in these 
cases the caller only received an empty string, which didn't help it 
very much either. So I just want to point out that it does not fully 
maintain compatibility, but I don't think there will be any problems 
whatsoever either.

Max

> It also updates all calls to bdrv_get_device_name() where it makes sense: 
> several error messages and the BLOCK_IMAGE_CORRUPTED event.
>
> I wrote this thinking on the case where any arbitrary node will be able to own 
> a block job, but I think it's useful already for the reasons explained above.
>
> Berto
>
> Alberto Garcia (3):
>    block: add bdrv_get_device_or_node_name()
>    block: use bdrv_get_device_or_node_name() in error messages
>    block: allow BLOCK_IMAGE_CORRUPTED to have a node name
>
>   block.c                 | 18 ++++++++++++------
>   block/qcow.c            |  4 ++--
>   block/qcow2.c           |  7 ++++---
>   block/qed.c             |  2 +-
>   block/quorum.c          |  5 +----
>   block/vdi.c             |  2 +-
>   block/vhdx.c            |  2 +-
>   block/vmdk.c            |  4 ++--
>   block/vpc.c             |  2 +-
>   block/vvfat.c           |  3 ++-
>   docs/qmp/qmp-events.txt |  2 +-
>   include/block/block.h   |  1 +
>   qapi/block-core.json    |  2 +-
>   13 files changed, 30 insertions(+), 24 deletions(-)