Re: [SPDK] Dynamic base bdev management for multi-tenant virtual bdev

[Walker, Benjamin <benjamin.walker at intel.com>]

[-- Attachment #1: Type: text/plain, Size: 5474 bytes --]

Hi Andrey,

I'm trying to get back to some messages that slipped through the cracks. I think
what you're doing here is important and needs to be addressed. Responses inline.

On Thu, 2018-05-31 at 14:16 +0300, Andrey Kuzmin wrote:
> Planning for a multi-tenant virtual bdev driver, I looked into the provided
> base bdev management capabilities and found them short of what I need. The
> issues I see are outlined below. Let me know if the analysis is correct and,
> if yes, are there any plans to provide for the dynamic base bdev management
> capabilities in the multi-tenant vbdev use case.
> 
> 1. Vbdev startup
> 
> spdk_vbdev_register at present allows one to register a completely assembled
> vbdev (with all base bdevs already examined) only. The root cause behind that
> fully-assembled requirement above is  spdk_vbdev_set_base_bdevs call that
> follows, which assumes that vbdev's base bdevs haven't been set up yet.
> 
> Apparently, a non-trivial multi-tenant vbdev should be allowed to start up in
> a partially assembled state; erasure code-based RAID provides a ready-made
> example of a vbdev that is expected to be/remain operational while an
> arbitrary number of base bdevs is missing permanently or temporarily, in
> particular (but not limited to) at startup time.
> 
> Furthermore, a vbdev like this should be able to register a hot-plugged base
> bdev at any point of runtime, yet again pointing to the need for a
> vbdev_register_base_bdev(vbdev, base_bdev) call in addition to/in replacement
> of the available spdk_vbdev_set_base_bdevs method (more on this under Bdev hot
> plug below).

I agree that bdev modules should be able to expose bdevs that are only partially
assembled and at run time add or remove base bdevs as necessary. I also agree
that the vbdev_* API has a lot of assumptions about when the base bdevs are
known, and that is not going to work for you. However, the vbdev_* APIs are
convenience wrappers only. You can perform every required operation in a bdev
module without using those, and I think that's what you're going to want to do
here. I think we need to audit the bdev module API and clarify which operations
are the "fundamental" ones and which are these convenience vbdev things that
work for 90% of vbdevs but not all of them. Since you sent this note, I've
created a public header file that is intended to define the bdev module API
officially (include/spdk/bdev_module.h). Now we just need to iterate on that to
make it clearer.

> 
> 2. Bdev surprise removal
> 
> SPDK bdev ops vector includes .hotremove method which, for each open base bdev
> descriptor, gives vbdev module an opportunity to clean up and/or do any
> redundancy-related base bdev management.
> 
> While .hotremove provides for the vbdev-internal bdev management on hot
> remove, spdk_bdev_unregister which completes hot-remove handling in the bdev
> layer does not remove base bdev from vbdev's base bdev list, so base bdev in
> question still sits on the list after being removed. The reason is likely the
> missing vbdev->base_bdevs dynamic management in general and
> vbdev_remove_base_bdev(vbdev, bdev) call in particular, required to manage
> vbdev->base_bdevs list on a single bdev removal.

Agreed - the vbdev wrappers need to either be improved, or you need to use the
lower level APIs.

> 
> 3 Bdev hot plug
> 
> At present virtual bdev design does seem to provide any support for base bdev
> hot-plug. Vbdev's extant .examine method seems to be geared toward initial
> vbdev setup in that it assumes no open vbdev descriptors (so that vbdev to
> base bdevs descriptor linkage occurs when vbdev is subsequently opened and its
> I/O channels are created). 
> 
> There is currently no .hotplug mechanism complementary to .hotremove that
> would propagate base bdev insertion throughout all open vbdev descriptors, so
> that vbdev has a chance to set up the I/O channel/do other house-keeping for
> the plugged base bdev on each vbdev descriptor/channel open at the moment of
> the base bdev insertion.

You need a mechanism such that on hot-insert a message is sent to each existing
I/O channel for the bdev to perform per-channel initialization? Can you just
call spdk_for_each_channel?

> 
> 4. Vbdev shutdown
> 
> It appears that, while bdev subsystem start-up proceeds in the expected
> bottom-up fashion, with vbdevs instantiated as the underlying base bdevs show
> up, the reverse is not true: on bdev subsystem shutdown, I see vbdev's
> .hotremove being called where I would expect vbdev being closed/unregistered.
> 
> Understandably, for a vbdev module author it would be very helpful to be able
> to differentiate between planned (sub)system shutdown and hot removal of a
> base bdev at run time; for this to happen, bdev subsystem shutdown should
> proceed top-down, with virtual bdevs unregistered prior to the underlying
> bdevs.

I agree - Pawel Wodkowski is working in this area.

I also wanted to mention that a few patches have gone in which allow the lvol
bdev module to perform its examine without write access. At least one more patch
is required still, but once complete we can merge your patch that separates
claiming bdevs from opening them.

Once that is done, we need to begin work to differentiate the various reasons a
bdev could be examined and the various reasons a bdev could be removed.

Thanks,
Ben