6:33 a.m.
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 -=|