Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix issue with multiple volume refs with same name #20381

Merged
merged 1 commit into from Feb 18, 2016
Merged

Fix issue with multiple volume refs with same name #20381

merged 1 commit into from Feb 18, 2016

Conversation

cpuguy83
Copy link
Member

Fixes #20378

@cpuguy83 cpuguy83 mentioned this pull request Feb 17, 2016
Closed
@cpuguy83
Fix issue with multiple volume refs with same name
0fe3130 
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
@thaJeztah thaJeztah added this to the 1.10.2 milestone Feb 17, 2016
@thaJeztah
Copy link
Member

@tiborvass @cpuguy83 given that we have two "panic" issues, should we have a 1.10.2 branch, and open these fixes on that branch?

coolljt0725
coolljt0725 reviewed Feb 17, 2016
View reviewed changes
volume/store/store.go
refs, exists := s.refs[v.Name()]
if !exists {
return
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this check is still need. Or else if s.refs[v.Name()] does not exists, the s.refs[v.Name()] = refs will make it exists, this may cause issue if some other operation relay on _, exists := s.refs[v.Name()]

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We aren't tracking existence anywhere as it's not what matters. What matters here is length (ie, 0 length it has no references, > 0 length it has references).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make sense if we aren't tracking existence anywhere :)

@thaJeztah
Copy link
Member

https://jenkins.dockerproject.org/job/Docker-PRs-WoW-TP4/1137/console

fatal: unable to access 'https://github.com/docker/docker.git/': Failed to connect to github.com port 443: Timed out
21:48:37 + ec=128
21:48:37 + set +x

@thaJeztah
Copy link
Member

@coolljt0725 does this LGTY?

@coolljt0725
Copy link
Contributor

@thaJeztah Yes, LGTM

@calavera
Copy link
Contributor

LGTM

calavera added a commit that referenced this pull request Feb 18, 2016
@calavera
Merge pull request #20381 from cpuguy83/20378_fix_multiple_deref
319bbff 
Fix issue with multiple volume refs with same name
@calavera calavera merged commit 319bbff into moby:master Feb 18, 2016
@cpuguy83 cpuguy83 deleted the 20378_fix_multiple_deref branch February 18, 2016 21:52
@tiborvass tiborvass mentioned this pull request Feb 20, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.10.2
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@cpuguy83 @thaJeztah @coolljt0725 @calavera @GordonTheTurtle