On Tue, Jan 30, 2007 at 05:42:21PM +0000, Richard W.M. Jones wrote:
> This patch is just for discussion. It's not in a state to be applied,
> even if it were accepted (which is a long-shot at present anyway).
>
> When looking at the patch, a good starting point is to search for
> "Architecture and notes" and read from there.
>
> Supports:
>
> * remote driver (just does the "open", "close", "type" and
> "version" calls at present as a proof of concept)
> * TLS transport (built using GnuTLS)
> * SSH transport (forks external ssh process)
> * TCP transport (unencrypted - just for testing)
> * Unix domain socket transport
> * arbitrary external program / shell script transport
> * IPv6-ready on client & server
>
> I've tested all the transports, and in limited tests they all
> seem to work. ie. You really can do:
>
> virsh -c remote:tls:server version
I don't think that's valid URI syntax - although we've got the historical
cruft of a simple 'xen' connect string, we're trying to get the rest of
the backends to use sane URIs. We need to be able to express the actual
backend driver in this URI, so that its possible to pass it into the
virConnectOpen method on the server side.
Current drivers have prefixes like
qemud:///
test:///default
We could make use of the host/port number component to signify that the
driver should use the libvirtd. So 'test:///default' would be procssed
by the driver locally, while 'test://example.com/default' would get
picked up by the 'remote' driver which would strip the host/port bit
and pass 'test:///default' to the virConnectOpen method on the remote
end.
> Shortcomings in this version:
>
> * in "open" call, name must be non-NULL (this is just a bug)
> * doesn't actually invoke libvirt on the server side; just
> prints out messages and returns dummy values
> * "ssh" not recognised as a service name by getaddrinfo, so
> you must always give a port number, ie. remote:ssh:server:22
That's a little odd - do other named services get recognised corectly ?
> * /tmp/socket should be cleaned up when the server exits
Two scenarios I imagine:
1. A privileged daemon started from the init scripts. The init
script would do the PID file based locking preventing 2
instances being started. Thus could simply unlink() the socket
upon startup before bind()ing to it
2. Or just use sockets in the abstract namespace which don't hit
the filesystem at all & thus are auto-cleaned up.
I prefer the latter really.
> * SunRPC is stateless so we need to hand out a cookie to
> represent the virConnectPtr handle on the server side.
> However if the client dies without explicitly calling
> close, we have no way to know, and so the cookie/handle
> on the server side lives forever.
I think you can get a handle to the socket representing the client
connection from without the remote impl of the methods, by using the
svc_getcaller() API, and thus associated state with the client socket.
Since we have to replace svc_run() with a hand crafted mainloop
we can then detect when a client socket is removed from the server
and trigger cleanup of any resources.
> * There's some confusion about the level of abstraction. At
> the moment I'm abstracting at the driver level, but that may
> be wrong and possibly I should be abstracting at the level
> of vir* calls. On the other hand, there's not a huge amount
> of difference.
The only interesting difference is probably in the virConnectOpen
method where we iterate over the drivers figuring out which one to
activate. Once you have an activated driver, then it doesn't matter
really whether you call the public, or driver APIs. I think I'd
probably start off at the vir* call level.
> * Security:
> Is it safe for libvirt to be connecting to arbitrary TCP
> sockets?
Well the app always has to pass an explicit URI into virConnectOpen
before we'll do any of the remote connection stuff, so I'd assume
that if they're giving us an explicit request to connect to port
XXX then they know what they're doing.
> Is it safe for libvirt to be able to run arbitrary programs?
I'm not such a fan of libraries which spawn arbitrary programs on
my behalf. But as long as it didn't actually spawn stuff unless ther
was some explicit request based on the URI it should be ok - that
way we can document that specific URI formats will result in the
spawning of /usr/bin/ssh.
> + if (strncasecmp (name, "remote:", 7) != 0)
> + return -1; /* Not for me. */
> +
> + /* Split the name at whitespace and parse it. */
> + const char *p;
> + for (p = name; *p;) {
> + // NB. All versioning is done by SunRPC so we don't need to worry
> + // that we are connected to an incompatible daemon.
> + break;
> + }
We can use libxml's existing routines for parsing URIs
#include <libxml/uri.h>
xmlURIPtr uri = xmlParseURI(connectstr)
> +
> + case proto_unix: {
> + // 108 is hard-coded into the header files as well.
> +#define UNIX_PATH_MAX 108
Can't we use sizeof(addr.sun_path) because this may be hardcoded
to a different value on non-Linux systems.
> +program LIBVIRTREMOTE {
> + version LIBVIRTREMOTE_VERS1 {
> + /* XXX The open interface should return a cookie to represent
> + * the virConnectPtr on the server side. Note also that SunRPC
> + * is stateless so it's unclear when cookies can be garbage
> + * collected.
> + */
> + int remote_rpc_open (string) = 1;
Do we actually need any to have any cookie for this - just assume 1 socket
connetion == virConnectPtr object and thus only allow remote_rpc_open to
be called a single time per connection.
Regards,
Dan.
--
|=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=|
|=- Perl modules: http://search.cpan.org/~danberr/ -=|
|=- Projects: http://freshmeat.net/~danielpb/ -=|
|=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|