On Fri, Jul 28, 2017 at 02:08:43PM +0200, Kevin Wolf wrote: > 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.
Nice thinking - write threshold can be a node too!
signature.asc
Description: PGP signature
