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.

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

hammer: build/ops: boost uuid makes valgrind complain #9741

Merged
merged 2 commits into from Aug 11, 2016
Merged

hammer: build/ops: boost uuid makes valgrind complain #9741

merged 2 commits into from Aug 11, 2016

Conversation

smithfarm
Copy link
Contributor

rohanmars and others added 2 commits June 16, 2016 08:51
@rohanmars @smithfarm
moved to use boost uuid implementation, based on commit 4fe89a7
174de7f 
Signed-off-by: Rohan Mars <code@rohanmars.com>
Reviewed-by: Casey Bodley <cbodley@redhat.com>
(cherry picked from commit 62bfc7a)

Conflicts:
	debian/control (trivial resolution)
	src/common/Makefile.am (trivial resolution)
	src/common/blkdev.cc (no get_device_by_uuid() function in hammer)
@liewegas @smithfarm
uuid: use boost::random:random_device
187d308 
The boost mt code uses uninitialized memory for extra randomness,
which is a bad idea in general but more importantly makes valgrind
unhappy.  Use /dev/urandom instead.

Unfortunately this introduces a link time dependency.. meh!

Fixes: ceph#12736
Signed-off-by: Sage Weil <sage@redhat.com>
(cherry picked from commit dbcaa54)

Conflicts:
	ceph.spec.in (trivial resolution)
@smithfarm smithfarm self-assigned this Jun 16, 2016
@smithfarm smithfarm added this to the hammer milestone Jun 16, 2016
smithfarm added a commit that referenced this pull request Jun 27, 2016
@smithfarm
Merge pull request #9741: hammer: boost uuid makes valgrind complain
ca5f8fb 
Reviewed-by: Nathan Cutler <ncutler@suse.com>
smithfarm added a commit that referenced this pull request Jul 18, 2016
@smithfarm
Merge pull request #9741: hammer: boost uuid makes valgrind complain
7019ff1 
Reviewed-by: Nathan Cutler <ncutler@suse.com>
smithfarm added a commit that referenced this pull request Jul 24, 2016
@smithfarm
Merge pull request #9741: hammer: boost uuid makes valgrind complain
ad66d64 
Reviewed-by: Nathan Cutler <ncutler@suse.com>
@smithfarm
Copy link
Contributor Author

@liewegas This PR is in the latest round of hammer-backports integration tests, which passed a rados run (the only failures are a valgrind false positive that has since been fixed by ceph/teuthology#915 and http://tracker.ceph.com/issues/15139 which is an infrastructure issue with two of the tests) - for details, see: http://tracker.ceph.com/issues/15895#note-18

Do you think this PR is OK to merge?

@smithfarm
Copy link
Contributor Author

@liewegas @athanatos This PR passed a /200 rados run on Ubuntu. None of the failures were reproducible. For details see http://tracker.ceph.com/issues/15895#note-18

Do you think it's OK to merge?

@athanatos
Copy link
Contributor

@smithfarm Sorry, I think the first one is ok, but I don't know what the implications of the second are.

@smithfarm
Copy link
Contributor Author

@liewegas This one has passed two rados runs. Sam is OK with the first commit, but unsure about the second. Do you think it's OK to merge?

@liewegas
Copy link
Member

I don't think the new dependency will be an issue.. @ktdreyer ? It's there in jewel...

@ktdreyer
Copy link
Member

Sure, looks good to me, thanks

@smithfarm smithfarm merged commit 1e01986 into ceph:hammer Aug 11, 2016
@smithfarm smithfarm deleted the wip-16343-hammer branch August 11, 2016 20:58
@smithfarm smithfarm changed the title hammer: boost uuid makes valgrind complain hammer: build/ops: boost uuid makes valgrind complain Aug 15, 2016
@smithfarm
Copy link
Contributor Author

Reverted by #10913

@0dig 0dig mentioned this pull request Apr 14, 2024
Open
@dengkun001 dengkun001 mentioned this pull request Apr 14, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@smithfarm smithfarm

Labels
bug-fix core
Projects
None yet
Milestone
hammer
5 participants
@smithfarm @athanatos @liewegas @ktdreyer @rohanmars