Re: [Qemu-block] [RFC] block-insert-node and block-job-delete

[Manos Pitsidianakis]

On Wed, Jul 26, 2017 at 04:12:21PM +0100, Stefan Hajnoczi wrote:
> On Wed, Jul 26, 2017 at 05:19:24PM +0300, Manos Pitsidianakis wrote:
> > This proposal follows a discussion we had with Kevin and Stefan on filter
> > node management.
> >
> > With block filter drivers arises a need to configure filter nodes on runtime
> > with QMP on live graphs. A problem with doing live graph modifications is
> > that some block jobs modify the graph when they are done and don't operate
> > under any synchronisation, resulting in a race condition if we try to insert
> > a filter node in the place of an existing edge.
>
> But maybe you are thinking about higher level race conditions between
> QMP commands.  Can you give an example of the race?

Exactly, synchronisation between the QMP/user actions. Here's an 
example, correct me if I'm wrong:

User issues a block-commit to this tree to commit A onto B:
   BB -> A -> B

This inserts the dummy mirror node at the top since this is a mirror job 
(active commit)
   BB -> dummy -> A -> B

User wants to add a filter node F below the BB. First we create the 
node:
   BB -> dummy -> A -> B
      F /

Commit finishes before we do block-insert-node, dummy and A is removed 
from the BB's chain, mirror_exit calls bdrv_replace_node()

   BB ------------> B
               F -> /

Now inserting F under BB will error since dummy does not exist any more.

The proposal doesn't guard against the following:
User issues a block-commit to this tree to commit B onto C:
   BB -> A -> B -> C

The dummy node (commit's top bs is A):
   BB -> A -> dummy -> B -> C

blockdev-add of a filter we wish to eventually be between A and C:
   BB -> A -> dummy -> B -> C
            F/

if commit finishes before we issue block-insert-node (commit_complete()
calls bdrv_set_backing_hd() which only touches A)
   BB -> A --------------> C
          F -> dummy -> B /
   resulting in error when issuing block-insert-node,

else if we set the job to manual-delete and insert F:
   BB -> A -> F -> dummy -> B -> C
deleting the job will result in F being lost since the job's top bs 
wasn't updated.

signature.asc
Description: PGP signature