[Top][All Lists]

[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.


Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]