storage: cleanup staged layer if unused

[giuseppe]

notify c/storage that the staged layer is not used and must be cleaned up on error.

[mtrmac]

Thanks!

This works, but it would be easy to forget next time again.

So I’m leaning towards

out, err := …
if err != nil { … }
succeeded := false
defer func() {
    if !succeeded { … CleanupStagedLayer … }
}()
…
…diffOutputs = …
s.lock.Unlock()
succeeded = true
return …

If you think that’s never going to be necessary, shrug, this PR is an improvement as is.

---

But, also, that code path should never be hit. Is that assumption incorrect?

[giuseppe]

If you think that’s never going to be necessary, shrug, this PR is an improvement as is.

your version is better, I've fixed it and pushed a new version

[mtrmac]

If I understand correctly, this is to follow the rules, allowing us to more precisely use locks in containers/storage#1916 .

But right now there’s containers/storage#1916 (comment) , so I’d prefer to wait a bit here until the that question is resolved.

[giuseppe]

containers/storage#1916 is merged, can we reconsider this?

[mtrmac]

AFAICS, on return from ApplyDiffFromStagingDirectory, the callers can’t tell whether the lock is now unlocked.

But, also, the locks only exist for directories created by MkdirTemp, and the unlock code is ~idempotent, so extra calls of CleanupStagedLayer don’t actually hurt.

LGTM.

[TomSweeneyRedHat]

LGTM
once the tests are hip