-
Notifications
You must be signed in to change notification settings - Fork 547
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
rbd: free snapshot resources after allocation #4514
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@Mergifyio queue |
🛑 The pull request has been removed from the queue
|
069cc10
to
402db77
Compare
/test ci/centos/k8s-e2e-external-storage/1.29 |
/test ci/centos/mini-e2e-helm/k8s-1.29 |
/test ci/centos/upgrade-tests-cephfs |
/test ci/centos/mini-e2e/k8s-1.29 |
/test ci/centos/upgrade-tests-rbd |
/test ci/centos/k8s-e2e-external-storage/1.27 |
/test ci/centos/mini-e2e-helm/k8s-1.27 |
/test ci/centos/k8s-e2e-external-storage/1.28 |
/test ci/centos/mini-e2e/k8s-1.27 |
/test ci/centos/mini-e2e-helm/k8s-1.28 |
/test ci/centos/mini-e2e/k8s-1.28 |
oh no, interesting failure!
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
^^ requested changes in above comment.
402db77
to
f2b0be6
Compare
Pull request has been modified.
/test ci/centos/mini-e2e/k8s-1.28 |
/test ci/centos/mini-e2e/k8s-1.29 |
/test ci/centos/k8s-e2e-external-storage/1.27 |
/test ci/centos/mini-e2e-helm/k8s-1.27 |
/test ci/centos/mini-e2e/k8s-1.27 |
/test ci/centos/upgrade-tests-cephfs |
/test ci/centos/upgrade-tests-rbd |
😭 it's Tuesday again and pulling container-images in the CI is problematic... |
/test ci/centos/mini-e2e-helm/k8s-1.29 |
1 similar comment
/test ci/centos/mini-e2e-helm/k8s-1.29 |
@Mergifyio rebase |
✅ Nothing to do for rebase action |
/test ci/centos/k8s-e2e-external-storage/1.29 |
/test ci/centos/mini-e2e-helm/k8s-1.29 |
/test ci/centos/mini-e2e/k8s-1.29 |
/test ci/centos/upgrade-tests-cephfs |
/test ci/centos/k8s-e2e-external-storage/1.27 |
/test ci/centos/upgrade-tests-rbd |
/test ci/centos/k8s-e2e-external-storage/1.28 |
/test ci/centos/mini-e2e-helm/k8s-1.27 |
/test ci/centos/mini-e2e-helm/k8s-1.28 |
/test ci/centos/mini-e2e/k8s-1.27 |
/test ci/centos/mini-e2e/k8s-1.28 |
Just like GenVolFromVolID() the genSnapFromSnapID() function can return
a snapshot. There is no need to allocated an empty snapshot and pass
that to the genSnapFromSnapID() function.
With this change, it is visible that not all snapshot objects are free'd correctly after they were allocated.
It is possible that some connections to the Ceph cluster were never
closed. This does not need to be a noticeable problem, as connections
are re-used where possible, but it isn't clean either.