Re: [Qemu-block] [Qemu-devel] [RFC] block: Is name of BlockBackend deprecated with -blockdev parameter?

[Anton Kuchin]

I want to make it consistent but I'm note sure I understand all aspects 
of current state and legacy clients we need to support.

The initial idea was to have single function blk_lookup(char* name) and 
search in all namespaces internally as they are guaranteed to have no 
intersection. This will work a little slower but I see no hot paths that 
will be affected. And this way transition will be transparent to users 
but I'm not sure this can be done in backward compatible way.
Do you have any suggestions how to do it correctly?

On 11/12/2018 12:47, Kevin Wolf wrote:
> [...]
> > Anton Kuchin <address@hidden> writes:
> > [...]
> >
> > As far as I understand all named backends are stored in
> > monitor_block_backends list, but I can't get what is the point of
> > having this list, and why parse_drive() function doesn't call
> > monitor_add_blk() like blockdev_init() does with -drive option. Can
> > someone give me a hint on this?

And what are monitor_block_backends used for? What does monitor 
reference mean in this context and how backends in this list differ from 
all others?

> > > I also noticed that some commands fallback to search by qdev_id or
> > > BDS->node_name,  but at the creation time (both in
> > > bdrv_assing_node_name and monitor_add_blk) it is already checked that
> > > names are unique across these namespaces so may be it would be useful
> > > to introduce generic search function?
> Yes, BlockBackend names are not supposed to be used in the external
> interfaces any more. If the QMP command is about the backend, it will
> take a node name, and if it's about the guest device (which may not even
> have a node attached with removable media), it will take a qdev ID.
>
> The implementation of qmp_x_block_latency_histogram_set() is incorrect,
> it shouldn't use blk_by_name(). I wonder where it got that pattern from
> because it was added long after all other QMP commands had switched to
> qmp_get_blk() or bdrv_lookup_bs().

Do we still need blk_by_name() to be public? May be we can replace it by 
new function like blk_lookup() and hide blk_by_name() for internal use 
only or fix its behavior to search by qdev_id first and if it fails 
fallback to old way of searching by device_id?

> hmp_commit() should indeed use bdrv_lookup_bs() to also accept node
> names.
But it also checks blk state before commit, so I'm not quite sure it can 
work correctly without blk.
> I think we reviewed QMP to make sure we converted everything
> and forgot about HMP commands that aren't mapped to QMP.
>
> Kevin
Anton