Skip to content

implement automatic device allocation; resolves #1833 #3729 #3754 #3730

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
Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

implement automatic device allocation; resolves #1833 #3729 #3754 #3730

wants to merge 7 commits into from
+612 −341

Conversation

tpashkin
Copy link
Collaborator

@tpashkin tpashkin commented Jun 21, 2025 •
edited
Loading

  • generalize netlink library to easily construct new messages
  • use sysfs to find free nbd device when using ioctl
  • let the driver find or allocate free device when using netlink
  • add config option to toggle between new and old behavior
  • remove 'reconfigure' parameter from TNetlinkDevice, because it was never used
  • modify nbd client to correctly handle retries

@tpashkin tpashkin requested review from antonmyagkov and drbasic June 21, 2025 15:25
This was linked to issues Jun 21, 2025
[NBS] use sysfs to get info about free nbd devices #1833
Open
[NBS] automatic nbd device allocation via netlink #3729
Open
@tpashkin tpashkin added blockstore Add this label to run only cloud/blockstore build and tests on PR msan Launch builds with memory sanitizer along with regular build asan Launch builds with address sanitizer along with regular build tsan Launch builds with thread sanitizer along with regular build ubsan Launch builds with undefined behaviour sanitizer along with regular build labels Jun 21, 2025
antonmyagkov
antonmyagkov reviewed Jun 23, 2025
View reviewed changes
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jun 23, 2025 •
edited
Loading

Note

This is an automated comment that will be appended during run.

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit e12488a.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
4990 4986 0 3 1 0

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit e12488a.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
3 3 0 0 0 0

@github-actions GitHub Actions
Copy link
Contributor

Note

This is an automated comment that will be appended during run.

🔴 linux-x86_64-release-asan: some tests FAILED for commit e12488a.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
4778 4777 0 1 0 0

@github-actions GitHub Actions
Copy link
Contributor

Note

This is an automated comment that will be appended during run.

🔴 linux-x86_64-release-msan: some tests FAILED for commit e12488a.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
4760 4759 0 1 0 0

@github-actions GitHub Actions
Copy link
Contributor

Note

This is an automated comment that will be appended during run.

🔴 linux-x86_64-release-tsan: some tests FAILED for commit e12488a.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
4747 4742 0 1 4 0

@github-actions GitHub Actions
Copy link
Contributor

Note

This is an automated comment that will be appended during run.

🔴 linux-x86_64-release-ubsan: some tests FAILED for commit e12488a.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
4791 4790 0 1 0 0

@github-actions GitHub Actions
Copy link
Contributor

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit dc9120e.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
5037 5036 0 0 1 0

@github-actions GitHub Actions
Copy link
Contributor

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-release-msan: all tests PASSED for commit dc9120e.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
4807 4807 0 0 0 0

@github-actions GitHub Actions
Copy link
Contributor

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-release-tsan: all tests PASSED for commit dc9120e.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
4794 4790 0 0 4 0

@github-actions GitHub Actions
Copy link
Contributor

Note

This is an automated comment that will be appended during run.

🔴 linux-x86_64-release-asan: some tests FAILED for commit dc9120e.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
4826 4825 0 1 0 0

@github-actions GitHub Actions
Copy link
Contributor

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-release-ubsan: all tests PASSED for commit dc9120e.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
4838 4838 0 0 0 0

@tpashkin tpashkin changed the title implement automatic device allocation; resolves #1833 and #3729 implement automatic device allocation; resolves #1833 #3729 #3754 Jun 24, 2025
@tpashkin tpashkin linked an issue Jun 24, 2025 that may be closed by this pull request
[NBS] nbd client crashes on retry #3754
Open
@tpashkin tpashkin added the recheck Add this label to relaunch checks, it will be automatically removed label Jun 24, 2025
@github-actions github-actions bot removed the recheck Add this label to relaunch checks, it will be automatically removed label Jun 24, 2025
@github-actions GitHub Actions
Copy link
Contributor

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit dc9120e.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
5037 5036 0 0 1 0

@github-actions GitHub Actions
Copy link
Contributor

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-release-msan: all tests PASSED for commit dc9120e.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
4807 4807 0 0 0 0

@github-actions GitHub Actions
Copy link
Contributor

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-release-asan: all tests PASSED for commit dc9120e.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
4825 4825 0 0 0 0

