[Libvir] [QEMU 3/3] the qemu driver & daemon

[Daniel Veillard]

On Mon, Aug 28, 2006 at 12:18:52AM +0100, Daniel P. Berrange wrote:
> Attached is the patch for the qemu driver & daemon process
> 
> Since there is no scalable way to determine active standalone QEMU instances,
> or access their monitor connection, the backend implements a management 
> daemon - called 'qemud' - which takes care of managing the complete lifecycle
> of QEMU virtual machines. The libvirt driver is a thin shim which talks to
> qemud via a UNIX domain socket - the wire protocol is modelled after the
> protocol used for libvirt_proxy, but obviously contains alot more functions
> since its full read/write, not just read-only.

   Okay, it would have been nicer if QEmu opened a socket upon launching
a new instance, but currently that sounds the only solution.

> 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 ?

> 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.

> The backend code is in src/qemu_internal.h, src/qemu_internal.c, the daemon
> code is in qemud/* with qemud/protocol.h defining the network wire protocol.
> This latter header file is also included by src/qemu_internal.c
> 
>  - qemud.c - the main daemon management code - handles all network I/O and 
>              forking / cleanup of QEMU processes. The implementation is
>              poll() based & completely non-blocking. All requests/replies
>              from a single client are, however, serialized.
>  - dispatch.c - takes care of serializing/de-serializing requests/replies
>                 from/to libvirt - called from qemud.c when a complete
>                 message is available.
>  - driver.c   - the implementation of each request from libvirt - called
>                 from dispatch.c
>  - config.c   - manages reading & writing of XML files for inactive domains
>                 in the $HOME/.qemud.d/*.xml. Also takes care of generating
>                 a suitable command line for execing qemu.
> 
> In terms of hardware, there is support for
> 
>  - harddisk - floppy, cdrom, harddisk
>  - graphics - VNC
>  - memory
>  - virtual CPUs
>  
> In particular no support for
> 
>  - networking
>  - sound
>  - non-native CPU types
>  - boot order
> 
> But its enough to demonstrate the practice.

  Well that looks excellent :-) , of course networking will be needed to really
make use of it but this is fantastic progress !

> +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.

> +    //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 ?

> +  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 :-)

> +static int qemudSaveConfig(struct qemud_server *server ATTRIBUTE_UNUSED,
> +			   struct qemud_vm *vm) {
> +  char *xml;
> +  int fd, ret = -1;
> +  int towrite;
> +  struct stat sb;
> +
> +  if (!(xml = qemudGenerateXML(vm))) {
> +    return -1;
> +  }
> +
> +  printf("Save VM %s\n", vm->configFile);
> +
> +  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.

> +  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 ...

> +  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.


I scanned quickly the protocol part but I need to get my head around it :-)
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 :-)

  Fantastic but that's a big chunk of code !

Daniel


-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard      | virtualization library  http://libvirt.org/
veillard at redhat.com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/