On 07/26/2017 02:23 PM, 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.
Exactly how much of a problem is this? You, the user, have instructed
QEMU to do two conflicting things:
(1) Squash A down into B, and
(2) Insert a filter above A
Shame on you for deleting the node you were trying to reference, no? And
as an added bonus, QEMU will even error out on your command instead of
trying to do something and getting it wrong.
Is this a problem?
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,
Because "B" and "dummy" have already actually been deleted, I assume.
(The diagram is confusing.)
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.
Seems like a fairly good reason not to pursue this avenue then, yes?
I think I agree with Stefan's concerns that this is more of a temporary
workaround for the benefit of jobs, but that there may well be other
reasons (especially in the future) where graph manipulations may occur
invisibly to the user.
Do you have any use cases for failure here that don't involve the user
themselves asking for two conflicting things? That is, QEMU doing some
graph reorganization without the user's knowledge coupled with the user
asking for a new filter node?
Are there any failures that result in anything more dangerous or bad
than a "404: node not found" style error where we simply reject the
premise of what the user was attempting to accomplish?