@github-actions GitHub Actions
Copy link
Contributor

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-release-tsan: all tests PASSED for commit dc9120e.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
4794 4790 0 0 4 0

@github-actions GitHub Actions
Copy link
Contributor

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-release-ubsan: all tests PASSED for commit dc9120e.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
4838 4838 0 0 0 0

@tpashkin tpashkin requested a review from antonmyagkov June 24, 2025 14:27
antonmyagkov
antonmyagkov previously approved these changes Jun 24, 2025
View reviewed changes
SvartMetal
SvartMetal reviewed Jun 24, 2025
View reviewed changes
SvartMetal
SvartMetal reviewed Jun 24, 2025
View reviewed changes
cloud/blockstore/libs/nbd/netlink_device.cpp
struct TNbdConfigureResponse {
TNetlinkHeader Header;
::nlattr IndexAttr;
ui32 Index;
Copy link
Collaborator

Choose a reason for hiding this comment

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

тоже дефолтная инициализация

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Не очень понимаю зачем – это структура, в которой содержится ответ ядра на нетлинк запрос, она никогда не используется сама по себе

@SvartMetal SvartMetal added the large-tests Launch large tests for PR label Jun 24, 2025
@github-actions GitHub Actions
Copy link
Contributor

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-release-tsan: all tests PASSED for commit dc9120e.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
4794 4790 0 0 4 0

@github-actions GitHub Actions
Copy link
Contributor

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-release-msan: all tests PASSED for commit dc9120e.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
4807 4807 0 0 0 0

@github-actions GitHub Actions
Copy link
Contributor

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-release-ubsan: all tests PASSED for commit dc9120e.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
4838 4838 0 0 0 0

@github-actions GitHub Actions
Copy link
Contributor

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-release-asan: all tests PASSED for commit dc9120e.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
4825 4825 0 0 0 0

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jun 24, 2025 •
edited
Loading

Note

This is an automated comment that will be appended during run.

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit dc9120e.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
5417 5415 0 1 1 0

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit dc9120e.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
24 24 0 0 0 0

drbasic
drbasic previously approved these changes Jun 25, 2025
View reviewed changes
@tpashkin tpashkin dismissed stale reviews from drbasic and antonmyagkov via 9b44b1c June 25, 2025 09:38
drbasic
drbasic previously approved these changes Jun 25, 2025
View reviewed changes
@tpashkin tpashkin added the recheck Add this label to relaunch checks, it will be automatically removed label Jun 25, 2025
@github-actions github-actions bot removed the recheck Add this label to relaunch checks, it will be automatically removed label Jun 25, 2025
@tpashkin tpashkin added the rebase Add this label if you want to rebase your PR for test run label Jun 25, 2025
tpashkin added 6 commits June 25, 2025 17:28
@tpashkin
implement automatic device allocation; resolves #1833 and #3729
10ddd81 
- generalize netlink library to easily construct new messages
- use sysfs to find free nbd device when using ioctl
- let the driver find or allocate free device when using netlink
- add config option to toggle between new and old behavior
- remove 'reconfigure' parameter from TNetlinkDevice, because it was
  never used
@tpashkin
clean up test
d20dc78
@tpashkin
fix tools/nbd
b6c9e57
@tpashkin
fix tests
fbaeb9a
@tpashkin
do not crash nbd client on retries
1e07b82
@tpashkin
cr1
b10267a
@tpashkin tpashkin force-pushed the users/tpashkin/automatic-device-allocation branch from 9b44b1c to b10267a Compare June 25, 2025 14:28
@tpashkin tpashkin removed the rebase Add this label if you want to rebase your PR for test run label Jun 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SvartMetal SvartMetal SvartMetal left review comments

@drbasic drbasic drbasic left review comments

@antonmyagkov antonmyagkov antonmyagkov left review comments

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
asan Launch builds with address sanitizer along with regular build blockstore Add this label to run only cloud/blockstore build and tests on PR large-tests Launch large tests for PR msan Launch builds with memory sanitizer along with regular build tsan Launch builds with thread sanitizer along with regular build ubsan Launch builds with undefined behaviour sanitizer along with regular build
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[NBS] nbd client crashes on retry [NBS] automatic nbd device allocation via netlink [NBS] use sysfs to get info about free nbd devices
4 participants
@tpashkin @drbasic @SvartMetal @antonmyagkov