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 <mre...@redhat.com> >>> >>> 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