Feature: Add postUnmount plugin callback

[tatokis]

This is useful for unmounting the remote drive over ssh after sshfs is unmounted.

If the remote drive is unmounted before sshfs with the existing callback, then sshfs can not be unmounted and the mountpoint shows up as

d????????? ? ?         ?            ?            ? mountpoint

until the remote drive is remounted.

The existing unmount() function was not renamed to preUnmount() in order to not break any existing plugins.

[buhtz]

Thank's a lot for your contribution.

Please be aware of the discussion in #1232 especially 1232#issuecomment-1146882621 .

In short: The project is not dead but it will take a while until we can take your PR into account.

[aryoda]

@tatokis I am trying to understand how the implementation supports your described use case:

useful for unmounting the remote drive over ssh after sshfs is unmounted.

Your proposed implementation introduces a new postUnmount() method but it is always called together with the unmount() user-callback method:

https://github.com/bit-team/backintime/blob/76a4b1bc1e0df8344069c15f5ec9b13cad688243/common/mount.py#L157,L179

So I am trying to understand why the extra postUnmount() is required at all.
Could it suffice for your use case to implement the unmount of sshfs and the remote drive together (sequentially) directly in the unmount() user-callback script only?

[tatokis]

@aryoda The key is that it's called after backend.umount(). That's when backintime unmounts the sshfs. The end user doesn't have control of the sshfs mount as it's handled by backintime itself.

The issue is that the remote drive that the sshfs points to would be unmounted before the sshfs itself if done in the unmount callback, rendering the sshfs non-unmountable afterwards by BIT.

The only way other way would be to try to detect which sshfs mountpoint BIT is using in the unmount callback, and manually unmount it before BIT tries to, but this is difficult and failure prone at best.

[aryoda]

THX! So the problem I think is that

backintime/common/mount.py

Line 178 in 76a4b1b

backend.umount()

is called by BiT after self.config.PLUGIN_MANAGER.unmount(self.profile_id) (=> user-callback)
which is strange I think because it should be called after above unmount so that we don't need a postUnmount at all.

Edit: Or deprecate "unmount" and introduce "beforeUnmount" and "afterUnmount" (like "processBegin" and "processEnd")...

I am not sure if there is already a good unit test to cover these lines of code with sshfs so I don't dare to change this at the moment.

I will come back to look deeper into this PR once I have got more time left (and after stabilizing BiT by fixing the most urgent bugs).

[buhtz]

I did some research about the basic functionality and behavior of user-callback. As a side-product I created some unit-tests (#1639) but they won't help solving this actual PR.

To my knowledge user-callback behavior is not covered by any of the current tests. Of course the code is executed in some tests (and covered in a technical way) but not tested.

IMHO the behavior to test would be if the callback reasons/steps are called on the right position on the timeline after/before some other specific steps of the taking a snapshot process.

My assumption is that this is nearly impossible in the current state of the productive code. The code is not isolated enough. And especially in testing user-callback behavior to many elements of code are involved. I see no way doing this with "unit tests" or "integration tests" in the near time.

A "dirty" approach could be to do a full snapshot run in a test and then parsing the log output for the expected messages (or their patterns) and if they appear in the correct order. We still have tests doing full backups. They do use some helper code from test/generic.py.

[buhtz]

is called by BiT after self.config.PLUGIN_MANAGER.unmount(self.profile_id) (=> user-callback) which is strange I think because it should be called after above unmount so that we don't need a postUnmount at all.

Edit: Or deprecate "unmount" and introduce "beforeUnmount" and "afterUnmount" (like "processBegin" and "processEnd")...

There is an old Issue (#648) from 2016 requesting also a "level 9" in the callback scripts to signaling that everything is done (after unmount).

[buhtz]

If no one else will work on it I vote to transform this into an Issue, linking this (closed) PR as a potential approach to solve the Issue.

[buhtz]

Closing this PR based on the comment above. Feel free to reopen and work further on it.
There is a follow-up issue #1869 to keep track of that contribution and not losing it.

Best regards,