Re: [Qemu-devel] [RFC] block: Is name of BlockBackend deprecated with -blockdev parameter?
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 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
Re: [Qemu-devel] [RFC] block: Is name of BlockBackend deprecated with -blockdev parameter?
Am 11.12.2018 um 08:28 hat Markus Armbruster geschrieben: > I figure Kevin knows, but you typoed his e-mail address. I fixed it for > you. > > Anton Kuchin writes: > > > Hello, > > > > I'm trying to switch from -drive parameter to -blockdev + -device and > > having problems. Looks like with this option I have no way to set the > > name of created BlockBackend, but some QMP and HMP commands are > > trying to find blk with blk_by_name() and fail to locate my device > > (e.g. hmp_commit, qmp_x_bloc_latency_histogram_set ...). Was it > > intentional and BB names are going to be deprecated? > > > > This also seems to be a the case for all block devices hotplugged with > > QMP as they use the same code path. > > > > 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? > > > > 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(). hmp_commit() should indeed use bdrv_lookup_bs() to also accept node names. I think we reviewed QMP to make sure we converted everything and forgot about HMP commands that aren't mapped to QMP. Kevin
Re: [Qemu-devel] [RFC] block: Is name of BlockBackend deprecated with -blockdev parameter?
I figure Kevin knows, but you typoed his e-mail address. I fixed it for you. Anton Kuchin writes: > Hello, > > I'm trying to switch from -drive parameter to -blockdev + -device and > having problems. Looks like with this option I have no way to set the > name of created BlockBackend, but some QMP and HMP commands are > trying to find blk with blk_by_name() and fail to locate my device > (e.g. hmp_commit, qmp_x_bloc_latency_histogram_set ...). Was it > intentional and BB names are going to be deprecated? > > This also seems to be a the case for all block devices hotplugged with > QMP as they use the same code path. > > 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? > > 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? > > Thanks, > Anton
