Move nbd to userspace #122
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
petrutlucian94
commented
May 17, 2023
petrutlucian94
force-pushed
the
move_nbd_to_userspace
branch
2 times, most recently
from
017405f
to
a8b6a14
Compare
We're updating the default console logger to include timestamps in the printed messages. Note that rbd-wnbd already provides its own logger function that includes timestamps. Signed-off-by: Lucian Petrut <lpetrut@cloudbasesolutions.com>
petrutlucian94
force-pushed
the
move_nbd_to_userspace
branch
2 times, most recently
from
c115d72
to
49db209
Compare
petrutlucian94
commented
May 19, 2023
petrutlucian94
commented
May 19, 2023
Right now, WNBD provides two IO transmission channels: * nbd - initial implementation * DeviceIoControl based API - used by rbd-wnbd rbd-wnbd no longer uses NBD, however we'll continue to offer the NBD client functionality for the following reasons: * one of the very few Windows NBD clients out there, pretty well received by the community * allows connecting to a variety of NBD servers, including qemu-nbd * useful for testing purposes We'll move the NBD client to userspace (libwnbd.dll), leveraging the DeviceIoControl API for driver communication. Main benefits: * simplifies the driver and allows us to drop the ksocket dependency * reduces the risk of crashes or security issues * easier to maintain While at it, we're bumping the c++ version to c++20, which is supported by VS 2019. Among other things, it simplifies structure initialization. Signed-off-by: Lucian Petrut <lpetrut@cloudbasesolutions.com>
Now that the nbd client implementation has been moved to libwnbd, we can drop it from the driver. We're keeping the WNBD_PROPERTIES structure mostly the same for backwards compatibility reasons, except for a small change: we're adding a separate flag to advertise the userspace nbd client in order to avoid incorrect behavior with older driver versions (e.g. having both libwnbd and the driver attempt to initiate NBD connections). The WNBD tests and CLI client are updated to use the libwnbd NBD client implementation. One thing to note here is that the "wnbd-client map" now becomes a blocking command, running the daemon in background. Signed-off-by: Lucian Petrut <lpetrut@cloudbasesolutions.com>
petrutlucian94
force-pushed
the
move_nbd_to_userspace
branch
from
49db209
to
9e5871d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Right now, WNBD provides two IO transmission channels:
rbd-wnbd no longer uses NBD, however we'll continue to offer the NBD client functionality for the following reasons:
We'll move the NBD client to userspace (libwnbd.dll), leveraging the DeviceIoControl API for driver communication. Main benefits:
The only potential downside to be aware of is that the
wnbd-client mapcommand becomes blocking. We might add an option to run the daemon in a separate process, however it would be killed when the session ends (i.e. winrm or ssh session). To avoid this, Ceph daemons are spawned by a centralized Windows service, however we aren't going to add such a service for NBD, at least for now. Other workarounds include NSSM or spawning the process using theWin32_ProcessWMI class.While at it, we're bumping the c++ version to c++20, which is supported by VS 2019. Among other things, it simplifies structure
initialization.
Signed-off-by: Lucian Petrut lpetrut@cloudbasesolutions.com