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@xxxxxxxxxxxxxxxxxxx> > > > > 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
