Re: [PATCH v2 20/36] block: add bdrv_attach_child_common() transaction action

[Kevin Wolf]

Am 04.02.2021 um 08:34 hat Vladimir Sementsov-Ogievskiy geschrieben:
> 04.02.2021 00:01, Kevin Wolf wrote:
> > Am 27.11.2020 um 15:45 hat Vladimir Sementsov-Ogievskiy geschrieben:
> > > Split out no-perm part of bdrv_root_attach_child() into separate
> > > transaction action. bdrv_root_attach_child() now moves to new
> > > permission update paradigm: first update graph relations then update
> > > permissions.
> > > 
> > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

> > > +static void bdrv_attach_child_common_abort(void *opaque)
> > > +{
> > > +    BdrvAttachChildCommonState *s = opaque;
> > > +    BdrvChild *child = *s->child;
> > > +    BlockDriverState *bs = child->bs;
> > > +
> > > +    bdrv_replace_child_noperm(child, NULL);
> > > +
> > > +    if (bdrv_get_aio_context(bs) != s->old_child_ctx) {
> > > +        bdrv_try_set_aio_context(bs, s->old_child_ctx, &error_abort);
> > 
> > Would failure actually be fatal? I think we can ignore it, the node is
> > in an AioContext that works for it.
> 
> As far as I explored the code, check-aio-context is transparent
> enough, nothing rely on IO, etc, and if we succeeded to change it we
> must success in revert.
> 
> And as I understand it is critical: if we failed to rollback
> aio-context change somewhere (but succeeded in reverting graph
> relation change), it means that we end up with different aio contexts
> inside one block subtree..

Ah, right, we're going to change the graph once again, so what is
working now doesn't have to be working for the changed graph.

Ok, let's leave this as &error_abort.

Kevin