On Mon, Sep 03, 2007 at 08:39:27AM -0400, Daniel Veillard wrote:
> On Mon, Sep 03, 2007 at 08:01:00AM -0400, Shuveb Hussain wrote:
> > Hi,
> >
> > Attached are the OpenVZ driver enhancements. Cleanups have been done
> > according to Daniel .V's suggestions. Please see my previous mail for
> > summary.
>
> Okay, that looks better, thanks a lot !
> I will apply them but there is a few things I still have issues with,
> I will try to fix them before commiting the patches:
Okay I commited to CVS earlier today
> - some really bizare changes of an xmlDoc name field:
> + if (!(xml->name = calloc(1, PATH_MAX))) {
> + openvzLog(OPENVZ_ERR, "Error in allocating memory to store xml URI");
> + xmlFreeDoc(xml);
> + return NULL;
> + }
> + if (displayName)
> + strncpy(xml->name, displayName, PATH_MAX - 1);
> + else
> + strncpy(xml->name, "domain.xml", PATH_MAX - 1);
> that's bad because:
> + it will probably leak the previous value of xml->name
> + xml->name should be allocated using xmlMalloc
> + why allocate MAX_PATH when you can allocate just the size
> so I will cleanup that part, still I don't undertsand what you
> tried to do there.
I removed that part. The URI information passed to the xmlDocRead earlier
should result in the same xml->URI being set, so I don't see the point
of that duplication, dropped for now.
> This should hit CVS later today when I cleaned up those things and possibly
> a couple of warnings,
I am also applying that second patch to clean thing up, this removes most
warnings at compile time, fix a serious deallocation problem on error when
listing the domains, and change strtoI to indicate it does not change the
input string.
There is still 3 serious warnings in openvz_conf.c around line 700, where
the return value of write is not checked and as I indicated previously we
should use a wrapper function handling interrupted calls at least. But that
can be done later,
thanks,
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/openvz_conf.c
===================================================================
RCS file: /data/cvs/libxen/src/openvz_conf.c,v
retrieving revision 1.5
diff -u -r1.5 openvz_conf.c
--- src/openvz_conf.c 3 Sep 2007 15:37:07 -0000 1.5
+++ src/openvz_conf.c 3 Sep 2007 16:18:08 -0000
@@ -59,9 +59,6 @@
static struct openvz_vm_def *openvzParseXML(virConnectPtr conn, xmlDocPtr xml);
static int openvzGetVPSUUID(int vpsid, char *uuidstr);
static int openvzSetUUID(int vpsid);
-static struct openvz_vm *openvzLoadConfig(struct openvz_driver *driver,
- const char *path,
- const char *xmlStr);
/* For errors internal to this library. */
static void
@@ -117,7 +114,7 @@
}
int
-strtoI(char *str)
+strtoI(const char *str)
{
int base = 10;
char *endptr;
@@ -340,7 +337,7 @@
}
/* rejecting VPS ID <= OPENVZ_RSRV_VM_LIMIT for they are reserved */
- if (strtoI(BAD_CAST obj->stringval) <= OPENVZ_RSRV_VM_LIMIT) {
+ if (strtoI((const char *) obj->stringval) <= OPENVZ_RSRV_VM_LIMIT) {
error(conn, VIR_ERR_INTERNAL_ERROR,
"VPS ID Error (must be an integer greater than 100");
goto bail_out;
@@ -531,13 +528,18 @@
*pnext = calloc(1, sizeof(struct openvz_vm));
if(!*pnext) {
error(conn, VIR_ERR_INTERNAL_ERROR, "calloc failed");
- return NULL;
+ goto error;
}
if(!vm)
vm = *pnext;
- fscanf(fp, "%d %s\n", &veid, status);
+ if (fscanf(fp, "%d %s\n", &veid, status) != 2) {
+ error(conn, VIR_ERR_INTERNAL_ERROR,
+ "Failed to parse vzlist output");
+ free(*pnext);
+ goto error;
+ }
if(strcmp(status, "stopped")) {
(*pnext)->status = VIR_DOMAIN_RUNNING;
driver->num_active ++;
@@ -546,14 +548,18 @@
else {
(*pnext)->status = VIR_DOMAIN_SHUTOFF;
driver->num_inactive ++;
- (*pnext)->vpsid = -1; /* inactive domains don't have their ID set in libvirt,
- thought this doesn't make sense for OpenVZ */
+ /*
+ * inactive domains don't have their ID set in libvirt,
+ * thought this doesn't make sense for OpenVZ
+ */
+ (*pnext)->vpsid = -1;
}
vmdef = calloc(1, sizeof(struct openvz_vm_def));
if(!vmdef) {
error(conn, VIR_ERR_INTERNAL_ERROR, "calloc failed");
- return NULL;
+ free(*pnext);
+ goto error;
}
snprintf(vmdef->name, OPENVZ_NAME_MAX, "%i", veid);
@@ -561,14 +567,27 @@
ret = virUUIDParse(uuidstr, vmdef->uuid);
if(ret == -1) {
- error(conn, VIR_ERR_INTERNAL_ERROR, "UUID in config file malformed");
- return NULL;
+ error(conn, VIR_ERR_INTERNAL_ERROR,
+ "UUID in config file malformed");
+ free(*pnext);
+ free(vmdef);
+ goto error;
}
(*pnext)->vmdef = vmdef;
pnext = &(*pnext)->next;
}
return vm;
+error:
+ while (vm != NULL) {
+ struct openvz_vm *next;
+
+ next = vm->next;
+ free(vm->vmdef);
+ free(vm);
+ vm = next;
+ }
+ return NULL;
}
static char
@@ -579,7 +598,7 @@
while(conf_dir_list[i]) {
if(!access(conf_dir_list[i], F_OK))
- return strdup(conf_dir_list[i]);
+ return strdup(conf_dir_list[i]);
i ++;
}
@@ -660,7 +679,7 @@
char uuidstr[VIR_UUID_STRING_BUFLEN];
unsigned char uuid[VIR_UUID_BUFLEN];
char *conf_dir;
- int fd, ret, i;
+ int fd, ret;
conf_dir = openvzLocateConfDir();
sprintf(conf_file, "%s/%d.conf", conf_dir, vpsid);
Index: src/openvz_conf.h
===================================================================
RCS file: /data/cvs/libxen/src/openvz_conf.h,v
retrieving revision 1.4
diff -u -r1.4 openvz_conf.h
--- src/openvz_conf.h 3 Sep 2007 15:37:07 -0000 1.4
+++ src/openvz_conf.h 3 Sep 2007 16:18:08 -0000
@@ -129,5 +129,5 @@
void openvzFreeDriver(struct openvz_driver *driver);
void openvzFreeVM(struct openvz_driver *driver, struct openvz_vm *vm, int checkCallee);
void openvzFreeVMDef(struct openvz_vm_def *def);
-int strtoI(char *str);
+int strtoI(const char *str);
#endif /* OPENVZ_CONF_H */
Index: src/openvz_driver.c
===================================================================
RCS file: /data/cvs/libxen/src/openvz_driver.c,v
retrieving revision 1.5
diff -u -r1.5 openvz_driver.c
--- src/openvz_driver.c 3 Sep 2007 15:37:07 -0000 1.5
+++ src/openvz_driver.c 3 Sep 2007 16:18:08 -0000
@@ -164,7 +164,7 @@
return dom;
}
-static char *openvzGetOSType(virDomainPtr dom)
+static char *openvzGetOSType(virDomainPtr dom ATTRIBUTE_UNUSED)
{
/* OpenVZ runs on Linux and runs only Linux */
return strdup("linux");
@@ -275,7 +275,8 @@
return ret;
}
-static int openvzDomainReboot(virDomainPtr dom, unsigned int flags) {
+static int openvzDomainReboot(virDomainPtr dom,
+ unsigned int flags ATTRIBUTE_UNUSED) {
char cmdbuf[CMDBUF_LEN];
int ret;
char *cmdExec[OPENVZ_MAX_ARG];
@@ -631,7 +632,7 @@
return got;
}
-static int openvzNumDomains(virConnectPtr conn) {
+static int openvzNumDomains(virConnectPtr conn ATTRIBUTE_UNUSED) {
return ovz_driver.num_active;
}
@@ -662,7 +663,7 @@
return got;
}
-static int openvzNumDefinedDomains(virConnectPtr conn) {
+static int openvzNumDefinedDomains(virConnectPtr conn ATTRIBUTE_UNUSED) {
return ovz_driver.num_inactive;
}
@@ -687,11 +688,11 @@
return 1;
}
-static int openvzCloseNetwork(virConnectPtr conn) {
+static int openvzCloseNetwork(virConnectPtr conn ATTRIBUTE_UNUSED) {
return 0;
}
-static virDrvOpenStatus openvzOpenNetwork(virConnectPtr conn,
+static virDrvOpenStatus openvzOpenNetwork(virConnectPtr conn ATTRIBUTE_UNUSED,
const char *name ATTRIBUTE_UNUSED,
int flags ATTRIBUTE_UNUSED) {
return VIR_DRV_OPEN_SUCCESS;
@@ -747,6 +748,11 @@
NULL, /* domainGetSchedulerType */
NULL, /* domainGetSchedulerParameters */
NULL, /* domainSetSchedulerParameters */
+ NULL, /* domainMigratePrepare */
+ NULL, /* domainMigratePerform */
+ NULL, /* domainMigrateFinish */
+ NULL, /* domainBlockStats */
+ NULL, /* domainInterfaceStats */
};
static virNetworkDriver openvzNetworkDriver = {
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list