Re: [Qemu-block] [PATCH v3 08/14] blockdev: Remove blk_hide_on_behalf_of_hmp_drive_del()

[Max Reitz]

On 17.02.2016 15:18, Kevin Wolf wrote:
> Am 16.02.2016 um 19:08 hat Max Reitz geschrieben:
>> This function first removed the BlockBackend from the blk_backends list
>> and cleared its name so it would no longer be found by blk_name(); since
>> blk_next() now iterates through monitor_block_backends (which the BB is
>> removed from in hmp_drive_del()), this is no longer necessary.
> 
> Not clearing the name any more means that you can't create a new
> BlockBackend with the same ID until the hidden BB finally goes away.
> 
> As it happens, you already can't do that with drive_add, because we also
> keep the ID reserved in the QemuOpts, so it's already broken today.
> However, before this patch, you can still use blockdev-add to create a
> new BB with the ID of a BB that drive_del removed; after it, you get an
> error:
> 
> {"error": {"class": "GenericError", "desc": "Device with id 'disk'
> already exists"}}
> 
> In order to fix this, I suggested on IRC that we view the BB name as
> belonging to the monitor reference logically, i.e. we add a name with
> monitor_add_blk() (which would get a new ID parameter) and clear it
> again in monitor_remove_blk().

Moving the @name parameter from blk_new() to monitor_add_blk() seems
reasonable but would necessitate pulling in two patches from the
follow-up series to this (there are places which try to emit the BB's
name while its BDS is created, which these two patches change so they no
longer do this).

Alternatively, as I said, we might want to call monitor_add_blk() in
blk_new() if the name is non-NULL (and allow NULL names).

Max

>> Second, bdrv_make_anon() was called on the BDS. This was intended for
>> cases where the BDS was owned by that BB alone; in which case the BDS
>> will no longer exist at this point thanks to the blk_remove_bs() in
>> hmp_drive_del().
>>
>> Therefore, this function does nothing useful anymore. Remove it.
>>
>> Signed-off-by: Max Reitz <address@hidden>
> 
> Kevin
>

signature.asc
Description: OpenPGP digital signature