storage: add dynamic managed storage dispatcher#442
storage: add dynamic managed storage dispatcher#442sp-viktori wants to merge 2 commits intooVirt:masterfrom
Conversation
sp-viktori
requested review from
aesteve-rh,
almusil,
mz-pdm and
tinez
as code owners
sp-viktori
force-pushed
the
managed-storage-dispatcher
branch
from
90d1601 to
394fa30
Compare
sp-viktori
force-pushed
the
managed-storage-dispatcher
branch
2 times, most recently
from
5cc41fc to
d0a34b5
Compare
sp-viktori
force-pushed
the
managed-storage-dispatcher
branch
from
d0a34b5 to
b026231
Compare
lib/vdsm/api/vdsm-api.yml
Outdated
| description: Adapter vendor | ||
| name: adapter | ||
| type: string | ||
| added: '4.5.6' |
There was a problem hiding this comment.
4.5.8 it would be if merged now
There was a problem hiding this comment.
thanks for catching this, updated
doc/managed-volume-adapters.md
Outdated
| the default `cinderlib-client.py` helper. | ||
|
|
||
| The vendor packaging is expected to install a symlink in | ||
| `/usr/share/ovirt-engine/cinderlib/` named `{adapter}-adapter`. |
There was a problem hiding this comment.
Just in doubt if this is the right directory to put this in.
/usr/share/ovirt-engine/managedvolume/ perhaps and move cinderlib into it?
There was a problem hiding this comment.
Right, I've changed this to reflect the oVirt Engine PR (the updated proposal indeed uses a managedblock directory instead of cinderlib).
lib/vdsm/storage/managedvolume.py
Outdated
|
|
||
| try: | ||
| attachment = run_helper("attach", connection_info) | ||
| attachment = run_helper("attach", connection_info, connection_info.get("adapter")) |
There was a problem hiding this comment.
This is useless? As you pass connection_info, you can also get the adapter in the run_helper function then.
There was a problem hiding this comment.
In this case, yes, but the run_helper function has a union type: in this case it is connection_info but when calling for detach it is vol_info. Good news is that vol_info has connection_info as a field (which has adapter) but it becomes very convoluted to figure it out.
I'll think on simplifying the whole thing while keeping it robust, and push an update soon.
There was a problem hiding this comment.
I've pushed a different approach, and also split it in two:
- A change in how managedvolume-helper is called that makes the protocol a bit stricter (so there is no ambiguity what is sent as input: it is always volume_info struct).
- The adapter dispatch which now knows exactly where to look for the adapter field.
sp-viktori
force-pushed
the
managed-storage-dispatcher
branch
from
b026231 to
25159ca
Compare
Make the protocol used to communicate with the managed volume helper scripts stricter. Previously, the input could be: - when attaching: connection_info struct; - when detaching: volume_info struct which contains connection_info, attachment, path, etc.). Now it is always a volume_info struct, and in the case when we're attaching the struct only has the connection_info field populated. Signed-off-by: Viktor Ivanov <[email protected]>
Add an optional "adapter" field in the ManagedVolumeConnection struct, allowing dispatching to vendor managedvolume-helper adapters. Signed-off-by: Viktor Ivanov <[email protected]>
sp-viktori
force-pushed
the
managed-storage-dispatcher
branch
from
25159ca to
ce5b7f0
Compare
2 participants
Dispatch mechanism that third-party storage vendors can use to redirect managed storage operations.
Add an optional
adapterfield in theManagedVolumeConnectionstruct.If there is an
adapterfield, direct storage requests tomanagedvolume-helper-{adapter}, otherwise use the default (os-brick) implementation.Works together with a simplar PR in ovirt-engine that allows redirection of the rest of the managed storage operations.