linux countainers cleanup patches

* src/lxc_conf.c src/lxc_conf.h: cleanup patch for the conf driver
  of linux countainers, reuse XPath helpers, make string fields dynamic
  and remove a memory leak.
* src/lxc_driver.c: avoid some problems when the config directory is
  not accessible and for regression tests
Daniel

diff --git a/ChangeLog b/ChangeLog
index 77375a340b540296c6f719cf415f2e31e6ee5692..be803c9167629290b248bad39645b5b9faa83218 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+Mon Mar 31 13:58:25 CEST 2008 Daniel Veillard <veillard@redhat.com>
+
+       * src/lxc_conf.c src/lxc_conf.h: cleanup patch for the conf driver
+         of linux countainers, reuse XPath helpers, make string fields dynamic
+         and remove a memory leak.
+       * src/lxc_driver.c: avoid some problems when the config directory is
+         not accessible and for regression tests
+
 Fri Mar 28 16:34:56 EDT 2008 Daniel P. Berrange <berrange@redhat.com>
 
        * src/network.rng: Add new routed networking schema
@@ -6,8 +14,8 @@ Fri Mar 28 16:34:56 EDT 2008 Daniel P. Berrange <berrange@redhat.com>
        * src/qemu_conf.h: Add attribute for routed networking
        * src/qemu_conf.c: Parse / format new networking attributes
        * src/qemu_driver.c: Support routed networking config
-       (patches from Mads Chr. Olesen)
-       
+         (patches from Mads Chr. Olesen)
+
 Fri Mar 28 13:55:56 EDT 2008 Daniel P. Berrange <berrange@redhat.com>
 
        * src/storage_conf.c: Fix XML output tag for FS storage pools

diff --git a/src/lxc_conf.c b/src/lxc_conf.c
index 4040bd12e6e3dc67667f10e178ab4272cadf91de..c816863763efe70330653e471c13f72672a09902 100644 (file)
--- a/src/lxc_conf.c
+++ b/src/lxc_conf.c
@@ -41,6 +41,7 @@
 #include "buf.h"
 #include "util.h"
 #include "uuid.h"
+#include "xml.h"
 
 #include "lxc_conf.h"
 
@@ -70,18 +71,6 @@ void lxcError(virConnectPtr conn, virDomainPtr dom, int code,
                     codeErrorMessage, errorMessage);
 }
 
