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

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

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;
+    char *res;
-    xmlXPathObjectPtr xpathObj = NULL;
 
-    xpathObj = xmlXPathEval(BAD_CAST "string(/domain/name[1])", contextPtr);
+    res = virXPathString("string(/domain/name[1])", contextPtr);
-    if (lxcIsEmptyXPathStringObj(xpathObj)) {
+    if (res == NULL) {
         lxcError(conn, NULL, VIR_ERR_NO_NAME, NULL);
-        goto parse_complete;
+        return(-1);
     }
 
-    *name = strdup((const char *)xpathObj->stringval);
+    *name = res;
-    if (NULL == *name) {
+    return(0);
-        lxcError(conn, NULL, VIR_ERR_NO_MEMORY, NULL);
-        goto parse_complete;
-    }
-
-
-    rc = 0;
-
-parse_complete:
-    xmlXPathFreeObject(xpathObj);
-    return rc;
 }
 
 static int lxcParseDomainUUID(virConnectPtr conn, unsigned char *uuid,
                               xmlXPathContextPtr contextPtr)
 {
-    int rc = -1;
+    char *res;
-    xmlXPathObjectPtr xpathObj = NULL;
 
-    xpathObj = xmlXPathEval(BAD_CAST "string(/domain/uuid[1])", contextPtr);
+    res = virXPathString("string(/domain/uuid[1])", contextPtr);
-    if (lxcIsEmptyXPathStringObj(xpathObj)) {
+    if (res == NULL) {
-        if ((rc = virUUIDGenerate(uuid))) {
+        if (virUUIDGenerate(uuid)) {
             lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR,
-                     _("failed to generate uuid: %s"), strerror(rc));
+                     _("failed to generate uuid"));
-            goto parse_complete;
+            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);
     }
-
+    return(0);
-    rc = 0;
-
-parse_complete:
-    xmlXPathFreeObject(xpathObj);
-    return rc;
 }
 
 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",
+    res = virXPathNodeSet("/domain/devices/filesystem", contextPtr, &list);
-                            contextPtr);
+    if (res > 0) {
-    if ((xpathObj != NULL) &&
+        for (i = 0; i < res; ++i) {
-        (xpathObj->type == XPATH_NODESET) &&
-        (xpathObj->nodesetval != NULL) &&
-        (xpathObj->nodesetval->nodeNr >= 0)) {
-        for (i = 0; i < xpathObj->nodesetval->nodeNr; ++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],
+            rc = lxcParseMountXML(conn, list[i], mountObj);
-                                  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;
+    char *res;
-    xmlXPathObjectPtr xpathObj = NULL;
 
-    xpathObj = xmlXPathEval(BAD_CAST "string(/domain/os/init[1])",
+    res = virXPathString("string(/domain/os/init[1])", contextPtr);
-                           contextPtr);
+    if (res == NULL) {
-    if (lxcIsEmptyXPathStringObj(xpathObj)) {
         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);
+    *init = res;
 
-    rc = 0;
+    return(0);
-
-parse_complete:
-    xmlXPathFreeObject(xpathObj);
-
-    return rc;
 }
 
 
-static int lxcParseDomainTty(virConnectPtr conn, char *tty, xmlXPathContextPtr contextPtr)
+static int lxcParseDomainTty(virConnectPtr conn, char **tty, xmlXPathContextPtr contextPtr)
 {
-    int rc = -1;
+    char *res;
-    xmlXPathObjectPtr xpathObj = NULL;
 
-    xpathObj = xmlXPathEval(BAD_CAST "string(/domain/devices/console[1]/@tty)",
+    res = virXPathString("string(/domain/devices/console[1]/@tty)", contextPtr);
-                            contextPtr);
+    if (res == NULL) {
-    if (lxcIsEmptyXPathStringObj(xpathObj)) {
         /* 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 */
+        *tty = res;
-        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);
     }
 
-    rc = 0;
+    return(0);
-
-parse_complete:
-    xmlXPathFreeObject(xpathObj);
-
-    return rc;
 }
 
 static int lxcParseDomainMemory(virConnectPtr conn, int* memory, xmlXPathContextPtr contextPtr)
 {
-    int rc = -1;
+    long res;
-    xmlXPathObjectPtr xpathObj = NULL;
+    int rc;
-    char *endChar = NULL;
 
-    xpathObj = xmlXPathEval(BAD_CAST "string(/domain/memory[1])", contextPtr);
+    rc = virXPathLong("string(/domain/memory[1])", contextPtr, &res);
-    if (lxcIsEmptyXPathStringObj(xpathObj)) {
+    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;
     } else {
-        *memory = strtoll((const char*)xpathObj->stringval,
+	*memory = (int) res;
-                                          &endChar, 10);
-        if ((endChar == (const char*)xpathObj->stringval) ||
-           (*endChar != '\0')) {
-            lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR,
-                     _("invalid memory value"));
-            goto parse_complete;
     }
-    }
+    return(0);
-
-    rc = 0;
-
-parse_complete:
-    xmlXPathFreeObject(xpathObj);
-
-    return rc;
 }
 
 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)

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;

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;
+    if (lxc_driver == NULL)
-
+        return(NULL);
-    lxcFreeVMs(vms);
+    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
      */