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

Design for on/off control of guest RDP service #12

Merged
merged 1 commit into from May 27, 2015

Conversation

thomassa
Copy link

@thomassa thomassa commented Dec 2, 2014

Design doc for on/off control of guest RDP service

@djs55
Copy link
Contributor

djs55 commented Dec 2, 2014

Thanks, will take a look!

@simonjbeaumont
Copy link
Contributor

Yay!! Contributions all round! 👍

@simonjbeaumont
Copy link
Contributor

This needs an entry in the _data/navbar.yml. See the README section: https://github.com/xapi-project/xapi-project.github.io#contributing-documentation

@simonjbeaumont
Copy link
Contributor

Query: should this be in cross-cutting features (as it is now) or should it be in the Xapi subdir. Deciding where to put it on the navbar might inform your decision.

@thomassa
Copy link
Author

thomassa commented Dec 3, 2014

I am not sure of the best place for this (hence deliberately not adding anything to navbar.yml yet: I wanted to put the content up first to inform the discussion of where it should go).

The argument for putting it in cross-cutting features is that the code for this feature is (or will be) spread across several repositories and their corresponding output components:

@simonjbeaumont
Copy link
Contributor

All sounds plausible. Good to get the content in a staging area like this. Let's just remember to add the navbar entry before merging. We could add one with where it is now and change if we change our mind.

@simonjbeaumont
Copy link
Contributor

@thomassa any chance you could squash your small fixup and refresh this pull request? It will have the bonus of waking up Travis (since he doesn't know about existing pull reqs)

@thomassa
Copy link
Author

thomassa commented Dec 4, 2014

Done (and already github says the Travis CI build is in progress).

@simonjbeaumont
Copy link
Contributor

Great, thanks Thomas!

@djs55
Copy link
Contributor

djs55 commented Dec 15, 2014

I followed @robhoes's example (for better or worse) and added a line-note to the code which probably should be a design-level comment instead. For the record here's what I said:

Looking at the other FEATURE_s in the context (shutdown, suspend, vcpu_hotplug) they all correspond to operations on the VM "virtual hardware". The XenAPI VM.* is mostly about manipulating and monitoring the "virtual hardware" and mostly not about performing operations within the guest operating system (although the implementation of VM.clean_shutdown happens to do this as part of its implementation it could have been implemented in terms of an ACPI button push). This means that the VM.* functions don't really care whether you're running Linux, FreeBSD or Windows.

Since this feature seems very Windows-dependent ("terminal services") and is tied to our particular guest agent (and is not part of the standard Xen guest interface which is more minimal), I think we should reflect that in the name. Perhaps we should call the API VM_guest_agent.foo?

@thomassa
Copy link
Author

In principle a guest agent could do the same thing in any guest regardless of operating system; there's at least one RDP server implementation for Linux.
Still, this is a Windows-only feature at present and for the foreseeable future.

So, taking all the suggestions together, we'd end up with calls like VM.guest_agent_request_ts_on.
That's adding "guest_agent" for (informative context),
replacing "beg" with "request" (because people are uncomfortable with "beg"), and
replacing "rdp" with "ts" for consistency throughout.
(The alternative to "ts" would be to change everything except the xenstore node-names from "ts" to "rdp", but this would involve some nasty fiddling about with names where at present we just copy everything under the control xenstore node into identically named mappings in the other map in VM guest metrics.)

@djs55
Copy link
Contributor

djs55 commented Dec 17, 2014

Here are some further thoughts on the XenAPI naming:

General thoughts

VMware has APIs which talk to their guest agent, such as vm.guest.ProcessManager.GuestProcessManger.ListProcessesInGuest. I think this is a bit verbose but I like that it's inside a vm.guest namespace -- it's clearly separate from VM power control operations. In KVM land they have guest agent protocols like SPICE which are binary protocols run over virtual serial links -- control-plane software like libvirt doesn't act as an RPC proxy.

Ideally in future we will be able to write guest agent code which exposes new APIs without having to add them to the XenAPI at all. Perhaps we would do this by transparently forwarding everything with an function name prefix of "VM.Guest" to the VM via some future protocol (over vchan or channels or vsock)

Option 1: move the functions to a new VM_guest namespace

In datamodel.ml we could create a new namespace like this:

let vm_guest = 
  create_obj ...
  ~contents:[]
  ~messages: [ function1; function2 ]

This would clearly separate the VM power control functions (e.g. VM.start) from the guest-agent specific functions. Perhaps in future we could remove the implementation from xapi and turn xapi into a proxy for the VM_guest prefix.

Option 2: add a call_plugin-style of API

We already have Host.call_plugin which allows people to add extension APIs. We could add VM.call_plugin and then define the RDP control to be a specific named "plugin". This would be less typesafe (since the API arguments are all strings) and would generate less documentation automatically but it minimises the amount of pollution in the VM namespace

@thomassa
Copy link
Author

I rather like Dave's second option because (once implemented properly) it would let us add a new feature to the guest agent and start using it from a client of XenAPI, all without needing to make any change to any software that runs on the XenServer host.

We shouldn't do this right now for the RDP control feature though.
For that we could just fake it: we would add a new API call VM.call_plugin which would throw an error like no_such_plugin unless the plugin call is to "request_rdp_on" or similar, in which case it would call the code in the existing pull-requests at xen-api#2042 etc.

Dave and I have discussed it some more in person and we think that's the best approach for now.

Signed-off-by: Thomas Sanders <thomas.sanders@citrix.com>
@thomassa
Copy link
Author

Today I have updated RDP.md to reflect the feature in its final form as implemented and released.
I've added design_doc: true to the metadata section so it will appear in the design-docs section; I don't think it warrants an entry of its own in the navbar.

Now this needs another review.

@thomassa thomassa changed the title Rdp control Design for on/off control of guest RDP service May 18, 2015
@thomassa thomassa removed their assignment May 18, 2015
robhoes added a commit that referenced this pull request May 27, 2015
Design for on/off control of guest RDP service
@robhoes robhoes merged commit 934f162 into xapi-project:master May 27, 2015
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.

None yet

5 participants