On Mon, Mar 14, 2022 at 1:23 PM Stefan Hajnoczi <stefa...@redhat.com> wrote: > > On Tue, Feb 15, 2022 at 06:59:38PM +0800, Xie Yongji wrote: > > This supports passing NULL ops to blk_set_dev_ops() > > so that we can remove stale ops in some cases. > > > > Signed-off-by: Xie Yongji <xieyon...@bytedance.com> > > --- > > block/block-backend.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/block/block-backend.c b/block/block-backend.c > > index 4ff6b4d785..08dd0a3093 100644 > > --- a/block/block-backend.c > > +++ b/block/block-backend.c > > @@ -1015,7 +1015,7 @@ void blk_set_dev_ops(BlockBackend *blk, const > > BlockDevOps *ops, > > blk->dev_opaque = opaque; > > > > /* Are we currently quiesced? Should we enforce this right now? */ > > - if (blk->quiesce_counter && ops->drained_begin) { > > + if (blk->quiesce_counter && ops && ops->drained_begin) { > > ops->drained_begin(opaque); > > } > > } > > John: You added this code in f4d9cc88ee6. Does blk_set_dev_ops() need to > call ->drained_end() when ops is set to NULL? > > Stefan
I'm not sure I trust my memory from five years ago. >From what I recall, the problem was that block jobs weren't getting drained/paused when the backend was getting quiesced -- we wanted to be sure that a blockjob wasn't continuing to run and submit requests against a backend we wanted to have on ice during a sensitive operation. This conditional stanza here is meant to check if the node we're already attached to is *already quiesced* and we missed the signal (so-to-speak), so we replay the drained_begin() request right there. i.e. there was some case where blockjobs were getting added to an already quiesced node, and this code here post-hoc relays that drain request to the blockjob. This gets used in 600ac6a0ef5c06418446ef2f37407bddcc51b21c to pause/unpause jobs. Original thread is here: https://lists.gnu.org/archive/html/qemu-devel/2017-03/msg03416.html Now, I'm not sure why you want to set ops to NULL here. If we're in a drained section, that sounds like it might be potentially bad because we lose track of the operation to end the drained section. If your intent is to destroy the thing that we'd need to call drained_end on, I guess it doesn't matter -- provided you've cleaned up the target object correctly. Just calling drained_end() pre-emptively seems like it might be bad, what if it unpauses something you're in the middle of trying to delete? I might need slightly more context to know what you're hoping to accomplish, but I hope this info helps contextualize this code somewhat. --js