Kevin Wolf <kw...@redhat.com> writes:
> Am 02.10.2014 um 11:04 hat Markus Armbruster geschrieben:
>> Make the BlockBackend own the DriveInfo. Change blockdev_init() to
>> return the BlockBackend instead of the DriveInfo.
>>
>> Signed-off-by: Markus Armbruster <arm...@redhat.com>
>> Reviewed-by: Max Reitz <mre...@redhat.com>
>
>> @@ -200,11 +202,11 @@ DriveInfo *drive_get_next(BlockInterfaceType type)
>>
>> DriveInfo *drive_get_by_blockdev(BlockDriverState *bs)
>> {
>> - DriveInfo *dinfo;
>> + BlockBackend *blk;
>>
>> - QTAILQ_FOREACH(dinfo, &drives, next) {
>> - if (dinfo->bdrv == bs) {
>> - return dinfo;
>> + for (blk = blk_next(NULL); blk; blk = blk_next(blk)) {
>> + if (blk == bs->blk) {
>> + return blk_legacy_dinfo(blk);
>> }
>> }
>> return NULL;
>
> In v3, I asked why you don't use bs->blk here. Apparently you understood
> this as a suggestion to change the if condition from:
>
> if (blk_bs(blk) == bs)
>
> to:
>
> if (blk == bs->blk)
>
> Which isn't a wrong change, it just doesn't change a lot. What I really
> meant is something like this, removing the loop:
>
> DriveInfo *drive_get_by_blockdev(BlockDriverState *bs)
> {
> return bs->blk ? blk_legacy_dinfo(bs->blk) : NULL;
> }
>
> This would only behave differently if there were BlockBackends that can
> be assigned to bs->blk, but aren't iterated by blk_next(). But such
> BlockBackends don't exist, blk_next() includes all of them.
Actually, there are: blk_hide_on_behalf_of_do_drive_del() removes from
blk_backends without also zapping bs->blk.
However, your version is *less* of a change, because it also doesn't
remove from drives.
In short: sold, thanks!
