[Libvir] PATCH 1/2: Support QEMU (+KVM) in libvirt

[Karel Zak]

On Fri, Jan 05, 2007 at 09:16:54PM +0000, Daniel P. Berrange wrote:
> +static int qemudParseUUID(const char *uuid,
> +                          unsigned char *rawuuid) {
> +    const char *cur;
> +    int i;
> +
> +    /*
> +     * do a liberal scan allowing '-' and ' ' anywhere between character
> +     * pairs as long as there is 32 of them in the end.
> +     */
> +    cur = uuid;
> +    for (i = 0;i < 16;) {
> +        rawuuid[i] = 0;
> +        if (*cur == 0)
> +            goto error;
> +        if ((*cur == '-') || (*cur == ' ')) {
> +            cur++;
> +            continue;
> +        }
> +        if ((*cur >= '0') && (*cur <= '9'))

 isdigit() ? :-)

> +/* Return the default machine type for a given architecture */
> +static const char *qemudDefaultMachineForArch(const char *arch) {
> +    int i;
> +
> +    for (i = 0 ; i < (int)(sizeof(archs) / sizeof(struct qemu_arch_info)) ; i++) {

#define NR_ITEMS(x)     (int)(sizeof(x)/ sizeof(*x))


> +/*
> + * Parses a libvirt XML definition of a guest, and populates the
> + * the qemud_vm struct with matching data about the guests config
> + */
> +static int qemudParseXML(struct qemud_server *server,
> +                         xmlDocPtr xml,
> +                         struct qemud_vm *vm) {
> +    xmlNodePtr root = NULL;
> +    xmlChar *prop = NULL;
> +    xmlXPathContextPtr ctxt = NULL;
> +    xmlXPathObjectPtr obj = NULL;
> +    char *conv = NULL;
> +
> +    /* Prepare parser / xpath context */
> +    root = xmlDocGetRootElement(xml);
> +    if ((root == NULL) || (!xmlStrEqual(root->name, BAD_CAST "domain"))) {
> +        qemudReportError(server, VIR_ERR_INTERNAL_ERROR, "%s", "incorrect root element");
> +        goto error;
> +    }
> +
> +    ctxt = xmlXPathNewContext(xml);
> +    if (ctxt == NULL) {
> +        qemudReportError(server, VIR_ERR_NO_MEMORY, "xmlXPathContext");
> +        goto error;
> +    }
> +
> +
> +    /* Find out what type of QEMU virtualization to use */
> +    if (!(prop = xmlGetProp(root, BAD_CAST "type"))) {
> +        qemudReportError(server, VIR_ERR_INTERNAL_ERROR, "%s", "missing domain type attribute");
> +        goto error;
> +    }

 ....

> +
> + error:
> +    /* XXX free all the stuff in the qemud_vm struct, or leave it upto
> +       the caller ? */
> +    if (prop)
> +        free(prop);
> +    if (obj)
> +        xmlXPathFreeObject(obj);

       if (ctxt)
           xmlXPathFreeContext(ctxt);

> +    return -1;
> +}
> +
> +/*
> + * Constructs a argv suitable for launching qemu with config defined
> + * for a given virtual machine.
> + */
> +int qemudBuildCommandLine(struct qemud_server *server,
> +                          struct qemud_vm *vm,
> +                          char ***argv,
> +                          int *argc) {
> +    int n = -1;
> +    char memory[50];
> +    char vcpus[50];
> +    struct qemud_vm_disk_def *disk = vm->def.disks;
> +    struct qemud_vm_net_def *net = vm->def.nets;
> +
> +    *argc = 1 + /* qemu */
> +        2 + /* machine type */
> +        (vm->def.virtType == QEMUD_VIRT_QEMU ? 1 : 0) + /* Disable kqemu */
> +        2 * vm->def.ndisks + /* disks*/
> +        (vm->def.nnets > 0 ? (4 * vm->def.nnets) : 2) + /* networks */
> +        2 + /* memory*/
> +        2 + /* cpus */
> +        (vm->def.graphicsType == QEMUD_GRAPHICS_VNC ? 2 :
> +         (vm->def.graphicsType == QEMUD_GRAPHICS_SDL ? 0 : 1)); /* graphics */
> +
> +    sprintf(memory, "%d", vm->def.memory/1024);

       vm->def.memory >> 10 

 (professional deformation from BaseOS packages... :-)

> +    sprintf(vcpus, "%d", vm->def.vcpus);
> +
> +    if (!(*argv = malloc(sizeof(char *) * (*argc +1))))
> +        goto no_memory;
> +    if (!((*argv)[++n] = strdup(vm->def.os.binary)))
> +        goto no_memory;
> +    if (!((*argv)[++n] = strdup("-M")))
> +        goto no_memory;

 Hmm... you reallocate static strings. I think you don't have to care
 about argv[] if you create it after fork() in a child process.  It
 seems odd allocate and dealocate it in a parent if you need't it the
 parent process.

> +/* Scan for all guest config files */
> +int qemudScanConfigs(struct qemud_server *server) {
> +    DIR *dir;
> +    struct dirent *entry;
> +
> +    if (!(dir = opendir(server->configDir))) {
> +        if (errno == ENOENT)
> +            return 0;
> +        return -1;
> +    }
> +
> +    while ((entry = readdir(dir))) {
> +        char file[PATH_MAX];
> +        if (entry->d_name[0] == '.')
> +            continue;

 is ".myconfig" forbidden filename? Otherwise:

   if (!strcmp(entry->d_name, ".") || !strcmp(entry->d_name, ".."))

> +/* Simple grow-on-demand string buffer */

 Hmm... It seems like duplicate code to virBufferAdd() (xml.c)

> +/* XXX re-factor to shared library */
> +struct qemudBuffer {
> +    char *data;
> +    int len;
> +    int used;
> +};
> +
> +static
> +int qemudBufferAdd(struct qemudBuffer *buf, const char *str) {
> +    int need = strlen(str);


 It's huge patch. Now (at 2:00 AM) I'm too tired... night! ;-)

    Karel

-- 
 Karel Zak  <kzak at redhat.com>