Re: [Qemu-block] [Qemu-devel] Meeting notes on -blockdev, dynamic backend reconfiguration

[Fam Zheng]

On Mon, 12/12 14:58, Markus Armbruster wrote:
> Fam Zheng <address@hidden> writes:
> 
> > On Thu, 12/08 13:46, Markus Armbruster wrote:
> >> Fam Zheng <address@hidden> writes:
> >> 
> >> > On Wed, 12/07 10:48, Kevin Wolf wrote:
> >> >> > If so I think there is no race to worry about, mirror-filter should go
> >> >> > away only after a QMP command.
> >> >> 
> >> >> Currently, a mirror job goes away whenever it is done. This is not
> >> >> directly tied to a QMP command.
> >> >
> >> > Ah right, block-job-complete is only "start to complete" and the job 
> >> > goes away
> >> > at some later point. I thought this is "the" QMP command but it is not.
> >> >
> >> >> 
> >> >> Of course, in the new job API we want an explicit job-delete, so in
> >> >> that case it wouldn't happen, but we need to keep the old case for
> >> >> compatibility.
> >> >
> >> > Another possibility is to add a placeholder node in the right location 
> >> > first
> >> > then fill in the actual throttling node once created. QMP owns the 
> >> > placeholder
> >> > node so it won't suddenly vanish when mirror job goes away.
> >> 
> >> I'm not sure I understand this idea.  Could you explain it in a bit more
> >> detail, perhaps even with a bit of ASCII art?
> >
> > OK, I'll base on your version:
> >
> > == Basic dynamic reconfiguration operation ==
> >
> > The two primitives are "insert placeholder" and bdrv_replace_child;
> >
> > A placeholder BDS is a dummy "pass through everything" filter. Inserting
> > placeholder is something like:
> >
> >     void bdrv_insert_placeholder(BdrvChild *child) {
> >         BlockDriverState *ph = bdrv_new_placeholder();
> >         ph->file->bs = child->bs;
> >         child->bs = ph;
> >     }
> >
> > Consider:
> >
> >           BB
> >           |
> >     mirror-filter
> >           |
> >          BDS
> >
> > Add a throttle filter under BB while the mirror job is running.  First step,
> > create the dummy _placeholder_ node and throttle-filter node (the existence 
> > of
> > placeholder node can be an implementation detail hidden from user):
> >
> >     placeholder = bdrv_insert_placeholder(BB->root);
> >     throttle_filter = bdrv_create_throttle_filter(placeholder);
> >
> > we get:
> >
> >           BB     throttle-filter
> >           |     /
> >      placeholder
> >           |
> >     mirror-filter
> >           |
> >          BDS
> >
> > Second step, move throttle filter in:
> >
> >     bdrv_replace_child(BB->root, throttle_filter);
> >     bdrv_replace_child(throttle_filter->file, 
> > throttle_filter->file->bs->file->bs);
> >     bdrv_unref(placeholder);
> >
> > we get:
> >
> >           BB
> >           |
> >    throttle-filter
> >           |
> >     mirror-filter
> >           |
> >          BDS
> >
> > Does this make us safe from racing with mirror-filter disappearing?
> 
> Thanks!  I figure your placeholder node does immunize the splicing in of
> the throttle-filter against the removal of mirror-filter.
> 
> Given a specific dynamic reconfiguration, where do we have to put
> placeholder nodes to immunize it against arbitrary other dynamic
> changes?

Generally, a placeholder node is essentially an insertion to the middle of an
existing edge, namely to change:

         NodeA
       /   |   \
      /    |    \
 [...]   NodeB   [...]

into:

         NodeA
       /   |   \
      /    |    \
 [...]  p-holdr  [...]
           |
           |
         NodeB

and the operation is safe because of BQL. Our later bdrv_replace_child()
operations on it will always work because a dynamic reconfiguration won't delete
the placeholder node in any case.

On the other hand, concurrent dynamic changes could yield usual outcomes from
user PoV (like the involved node is no longer in the same place or disappears),
but I think it is still in some degree under user's control. E.g. a
mirror-filter won't go away automatically until user does
block-job-complete/cancel.

Fam