cmd/mount: honour --no-lock flag #2821

renard · 2020-07-07T05:40:32Z

Do not lock the repository if --no-lock flag is set. This allows to
mount repositories which are archived on a read only system.

Signed-off-by: Sébastien Gross <seb•ɑƬ•chezwam•ɖɵʈ•org>

What is the purpose of this change? What does it change?

If remote repository is on a read-only partition no lock can be acquired making the repo un-mountable.

This small patch fixes this by honouring the --no-lock global flag.

Was the change discussed in an issue or in the forum before?

https://forum.restic.net/t/mount-without-locking/2887

Fixes #1597

Checklist

I have read the Contribution Guidelines
I have enabled maintainer edits for this PR
I have added tests for all changes in this PR
I have added documentation for the changes (in the manual)
There's a new file in changelog/unreleased/ that describes the changes for our users (template here)
I have run gofmt on the code in all commits
All commit messages are formatted in the same style as the other commits in the repo
I'm done, this Pull Request is ready for review

aawsome · 2020-07-07T05:55:38Z

cmd/restic/cmd_mount.go

-	if err != nil {
-		return err
+	if !gopts.NoLock {
+		Verbosef("create lock for repository\n")


Instead of printing out an info when the repo is locked, I would print out a warning when the repo is not locked.

Commit updated. Thanks.

aawsome

Looks good to me!

greatroar · 2020-07-09T10:53:44Z

I'm not sure why mount should print a warning when no other command does that, but LGTM too.

aawsome · 2020-07-09T10:57:47Z

I'm not sure why mount should print a warning when no other command does that, but LGTM too.

I think that that other commands should warn as well. 😉
IMO for mount this is even more important as the mount command only quits when the user cancels it - that might be quite a long time. However, not too much to worry as mount is a read-only command.

greatroar · 2020-07-09T14:19:11Z

Fine with me, let's see what the maintainers think.

MichaelEischer

This PR would also fix #1597, for some reason Github doesn't let me link this issue to the PR.

MichaelEischer · 2020-08-01T17:06:15Z

cmd/restic/cmd_mount.go

-	if err != nil {
-		return err
+	if gopts.NoLock {
+		Warnf("not locking the repository\n")


I don't think there's a need to print this warning. A user hopefully still remembers that they passed the --no-lock flag. So let's keep this consistent with the other commands which respect --no-lock and remove the warning.

@MichaelEischer done in commit a6c94ac.

Let me know if this is good for you.

MichaelEischer

Please also add a short changelog entry file to this PR. After this and the warning message are fixed, this PR is ready for merging.

changelog/unreleased/pull-2821

rawtaz

@renard I guess GitHub is acting up and you haven't seen the two remaining comments I made on the changelog file. Please check the "Files changed" tab to see them :)

rawtaz · 2020-08-02T22:43:24Z

changelog/unreleased/pull-2821

@@ -0,0 +1,7 @@
+cmd/mount: honour --no-lock flag


This isn't formatted like the changelog entry template. I suggest changing this line to (yeah, the shorter "honor" instead of "honour"): Change: Honor the --no-lock flag in the mount command

changelog/unreleased/pull-2821

rawtaz · 2020-08-03T17:39:24Z

The subject line in the changelog is still not changed.

Do not lock the repository if --no-lock global flag is set. This allows to mount repositories which are archived on a read only system. Signed-off-by: Sébastien Gross <seb•ɑƬ•chezwam•ɖɵʈ•org>

rawtaz

Thank you very much!

ATLief · 2020-09-19T22:00:00Z

This is a really interesting addition. Would it be possible to modify the repository via another process while it’s mounted with the —no-lock option? If so what would happen?

I’m thinking that this could be useful to continue backing up my system while I restore or read a backup of a file, but I also see the potential for this to have unintended side-effects; for example, if the snapshot where a file was being accessed was forgotten or pruned.

aawsome · 2020-09-20T05:01:29Z

@ATLief as long as you just want to backup while having your repo mounted, there is not need to use the --no-lock option: It already works as expected without this option. Mount will even update the directory structure after a short delay and includes the newly added snapshots.
However, I would not recommend using mount --no-lock with commands that delete things from the repository. This won't break your repo as mount does only read, but may give errors or even may crash.

This option is IMO only interesting for the use case that you can no longer write to your repopository but still want to access it.

aawsome reviewed Jul 7, 2020

View reviewed changes

renard force-pushed the mount-nolock branch from 1837fc4 to ed3e9d0 Compare July 8, 2020 15:41

aawsome approved these changes Jul 8, 2020

View reviewed changes

MichaelEischer reviewed Aug 1, 2020

View reviewed changes

renard force-pushed the mount-nolock branch 2 times, most recently from c0b4bc2 to a6c94ac Compare August 2, 2020 17:48

rawtaz reviewed Aug 2, 2020

View reviewed changes

changelog/unreleased/pull-2821 Outdated Show resolved Hide resolved

renard force-pushed the mount-nolock branch from a6c94ac to 02c2ba9 Compare August 2, 2020 22:42

renard requested review from MichaelEischer and rawtaz August 3, 2020 13:05

rawtaz requested changes Aug 3, 2020

View reviewed changes

renard force-pushed the mount-nolock branch from 02c2ba9 to c388d10 Compare August 3, 2020 17:29

renard requested a review from rawtaz August 3, 2020 17:30

cmd/mount: honour --no-lock flag

9be4fe3

Do not lock the repository if --no-lock global flag is set. This allows to mount repositories which are archived on a read only system. Signed-off-by: Sébastien Gross <seb•ɑƬ•chezwam•ɖɵʈ•org>

renard force-pushed the mount-nolock branch from c388d10 to 9be4fe3 Compare August 3, 2020 17:45

rawtaz approved these changes Aug 3, 2020

View reviewed changes

rawtaz merged commit 5af2815 into restic:master Aug 3, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmd/mount: honour --no-lock flag #2821

cmd/mount: honour --no-lock flag #2821

renard commented Jul 7, 2020 •

edited by rawtaz

Loading

aawsome Jul 7, 2020 •

edited

Loading

renard Jul 8, 2020

aawsome left a comment

greatroar commented Jul 9, 2020

aawsome commented Jul 9, 2020

greatroar commented Jul 9, 2020

MichaelEischer left a comment

MichaelEischer Aug 1, 2020

renard Aug 2, 2020

MichaelEischer left a comment

rawtaz left a comment

rawtaz Aug 2, 2020

rawtaz commented Aug 3, 2020 •

edited

Loading

rawtaz left a comment

ATLief commented Sep 19, 2020 •

edited

Loading

aawsome commented Sep 20, 2020

cmd/mount: honour --no-lock flag #2821

cmd/mount: honour --no-lock flag #2821

Conversation

renard commented Jul 7, 2020 • edited by rawtaz Loading

What is the purpose of this change? What does it change?

Was the change discussed in an issue or in the forum before?

Checklist

aawsome Jul 7, 2020 • edited Loading

Choose a reason for hiding this comment

renard Jul 8, 2020

Choose a reason for hiding this comment

aawsome left a comment

Choose a reason for hiding this comment

greatroar commented Jul 9, 2020

aawsome commented Jul 9, 2020

greatroar commented Jul 9, 2020

MichaelEischer left a comment

Choose a reason for hiding this comment

MichaelEischer Aug 1, 2020

Choose a reason for hiding this comment

renard Aug 2, 2020

Choose a reason for hiding this comment

MichaelEischer left a comment

Choose a reason for hiding this comment

rawtaz left a comment

Choose a reason for hiding this comment

rawtaz Aug 2, 2020

Choose a reason for hiding this comment

rawtaz commented Aug 3, 2020 • edited Loading

rawtaz left a comment

Choose a reason for hiding this comment

ATLief commented Sep 19, 2020 • edited Loading

aawsome commented Sep 20, 2020

renard commented Jul 7, 2020 •

edited by rawtaz

Loading

aawsome Jul 7, 2020 •

edited

Loading

rawtaz commented Aug 3, 2020 •

edited

Loading

ATLief commented Sep 19, 2020 •

edited

Loading