mirage / mirage-qubes

Mirage support for writing QubesOS AppVM unikernels
BSD 2-Clause "Simplified" License
62 stars 11 forks source link

Implement `request_service` and some attendant changes #35

Closed yomimono closed 5 years ago

yomimono commented 5 years ago

This change implements a request_service function, which lets users request that a service run on their behalf on another VM. I believe it fixes #23 .

It requires some minor API-breaking changes in the Flow interface, which previously assumed that users would be operating in the client context only.

The signature for request_service may be a bit controversial as I don't see result types used elsewhere in the library; I'm happy to rework it to fit in better with other functions here if there's a specific signature that seems more sensible.

@linse and I implemented this to allow a unikernel to request dom0 to run a service on its behalf, and we've tested it only in that context. The protocol also allows domY to request a service run on domX (where X is not 0), and while this should work, we haven't tested it.

talex5 commented 5 years ago

This API doesn't make much sense to me. I may be misunderstanding the API or the protocol, but it seems to be trying to use the same API for both clients and servers. As I understand it, these roles are different.

A server can:

A client can:

For the server-side API, we currently provide something similar to a normal Mirage FLOW, but extended with the ability to write to stderr.

For a client, we need something different. It can't just be a normal bidirectional flow extended to read stderr, because we can't control whether we receive data from stdout or stderr. It looks like this PR just throws an exception if you try to read stdout but the remote end sends stderr. It would be possible to provide a bi-directional flow that just sends stderr values to the log, but it's probably better to let the caller handle that.

The PR replaces the old server FLOW with 3 FLOWs (each with the same type): stdin, stdout and stderr. I don't believe this is useful. The point of a FLOW is that you can pass it to something that needs to read and write, but these flows are really unidirectional. e.g. a client can write to its stdin, but it would be an error to try to read from it.

It also tries to reuse the handler type for the client and server, but then has to provide a meaningless value for the user argument and ignore the return value.

I think it would make more sense to leave the existing RExec module as it is and create a new RExec_client with its own API.