Okay, that's something i promised last week, but it has some significant
changes:
- cleanup the XPath methods to access the XML informations, reusing the
existing methods from xml.h
- make string fields from __lxc_vm_def dynamic (except the UUID)
- fix what looked like a funny leak situation when vm def structures were
deallocated (see changes around lxcFreeVM/lxcFreeVMs/lxcFreeVMDef),
i hope i didn't overlooked something but valgrind seems happy now leak
wise.
Overall it looks a bit simpler to me :-)
Valgrind only report left is
==1== Invalid write of size 4
==1== at 0x470C8C: lxcCheckContainerSupport (lxc_driver.c:91)
==1== Address 0xFFFFFFFFFFFFFFEC is not stack'd, malloc'd or (recently) free'd
==1==
==1== Process terminating with default action of signal 11 (SIGSEGV)
==1== Access not within mapped region at address 0xFFFFFFFFFFFFFFEC
==1== at 0x470C8C: lxcCheckContainerSupport (lxc_driver.c:91)
which is related to the probe code of the driver
---------------------------
stack = malloc(getpagesize() * 4);
if(!stack) {
DEBUG0("Unable to allocate stack");
rc = -1;
goto check_complete;
}
childStack = stack + (getpagesize() * 4);
cpid = clone(lxcDummyChild, childStack, flags, NULL);
---------------------------
I would assume it's valgrind being a bit pedantic because we pass the address
just over the allocated stack limit, which should be fine since stack is
addressed in predecrementing mode (BTW isn't that a portability bug in the
code stack direction should probably be checked no ?). Still maybe it's a
good idea to pass a pointer in the allocated area ...
Daniel
--
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard | virtualization library http://libvirt.org/
veillard@xxxxxxxxxx | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
Index: src/lxc_conf.c
===================================================================
RCS file: /data/cvs/libxen/src/lxc_conf.c,v
retrieving revision 1.3
diff -p -r1.3 lxc_conf.c
*** src/lxc_conf.c 27 Mar 2008 14:02:57 -0000 1.3
--- src/lxc_conf.c 28 Mar 2008 13:29:58 -0000
***************
*** 41,46 ****
--- 41,47 ----
#include "buf.h"
#include "util.h"
#include "uuid.h"
+ #include "xml.h"
#include "lxc_conf.h"
*************** void lxcError(virConnectPtr conn, virDom
*** 70,87 ****
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)
{
--- 71,76 ----
*************** static int lxcParseMountXML(virConnectPt
*** 92,98 ****
int strLen;
int rc = -1;
! if (NULL == (fsType = xmlGetProp(nodePtr, BAD_CAST "type"))) {
lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR,
_("missing filesystem type"));
goto error;
--- 81,88 ----
int strLen;
int rc = -1;
! fsType = xmlGetProp(nodePtr, BAD_CAST "type");
! if (NULL == fsType) {
lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR,
_("missing filesystem type"));
goto error;
*************** static int lxcParseMountXML(virConnectPt
*** 155,160 ****
--- 145,151 ----
rc = 0;
error:
+ xmlFree(fsType);
xmlFree(mountSource);
xmlFree(mountTarget);
*************** error:
*** 164,218 ****
static int lxcParseDomainName(virConnectPtr conn, char **name,
xmlXPathContextPtr contextPtr)
{
! int rc = -1;
! xmlXPathObjectPtr xpathObj = NULL;
! xpathObj = xmlXPathEval(BAD_CAST "string(/domain/name[1])", contextPtr);
! if (lxcIsEmptyXPathStringObj(xpathObj)) {
lxcError(conn, NULL, VIR_ERR_NO_NAME, NULL);
! goto parse_complete;
}
! *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;
}
static int lxcParseDomainUUID(virConnectPtr conn, unsigned char *uuid,
xmlXPathContextPtr contextPtr)
{
! int rc = -1;
! xmlXPathObjectPtr xpathObj = NULL;
! xpathObj = xmlXPathEval(BAD_CAST "string(/domain/uuid[1])", contextPtr);
! if (lxcIsEmptyXPathStringObj(xpathObj)) {
! if ((rc = virUUIDGenerate(uuid))) {
lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR,
! _("failed to generate uuid: %s"), strerror(rc));
! goto parse_complete;
}
} else {
! if (virUUIDParse((const char *)xpathObj->stringval, uuid) < 0) {
lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR,
_("invalid uuid element"));
! goto parse_complete;
}
}
!
! rc = 0;
!
! parse_complete:
! xmlXPathFreeObject(xpathObj);
! return rc;
}
static int lxcParseDomainMounts(virConnectPtr conn,
--- 155,194 ----
static int lxcParseDomainName(virConnectPtr conn, char **name,
xmlXPathContextPtr contextPtr)
{
! char *res;
! res = virXPathString("string(/domain/name[1])", contextPtr);
! if (res == NULL) {
lxcError(conn, NULL, VIR_ERR_NO_NAME, NULL);
! return(-1);
}
! *name = res;
! return(0);
}
static int lxcParseDomainUUID(virConnectPtr conn, unsigned char *uuid,
xmlXPathContextPtr contextPtr)
{
! char *res;
! res = virXPathString("string(/domain/uuid[1])", contextPtr);
! if (res == NULL) {
! if (virUUIDGenerate(uuid)) {
lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR,
! _("failed to generate uuid"));
! return(-1);
}
} else {
! if (virUUIDParse(res, uuid) < 0) {
lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR,
_("invalid uuid element"));
! free(res);
! return(-1);
}
+ free(res);
}
! return(0);
}
static int lxcParseDomainMounts(virConnectPtr conn,
*************** static int lxcParseDomainMounts(virConne
*** 220,246 ****
xmlXPathContextPtr contextPtr)
{
int rc = -1;
- xmlXPathObjectPtr xpathObj = NULL;
int i;
lxc_mount_t *mountObj;
lxc_mount_t *prevObj = NULL;
int nmounts = 0;
! 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) {
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);
if (0 > rc) {
free(mountObj);
goto parse_complete;
--- 196,218 ----
xmlXPathContextPtr contextPtr)
{
int rc = -1;
int i;
lxc_mount_t *mountObj;
lxc_mount_t *prevObj = NULL;
int nmounts = 0;
+ xmlNodePtr *list;
+ int res;
! 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, list[i], mountObj);
if (0 > rc) {
free(mountObj);
goto parse_complete;
*************** static int lxcParseDomainMounts(virConne
*** 256,356 ****
}
prevObj = mountObj;
}
}
rc = nmounts;
parse_complete:
- xmlXPathFreeObject(xpathObj);
-
return rc;
}
! static int lxcParseDomainInit(virConnectPtr conn, char* init, xmlXPathContextPtr contextPtr)
{
! int rc = -1;
! xmlXPathObjectPtr xpathObj = NULL;
! xpathObj = xmlXPathEval(BAD_CAST "string(/domain/os/init[1])",
! contextPtr);
! if (lxcIsEmptyXPathStringObj(xpathObj)) {
lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR,
_("invalid or missing init element"));
! goto parse_complete;
}
! if (strlen((const char*)xpathObj->stringval) >= PATH_MAX - 1) {
lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR,
_("init string too long"));
! goto parse_complete;
}
! strcpy(init, (const char *)xpathObj->stringval);
!
! rc = 0;
!
! parse_complete:
! xmlXPathFreeObject(xpathObj);
! return rc;
}
! static int lxcParseDomainTty(virConnectPtr conn, char *tty, xmlXPathContextPtr contextPtr)
{
! int rc = -1;
! xmlXPathObjectPtr xpathObj = NULL;
! xpathObj = xmlXPathEval(BAD_CAST "string(/domain/devices/console[1]/@tty)",
! contextPtr);
! if (lxcIsEmptyXPathStringObj(xpathObj)) {
/* make sure the tty string is empty */
! tty[0] = 0x00;
} 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);
}
! rc = 0;
!
! parse_complete:
! xmlXPathFreeObject(xpathObj);
!
! return rc;
}
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)) {
/* not an error, default to an invalid value so it's not used */
! *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;
! }
}
!
! rc = 0;
!
! parse_complete:
! xmlXPathFreeObject(xpathObj);
!
! return rc;
}
static lxc_vm_def_t * lxcParseXML(virConnectPtr conn, xmlDocPtr docPtr)
--- 228,303 ----
}
prevObj = mountObj;
}
+ free(list);
}
rc = nmounts;
parse_complete:
return rc;
}
! static int lxcParseDomainInit(virConnectPtr conn, char** init,
! xmlXPathContextPtr contextPtr)
{
! char *res;
! res = virXPathString("string(/domain/os/init[1])", contextPtr);
! if (res == NULL) {
lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR,
_("invalid or missing init element"));
! return(-1);
}
! if (strlen(res) >= PATH_MAX - 1) {
lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR,
_("init string too long"));
! free(res);
! return(-1);
}
! *init = res;
! return(0);
}
! static int lxcParseDomainTty(virConnectPtr conn, char **tty, xmlXPathContextPtr contextPtr)
{
! char *res;
! res = virXPathString("string(/domain/devices/console[1]/@tty)", contextPtr);
! if (res == NULL) {
/* make sure the tty string is empty */
! *tty = strdup("");
! if (*tty == NULL) {
! lxcError(conn, NULL, VIR_ERR_NO_MEMORY, NULL);
! return(-1);
! }
} else {
! *tty = res;
}
! return(0);
}
static int lxcParseDomainMemory(virConnectPtr conn, int* memory, xmlXPathContextPtr contextPtr)
{
! 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;
} else {
! *memory = (int) res;
}
! return(0);
}
static lxc_vm_def_t * lxcParseXML(virConnectPtr conn, xmlDocPtr docPtr)
*************** static lxc_vm_def_t * lxcParseXML(virCon
*** 411,417 ****
goto error;
}
! if (lxcParseDomainInit(conn, containerDef->init, contextPtr) < 0) {
goto error;
}
--- 358,364 ----
goto error;
}
! if (lxcParseDomainInit(conn, &(containerDef->init), contextPtr) < 0) {
goto error;
}
*************** static lxc_vm_def_t * lxcParseXML(virCon
*** 425,431 ****
goto error;
}
! if (lxcParseDomainTty(conn, containerDef->tty, contextPtr) < 0) {
goto error;
}
--- 372,378 ----
goto error;
}
! if (lxcParseDomainTty(conn, &(containerDef->tty), contextPtr) < 0) {
goto error;
}
*************** int lxcLoadContainerInfo(lxc_driver_t *d
*** 708,714 ****
rc = 0;
} else {
lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR,
! _("failed to open config directory: %s"), strerror(errno));
}
goto load_complete;
--- 655,662 ----
rc = 0;
} else {
lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR,
! _("failed to open config directory %s: %s"),
! driver->configDir, strerror(errno));
}
goto load_complete;
*************** no_memory:
*** 837,845 ****
void lxcFreeVMDef(lxc_vm_def_t *vmdef)
{
! lxc_mount_t *curMount = vmdef->mounts;
lxc_mount_t *nextMount;
while (curMount) {
nextMount = curMount->next;
free(curMount);
--- 785,797 ----
void lxcFreeVMDef(lxc_vm_def_t *vmdef)
{
! lxc_mount_t *curMount;
lxc_mount_t *nextMount;
+ if (vmdef == NULL)
+ return;
+
+ curMount = vmdef->mounts;
while (curMount) {
nextMount = curMount->next;
free(curMount);
*************** void lxcFreeVMDef(lxc_vm_def_t *vmdef)
*** 847,854 ****
}
free(vmdef->name);
! vmdef->name = NULL;
!
}
void lxcFreeVMs(lxc_vm_t *vms)
--- 799,807 ----
}
free(vmdef->name);
! free(vmdef->init);
! free(vmdef->tty);
! free(vmdef);
}
void lxcFreeVMs(lxc_vm_t *vms)
*************** void lxcFreeVMs(lxc_vm_t *vms)
*** 859,865 ****
while (curVm) {
lxcFreeVM(curVm);
nextVm = curVm->next;
- free(curVm);
curVm = nextVm;
}
}
--- 812,817 ----
*************** void lxcFreeVMs(lxc_vm_t *vms)
*** 867,873 ****
void lxcFreeVM(lxc_vm_t *vm)
{
lxcFreeVMDef(vm->def);
! free(vm->def);
}
lxc_vm_t *lxcFindVMByID(const lxc_driver_t *driver, int id)
--- 819,825 ----
void lxcFreeVM(lxc_vm_t *vm)
{
lxcFreeVMDef(vm->def);
! free(vm);
}
lxc_vm_t *lxcFindVMByID(const lxc_driver_t *driver, int id)
Index: src/lxc_conf.h
===================================================================
RCS file: /data/cvs/libxen/src/lxc_conf.h,v
retrieving revision 1.2
diff -p -r1.2 lxc_conf.h
*** src/lxc_conf.h 27 Mar 2008 09:34:06 -0000 1.2
--- src/lxc_conf.h 28 Mar 2008 13:29:58 -0000
*************** struct __lxc_vm_def {
*** 51,57 ****
int id;
/* init command string */
! char init[PATH_MAX];
int maxMemory;
--- 51,57 ----
int id;
/* init command string */
! char *init;
int maxMemory;
*************** struct __lxc_vm_def {
*** 60,66 ****
lxc_mount_t *mounts;
/* tty device */
! char tty[LXC_MAX_TTY_NAME];
};
typedef struct __lxc_vm lxc_vm_t;
--- 60,66 ----
lxc_mount_t *mounts;
/* tty device */
! char *tty;
};
typedef struct __lxc_vm lxc_vm_t;
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list