Re: [PATCH v18 12/21] dm: add finalize hook to target_type

Mikulas Patocka
2024-05-09T17:07:21Z

On Wed, 8 May 2024, Fan Wu wrote:

>
>
> On 5/8/2024 10:17 AM, Mikulas Patocka wrote:
> >
> >
> > On Fri, 3 May 2024, Fan Wu wrote:
> >
> >> This patch adds a target finalize hook.
> >>
> >> The hook is triggered just before activating an inactive table of a
> >> mapped device. If it returns an error the __bind get cancelled.
> >>
> >> The dm-verity target will use this hook to attach the dm-verity's
> >> roothash metadata to the block_device struct of the mapped device.
> >>
> >> Signed-off-by: Fan Wu <wufan@linux.microsoft.com>
> >
> > Hi
> >
> > Why not use the preresume callback?
> >
> > Is there some reason why do we need a new callback instead of using the
> > existing one?
> >
> > Mikulas
> Thanks for the suggestion.
>
> Mike suggested the new finalize() callback. I think the reason for not using
> the preresume() callback is that there are multiple points that can fail
> before activating an inactive table of a mapped device which can potentially
> lead to inconsistent state.
>
> In our specific case, we are trying to associate dm-verity's roothash metadata
> with the block_device struct of the mapped device inside the callback.
>
> If we use the preresume() callback for the work and an error occurs between
> the callback and the table activation, this leave the block_device struct in
> an inconsistent state.

The preresume callback is the final GO/NO-GO decision point. If all the
targets return zero in their preresume callback, then there's no turning
back and the table will be activated.

> This is because now the block device contains the roothash metadata of it's
> inactive table due to the preresume() callback, but the activation failed so
> the mapped device is still using the old table.
>
> The new finalize() callback guarantees that the callback happens just before
> the table activation, thus avoiding the inconsistency.

In your patch, it doesn't guarantee that.

do_resume calls dm_swap_table, dm_swap_table calls __bind, __bind calls
ti->type->finalize. Then we go back to do_resume and call dm_resume which
calls __dm_resume which calls dm_table_resume_targets which calls the
preresume callback on all the targets. If some of them fails, it returns a
failure (despite the fact that ti->type->finalize succeeded), if all of
them succeed, it calls the resume callback on all of them.

So, it seems that the preresume callback provides the guarantee that you
looking for.

> -Fan

Mikulas