Skip to content

Conversation

@knard38
Copy link
Contributor

@knard38 knard38 commented Sep 27, 2025

Description

Instrument mercury with libasan according to build option SANITIZERS

Steps for the author:

  • Commit message follows the guidelines.
  • Appropriate Features or Test-tag pragmas were used.
  • Appropriate Functional Test Stages were run.
  • At least two positive code reviews including at least one code owner from each category referenced in the PR.
  • Testing is complete. If necessary, forced-landing label added and a reason added in a comment.

After all prior steps are complete:

  • Gatekeeper requested (daos-gatekeeper added as a reviewer).

@github-actions
Copy link

Ticket title is 'LRZ: m02r01s10dao coredump - invalid free'
Status is 'Resolved'
Labels: 'lrz'
https://daosio.atlassian.net/browse/DAOS-16501

@daosbuild3
Copy link
Collaborator

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]>
@knard38 knard38 force-pushed the ckochhof/dev/master/daos-16501-part1 branch from 6646951 to 7fe6ec3 Compare October 8, 2025 05:08
@knard38 knard38 marked this pull request as ready for review October 8, 2025 05:10
@knard38 knard38 requested review from a team as code owners October 8, 2025 05:10
@knard38 knard38 requested review from jolivier23 and soumagne October 8, 2025 05:10
@knard38 knard38 self-assigned this Oct 8, 2025
@knard38 knard38 requested a review from liw October 8, 2025 05:12
@daosbuild3
Copy link
Collaborator

@daosbuild3
Copy link
Collaborator

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

@daosbuild3
Copy link
Collaborator

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

@daosbuild3
Copy link
Collaborator

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

@daosbuild3
Copy link
Collaborator

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

@daosbuild3
Copy link
Collaborator

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

liw
liw previously approved these changes Oct 20, 2025
Copy link
Contributor

@jolivier23 jolivier23 left a 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.

https://github.com/daos-stack/daos/blob/master/docs/dev/development.md#updating-a-3rd-party-component

Copy link
Collaborator

@soumagne soumagne left a 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)

@soumagne
Copy link
Collaborator

@knard38 see new option I have added in mercury-hpc/mercury@74dda4a

@knard38
Copy link
Contributor Author

knard38 commented Oct 27, 2025

@knard38 see new option I have added in mercury-hpc/mercury@74dda4a

I am going to update the PR accordingly

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]>
@knard38
Copy link
Contributor Author

knard38 commented Oct 27, 2025

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.

https://github.com/daos-stack/daos/blob/master/docs/dev/development.md#updating-a-3rd-party-component

  • Fix CI integration

@knard38
Copy link
Contributor Author

knard38 commented Oct 27, 2025

@knard38 see new option I have added in mercury-hpc/mercury@74dda4a

  • Use official mercury patch 74dda4a

@knard38 knard38 requested a review from a team as a code owner October 27, 2025 14:17
@knard38
Copy link
Contributor Author

knard38 commented Oct 27, 2025

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.
https://github.com/daos-stack/daos/blob/master/docs/dev/development.md#updating-a-3rd-party-component

  • Fix CI integration

Fixed with commit 5cb9bbe

@knard38
Copy link
Contributor Author

knard38 commented Oct 27, 2025

@knard38 see new option I have added in mercury-hpc/mercury@74dda4a

  • Use official mercury patch 74dda4a

Fixed with commit 5cb9bbe

Version: 2.7.101
Release: 16%{?relval}%{?dist}
Release: 17%{?relval}%{?dist}
Summary: DAOS Storage Engine
Copy link
Collaborator

@soumagne soumagne Oct 27, 2025

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.

Copy link
Contributor

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?

Copy link
Contributor Author

@knard38 knard38 Oct 30, 2025

Choose a reason for hiding this comment

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

  • Remove daos.spec update

Copy link
Contributor Author

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

Copy link
Collaborator

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.

Copy link
Collaborator

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

Copy link
Collaborator

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

Copy link
Contributor Author

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 ?

Copy link
Contributor Author

@knard38 knard38 Nov 6, 2025

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

Copy link
Contributor Author

@knard38 knard38 Nov 6, 2025

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

Fixed with commit 89f602f and 967bc9f

debian/changelog Outdated
[ Jeff Olivier]
* Make daos-spdk conflict with spdk

-- Jeff Olivier <[email protected]> Mon, 19 May 2025 14:12:00 -0700
Copy link
Contributor

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

Copy link
Contributor Author

@knard38 knard38 Oct 30, 2025

Choose a reason for hiding this comment

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

  • Remove Debian packaging update

Copy link
Contributor Author

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

@jolivier23
Copy link
Contributor

I pushed this: #17050

Integrate reviewers comments:
- Remove daos.spec update
- Remove Debian packaging update

Signed-off-by: Cedric Koch-Hofer <[email protected]>
@jolivier23
Copy link
Contributor

@soumagne this seems like a better option for the change logs

#17063 (review)

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]>
@knard38
Copy link
Contributor Author

knard38 commented Nov 6, 2025

@soumagne this seems like a better option for the change logs

#17063 (review)

It seems that the integration of the changelog is not compatible as it with Mercury.
I am going to investigate.

@soumagne
Copy link
Collaborator

soumagne commented Nov 6, 2025

@soumagne this seems like a better option for the change logs
#17063 (review)

It seems that the integration of the changelog is not compatible as it with Mercury. I am going to investigate.

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]>
@knard38
Copy link
Contributor Author

knard38 commented Nov 6, 2025

@soumagne this seems like a better option for the change logs
#17063 (review)

It seems that the integration of the changelog is not compatible as it with Mercury. I am going to investigate.

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

I have fixed it: locally tested.
Also takes the opportunity to improve the fpm error messages.
I will wait after the merge of your PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

7 participants