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

fix(nvmx): fixing async qpair connection bugs #1405

Merged
merged 2 commits into from
Jun 9, 2023

Conversation

dsavitskiy
Copy link
Contributor

No description provided.

@auto-assign auto-assign bot requested a review from blaisedias June 8, 2023 17:40
@dsavitskiy dsavitskiy force-pushed the feature/async-qpair-crash branch from 0c5d00d to 40c887e Compare June 8, 2023 17:48
@dsavitskiy
Copy link
Contributor Author

bors try

bors bot pushed a commit that referenced this pull request Jun 8, 2023
@bors
Copy link
Contributor

bors bot commented Jun 8, 2023

try

Build failed:

@dsavitskiy dsavitskiy force-pushed the feature/async-qpair-crash branch from 40c887e to a75f2d9 Compare June 8, 2023 18:02

fn remove_qpair(&mut self) -> Option<QPair> {
if let Some(q) = &self.qpair {
debug!("removing qpair {q:p}", q = q.as_ptr());
Copy link
Contributor

Choose a reason for hiding this comment

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

should this be trace?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes

Comment on lines 69 to 70
.parse()
.unwrap()
Copy link
Contributor

Choose a reason for hiding this comment

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

is to_string() enough ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah, indeed. I just copied this piece of code over from the old implementation.

@dsavitskiy dsavitskiy force-pushed the feature/async-qpair-crash branch 2 times, most recently from e9e78a5 to 46671c5 Compare June 9, 2023 11:23
@dsavitskiy
Copy link
Contributor Author

bors try

@bors
Copy link
Contributor

bors bot commented Jun 9, 2023

try

Merge conflict.

@dsavitskiy dsavitskiy force-pushed the feature/async-qpair-crash branch from 46671c5 to 6268470 Compare June 9, 2023 11:31
@dsavitskiy dsavitskiy force-pushed the feature/async-qpair-crash branch from 6268470 to 537ac2f Compare June 9, 2023 11:35
@dsavitskiy
Copy link
Contributor Author

bors try

bors bot pushed a commit that referenced this pull request Jun 9, 2023
This commit fixes a use-after-free in a situation when
a qpair instance was dropped while an async connection was in progress.
A component-level test for such situation is added.

Signed-off-by: Dmitry Savitskiy <[email protected]>
@dsavitskiy dsavitskiy force-pushed the feature/async-qpair-crash branch from 537ac2f to 3875ce9 Compare June 9, 2023 11:40
@bors
Copy link
Contributor

bors bot commented Jun 9, 2023

try

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@dsavitskiy
Copy link
Contributor Author

bors merge

@bors
Copy link
Contributor

bors bot commented Jun 9, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit 52956b9 into openebs:develop Jun 9, 2023
@dsavitskiy dsavitskiy deleted the feature/async-qpair-crash branch June 9, 2023 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants