[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v7 12/24] virtio-blk: Functions for op blocker m
From: |
Max Reitz |
Subject: |
Re: [Qemu-block] [PATCH v7 12/24] virtio-blk: Functions for op blocker management |
Date: |
Wed, 25 Nov 2015 17:26:02 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 25.11.2015 17:18, Kevin Wolf wrote:
> Am 25.11.2015 um 17:03 hat Max Reitz geschrieben:
>> On 25.11.2015 16:57, Kevin Wolf wrote:
>>> Am 09.11.2015 um 23:39 hat Max Reitz geschrieben:
>>>> Put the code for setting up and removing op blockers into an own
>>>> function, respectively. Then, we can invoke those functions whenever a
>>>> BDS is removed from an virtio-blk BB or inserted into it.
>>>>
>>>> Signed-off-by: Max Reitz <address@hidden>
>>>
>>> Do you know of a case where this is observable?
>>
>> Actually, no.
>>
>>> I don't think you can
>>> change the medium of a virtio-blk device, and blk_set_bs() isn't
>>> converted to a wrapper around blk_remove/insert_bs() yet. If this patch
>>> is necessary to fix something observable, the latter is probably a bug.
>>
>> So I guess that implies "Otherwise, this patch should be dropped"?
>
> I'm not sure. I guess you had a reason to include these patches other
> than putting the notifiers to use?
I'm not sure, it has been so long. :-)
I can very well imagine having included it because a similar change is
necessary to virtio-scsi, so I included it just because I was already
doing work for virtio. Maybe I imagined that virtio-blk may reasonably
offer tray devices in the future, but I just now read you saying
somewhere else:
> Please write your code so that it makes sense today.
So that doesn't hold up. :-)
Maybe I just saw it unblocking the EJECT op blocker, and so I thought
there might be a reason for that.
> With blk_set_bs() changed, I think it would have an effect: The op
> blockers would move from the old BDS to the new top-level one. This
> sounds desirable to me.
Hm, thanks for saving me. Yes, that seems useful indeed.
[...]
>>> This makes me wonder: What do we even block here any more? If I didn't
>>> miss anything, it's only BLOCK_OP_TYPE_BACKUP_TARGET, and I'm not sure
>>> why this needs to be blocked, or if we simply forgot to enable it.
>>
>> Well, even though in practice this wall of code doesn't make much sense,
>> of course it will be safe for potential additions of new op blockers.
>>
>> And of course we actually don't want these blockers at all anymore...
>
> Yes, but dataplane shouldn't really be special enough any more that we
> want to disable features for it initially. By now it sounds more like an
> easy way to forget unblocking a new feature even though it would work.
>
> So perhaps we should really just remove the blockers from dataplane.
> Then we don't have to answer the question above...
Well, maybe. I guess this is up to Stefan.
Max
signature.asc
Description: OpenPGP digital signature
- [Qemu-block] [PATCH v7 09/24] iotests: Make redirecting qemu's stderr optional, (continued)
[Qemu-block] [PATCH v7 13/24] virtio-scsi: Catch BDS-BB removal/insertion, Max Reitz, 2015/11/09
[Qemu-block] [PATCH v7 14/24] nbd: Switch from close to eject notifier, Max Reitz, 2015/11/09
[Qemu-block] [PATCH v7 15/24] block: Remove BDS close notifier, Max Reitz, 2015/11/09