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

8 Jan 2007


      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@redhat.com>