On Mon, Aug 28, 2006 at 04:55:09AM -0400, Daniel Veillard wrote:
On Mon, Aug 28, 2006 at 12:18:52AM +0100, Daniel P. Berrange wrote:
There is intended to be one qemud instance per UNIX user - the so called 'session' instance, and optionally one system global instance runing as root (or a system daemon account?) - the so called 'system' instance. When using libvirt, the URIs for each are 'qemu:///session' and 'qemu:///system' respectively.
I'm wondering what would be the role of the system instance. Aggregate the informations from the set of users running qemu when running as root ?
Well, the session instance is intended to fill the desktop user use-case, while the system instance is more aimed at a data-center use case where an admin might set up a bunch of persistently running QEMU instances on a server.
The UNIX socket for session instances is $HOME/.qemud and is chmod 0700 - no support is provided for users connecting to each other's instances. Although we could trivially extend to create a read-only socket $HOME/.qemud-ro if desired. The system instance is currently not finished but intended to run in /var/run/qemud or some such.
hum .... I'm not too fond of putting the socket directly in the user home directory, I would feel better with a .qemud directory, protected 700 by default and then have the socket mapped in that directory, I think it is more secure, a bit cleaner to not have socket in home dir, and gives a place to store informations if needed. Actually you already create .qemud.d for storage, can we just make it .qemud and store the socket there ?
Or we can use a socket not mapped in the filesystem at all, based on the user name, which usually avoid the race when checking and then opening.
Actually, the socket is already in the abstract namespace - i use the abstract path '\0$HOME/.qemud' - could easily put it in a dir below though
+static int qemudParseUUID(const char *uuid, + unsigned char *rawuuid) {
Should we make a lib subdir to avoid duplicating code like this ? Alone that doesn't sound worth it, but if there is more routines duplicated could be useful.
Ues, as karel pointed out, there are a number of utility functions from libvirt that I duplicated in the qemud/ directory primarily because I wanted to avoid the circular depedancy back onto libvirt from the daemon. I think it would be worthwhile having the various helper routines in some lib that we can then pull into both libvirt & the daemon as needed.
+ //virXMLError(VIR_ERR_NO_SOURCE, (const char *) target, 0); + //virXMLError(VIR_ERR_NO_TARGET, (const char *) source, 0); + printf("Bogus floppy\n"); + printf("Bogus cdomr\n"); + printf("alse %s %s\n", device, target);
Need to unify and make the usual wrappers for error code. Even if not linked in the libvirt lib, so that error messages could be carried back at the protocol level, if we put much logic in the qemud we will need some logging facilities, or transport errors, right ?
Yes, I need to rip out the debug code & formalize the set of error codes which can be returned to libvirt. Currently I catch every error and just return a generic -1, internal error. Obviously this needs to be better before the patch is merged.
+ obj = xmlXPathEval(BAD_CAST "/domain/devices/graphics", ctxt); + if ((obj == NULL) || (obj->type != XPATH_NODESET) || + (obj->nodesetval == NULL) || (obj->nodesetval->nodeNr == 0)) { + vm->def.graphicsType = QEMUD_GRAPHICS_NONE; + } else { + prop = xmlGetProp(obj->nodesetval->nodeTab[0], BAD_CAST "type"); + if (!strcmp((char *)prop, "vnc")) { + vm->def.graphicsType = QEMUD_GRAPHICS_VNC; + prop = xmlGetProp(obj->nodesetval->nodeTab[0], BAD_CAST "port"); + if (prop) { + conv = NULL; + vm->def.vncPort = strtoll((const char*)prop, &conv, 10); + } else { + vm->def.vncPort = 0; + } + } + }
Hum ... if we have no graphic device what's going on ? the QEmu process is forked by a daemon, so there is no way to get back to the console... The console access is something we didn't tried to address so far in libvirt. At least some console logs would be useful, otherwise we have the problem of hitting asynchronous interface is we really want to give interactive access... Well we can probably live without :-)
There's a couple of different ways this can work - the VNC console, or we can hook up a serial device to the guest (and assume user puts appropriate kernl boot command line args in 'console=ttyS0'. I also expect to save the stderr and stdout from QEMU to a log file so user can see if stuff goes wrong. I expect VNC is the primary console though.
+ if (stat(server->configDir, &sb) < 0) { + if (errno == ENOENT) { + if (mkdir(server->configDir, 0700) < 0) + return -1; + } else { + return -1; + } + } else if (!S_ISDIR(sb.st_mode)) { + return -1; + }
Shouldn't we check the mode and owner if the directory already exists ? that should be in the stat informations.
Yes, worthwhile doing.
+ if (qemudFindVMByUUID(server, vm->def.uuid)) { + qemudFreeVM(vm); + printf("Duplicate domains, discarding %s\n", file); + return NULL; + }
UUID clashes due to users manually copying files may be more frequent than expected. Cloning is gonna be fun in general ...
Yes, I'm not entirely sure what we can do there - either document - "do not do that", or automatically re-generate the UUID if they have a duplicate (re-saving the cofig file to disk with the new UUID). I believe VMWare automatically re-generates the UUID if you manually copy a file, so its a reasonable approach to take. Could do the same with the Name if neccessary - just append '-1', '-2', or something on to the end of it.
+ if (qemudFindVMByName(server, vm->def.name)) { + qemudFreeVM(vm); + printf("Duplicate domains, discarding %s\n", file); + return NULL; + } [...] +int qemudScanConfigs(struct qemud_server *server) {
2 things here: 1/ caching if the modification timestamps of the directory and file didn't changed. I don't know how often the scanning will be done but it's not lightweight since it reparses everything 2/ name and uuid clashes could be detected here or when adding some.
If we wanted to be really clever we could use INotify to automatically re-scan. A simple approach though would just re-scan the directory comparing timestamps every minute or so & re-loading configs which have changed.
I scanned quickly the protocol part but I need to get my head around it :-)
There's a couple of important assumptions I apply in the network code which I should document further to help explain the way it works.
That's a lot of code ! One think I would like is to try to get all commenting at the same level, i.e. having every function carrying a comment header, That's something I will help with, maybe as a side effect of reviewing the code. Tell me how we could do that ? Maybe I could start with the simplest code from qemud.
-#define MAX_DRIVERS 5 +#define MAX_DRIVERS 10
I didn't expect that initially :-)
Hehe - took me a little while to track down that issue too - I just could not work out why nothing was working at first :-) 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 -=|