On 04.07.2016 12:43, Kevin Wolf wrote: > Am 02.07.2016 um 17:33 hat Max Reitz geschrieben: >> On 30.06.2016 16:13, Kevin Wolf wrote: >>> If a node name instead of a BlockBackend name is specified as the driver >>> for a guest device, an anonymous BlockBackend is created now. >>> >>> Signed-off-by: Kevin Wolf <kw...@redhat.com> >>> --- >>> hw/core/qdev-properties-system.c | 22 +++++++++++++++++++--- >>> 1 file changed, 19 insertions(+), 3 deletions(-) >> >> I just noticed this won't work with (at least) USB mass storage devices. >> scsi_bus_legacy_add_drive() creates a new device and uses >> qdev_prop_set_drive() to set its drive, and that function uses >> blk_name() to get the required value for .drive. >> >> That can of course be fixed by using bdrv_get_node_name(blk_bs(value)) >> there if blk_name(value) is empty. But in addition we need to make sure >> that the BB is not deleted in usb_msd_realize_storage() (because that >> function has to detach the device from the BB before creating the SCSI >> disk), so we need to wrap the explicitly hacky block there in >> blk_ref(blk) and blk_unref(blk). > > I'm not sure if I even want to fix this here... Maybe it's better to > leave node names broken for usb-storage, as a motivation for someone to > try and clean up this mess for good. I mean, it's not the first time > that this hack breaks.
Well, you'll at least have to add the blk_ref()+blk_unref() wrapping, or we'll get accesses to freed memory. Realistically speaking, I doubt deliberately making USB mass storage devices not support the new drive syntax would actually motivate someone to clean the whole thing up. In my opinion, just making qdev_prop_set_drive() support anonymous BBs is simple enough, so it'd be worth it. I don't think we gain anything by not doing it. Max
signature.asc
Description: OpenPGP digital signature
