Am 27.07.2017 um 12:07 hat Stefan Hajnoczi geschrieben:
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
commands.
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.
We can expect that users of block-insert-node also know about block job
filter nodes, simply because the former is newer than the latter.
(I also don't like the name "dummy", this makes it sound like it's not
really a proper node. In reality, there is little reason to treat it
specially.)
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).
Yes, that becomes a legacy QMP command then. The new way is blockdev-add
and block-insert-node for write threshold, too.
Apart from that, a write threshold node never disappears by itself, it
is only inserted or removed in the context of a QMP command. That makes
it a lot less dangerous than automatic block completion.
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.
Yeah, no reason to do so. Just a manually created filter node.
signature.asc