-
Notifications
You must be signed in to change notification settings - Fork 338
DAOS-16501 build: Support of mercury with ASan #16917
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
base: master
Are you sure you want to change the base?
Conversation
|
Ticket title is 'LRZ: m02r01s10dao coredump - invalid free' |
|
Test stage Functional Hardware Large MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-16917/1/execution/node/1462/log |
Instrument mercury with libasan according to build option SANITIZERS Signed-off-by: Cedric Koch-Hofer <[email protected]>
6646951 to
7fe6ec3
Compare
|
Test stage NLT on EL 8.8 completed with status UNSTABLE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos//view/change-requests/job/PR-16917/2/testReport/ |
…/daos-16501-part1
|
Test stage NLT on EL 8.8 completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos/job/PR-16917/3/display/redirect |
|
Test stage Unit Test bdev with memcheck on EL 8.8 completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos/job/PR-16917/3/display/redirect |
|
Test stage Unit Test on EL 8.8 completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos/job/PR-16917/3/display/redirect |
|
Test stage Unit Test bdev on EL 8.8 completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos/job/PR-16917/3/display/redirect |
|
Test stage Unit Test with memcheck on EL 8.8 completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos/job/PR-16917/3/display/redirect |
…/daos-16501-part1
jolivier23
left a comment
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.
You need to add an entry to daos.spec changelog, bump the daos and mercury versions. See this:
Without updating the mercury version, this will never build in CI because there is already a mercury to use.
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.
ok based on Jeff's comment, I'll ask you to hold on this PR for now until I update mercury myself with this patch included. (There is also not much that you gain currently from building mercury with asan options, except slightly better traces)
|
@knard38 see new option I have added in mercury-hpc/mercury@74dda4a |
I am going to update the PR accordingly |
…/daos-16501-part1
Integrate reviewers comments: - Fix CI integration - Use of official mercury patch 74dda4a Miscellaneous fixes related to package build. Signed-off-by: Cedric Koch-Hofer <[email protected]>
|
|
Fixed with commit 5cb9bbe |
Fixed with commit 5cb9bbe |
| Version: 2.7.101 | ||
| Release: 16%{?relval}%{?dist} | ||
| Release: 17%{?relval}%{?dist} | ||
| Summary: DAOS Storage Engine |
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.
@jolivier23 it does not make sense to me, why would someone need to update the daos.spec file in this case? we're not changing the min required version of mercury etc.
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.
Because the daos changelog is now the only documentation we have that something changed. I'm open to other options though. Or maybe the gitlog on utils/rpms/package_info.sh is enough?
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.
- Remove daos.spec update
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.
- Remove daos.spec update
Fixed with commit 8eb204a
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.
@jolivier23 is there not a way to include a changelog with fpm ? imo we want the changelog to be in the mercury rpm itself, not the DAOS one.
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.
I see in the doc they have
--rpm-changelog FILEPATH
Add changelog from FILEPATH contents
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.
ignore my last comment, looks like I can't read past 2 comments :D I see your comment now in #17063
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.
For the Mercury change log update, I will wait for #17063 to be merged.
Is that OK for you @soumagne and @jolivier23 ?
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.
For the Mercury change log update, I will wait for #17063 to be merged. Is that OK for you @soumagne and @jolivier23 ?
- Restore Mercury changelog
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.
For the Mercury change log update, I will wait for #17063 to be merged. Is that OK for you @soumagne and @jolivier23 ?
- Restore Mercury changelog
…/daos-16501-part1
debian/changelog
Outdated
| [ Jeff Olivier] | ||
| * Make daos-spdk conflict with spdk | ||
|
|
||
| -- Jeff Olivier <[email protected]> Mon, 19 May 2025 14:12:00 -0700 |
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.
The whole debian folder is no longer used since we produce .deb files using fpm. We shoudl just remove the folder rather than updating this file
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.
- Remove Debian packaging update
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.
- Remove Debian packaging update
Fixed with commit 8eb204a
|
I pushed this: #17050 |
Integrate reviewers comments: - Remove daos.spec update - Remove Debian packaging update Signed-off-by: Cedric Koch-Hofer <[email protected]>
|
@soumagne this seems like a better option for the change logs |
…/daos-16501-part1
Integrate reviewers comments: - Restore Mercury Changelog Signed-off-by: Cedric Koch-Hofer <[email protected]>
Fix mercury release number Signed-off-by: Cedric Koch-Hofer <[email protected]>
It seems that the integration of the changelog is not compatible as it with Mercury. |
don't worry about it for now, maybe it's best that you wait that I merge my PR first that updates mercury to 2.4.1 first and includes your patch, I have also added the changeling there. See #16943 |
- Fix invalid date in mercury changelog - add verbose option to fpm to have meaningull error messages Signed-off-by: Cedric Koch-Hofer <[email protected]>
I have fixed it: locally tested. |
Description
Instrument mercury with libasan according to build option SANITIZERS
Steps for the author:
After all prior steps are complete: