5:31 p.m.
2010/3/5 Sharadha Prabhakar (3P) <sharadha.prabhakar(a)citrix.com>:
Patch includes
1) Modification of xenapiDomainGetOSType
1) global url and SSL flags removed and placed in private driver struct.
2) SSL verify on url parsed using function similar to the one in ESX.
3) mapDomainPinVcpu updated in xenapi_utils.c
diff -Nur ./libvirt_org/src/xenapi/xenapi_driver.c
./libvirt/src/xenapi/xenapi_driver.c
--- ./libvirt_org/src/xenapi/xenapi_driver.c 1970-01-01 01:00:00.000000000 +0100
+++ ./libvirt/src/xenapi/xenapi_driver.c 2010-03-04 16:59:01.000000000 +0000
@@ -0,0 +1,1738 @@
+
+/*
+ * xenapi_driver.c: Xen API driver.
+ * Copyright (C) 2009 Citrix Ltd.
+ * Sharadha Prabhakar <sharadha.prabhakar(a)citrix.com>
+*/
A license statement is missing. Using LGPL like the rest of libvirt
would be a good choice.
+
+/*
+*getCapsObject
+*
+*Build the capabilities of the hypervisor
+*Return virCapsPtr on success or NULL on failure
+*/
+static virCapsPtr
+getCapsObject (void)
+{
+ virCapsPtr caps = virCapabilitiesNew("x86_64", 0, 0);
You're ignoring the actual host architecture here and assume x86_64
even if the host is just x86.
+ if (!caps) {
+ virReportOOMError();
+ return NULL;
+ }
+ virCapsGuestPtr guest1 = virCapabilitiesAddGuest(caps, "hvm",
"x86_64", 0, "", "", 0, NULL);
+ if (!guest1)
+ goto error_cleanup;
+ virCapsGuestDomainPtr domain1 = virCapabilitiesAddGuestDomain(guest1,
"xen", "", "", 0, NULL);
+ if (!domain1)
+ goto error_cleanup;
+ virCapsGuestPtr guest2 = virCapabilitiesAddGuest(caps, "xen",
"x86_64", 0, "", "", 0, NULL);
+ if (!guest2)
+ goto error_cleanup;
+ virCapsGuestDomainPtr domain2 = virCapabilitiesAddGuestDomain(guest2,
"xen", "", "", 0, NULL);
+ if (!domain2)
+ goto error_cleanup;
+
+ return caps;
+
+ error_cleanup:
+ virCapabilitiesFree(caps);
+ return NULL;
+}
+
+/*
+*XenapiOpen
+*
+*Authenticates and creates a session with the server
+*Return VIR_DRV_OPEN_SUCCESS on success, else VIR_DRV_OPEN_ERROR
+*/
+static virDrvOpenStatus
+xenapiOpen (virConnectPtr conn, virConnectAuthPtr auth , int flags ATTRIBUTE_UNUSED)
+{
+ char *passwd;
Initialize passwd to NULL.
+ xen_session *session;
+ struct _xenapiPrivate *privP;
+ int noVerify=0;
+
+ if (!STRCASEEQ(conn->uri->scheme,"XenAPI")) {
+ xenapiSessionErrorHandler(conn, VIR_ERR_AUTH_FAILED ,"Check URI format:
'XenAPI://user@server'");
The given URI is not for you, this is no error, so don't report an
error, just return VIR_DRV_OPEN_DECLINED.
+ return VIR_DRV_OPEN_DECLINED;
+ }
+ if (conn->uri->server==NULL) {
+ xenapiSessionErrorHandler(conn, VIR_ERR_AUTH_FAILED ,"Server name not in
URI");
+ return VIR_DRV_OPEN_ERROR;
+ }
+ if (auth) {
+ passwd =
xenapiUtil_RequestPassword(auth,conn->uri->user,conn->uri->server);
+ } else {
+ xenapiSessionErrorHandler(conn, VIR_ERR_AUTH_FAILED ,"Authentication
Credentials not found");
+ return VIR_DRV_OPEN_ERROR;
+ }
+ if (!passwd && !conn->uri->user) {
You should use || instead of &&, otherwise you won't catch cases where
passwd is NULL but conn->uri->user isn't and via versa.
+ xenapiSessionErrorHandler(conn, VIR_ERR_AUTH_FAILED
,"Username/Password not valid");
You may leak passwd here. Free is using VIR_FREE(passwd).
+ return VIR_DRV_OPEN_ERROR;
+ }
+ if (VIR_ALLOC(privP) < 0) {
+ virReportOOMError();
You leak passwd here too.
+ return VIR_DRV_OPEN_ERROR;
+ }
+ if (virAsprintf(&(privP->url),"https://%s",conn->uri->server)
< 0) {
+ virReportOOMError();
You may leak passwd here too.
+ return VIR_DRV_OPEN_ERROR;
+ }
+ privP->SSLflag=2;
+ xenapiUtil_ParseQuery(conn, conn->uri,&noVerify);
+ if (noVerify==1) privP->SSLflag=0;
+ xmlInitParser();
+ xmlKeepBlanksDefault(0);
+ xen_init();
+ curl_global_init(CURL_GLOBAL_ALL);
+
+ session = xen_session_login_with_password (call_func, (void *)privP,
conn->uri->user, "xenroot", xen_api_latest_version);
No need to cast privP to void *.
Shouldn't the parameter after the username be passwd instead of
hardcoding "xenroot"?
+
+ if ( session && session->ok ) {
+ privP->session = session;
+ virCapsPtr caps = getCapsObject();
+ if (caps)
+ privP->caps = caps;
Why not directly assign caps:
privP->caps = getCapsObject();
getCapsObject() may fail you need to check the return value for NULL,
and report an error.
+ conn->privateData = privP;
You leak passwd here too.
+ return VIR_DRV_OPEN_SUCCESS;
+ } else {
+ xenapiSessionErrorHandler(conn, VIR_ERR_AUTH_FAILED ,"");
+ VIR_FREE(privP);
You may also get here if session is not NULL but session->ok is false.
I assume session is allocated, so you need to free it here by some
free function, maybe xen_session_logout does this.
You leak passwd here too.
+ return VIR_DRV_OPEN_ERROR;
+ }
+}
+
+
+
+/*
+* xenapiGetVersion:
+*
+* Gets the version of XenAPI
+*
+*/
+static int
+xenapiGetVersion (virConnectPtr conn, unsigned long *hvVer)
+{
+ xen_host host;
+ xen_session *session = ((struct _xenapiPrivate
*)(conn->privateData))->session;
+ xen_string_string_map *result=NULL;
+ if (!(xen_session_get_this_host(session, &host,session))) {
+ xenapiSessionErrorHandler(conn, VIR_ERR_INTERNAL_ERROR ,NULL);
+ return -1;
+ }
+ if (!(xen_host_get_software_version(session, &result, host))) {
+ xenapiSessionErrorHandler(conn, VIR_ERR_INTERNAL_ERROR ,NULL);
+ xen_host_free(host);
+ return -1;
+ }
+ xen_host_free(host);
+ if (result && result->size>0) {
+ int i;
+ char *version=NULL;
+ for (i=0; i<result->size; i++) {
+ if (STREQ(result->contents[i].key,"xen")) {
+ if (!(version = strdup(result->contents[i].val))) {
+ xen_string_string_map_free(result);
+ virReportOOMError();
+ return -1;
+ }
+ break;
+ }
+ }
+ if (version) {
+ unsigned long major=0,minor=0,release=0;
+ if
(sscanf(version,"%ld.%ld.%ld",&major,&minor,&release)!=3) {
+ virReportOOMError();
If sscanf fails to parse 3 items, then this is no OOM error.
+ xen_string_string_map_free(result);
+ VIR_FREE(version);
+ return -1;
+ }
+ *hvVer = major * 1000000 + minor * 1000 + release;
+ VIR_FREE(version);
+ xen_string_string_map_free(result);
+ return 0;
+ }
+ }
+ return -1;
+}
+
+
+
+/*
+* xenapiGetCapabilities:
+*
+*
+* Returns capabilities as an XML string
+*/
+static char *
+xenapiGetCapabilities (virConnectPtr conn)
+{
+ virCapsPtr caps = ((struct _xenapiPrivate *)(conn->privateData))->caps;
+ if (caps) {
+ char *xml = virCapabilitiesFormatXML(caps);
+ return xml;
You need to report an OOM error if virCapabilitiesFormatXML returns NULL.
+ }
+ xenapiSessionErrorHandler(conn, VIR_ERR_INTERNAL_ERROR ,"Capabilities not
available");
+ return NULL;
+}
+
+
+/*
+* xenapiListDomains
+*
+* Collects the list of active domains, and store their ID in @maxids
+* Returns the number of domain found or -1 in case of error
+*/
+static int
+xenapiListDomains (virConnectPtr conn, int *ids, int maxids)
+{
+ /* vm.list */
+ xen_host host;
+ xen_vm_set *result=NULL;
+ xen_session *session = ((struct _xenapiPrivate
*)(conn->privateData))->session;
+ if (xen_session_get_this_host(session, &host, session)) {
+ xen_host_get_resident_vms(session, &result, host);
+ xen_host_free(host);
+ } else
+ xenapiSessionErrorHandler(conn, VIR_ERR_INTERNAL_ERROR ,NULL);
+
+ if (result != NULL) {
+ int i;
+ for ( i=0; (i < (result->size)) && (i<maxids) ; i++ ) {
+ int64_t t0;
+ xen_vm_get_domid(session, &t0, result->contents[i]);
Maybe you should check if the ID doesn't fit into an int and report an
error in that case, instead of just masking out the upper 32bit.
+ ids[i] = (int)(t0 & 0xffffffff);
+ }
+ xen_vm_set_free(result);
+ return i;
+ }
+ return -1;
+}
+
+
+/*
+* xenapiDomainCreateXML
+*
+* Launches a new domain based on the XML description
+* Returns the domain pointer or NULL in case of error
+*/
+static virDomainPtr
+xenapiDomainCreateXML (virConnectPtr conn,
+ const char *xmlDesc, ATTRIBUTE_UNUSED unsigned int flags)
Normally ATTRIBUTE_UNUSED is placed behind the parameter, not in front of it.
+{
+ xen_vm_record *record=NULL;
+ xen_vm vm=NULL;
+ virDomainPtr domP=NULL;
+ xen_session *session = ((struct _xenapiPrivate
*)(conn->privateData))->session;
+ virCapsPtr caps = ((struct _xenapiPrivate *)(conn->privateData))->caps;
+ if (!caps)
+ return NULL;
+
+ virDomainDefPtr defPtr = virDomainDefParseString(caps, xmlDesc, flags);
+ createVMRecordFromXml(conn, defPtr, &record, &vm);
+ virDomainDefFree(defPtr);
+ if (record) {
+ unsigned char raw_uuid[VIR_UUID_BUFLEN];
+ virUUIDParse(record->uuid,raw_uuid);
+ if (vm) {
+ if (xen_vm_start(session, vm, false, false)) {
+ domP = virGetDomain(conn, record->name_label, raw_uuid);
+ if (!domP) {
+ xen_vm_record_free(record);
+ xenapiSessionErrorHandler(conn, VIR_ERR_INTERNAL_ERROR, "Domain
Pointer is invalid");
+ return domP;
+ }
+ domP->id = record->domid;
+ xen_vm_free(vm);
+ }
+ else
+ xenapiSessionErrorHandler(conn, VIR_ERR_INTERNAL_ERROR, NULL);
+ }
+ xen_vm_record_free(record);
+ }
+ else
+ xenapiSessionErrorHandler(conn, VIR_ERR_INTERNAL_ERROR, NULL);
+ return domP;
+}
+
+
+/*
+* xenapiDomainLookupByName
+*
+* Returns the domain pointer of domain with matching name
+* or -1 in case of error
+*/
+static virDomainPtr
+xenapiDomainLookupByName (virConnectPtr conn,
+ const char *name)
+{
+ /* vm.get_by_name_label */
+ xen_vm_set *vms=NULL;
+ xen_vm vm;
+ char *uuid=NULL;
+ unsigned char raw_uuid[VIR_UUID_BUFLEN];
+ virDomainPtr domP=NULL;
+ xen_session *session = ((struct _xenapiPrivate
*)(conn->privateData))->session;
+ if (xen_vm_get_by_name_label(session, &vms, (char *)name)) {
+ if (vms->size!=1) {
+ xenapiSessionErrorHandler(conn, VIR_ERR_INTERNAL_ERROR,"Domain name is
not unique");
You should distinguish between vms->size < 1 and vms->size > 1. If
there is no domain with a given name you'll report an error that says
that the domain name is not unique.
Or is vms->size >= 1 guaranteed if xen_vm_get_by_name_label succeeds?
If not then this applies to all other fucntions too, where you check
vms->size != 1.
+ xen_vm_set_free(vms);
+ return NULL;
+ }
+ vm = vms->contents[0];
+ xen_vm_get_uuid(session, &uuid, vm);
+ if (uuid!=NULL) {
+ virUUIDParse(uuid,raw_uuid);
+ domP = virGetDomain(conn, name, raw_uuid);
+ if (domP != NULL) {
+ int64_t domid=-1;
+ xen_vm_get_domid(session, &domid, vm);
+ domP->id = domid;
+ xen_uuid_free(uuid);
+ xen_vm_set_free(vms);
+ return domP;
+ } else {
+ xen_uuid_free(uuid);
+ xen_vm_set_free(vms);
+ if (!(session->ok))
+ xenapiSessionErrorHandler(conn, VIR_ERR_NO_DOMAIN, NULL);
+ return NULL;
+ }
+ }
You're leaking vms here.
+ }
+ xenapiSessionErrorHandler(conn, VIR_ERR_NO_DOMAIN, NULL);
+ return NULL;
+}
+
+/*
+* xenapiDomainGetMaxMemory
+*
+* Returns maximum static memory for VM on success
+* or 0 in case of error
+*/
+static unsigned long
+xenapiDomainGetMaxMemory (virDomainPtr dom)
+{
+ int64_t mem_static_max=0;
+ xen_vm vm;
+ xen_vm_set *vms;
+ xen_session *session = ((struct _xenapiPrivate
*)(dom->conn->privateData))->session;
+ if (xen_vm_get_by_name_label(session, &vms, dom->name)) {
+ if (vms->size!=1) {
+ xenapiSessionErrorHandler(dom->conn, VIR_ERR_INTERNAL_ERROR,"Domain
name is not unique");
+ xen_vm_set_free(vms);
+ return 0;
+ }
+ vm = vms->contents[0];
+ xen_vm_get_memory_static_max(session, &mem_static_max, vm);
+ xen_vm_set_free(vms);
+ return (unsigned long)(mem_static_max/1024);
Is mem_static_max a value in byte? libvirt expects this value in kilobyte.
+ } else {
+ xenapiSessionErrorHandler(dom->conn, VIR_ERR_NO_DOMAIN, NULL);
+ return 0;
+ }
+}
+
+/*
+* xenapiDomainSetMaxMemory
+*
+* Sets maximum static memory for VM on success
+* Returns 0 on success or -1 in case of error
+*/
+static int
+xenapiDomainSetMaxMemory (virDomainPtr dom, unsigned long memory)
+{
+ /* vm.set_memory_static_max */
+ xen_vm vm;
+ struct xen_vm_set *vms;
+ xen_session *session = ((struct _xenapiPrivate
*)(dom->conn->privateData))->session;
+ if (xen_vm_get_by_name_label(session, &vms, dom->name)) {
+ if (vms->size!=1) {
+ xenapiSessionErrorHandler(dom->conn, VIR_ERR_INTERNAL_ERROR,"Domain
name is not unique");
+ xen_vm_set_free(vms);
+ return -1;
+ }
+ vm = vms->contents[0];
If mem_static_max is in byte then you need to multiply memory by 1024,
because memory is in kilobyte.
+ if (!(xen_vm_set_memory_static_max(session, vm, memory))) {
+ xenapiSessionErrorHandler(dom->conn, VIR_ERR_INTERNAL_ERROR, NULL);
+ xen_vm_set_free(vms);
+ return -1;
+ }
+ xen_vm_set_free(vms);
+ } else {
+ xenapiSessionErrorHandler(dom->conn, VIR_ERR_NO_DOMAIN, NULL);
+ return -1;
+ }
+ return 0;
+}
+
+
+/*
+* xenapiDomainGetVcpus
+*
+* Gets Vcpu information
+* Return number of structures filled on success or -1 in case of error
+*/
+static int
+xenapiDomainGetVcpus (virDomainPtr dom,
+ virVcpuInfoPtr info, int maxinfo,
+ unsigned char *cpumaps, int maplen)
+{
+
+ xen_vm_set *vms=NULL;
+ xen_vm vm=NULL;
+ xen_string_string_map *vcpu_params=NULL;
+ int nvcpus=0,cpus=0,i;
+ virDomainInfoPtr domInfo;
+ virNodeInfo nodeInfo;
+ virVcpuInfoPtr ifptr;
+ xen_session *session = ((struct _xenapiPrivate
*)(dom->conn->privateData))->session;
+ char *mask=NULL;
+ if((cpumaps!=NULL) && (maplen < 1))
+ return -1;
+ if (VIR_ALLOC(domInfo)<0) {
Don't allocate domInfo here, just use a virDomainInfo directly on the
stack, like you do it with virNodeInfo.
+ virReportOOMError();
+ return -1;
+ }
+ if (virDomainGetInfo(dom,domInfo)==0) {
You should not call libvirt public API functions from here. You can
call xenapiDomainGetInfo directly.
+ nvcpus = domInfo->nrVirtCpu;
+ VIR_FREE(domInfo);
If you put domInfo on the stack, remove this free call.
+ } else {
+ xenapiSessionErrorHandler(dom->conn, VIR_ERR_INTERNAL_ERROR,
"Couldn't fetch Domain Information");
+ return -1;
+ }
+ if ( virNodeGetInfo(dom->conn,&nodeInfo)==0)
Just call xenapiNodeGetInfo directly here.
+ cpus = nodeInfo.cpus;
+ else {
+ xenapiSessionErrorHandler(dom->conn, VIR_ERR_INTERNAL_ERROR,
"Couldn't fetch Node Information");
+ return -1;
+ }
+ if(nvcpus > maxinfo) nvcpus = maxinfo;
+
+ if (cpumaps != NULL)
+ memset(cpumaps, 0, maxinfo * maplen);
+ if (!xen_vm_get_by_name_label(session, &vms, dom->name)) return -1;
+ if (vms->size!=1) {
+ xenapiSessionErrorHandler(dom->conn, VIR_ERR_INTERNAL_ERROR,"Domain name
is not unique");
+ xen_vm_set_free(vms);
+ return -1;
+ }
+ vm = vms->contents[0];
+ if (!xen_vm_get_vcpus_params(session, &vcpu_params, vm)) {
+ xen_vm_set_free(vms);
+ xenapiSessionErrorHandler(dom->conn, VIR_ERR_INTERNAL_ERROR, NULL);
+ return -1;
+ }
+ for (i=0; i<vcpu_params->size; i++) {
+ if (STREQ(vcpu_params->contents[i].key,"mask")) {
+ if (!(mask = strdup(vcpu_params->contents[i].val))){
+ xen_vm_set_free(vms);
+ xen_string_string_map_free(vcpu_params);
+ virReportOOMError();
+ return -1;
+ }
+ break;
+ }
+ }
+ xen_string_string_map_free(vcpu_params);
+ for (i=0,ifptr=info ;i<nvcpus; i++,ifptr++) {
+ ifptr->number = i;
+ ifptr->state = VIR_VCPU_RUNNING;
+ ifptr->cpuTime = 0;
+ ifptr->cpu = 0;
+ if (mask !=NULL)
+ getCpuBitMapfromString(mask,VIR_GET_CPUMAP(cpumaps,maplen,i),maplen);
+ }
+ VIR_FREE(mask);
+ xen_vm_set_free(vms);
+ return i;
+}
+
+
+/*
+* xenapiDomainDumpXML
+*
+*
+* Returns XML string of the domain configuration on success or -1 in case of error
+*/
+static char *
+xenapiDomainDumpXML (virDomainPtr dom, ATTRIBUTE_UNUSED int flags)
Put ATTRIBUTE_UNUSED behind the parameter.
+{
+ xen_vm vm=NULL;
+ xen_vm_set *vms;
+ xen_string_string_map *result=NULL;
+ xen_session *session = ((struct _xenapiPrivate
*)(dom->conn->privateData))->session;
+ virDomainDefPtr defPtr = NULL;
+
+ if(!xen_vm_get_by_name_label(session, &vms, dom->name)) return NULL;
+ if (vms->size!=1) {
+ xenapiSessionErrorHandler(dom->conn, VIR_ERR_INTERNAL_ERROR,"Domain name
is not unique");
+ xen_vm_set_free(vms);
+ return NULL;
+ }
+ if (VIR_ALLOC(defPtr)<0) {
+ virReportOOMError();
+ xen_vm_set_free(vms);
+ return NULL;
+ }
+ vm = vms->contents[0];
+ defPtr->virtType = VIR_DOMAIN_VIRT_XEN;
+ defPtr->id = dom->id;
+ memcpy((char *)defPtr->uuid,(char *)dom->uuid,VIR_UUID_BUFLEN);
+ if (!(defPtr->name = strdup(dom->name)))
+ goto error_cleanup;
+ char *boot_policy=NULL;
+ xen_vm_get_hvm_boot_policy(session, &boot_policy, vm);
+ if (STREQ(boot_policy,"BIOS order")) {
+ if (!(defPtr->os.type = strdup("hvm"))) {
+ VIR_FREE(boot_policy);
+ goto error_cleanup;
+ }
+ xen_vm_get_hvm_boot_params(session, &result, vm);
+ if (result!=NULL) {
+ int i;
+ for (i=0; i<(result->size); i++) {
+ if (STREQ(result->contents[i].key,"order")) {
+ int cnt=0;
+ while(result->contents[i].val[cnt]!='\0') {
+ defPtr->os.bootDevs[cnt] =
map2LibvirtBootOrder(result->contents[i].val[cnt]);
+ cnt++;
+ }
+ defPtr->os.nBootDevs = cnt;
I think you can break the for loop here.
+ }
+ }
+ xen_string_string_map_free(result);
+ }
+ VIR_FREE(boot_policy);
+ } else {
+ if(!(defPtr->os.type = strdup("xen"))) {
+ VIR_FREE(boot_policy);
+ goto error_cleanup;
+ }
+ if(!(defPtr->os.loader = strdup("pygrub"))) {
+ VIR_FREE(boot_policy);
+ goto error_cleanup;
+ }
+ char *value=NULL;
+ xen_vm_get_pv_kernel(session, &value, vm);
+ if (STRNEQ(value,"")) {
+ if(!(defPtr->os.kernel = strdup(value))) {
+ VIR_FREE(boot_policy);
+ VIR_FREE(value);
+ goto error_cleanup;
+ }
+ VIR_FREE(value);
+ }
+ xen_vm_get_pv_ramdisk(session, &value, vm);
+ if (STRNEQ(value,"")) {
+ if(!(defPtr->os.initrd = strdup(value))) {
+ VIR_FREE(boot_policy);
+ VIR_FREE(value);
+ goto error_cleanup;
+ }
+ VIR_FREE(value);
+ }
+ xen_vm_get_pv_args(session, &value, vm);
+ if (STRNEQ(value,"")) {
+ if(!(defPtr->os.cmdline = strdup(value))) {
+ VIR_FREE(boot_policy);
+ VIR_FREE(value);
+ goto error_cleanup;
+ }
+ VIR_FREE(value);
+ }
+ VIR_FREE(boot_policy);
+ if(!(defPtr->os.bootloader = strdup("pygrub")))
+ goto error_cleanup;
+ }
+ char *val=NULL;
+ xen_vm_get_pv_bootloader_args(session, &val, vm);
+ if (STRNEQ(val,"")) {
+ if(!(defPtr->os.bootloaderArgs = strdup(val))) {
+ VIR_FREE(val);
+ goto error_cleanup;
+ }
+ VIR_FREE(val);
+ }
+ unsigned long memory=0;
+ memory = xenapiDomainGetMaxMemory(dom);
+ defPtr->maxmem = memory;
+ int64_t dynamic_mem=0;
+ if (xen_vm_get_memory_dynamic_max(session, &dynamic_mem, vm)) {
+ defPtr->memory = (unsigned long) (dynamic_mem/1024);
+ } else {
+ defPtr->memory = memory;
+ }
+ defPtr->vcpus = xenapiDomainGetMaxVcpus(dom);
+ enum xen_on_normal_exit action;
+ if (xen_vm_get_actions_after_shutdown(session, &action, vm)) {
+ defPtr->onPoweroff = xenapiNormalExitEnum2virDomainLifecycle(action);
+ }
+ if (xen_vm_get_actions_after_reboot(session, &action, vm)) {
+ defPtr->onReboot = xenapiNormalExitEnum2virDomainLifecycle(action);
+ }
+ enum xen_on_crash_behaviour crash;
+ if (xen_vm_get_actions_after_crash(session, &crash, vm)) {
+ defPtr->onCrash = xenapiCrashExitEnum2virDomainLifecycle(action);
+ }
+ xen_vm_get_platform(session, &result, vm);
+ if (result!=NULL) {
+ int i;
+ for(i=0; i< (result->size); i++) {
+ if (STREQ(result->contents[i].val,"true")) {
+ if (STREQ(result->contents[i].key,"acpi"))
+ defPtr->features = defPtr->features |
(1<<VIR_DOMAIN_FEATURE_ACPI);
+ else if (STREQ(result->contents[i].key,"apic"))
+ defPtr->features = defPtr->features |
(1<<VIR_DOMAIN_FEATURE_APIC);
+ else if (STREQ(result->contents[i].key,"pae"))
+ defPtr->features = defPtr->features |
(1<<VIR_DOMAIN_FEATURE_PAE);
+ }
+ }
+ xen_string_string_map_free(result);
+ }
+ struct xen_vif_set *vif_set=NULL;
+ xen_vm_get_vifs(session, &vif_set, vm);
+ if (vif_set) {
+ int i;
+ xen_vif vif;
+ xen_vif_record *vif_rec=NULL;
+ xen_network network;
+ char *bridge=NULL;
+ defPtr->nnets = vif_set->size;
+ if (VIR_ALLOC_N(defPtr->nets, vif_set->size)<0) {
+ xen_vif_set_free(vif_set);
+ goto error_cleanup;
+ }
+ for (i=0; i<vif_set->size; i++) {
+ if (VIR_ALLOC(defPtr->nets[i])<0) {
+ xen_vif_set_free(vif_set);
+ goto error_cleanup;
+ }
+ defPtr->nets[i]->type = VIR_DOMAIN_NET_TYPE_BRIDGE;
+ vif = vif_set->contents[i];
+ xen_vif_get_network(session, &network, vif);
+ if (network!=NULL) {
+ xen_network_get_bridge(session, &bridge, network);
+ if (bridge!=NULL)
+ defPtr->nets[i]->data.bridge.brname = bridge;
+ xen_network_free(network);
+ }
+ xen_vif_get_record(session, &vif_rec, vif);
+ if (vif_rec!=NULL) {
+ if(virParseMacAddr((const char
*)vif_rec->mac,defPtr->nets[i]->mac) < 0)
+ xenapiSessionErrorHandler(dom->conn, VIR_ERR_INTERNAL_ERROR,
"Unable to parse given mac address");
+ xen_vif_record_free(vif_rec);
+ }
+ }
+ xen_vif_set_free(vif_set);
+ }
+ if (vms) xen_vm_set_free(vms);
+ char *xml = virDomainDefFormat(defPtr,0);
+ virDomainDefFree(defPtr);
+ return xml;
+
+ error_cleanup:
+ virReportOOMError();
+ xen_vm_set_free(vms);
+ virDomainDefFree(defPtr);
+ return NULL;
+
+}
+
+/*
+* xenapiListDefinedDomains
+*
+* list the defined but inactive domains, stores the pointers to the names in @names
+* Returns number of names provided in the array or -1 in case of error
+*/
+static int
+xenapiListDefinedDomains (virConnectPtr conn, char **const names,
+ int maxnames)
+{
+ int i,j=0,doms;
+ xen_vm_set *result;
+ xen_vm_record *record;
+ xen_session *session = ((struct _xenapiPrivate
*)(conn->privateData))->session;
+ xen_vm_get_all(session, &result);
+ if (result != NULL) {
+ for (i=0; i< (result->size) && j< maxnames; i++) {
+ xen_vm_get_record(session, &record, result->contents[i]);
+ if ( record!=NULL ) {
+ if ( record->is_a_template == 0 ) {
+ char *usenames;
+ if (!(usenames = strdup(record->name_label))) {
+ virReportOOMError();
+ xen_vm_record_free(record);
+ xen_vm_set_free(result);
If you've already srtdup'ed some names when an error occurs, then
you're leaking this already strdup'ed names here.
+ return -1;
+ }
+ names[j++]=usenames;
+ }
+ xen_vm_record_free(record);
+ } else {
+ xenapiSessionErrorHandler(conn, VIR_ERR_INTERNAL_ERROR,
"Couldn't get VM record");
+ xen_vm_set_free(result);
And here you may leak already strdup'ed names too. you need to free
them in bot cases.
+ return -1;
+ }
+ }
+ doms=j;
+ xen_vm_set_free(result);
+ return doms;
+ }
+ xenapiSessionErrorHandler(conn, VIR_ERR_INTERNAL_ERROR, NULL);
+ return -1;
+}
+
+
+/*
+* call_func
+* sets curl options, used with xen_session_login_with_password
+*/
+int
+call_func(const void *data, size_t len, void *user_handle,
+ void *result_handle, xen_result_func result_func)
+{
+ //(void)user_handle;
+ struct _xenapiPrivate *priv = (struct _xenapiPrivate *)user_handle;
+ #ifdef PRINT_XML
+
+ printf("\n\n---Data to server: -----------------------\n");
+ printf("%s\n",((char*) data));
+ fflush(stdout);
+ #endif
+ CURL *curl = curl_easy_init();
+ if (!curl) {
+ return -1;
+ }
+ xen_comms comms = {
+ .func = result_func,
+ .handle = result_handle
+ };
+ curl_easy_setopt(curl, CURLOPT_URL, priv->url);
+ curl_easy_setopt(curl, CURLOPT_NOPROGRESS, 1);
+ #ifdef CURLOPT_MUTE
+ curl_easy_setopt(curl, CURLOPT_MUTE, 1);
+ #endif
+ curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, &write_func);
+ curl_easy_setopt(curl, CURLOPT_WRITEDATA, &comms);
+ curl_easy_setopt(curl, CURLOPT_POST, 1);
+ curl_easy_setopt(curl, CURLOPT_POSTFIELDS, data);
+ curl_easy_setopt(curl, CURLOPT_POSTFIELDSIZE, len);
+ curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, priv->SSLflag);
+ curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, priv->SSLflag);
You set SSLflag to 0 or 2 depending on the no_verify query parameter
in xenapiOpen. For CURLOPT_SSL_VERIFYHOST 2 is the correct value, but
CURLOPT_SSL_VERIFYPEER expects 0 or 1.
Instead of having the SSLflag in the xenapiPrivate struct you could
have the noVerify value there and then decide here to what values this
maps. For example
curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, priv->noVerify ? 0 : 1);
curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, priv->noVerify ? 0 : 2);
+ CURLcode result = curl_easy_perform(curl);
+ curl_easy_cleanup(curl);
+ return result;
+
+}
+
+
+
diff -Nur ./libvirt_org/src/xenapi/xenapi_driver.h ./libvirt/src/xenapi/xenapi_driver.h
--- ./libvirt_org/src/xenapi/xenapi_driver.h 1970-01-01 01:00:00.000000000 +0100
+++ ./libvirt/src/xenapi/xenapi_driver.h 2010-02-26 13:21:50.000000000 +0000
@@ -0,0 +1,15 @@
+/*
+ * xenapi_driver.h.c: Xen API driver header file to be included in libvirt.c.
+ * Copyright (C) 2009 Citrix Ltd.
+ * Sharadha Prabhakar <sharadha.prabhakar(a)citrix.com>
+ */
As in xenapi_driver.c, a license statement is missing here.
+
+#ifndef __VIR_XENAPI_PRIV_H__
+#define __VIR_XENAPI_PRIV_H__
+
+
+extern int xenapiRegister (void);
+
+
+#endif /* __VIR_XENAPI_PRIV_H__ */
diff -Nur ./libvirt_org/src/xenapi/xenapi_driver_private.h
./libvirt/src/xenapi/xenapi_driver_private.h
--- ./libvirt_org/src/xenapi/xenapi_driver_private.h 1970-01-01 01:00:00.000000000
+0100
+++ ./libvirt/src/xenapi/xenapi_driver_private.h 2010-03-03 16:09:47.000000000
+0000
@@ -0,0 +1,49 @@
+/*
+ * xenapi_driver_private.h: Xen API driver's private header file.
+ * Copyright (C) 2009 Citrix Ltd.
+ * Sharadha Prabhakar <sharadha.prabhakar(a)citrix.com>
+ */
A license statement is missing here too.
+
+#ifndef __VIR_XENAPI_H__
+#define __VIR_XENAPI_H__
+
+#include <xen/api/xen_common.h>
+#include <libxml/tree.h>
+#include "virterror_internal.h"
+
+//#define PRINT_XML
+#define VIR_FROM_THIS VIR_FROM_XENAPI
+#define LIBVIRT_MODELNAME_LEN (32)
+#define xenapiSessionErrorHandler(conn,errNum,buf) xenapiSessionErrorHandle(conn,
errNum, \
+
buf,__FILE__,__FUNCTION__,__LINE__)
+
+void
+xenapiSessionErrorHandle(virConnectPtr conn, virErrorNumber errNum,
+ const char *buf, const char *filename, const char *func, size_t
lineno);
+
+typedef struct
+{
+ xen_result_func func;
+ void *handle;
+} xen_comms;
+
+
+int
+call_func(const void *data, size_t len, void *user_handle,
+ void *result_handle, xen_result_func result_func);
+size_t
+write_func(void *ptr, size_t size, size_t nmemb, void *comms);
+
+/* xenAPI driver's private data structure */
+struct _xenapiPrivate {
+ xen_session *session;
+ void *handle;
+ char *uname;
+ char *pwd;
I think handle, uname, and pwd are actually unused. If that's true you
could remove them from the struct.
+ char *url;
+ int SSLflag;
+ virCapsPtr caps;
+};
+
+#endif /* __VIR_XENAPI_H__ */
diff -Nur ./libvirt_org/src/xenapi/xenapi_utils.c ./libvirt/src/xenapi/xenapi_utils.c
--- ./libvirt_org/src/xenapi/xenapi_utils.c 1970-01-01 01:00:00.000000000 +0100
+++ ./libvirt/src/xenapi/xenapi_utils.c 2010-03-03 18:01:06.000000000 +0000
@@ -0,0 +1,578 @@
+/*
+ * xenapi_utils.c: Xen API driver -- utils parts.
+ * Copyright (C) 2009 Citrix Ltd.
+ * Sharadha Prabhakar <sharadha.prabhakar(a)citrix.com>
+ */
A license statement is missing here too.
+
+/* obtains the CPU bitmap from the string passed */
+void
+getCpuBitMapfromString(char *mask, unsigned char *cpumap, int maplen)
+{
+ int pos;
+ int max_bits = maplen * 8;
+ char *num = NULL,*bp=NULL;
+ bzero(cpumap, maplen);
+ num = strtok_r (mask, ",", &bp);
+ while (num != NULL) {
+ if (sscanf (num, "%d", &pos)!=1)
+ virReportOOMError();
It isn't an OOM error if sscanf fails.
+ if (pos<0 || pos>max_bits-1)
+ VIR_WARN ("number in str %d exceeds cpumap's max bits %d\n",
pos, max_bits);
Do not append \n, that'll be done automatically.
+ else
+ (cpumap)[pos/8] |= (1<<(pos%8));
+ num = strtok_r (NULL, ",", &bp);
+ }
+}
+
+
+
+/* allocate a flexible array and fill values(key,val) */
+int
+allocStringMap (xen_string_string_map **strings, char *key, char *val)
+{
+ int sz = ((*strings) == NULL)?0:(*strings)->size;
+ sz++;
+ if(VIR_REALLOC_N(*strings, sizeof(xen_string_string_map)+
+ sizeof(xen_string_string_map_contents)*sz)<0) {
+ virReportOOMError();
+ return -1;
+ }
+ (*strings)->size = sz;
+ (*strings)->contents[sz-1].key = strdup(key);
+ (*strings)->contents[sz-1].val = strdup(val);
You should check if strdup returned NULL.
+ return 0;
+}
+
+/* Error handling function returns error messages from the server if any */
+void
+xenapiSessionErrorHandle(virConnectPtr conn, virErrorNumber errNum,
+ const char *buf, const char *filename, const char *func, size_t
lineno)
+{
+ if (buf==NULL) {
+ char *ret=NULL;
+ ret = returnErrorFromSession(((struct _xenapiPrivate
*)(conn->privateData))->session);
+ virReportErrorHelper (conn, VIR_FROM_XENAPI, errNum, filename, func, lineno,
_("%s\n"), ret);
+ xen_session_clear_error(((struct _xenapiPrivate
*)(conn->privateData))->session);
+ VIR_FREE(ret);
+ } else {
+ virReportErrorHelper (conn, VIR_FROM_XENAPI, errNum, filename, func, lineno,
_("%s\n"), buf);
+ }
No need to append \n here too.
+}
+
+
+/* Create a VM record from the XML description */
+int
+createVMRecordFromXml (virConnectPtr conn, virDomainDefPtr def,
+ xen_vm_record **record, xen_vm *vm)
+{
+ char uuidStr[VIR_UUID_STRING_BUFLEN];
+ *record = xen_vm_record_alloc();
+ if (!((*record)->name_label = strdup(def->name)))
+ goto error_cleanup;
+ if (def->uuid) {
+ virUUIDFormat(def->uuid,uuidStr);
+ if (!((*record)->uuid = strdup(uuidStr)))
+ goto error_cleanup;
+ }
+ if (STREQ(def->os.type,"hvm")) {
+ if(!((*record)->hvm_boot_policy = strdup("BIOS order")))
+ goto error_cleanup;
+ char *boot_order = NULL;
+ if (def->os.nBootDevs!=0)
+ boot_order = createXenAPIBootOrderString(def->os.nBootDevs,
&def->os.bootDevs[0]);
+ if (boot_order!=NULL) {
+ xen_string_string_map *hvm_boot_params=NULL;
+ allocStringMap(&hvm_boot_params, (char *)"order",boot_order);
+ (*record)->hvm_boot_params = hvm_boot_params;
+ VIR_FREE(boot_order);
+ }
+ } else if (STREQ(def->os.type,"xen")) {
+ if (!((*record)->pv_bootloader = strdup("pygrub")))
+ goto error_cleanup;
+ if (def->os.kernel){
+ if (!((*record)->pv_kernel = strdup(def->os.kernel)))
+ goto error_cleanup;
+ }
+ if (def->os.initrd) {
+ if (!((*record)->pv_ramdisk = strdup(def->os.initrd)))
+ goto error_cleanup;
+ }
+ if(def->os.cmdline) {
+ if (!((*record)->pv_args = strdup(def->os.cmdline)))
+ goto error_cleanup;
+ }
+ (*record)->hvm_boot_params = xen_string_string_map_alloc(0);
+ }
+ if (def->os.bootloaderArgs)
+ if(!((*record)->pv_bootloader_args = strdup(def->os.bootloaderArgs)))
+ goto error_cleanup;
+
+ if (def->memory)
+ (*record)->memory_static_max = (int64_t) (def->memory * 1024);
+ if (def->maxmem)
+ (*record)->memory_dynamic_max = (int64_t) (def->maxmem * 1024);
+ else
+ (*record)->memory_dynamic_max = (*record)->memory_static_max;
+
+ if (def->vcpus) {
+ (*record)->vcpus_max = (int64_t) def->vcpus;
+ (*record)->vcpus_at_startup = (int64_t) def->vcpus;
+ }
+ if (def->onPoweroff)
+ (*record)->actions_after_shutdown =
actionShutdownLibvirt2XenapiEnum(def->onPoweroff);
+ if (def->onReboot)
+ (*record)->actions_after_reboot =
actionShutdownLibvirt2XenapiEnum(def->onReboot);
+ if (def->onCrash)
+ (*record)->actions_after_crash =
actionCrashLibvirt2XenapiEnum(def->onCrash);
+
+ xen_string_string_map *strings=NULL;
+ if (def->features) {
+ if (def->features & (1<<VIR_DOMAIN_FEATURE_ACPI))
+ allocStringMap(&strings,(char *)"acpi",(char
*)"true");
+ if (def->features & (1<<VIR_DOMAIN_FEATURE_APIC))
+ allocStringMap(&strings,(char *)"apic",(char
*)"true");
+ if (def->features & (1<<VIR_DOMAIN_FEATURE_PAE))
+ allocStringMap(&strings,(char *)"pae",(char
*)"true");
+ }
+ if (strings!=NULL)
+ (*record)->platform = strings;
+
+ (*record)->vcpus_params = xen_string_string_map_alloc(0);
+ (*record)->other_config = xen_string_string_map_alloc(0);
+ (*record)->last_boot_cpu_flags = xen_string_string_map_alloc(0);
+ (*record)->xenstore_data = xen_string_string_map_alloc(0);
+ (*record)->hvm_shadow_multiplier = 1.000;
+ if (!xen_vm_create(((struct _xenapiPrivate *)(conn->privateData))->session,
+ vm, *record)) {
+ xenapiSessionErrorHandler(conn,VIR_ERR_INTERNAL_ERROR,NULL);
+ return -1;
+ }
+
+ int device_number=0;
+ char *bridge=NULL,*mac=NULL;
+ int i;
+ for (i=0;i<def->nnets;i++) {
+ if (def->nets[i]->type == VIR_DOMAIN_NET_TYPE_BRIDGE) {
+ if (def->nets[i]->data.bridge.brname)
+ if(!(bridge = strdup(def->nets[i]->data.bridge.brname)))
+ goto error_cleanup;
+ if (def->nets[i]->mac) {
+ char macStr[VIR_MAC_STRING_BUFLEN];
+ virFormatMacAddr(def->nets[i]->mac, macStr);
+ if(!(mac = strdup(macStr))) {
+ if (bridge) VIR_FREE(bridge);
+ goto error_cleanup;
+ }
+ }
+ if (mac!=NULL && bridge!=NULL) {
+ char device[NETWORK_DEVID_SIZE]="\0";
You defined NETWORK_DEVID_SIZE to be 10, but 10 is to small for an int
value. You need at least two more chars, one for a possible minus sign
and one for the null terminator.
> + sprintf(device,"%d",device_number);
> + createVifNetwork(conn, *vm, device, bridge, mac);
> + VIR_FREE(bridge);
> + device_number++;
> + }
> + if (bridge) VIR_FREE(bridge);
> + /*if (mac!=NULL && bridge!=NULL) {
> + char device[NETWORK_DEVID_SIZE]="\0";
> + sprintf(device,"%d",device_number);
> + if (createVifNetwork(conn, *vm, device, bridge, mac)<0) {
> + VIR_FREE(mac);
> + VIR_FREE(bridge);
> + xen_vm_record_free(*record);
> + xenapiSessionErrorHandler(conn,VIR_ERR_INTERNAL_ERROR,NULL);
> + return -1;
> + }
> + VIR_FREE(bridge);
> + device_number++;
> + } else {
> + if (bridge)
> + VIR_FREE(bridge);
> + if (mac)
> + VIR_FREE(mac);
> + xen_vm_record_free(*record);
> + xenapiSessionErrorHandler(conn,VIR_ERR_INTERNAL_ERROR,NULL);
> + return -1;
> + }*/
> + }
> + }
> + return 0;
> +
> + error_cleanup:
> + virReportOOMError();
> + xen_vm_record_free(*record);
> + return -1;
+}
+
> diff -Nur ./libvirt_org/src/xenapi/xenapi_utils.h
./libvirt/src/xenapi/xenapi_utils.h
> --- ./libvirt_org/src/xenapi/xenapi_utils.h 1970-01-01 01:00:00.000000000 +0100
> +++ ./libvirt/src/xenapi/xenapi_utils.h 2010-03-03 18:01:19.000000000 +0000
> @@ -0,0 +1,93 @@
> +/*
> + * xenapi_utils.h: Xen API driver -- utils header
> + * Copyright (C) 2009 Citrix Ltd.
> + * Sharadha Prabhakar <sharadha.prabhakar(a)citrix.com>
> + */
> +
A license statement is missing here too.
+
+#define NETWORK_DEVID_SIZE (10)
+
Should be at least 12.
Another thing you probably should fix is the definition of varaibles
in the middle of blocks. Variables should be defined at the beginning
of a block.
Okay, mostly minor issues. I think the next version of this patch can
be applied and remaining issues can be fixed by additional patches.
Matthias