-static inline int lxcIsEmptyXPathStringObj(xmlXPathObjectPtr xpathObj)
-{
-    if ((xpathObj == NULL) ||
-        (xpathObj->type != XPATH_STRING) ||
-        (xpathObj->stringval == NULL) ||
-        (xpathObj->stringval[0] == 0)) {
-        return 1;
-    } else {
-        return 0;
-    }
-}
-
 static int lxcParseMountXML(virConnectPtr conn, xmlNodePtr nodePtr,
                             lxc_mount_t *lxcMount)
 {
@@ -92,7 +81,8 @@ static int lxcParseMountXML(virConnectPtr conn, xmlNodePtr nodePtr,
     int strLen;
     int rc = -1;
 
-    if (NULL == (fsType = xmlGetProp(nodePtr, BAD_CAST "type"))) {
+    fsType = xmlGetProp(nodePtr, BAD_CAST "type");
+    if (NULL == fsType) {
         lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR,
                  _("missing filesystem type"));
         goto error;
@@ -155,6 +145,7 @@ static int lxcParseMountXML(virConnectPtr conn, xmlNodePtr nodePtr,
     rc = 0;
 
 error:
+    xmlFree(fsType);
     xmlFree(mountSource);
     xmlFree(mountTarget);
 
@@ -164,55 +155,40 @@ error:
 static int lxcParseDomainName(virConnectPtr conn, char **name,
                               xmlXPathContextPtr contextPtr)
 {
-    int rc = -1;
-    xmlXPathObjectPtr xpathObj = NULL;
+    char *res;
 
-    xpathObj = xmlXPathEval(BAD_CAST "string(/domain/name[1])", contextPtr);
-    if (lxcIsEmptyXPathStringObj(xpathObj)) {
+    res = virXPathString("string(/domain/name[1])", contextPtr);
+    if (res == NULL) {
         lxcError(conn, NULL, VIR_ERR_NO_NAME, NULL);
-        goto parse_complete;
+        return(-1);
     }
 
-    *name = strdup((const char *)xpathObj->stringval);
-    if (NULL == *name) {
-        lxcError(conn, NULL, VIR_ERR_NO_MEMORY, NULL);
-        goto parse_complete;
-    }
-
-
-    rc = 0;
-
-parse_complete:
-    xmlXPathFreeObject(xpathObj);
-    return rc;
+    *name = res;
+    return(0);
 }
 
 static int lxcParseDomainUUID(virConnectPtr conn, unsigned char *uuid,
                               xmlXPathContextPtr contextPtr)
 {
-    int rc = -1;
-    xmlXPathObjectPtr xpathObj = NULL;
+    char *res;
 
-    xpathObj = xmlXPathEval(BAD_CAST "string(/domain/uuid[1])", contextPtr);
-    if (lxcIsEmptyXPathStringObj(xpathObj)) {
-        if ((rc = virUUIDGenerate(uuid))) {
+    res = virXPathString("string(/domain/uuid[1])", contextPtr);
+    if (res == NULL) {
+        if (virUUIDGenerate(uuid)) {
             lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR,
-                     _("failed to generate uuid: %s"), strerror(rc));
-            goto parse_complete;
+                     _("failed to generate uuid"));
+            return(-1);
         }
     } else {
-        if (virUUIDParse((const char *)xpathObj->stringval, uuid) < 0) {
+        if (virUUIDParse(res, uuid) < 0) {
             lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR,
                      _("invalid uuid element"));
-            goto parse_complete;
+           free(res);
+            return(-1);
         }
+       free(res);
     }
-
-    rc = 0;
-
-parse_complete:
-    xmlXPathFreeObject(xpathObj);
-    return rc;
+    return(0);
 }
 
 static int lxcParseDomainMounts(virConnectPtr conn,
@@ -220,27 +196,23 @@ static int lxcParseDomainMounts(virConnectPtr conn,
                                 xmlXPathContextPtr contextPtr)
 {
     int rc = -1;
-    xmlXPathObjectPtr xpathObj = NULL;
     int i;
     lxc_mount_t *mountObj;
     lxc_mount_t *prevObj = NULL;
     int nmounts = 0;
+    xmlNodePtr *list;
+    int res;
 
-    xpathObj = xmlXPathEval(BAD_CAST "/domain/devices/filesystem",
-                            contextPtr);
-    if ((xpathObj != NULL) &&
-        (xpathObj->type == XPATH_NODESET) &&
-        (xpathObj->nodesetval != NULL) &&
-        (xpathObj->nodesetval->nodeNr >= 0)) {
-        for (i = 0; i < xpathObj->nodesetval->nodeNr; ++i) {
+    res = virXPathNodeSet("/domain/devices/filesystem", contextPtr, &list);
+    if (res > 0) {
+        for (i = 0; i < res; ++i) {
             mountObj = calloc(1, sizeof(lxc_mount_t));
             if (NULL == mountObj) {
                 lxcError(conn, NULL, VIR_ERR_NO_MEMORY, "mount");
                 goto parse_complete;
             }
 
-            rc = lxcParseMountXML(conn, xpathObj->nodesetval->nodeTab[i],
-                                  mountObj);
+            rc = lxcParseMountXML(conn, list[i], mountObj);
             if (0 > rc) {
                 free(mountObj);
                 goto parse_complete;
@@ -256,101 +228,76 @@ static int lxcParseDomainMounts(virConnectPtr conn,
             }
             prevObj = mountObj;
         }
+       free(list);
     }
 
     rc = nmounts;
 
 parse_complete:
-    xmlXPathFreeObject(xpathObj);
-
     return rc;
 }
 
-static int lxcParseDomainInit(virConnectPtr conn, char* init, xmlXPathContextPtr contextPtr)
+static int lxcParseDomainInit(virConnectPtr conn, char** init,
+                              xmlXPathContextPtr contextPtr)
 {
-    int rc = -1;
-    xmlXPathObjectPtr xpathObj = NULL;
+    char *res;
 
-    xpathObj = xmlXPathEval(BAD_CAST "string(/domain/os/init[1])",
-                           contextPtr);
-    if (lxcIsEmptyXPathStringObj(xpathObj)) {
+    res = virXPathString("string(/domain/os/init[1])", contextPtr);
+    if (res == NULL) {
         lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR,
                  _("invalid or missing init element"));
-        goto parse_complete;
+        return(-1);
     }
 
-    if (strlen((const char*)xpathObj->stringval) >= PATH_MAX - 1) {
+    if (strlen(res) >= PATH_MAX - 1) {
         lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR,
                  _("init string too long"));
-        goto parse_complete;
+        free(res);
+       return(-1);
     }
 
-    strcpy(init, (const char *)xpathObj->stringval);
-
-    rc = 0;
+    *init = res;
 
-parse_complete:
-    xmlXPathFreeObject(xpathObj);
-
-    return rc;
+    return(0);
 }
 
 
-static int lxcParseDomainTty(virConnectPtr conn, char *tty, xmlXPathContextPtr contextPtr)
+static int lxcParseDomainTty(virConnectPtr conn, char **tty, xmlXPathContextPtr contextPtr)
 {
-    int rc = -1;
-    xmlXPathObjectPtr xpathObj = NULL;
+    char *res;
 
-    xpathObj = xmlXPathEval(BAD_CAST "string(/domain/devices/console[1]/@tty)",
-                            contextPtr);
-    if (lxcIsEmptyXPathStringObj(xpathObj)) {
+    res = virXPathString("string(/domain/devices/console[1]/@tty)", contextPtr);
+    if (res == NULL) {
         /* make sure the tty string is empty */
-        tty[0] = 0x00;
+        *tty = strdup("");
+       if (*tty == NULL) {
+           lxcError(conn, NULL, VIR_ERR_NO_MEMORY, NULL);
+           return(-1);
+       }
     } else {
-        /* check the source string length */
-        if (strlen((const char*)xpathObj->stringval) >= LXC_MAX_TTY_NAME - 1) {
-            lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR,
-                     _("tty name is too long"));
-            goto parse_complete;
-        }
-        strcpy(tty, (const char *)xpathObj->stringval);
+        *tty = res;
     }
 
-    rc = 0;
-
-parse_complete:
-    xmlXPathFreeObject(xpathObj);
-
-    return rc;
+    return(0);
 }
 
 static int lxcParseDomainMemory(virConnectPtr conn, int* memory, xmlXPathContextPtr contextPtr)
 {
-    int rc = -1;
-    xmlXPathObjectPtr xpathObj = NULL;
-    char *endChar = NULL;
-
-    xpathObj = xmlXPathEval(BAD_CAST "string(/domain/memory[1])", contextPtr);
-    if (lxcIsEmptyXPathStringObj(xpathObj)) {
+    long res;
+    int rc;
+
+    rc = virXPathLong("string(/domain/memory[1])", contextPtr, &res);
+    if ((rc == -2) || ((rc == 0) && (res <= 0))) {
+       *memory = -1;
+       lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR,
+                _("invalid memory value"));
+    } else if (rc < 0) {
         /* not an error, default to an invalid value so it's not used */
-        *memory = -1;
+       *memory = -1;
     } else {
-        *memory = strtoll((const char*)xpathObj->stringval,
-                                          &endChar, 10);
-        if ((endChar == (const char*)xpathObj->stringval) ||
-           (*endChar != '\0')) {
-            lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR,
-                     _("invalid memory value"));
-            goto parse_complete;
-        }
+       *memory = (int) res;
     }
-
-    rc = 0;
-
-parse_complete:
-    xmlXPathFreeObject(xpathObj);
-
-    return rc;
+    return(0);
 }
 
 static lxc_vm_def_t * lxcParseXML(virConnectPtr conn, xmlDocPtr docPtr)
@@ -411,7 +358,7 @@ static lxc_vm_def_t * lxcParseXML(virConnectPtr conn, xmlDocPtr docPtr)
         goto error;
     }
 
-    if (lxcParseDomainInit(conn, containerDef->init, contextPtr) < 0) {
+    if (lxcParseDomainInit(conn, &(containerDef->init), contextPtr) < 0) {
         goto error;
     }
 
@@ -425,7 +372,7 @@ static lxc_vm_def_t * lxcParseXML(virConnectPtr conn, xmlDocPtr docPtr)
         goto error;
     }
 
-    if (lxcParseDomainTty(conn, containerDef->tty, contextPtr) < 0) {
+    if (lxcParseDomainTty(conn, &(containerDef->tty), contextPtr) < 0) {
         goto error;
     }
 
@@ -708,7 +655,8 @@ int lxcLoadContainerInfo(lxc_driver_t *driver)
             rc = 0;
         } else {
             lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR,
-                     _("failed to open config directory: %s"), strerror(errno));
+                     _("failed to open config directory %s: %s"),
+                    driver->configDir, strerror(errno));
         }
 
         goto load_complete;
@@ -837,9 +785,13 @@ no_memory:
 
 void lxcFreeVMDef(lxc_vm_def_t *vmdef)
 {
-    lxc_mount_t *curMount = vmdef->mounts;
+    lxc_mount_t *curMount;
     lxc_mount_t *nextMount;
 
+    if (vmdef == NULL)
+        return;
+
+    curMount = vmdef->mounts;
     while (curMount) {
         nextMount = curMount->next;
         free(curMount);
@@ -847,8 +799,9 @@ void lxcFreeVMDef(lxc_vm_def_t *vmdef)
     }
 
     free(vmdef->name);
-    vmdef->name = NULL;
-
+    free(vmdef->init);
+    free(vmdef->tty);
+    free(vmdef);
 }
 
 void lxcFreeVMs(lxc_vm_t *vms)
@@ -859,7 +812,6 @@ void lxcFreeVMs(lxc_vm_t *vms)
     while (curVm) {
         lxcFreeVM(curVm);
         nextVm = curVm->next;
-        free(curVm);
         curVm = nextVm;
     }
 }
@@ -867,7 +819,7 @@ void lxcFreeVMs(lxc_vm_t *vms)
 void lxcFreeVM(lxc_vm_t *vm)
 {
     lxcFreeVMDef(vm->def);
-    free(vm->def);
+    free(vm);
 }
 
 lxc_vm_t *lxcFindVMByID(const lxc_driver_t *driver, int id)

diff --git a/src/lxc_conf.h b/src/lxc_conf.h
index 450093bf1a1ea362130b6f0f23401bbb11ac97d1..a0f1737b84f6a359b1cb559d43fcec40db86e05d 100644 (file)
--- a/src/lxc_conf.h
+++ b/src/lxc_conf.h
@@ -51,7 +51,7 @@ struct __lxc_vm_def {
     int id;
 
     /* init command string */
-    char init[PATH_MAX];
+    char *init;
 
     int maxMemory;
 
@@ -60,7 +60,7 @@ struct __lxc_vm_def {
     lxc_mount_t *mounts;
 
     /* tty device */
-    char tty[LXC_MAX_TTY_NAME];
+    char *tty;
 };
 
 typedef struct __lxc_vm lxc_vm_t;

diff --git a/src/lxc_driver.c b/src/lxc_driver.c
index c88fc2f0fb70503399efc983ff0e5bd72ec4d92a..a9afae6a5b08a6c4e1887e52258117c30a82cbaa 100644 (file)
--- a/src/lxc_driver.c
+++ b/src/lxc_driver.c
@@ -61,7 +61,7 @@
 
 static int lxcStartup(void);
 static int lxcShutdown(void);
-static lxc_driver_t *lxc_driver;
+static lxc_driver_t *lxc_driver = NULL;
 
 /* Functions */
 static int lxcDummyChild( void *argv ATTRIBUTE_UNUSED )
@@ -378,6 +378,13 @@ static char *lxcDomainDumpXML(virDomainPtr dom,
 
 static int lxcStartup(void)
 {
+    uid_t uid = getuid();
+
+    /* Check that the user is root */
+    if (0 != uid) {
+        return -1;
+    }
+
     lxc_driver = calloc(1, sizeof(lxc_driver_t));
     if (NULL == lxc_driver) {
         return -1;
@@ -412,11 +419,12 @@ static void lxcFreeDriver(lxc_driver_t *driver)
 
 static int lxcShutdown(void)
 {
-    lxc_vm_t *vms = lxc_driver->vms;
-
-    lxcFreeVMs(vms);
+    if (lxc_driver == NULL)
+        return(NULL);
+    lxcFreeVMs(lxc_driver->vms);
     lxc_driver->vms = NULL;
     lxcFreeDriver(lxc_driver);
+    lxc_driver = NULL;
 
     return 0;
 }
@@ -430,6 +438,8 @@ static int lxcShutdown(void)
  */
 static int
 lxcActive(void) {
+    if (lxc_driver == NULL)
+        return(0);
     /* If we've any active networks or guests, then we
      * mark this driver as active
      */