This looks great, Alex! I haven't studied intently, but I have a
couple quick comments:

Line 16: Yes, I think it should be a parameter on create_volume. That
makes the implementation less awkward if a provider does not support
snapshotting.

Line 70: What does node= do on detach()? Is it a sanity check in case
your idea of state is wrong, or supporting attachment to multiple
nodes at a time?

I think some of the naming is wordy. As one example, you'll end up as
a client with something like:

node = ...
volume = ...
result = driver.node_attach_volume(node, volume, '/dev/xvdq')

To steal a Zed Shaw term, feels a bit like a feature chant ("node Node
NODE"), but it fits with the rest of the APIs in my eyes, so probably
a broader concern than just this design.

--
Jed Smith
[email protected]

Reply via email to