Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace python example code with minimal functional implementation. #105

Merged
merged 1 commit into from
Nov 28, 2019

Conversation

MarkSymsCtx
Copy link
Contributor

Signed-off-by: Mark Syms mark.syms@citrix.com

Copy link
Contributor

@lindig lindig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me but it's quite a bit of code.

@MarkSymsCtx
Copy link
Contributor Author

Looks good to me but it's quite a bit of code.

It's about the smallest amount that can actually be functional, unlike the removed example which didn't actually do anything other than respond to the requests. This example is good enough to install and run VMs on albeit with some limitations.

https://xapi-project.github.io/xapi-storage/?python#plugin. This
declares the name of the plugin, version, vendor, copyright etc and
the capabilities supported by the implementation of the
plugin. Simpler than the definition for a volume plugin as now
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean "no features are exposed"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, well spotted

Copy link
Contributor

@gaborigloi gaborigloi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great thanks!

@lindig
Copy link
Contributor

lindig commented Nov 25, 2019

Can we merge this?

"VDI_DEACTIVATE",
"VDI_UPDATE",
"VDI_RESIZE",
"SR_METADATA",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about the exact meaning of this, but it looks like this is needed only if the SR supports disaster recovery. SInce this example SR doesn't even support set_name or set_description I think we could safely remove this particular feature.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, very true.

@edwintorok
Copy link
Contributor

Looks good, one minor comment about SR_METADATA

Signed-off-by: Mark Syms <mark.syms@citrix.com>
@lindig
Copy link
Contributor

lindig commented Nov 28, 2019

Can we merge this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants