[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Stefan Hajnoczi
Subject: Re: [Qemu-devel] [RFC] block-insert-node and block-job-delete
Date: Thu, 27 Jul 2017 11:07:55 +0100
User-agent: Mutt/1.8.3 (2017-05-23)

On Wed, Jul 26, 2017 at 09:23:20PM +0300, Manos Pitsidianakis wrote:
> 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.

I see the diagrams but the flow isn't clear without the user's QMP

F is created using blockdev-add?  What are the parameters and how does
it know about dummy?  I think this is an interesting question in itself
because dummy is a transient internal node that QMP users don't
necessarily know about.

What is the full block-insert-node command?

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

manual-delete is a solution for block jobs.  The write threshold feature
is a plain QMP command that in the future will create a transient
internal node (before write notifier).

I'm not sure it makes sense to turn the write threshold feature into a
block job.  I guess it could work, but it seems a little unnatural and
maybe there will be other features that use transient internal nodes.

What I'm getting at is that there might be alternative to manual-delete
that work in the general case.  For example, if blockdev-add +
block-insert-node are part of a QMP 'transaction' command, can we lock
the graph to protect it against modifications?

Attachment: signature.asc
Description: PGP signature

reply via email to

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