rbd: promote rbd-nbd attach and detach at rbd integrated cli#41279
rbd: promote rbd-nbd attach and detach at rbd integrated cli#41279idryomov merged 3 commits intoceph:masterfrom
Conversation
|
@pkalever And you need to update |
There was a problem hiding this comment.
It's not that I don't like the idea -- on the contrary I'm all for a better, unified, more accessible UI. However:
- the UI should be safe to use and currently there is a glaring hole here. One single-digit typo in the
--deviceargument or an image spec pasted from the wrong clipboard and you get a kernel crash or, much worse, data corruption. Yes, it is already out there in the form ofrbd-nbd attachbut not as easily accessible. - only
nbddevice type is covered.krbdis never going to support attach/detach, but the remaining two might. It would be nice to cover at least one (perhaps just as a POC), to make sure that the new commands are accommodating enough.
I don't have a ggate/wnbd setup, I will check on it. |
I understand that a POC would be nice/wise, but both |
I thought the implementation of attach/detach exists already with wnbd/ggate and just the rbd CLI integration is missing. |
|
The V3 addresses all the comments from @idryomov and @trociny, except that promotion of attach/detach at rbd CLI is added only for nbd, as the backend attach/detach support is missing for ggate/wnbd at the moment. IMHO, we can promote attach/detach for ggate/wnbd once the backend support is Inplace. Thanks! |
|
jenkins test make check |
idryomov
left a comment
There was a problem hiding this comment.
I still have a problem with rbd attach being unsafe.
Let's add a --force option. For now, until the kernel support for backend_id is merged, it would basically be required and in order to attach the user would need to say rbd device attach --device /dev/nbd0 --force .... Later, on newer kernels, the user would either provide --device-cookie or --force.
How does that sound?
|
The man page (doc/man/8/rbd.rst) needs to be updated. |
Perfect!
Sure, thanks! |
|
@idryomov I have not touched about the latest |
|
@idryomov could you please take a look? Thanks! |
doc/man/8/rbd.rst
Outdated
| specific options (opt1,opt2=val,...). | ||
|
|
||
| :command:`device detach` [-t | --device-type *device-type*] [-o | --options *device-options*] *image-spec* | *snap-spec* | *device-path* | ||
| Detach the block device that was mapped/attached via supported device type. |
There was a problem hiding this comment.
| Detach the block device that was mapped/attached via supported device type. | |
| Detach the block device that was mapped or attached (currently only `nbd` | |
| on Linux). This operation is unsafe and should not be normally used. |
There was a problem hiding this comment.
Unless I'm missing something, it just sends SIGTERM to rbd-nbd without regard to in-flight I/O. Now, rbd-nbd catches SIGTERM and exits the polling loop so it is better than SIGKILL, but it looks like all in-flight user data is going to get lost unless you reattach in time.
|
"rbd: allow nbd attach on --force only" and "man: updated rbd man page with attach/detach options" can be folded into "rbd: promote rbd-nbd attach and detach at rbd integrated cli" (updating the example in the commit message for |
|
Now that 4f6370b is merged, need to update "test detach/attach" section of |
|
@idryomov everything incorporated. I had just changed this part.
This operation is unsafe and should be used very carefully. Thanks! |
Example: $ rbd device attach rbd-pool/image --device /dev/nbd0 --device-type nbd --force $ rbd device detach rpool/image --device-type nbd for now returning EOPNOTSUPP with krbd, ggate and wnbd Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
|
@idryomov Thanks for the quick review. I had incorporated the review comments. |
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
Description:
Example:
$ rbd device attach rpool/image --device /dev/nbd0 --device-type nbd --force
$ rbd device detach rpool/image --device-type nbd
for now returning EOPNOTSUPP with krbd, ggate and wnbd
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>