2010/2/26 Sharadha Prabhakar (3P) <sharadha.prabhakar@xxxxxxxxxx>:
> This patch contains XenAPI driver specific files.
> Files included are
> xenapi_driver.c
> xenapi_utils.c
> xenapi_driver.h
> xenapi_driver_private.h
> xenapi_utils.h
> This patch includes changes suggested in the first review.
>
>
> 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-02-26 15:27:00.000000000 +0000
> @@ -0,0 +1,1564 @@
> +
> +/*
> + * xenapi_driver.c: Xen API driver.
> + * Copyright (C) 2009 Citrix Ltd.
> + * Sharadha Prabhakar <sharadha.prabhakar@xxxxxxxxxx>
> +*/
> +
> +
> +char *url=NULL;
You should move this into the xenapiPrivate struct, otherwise you'll
have trouble using multiple XenAPI connections at the same time,
because multiple calls to xenapiOpen will overwrite the pointer an
leak the previous value.
> +/*
> +*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;
> + xen_session *session;
> + struct _xenapiPrivate *privP;
> +
> + if (!STREQ(conn->uri->scheme,"XenAPI")) {
> + xenapiSessionErrorHandler(conn, VIR_ERR_AUTH_FAILED ,"Check URI format: 'XenAPI://user@server'");
> + return VIR_DRV_OPEN_ERROR;
If the URI is not for you (scheme != XenAPI) then you must decline it
(return VIR_DRV_OPEN_DECLINED) and not return VIR_DRV_OPEN_ERROR.
All other schemes in libvirt are lowercase. I suggest you change it to
"xenapi" or switch the STREQ to STRCASEEQ to do a case-insensitive
comparison.
> + }
> + if (conn->uri->server==NULL) {
> + xenapiSessionErrorHandler(conn, VIR_ERR_AUTH_FAILED ,"Server name not in URI");
> + return VIR_DRV_OPEN_ERROR;
> + }
> + if (auth) {
> + passwd = esxUtil_RequestPassword(auth,conn->uri->user,conn->uri->server);
You cannot use ESX driver code here, because inter-driver dependencies
are forbidden. So we could either move this function into the
src/utils/utils.c or you duplicate the function in the XenAPI driver
code. I suggest the later because esxUtil_RequestPassword's behavior
is special for the ESX driver (setting the challenge to the hostname).
> + } else {
> + xenapiSessionErrorHandler(conn, VIR_ERR_AUTH_FAILED ,"Authentication Credentials not found");
> + return VIR_DRV_OPEN_ERROR;
> + }
> + if (!passwd && !conn->uri->user) {
> + xenapiSessionErrorHandler(conn, VIR_ERR_AUTH_FAILED ,"Username/Password not valid");
> + return VIR_DRV_OPEN_ERROR;
> + }
> + virAsprintf(&url,"https://%s",conn->uri->server);
You should check the return value from virAsprintf and report an OOM
error if the return value is negative.
> + xmlInitParser();
> + xmlKeepBlanksDefault(0);
I'm not sure if it's a good idea to change global libxml2 defaults. I
can't tell if this is the default anyway, or if it'll will affect the
other XML parsing code in libvirt in a negative way, because the
libxml2 documentation for this function is confusing to me.
> + xen_init();
> + curl_global_init(CURL_GLOBAL_ALL);
> +
> + session = xen_session_login_with_password (call_func, NULL, conn->uri->user, passwd, xen_api_latest_version);
> +
> + if ( session && session->ok ) {
> + if (VIR_ALLOC(privP)<0)
You need to call virReportOOMError() here, to report the allocation failure.
> + return VIR_DRV_OPEN_ERROR;
> + privP->session = session;
> + conn->privateData = privP;
> + return VIR_DRV_OPEN_SUCCESS;
> + } else {
> + xenapiSessionErrorHandler(conn, VIR_ERR_AUTH_FAILED ,"");
> + return VIR_DRV_OPEN_ERROR;
> + }
> +}
> +
> +
> +/*
> +*
> +* xenapiSupportsFeature
> +*
> +* Returns 0
> +*/
> +static int
> +xenapiSupportsFeature (virConnectPtr conn ATTRIBUTE_UNUSED, int feature)
> +{
> + switch (feature) {
> + case VIR_DRV_FEATURE_MIGRATION_V2:
> + case VIR_DRV_FEATURE_MIGRATION_P2P:
> + default:
> + return 0;
> + }
> +}
> +
> +
> +/*
> +* xenapiGetVersion:
> +*
> +* Gets the version of XenAPI
> +*
> +*/
> +static int
> +xenapiGetVersion (virConnectPtr conn ATTRIBUTE_UNUSED, unsigned long *hvVer)
Remove ATTRIBUTE_UNUSED from conn, as conn is used in this function.
> +{
> + xen_host host;
> + xen_session *session = ((struct _xenapiPrivate *)(conn->privateData))->session;
> + if (!(xen_session_get_this_host(session, &host,session))) {
> + xenapiSessionErrorHandler(conn, VIR_ERR_NO_DEVICE ,NULL);
You're abusing the VIR_ERR_NO_DEVICE error code here.
> + return -1;
> + }
> + xen_string_string_map *result=NULL;
You should define new variables always at the beginning of a block.
> + 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"))
> + version = strdup(result->contents[i].val);
You need to check the return value strdup of for NULL and report an
OOM error using virReportOOMError if it's NULL.
Also you need to break the for loop if you found the version string,
otherwise you may strdup and assign a string multiple times to version
and leak the previous allocated memeory.
> + }
> + if (version) {
> + unsigned long major=0,minor=0,release=0;
> + sscanf(version,"%ld.%ld.%ld",&major,&minor,&release);
You should check the return value of sscanf to see if it failed.
> + *hvVer = major * 1000000 + minor * 1000 + release;
> + VIR_FREE(version);
> + xen_string_string_map_free(result);
> + return 0;
> + }
> + }
> + return -1;
> +}
> +
> +
> +/*
> +* xenapiGetHostname:
> +*
> +*
> +* Returns the hostname on success, or NULL on failure
> +*/
> +static char *
> +xenapiGetHostname (virConnectPtr conn)
> +{
> + char *result=NULL;
> + xen_host host;
> + xen_session *session = ((struct _xenapiPrivate *)(conn->privateData))->session;
> + if (!(xen_session_get_this_host(session, &host, session))) {
> + xenapiSessionErrorHandler(conn, VIR_ERR_NO_DEVICE ,NULL);
You're abusing the VIR_ERR_NO_DEVICE error code here.
> + return NULL;
> + }
> + xen_host_get_hostname(session, &result, host);
> + xen_host_free(host);
> + return result;
> +}
> +
> +
> +/*
> +* xenapiNodeGetInfo:
> +*
> +*
> +* Returns Node details on success or else -1
> +*/
> +static int
> +xenapiNodeGetInfo (virConnectPtr conn, virNodeInfoPtr info)
> +{
> + int64_t memory,mhz;
> + xen_host_cpu_set *host_cpu_set;
> + xen_host_cpu host_cpu;
> + xen_host_metrics_set *xen_met_set;
> + char *modelname;
> + xen_session *session = ((struct _xenapiPrivate *)(conn->privateData))->session;
> + info->nodes = 1;
> + info->threads = 1;
> + info->sockets = 1;
> +
> + if (xen_host_metrics_get_all(session, &xen_met_set)) {
> + xen_host_metrics_get_memory_total(session, &memory, xen_met_set->contents[0]);
> + info->memory = (unsigned long)(memory/1024);
> + xen_host_metrics_set_free(xen_met_set);
> + } else {
> + xenapiSessionErrorHandler(conn, VIR_ERR_INTERNAL_ERROR ,"Unable to get host metric Information");
> + return -1;
> + }
> + if (xen_host_cpu_get_all(session, &host_cpu_set)) {
> + host_cpu = host_cpu_set->contents[0];
> + xen_host_cpu_get_modelname(session, &modelname, host_cpu);
> + strncpy(info->model, modelname, LIBVIRT_MODELNAME_LEN-2);
You should use virStrncpy here and check its return value.
> + info->model[LIBVIRT_MODELNAME_LEN-1]='\0';
> + xen_host_cpu_get_speed(session, &mhz, host_cpu);
> + info->mhz = (unsigned long)mhz;
> + info->cpus = host_cpu_set->size;
> + info->cores = host_cpu_set->size;
> +
> + xen_host_cpu_set_free(host_cpu_set);
> + VIR_FREE(modelname);
> + return 0;
> + }
> + xenapiSessionErrorHandler(conn, VIR_ERR_INTERNAL_ERROR ,"Unable to get Host CPU set");
> + return -1;
> +}
> +
> +static virCapsPtr
> +getCapsObject(void)
> +{
You're not supposed to build the caps structure by hand. Use the
virCapabilities* functions.
> + virCapsPtr caps;
> + if (VIR_ALLOC(caps) < 0)
> + return NULL;
> + caps->host.arch = strdup("x86_64");
Use virCapabilitiesNew instead.
> + caps->nguests = 2;
> + if (VIR_ALLOC_N(caps->guests,2) < 0)
> + return NULL;
> + int i;
> + for (i=0;i<2;i++) {
> + if (VIR_ALLOC(caps->guests[i]) < 0)
> + return NULL;
> + if (VIR_ALLOC_N(caps->guests[i]->arch.domains,1)<0)
> + return NULL;
> + if (VIR_ALLOC(caps->guests[i]->arch.domains[0])<0)
> + return NULL;
> + caps->guests[i]->arch.name = strdup("x86_64");
> + caps->guests[i]->arch.domains[0]->type = strdup("xen");
> + caps->guests[i]->arch.ndomains = 1;
> + }
> + caps->guests[0]->ostype = strdup("hvm");
> + caps->guests[1]->ostype = strdup("xen");
Use virCapabilitiesAddGuest and virCapabilitiesAddGuestDomain instead.
> + return caps;
> +}
> +
> +
> +/*
> +* xenapiGetCapabilities:
> +*
> +*
> +* Returns capabilities as an XML string
> +*/
> +static char *
> +xenapiGetCapabilities (virConnectPtr conn ATTRIBUTE_UNUSED)
> +{
> + virCapsPtr caps = getCapsObject();
> + if (caps!=NULL) {
> + char *xml = virCapabilitiesFormatXML(caps);
You could improve this by creating the caps once per connection and
store the virCapsPtr in the xenapiPrivate struct, instead of
recreating it over and over again.
> + VIR_FREE(caps);
> + return xml;
> + }
> + 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 */
> + int i,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_NO_DEVICE ,NULL);
You're abusing the VIR_ERR_NO_DEVICE error code here.
> +
> + if (result != NULL) {
> + for ( i=0; (i < (result->size)) && (i<maxids) ; i++ ) {
> + int64_t t0;
> + xen_vm_get_domid(session, &t0, result->contents[i]);
> + ids[i] = (int)(t0 & 0xffffffff);
> + }
> + list = result->size;
That's wrong. Assume the case that result->size is greater than
maxids, then you are reporting back more IDs than you actually wrote
to the ids array.
> + xen_vm_set_free(result);
> + return list;
> + }
> + return -1;
> +}
> +
> +/*
> +* xenapiNumOfDomains
> +*
> +*
> +* Returns the number of domains found or -1 in case of error
> +*/
> +static int
> +xenapiNumOfDomains (virConnectPtr conn)
> +{
> + /* #(vm.list) */
> + xen_vm_set *result=NULL;
> + xen_host host=NULL;
> + int numDomains=-1;
> + xen_session *session = ((struct _xenapiPrivate *)(conn->privateData))->session;
> +
> + xen_session_get_this_host(session, &host, session);
> + if ( host!=NULL ) {
> + xen_host_get_resident_vms(session, &result, host);
> + if ( result != NULL) {
> + numDomains = result->size;
> + xen_vm_set_free(result);
> + }
> + xen_host_free(host);
> + }
> + if (!(session->ok))
> + xenapiSessionErrorHandler(conn, VIR_ERR_NO_DEVICE ,NULL);
You're abusing the VIR_ERR_NO_DEVICE error code here.
> + return numDomains;
> +}
> +
> +/*
> +* 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)
> +{
> + xen_vm_record *record=NULL;
> + xen_vm vm=NULL;
> + virDomainPtr domP=NULL;
> + xen_session *session = ((struct _xenapiPrivate *)(conn->privateData))->session;
> + virCapsPtr caps = getCapsObject();
> + if (!caps)
> + return NULL;
> +
> + virDomainDefPtr defPtr = virDomainDefParseString(caps, xmlDesc, flags);
> + createVMRecordFromXml(conn, defPtr, &record, &vm);
> + if (record!=NULL) {
> + unsigned char raw_uuid[VIR_UUID_BUFLEN];
> + virUUIDParse(record->uuid,raw_uuid);
createVMRecordFromXml doesn't copy the UUID from the virDomainDefPtr
to the xen_vm_record, so parsing it here gives wrong results.
> + if (vm!=NULL) {
> + if (xen_vm_start(session, vm, false, false)) {
> + domP = virGetDomain(conn, record->name_label, raw_uuid);
You forgot to set the domain ID (domP->id).
> + 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);
You're leaking the caps object here.
> + return domP;
> +}
> +
> +/*
> +* xenapiDomainLookupByID
> +*
> +*
> +* Returns a valid domain pointer of the domain with ID same as the one passed
> +* or NULL in case of error
> +*/
> +static virDomainPtr
> +xenapiDomainLookupByID (virConnectPtr conn, int id)
> +{
> + int i;
> + int64_t domID;
> + char *uuid;
> + xen_host host;
> + xen_vm_set *result;
> + xen_vm_record *record;
> + unsigned char raw_uuid[VIR_UUID_BUFLEN];
> + virDomainPtr domP=NULL;
> + xen_session *session = ((struct _xenapiPrivate *)(conn->privateData))->session;
> +
> + xen_session_get_this_host(session, &host, session);
> + if (host!=NULL && session->ok) {
> + xen_host_get_resident_vms(session, &result, host);
> + if ( result !=NULL ) {
> + for( i=0; i < (result->size); i++) {
> + xen_vm_get_domid(session, &domID, result->contents[i]);
> + if ( domID == id ) {
> + xen_vm_get_record(session, &record, result->contents[i]);
> + xen_vm_get_uuid(session, &uuid, result->contents[i]);
> + virUUIDParse(uuid,raw_uuid);
> + domP = virGetDomain(conn, record->name_label, raw_uuid);
> + if (domP!=NULL) {
> + int64_t domid=-1;
> + xen_vm_get_domid(session, &domid, result->contents[i]);
> + domP->id = domid;
> + }
> + xen_uuid_free(uuid);
> + xen_vm_record_free(record);
You should break the for loop if the domain id found.
> + }
> + }
You should report an error if the domain is not found.
> + xen_vm_set_free(result);
> + } else {
> + xenapiSessionErrorHandler(conn, VIR_ERR_NO_DOMAIN ,NULL);
> + }
> + xen_host_free(host);
> + } else {
> + xenapiSessionErrorHandler(conn, VIR_ERR_NO_DEVICE ,NULL);
You're abusing the VIR_ERR_NO_DEVICE error code here.
> + }
> + return domP;
> +}
> +
> +/*
> +* xenapiDomainLookupByUUID
> +*
> +* Returns the domain pointer of domain with matching UUID
> +* or -1 in case of error
> +*/
> +static virDomainPtr
> +xenapiDomainLookupByUUID (virConnectPtr conn,
> + const unsigned char *uuid)
> +{
> + /* vm.get_by_uuid */
> + xen_vm vm;
> + xen_vm_record *record;
> + unsigned char raw_uuid[VIR_UUID_BUFLEN];
> + virDomainPtr domP=NULL;
> + xen_session *session = ((struct _xenapiPrivate *)(conn->privateData))->session;
> + if (xen_vm_get_by_uuid(session, &vm, (char *)uuid)) {
> + xen_vm_get_record(session, &record, vm);
> + if (record != NULL) {
> + virUUIDParse((char *)uuid,raw_uuid);
const unsigned char *uuid is already in raw format. Parsing it again
will give wrong results. Did you actually test this function?
> + domP = virGetDomain(conn, record->name_label, raw_uuid);
The domain ID should be set here.
> + xen_vm_record_free(record);
> + }
> + else
> + xenapiSessionErrorHandler(conn, VIR_ERR_NO_DOMAIN ,NULL);
> + xen_vm_free(vm);
> + } else
> + xenapiSessionErrorHandler(conn, VIR_ERR_NO_DOMAIN ,NULL);
> + return domP;
> +}
> +
> +
> +/*
> +* xenapiDomainSuspend
> +*
> +* a VM is paused
> +* Returns 0 on success or -1 in case of error
> +*/
> +static int
> +xenapiDomainSuspend (virDomainPtr dom)
> +{
> + /* vm.pause() */
> + 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)) {
> + xenapiSessionErrorHandler(dom->conn,VIR_ERR_NO_DOMAIN,NULL);
> + return -1;
> + }
Is vms->contents[0] guaranteed to be valid if xen_vm_get_by_name_label succeeds?
> + vm = vms->contents[0];
> + if (!xen_vm_pause(session, vm)) {
> + xenapiSessionErrorHandler(dom->conn, VIR_ERR_INTERNAL_ERROR, NULL);
> + xen_vm_set_free(vms);
> + return -1;
> + }
> + xen_vm_set_free(vms);
> + return 0;
> +}
> +
> +/*
> +* xenapiDomainResume
> +*
> +* Resumes a VM
> +* Returns 0 on success or -1 in case of error
> +*/
> +static int
> +xenapiDomainResume (virDomainPtr dom)
> +{
> + /* vm.unpause() */
> + 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)) {
> + xenapiSessionErrorHandler(dom->conn, VIR_ERR_NO_DOMAIN, NULL);
> + return -1;
> + }
Okay you check vms->size here before accessing vms->contents[0]. So
you should do the same in xenapiDomainSuspend.
> + if (vms!=NULL && vms->size!=0) {
> + vm = vms->contents[0];
> + if (!xen_vm_unpause(session, vm)) {
> + xenapiSessionErrorHandler(dom->conn, VIR_ERR_INTERNAL_ERROR, NULL);
> + xen_vm_set_free(vms);
> + return -1;
> + }
> + xen_vm_set_free(vms);
> + }
> + return 0;
> +}
> +
> +/*
> +* xenapiDomainShutdown
> +*
> +* shutsdown a VM
> +* Returns 0 on success or -1 in case of error
> +*/
> +static int
> +xenapiDomainShutdown (virDomainPtr dom)
> +{
> + /* vm.clean_shutdown */
> + 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)) {
> + xenapiSessionErrorHandler(dom->conn, VIR_ERR_NO_DOMAIN, NULL);
> + return -1;
> + }
vms->size check missing here as in xenapiDomainSuspend.
> + vm = vms->contents[0];
> + if (!xen_vm_clean_shutdown(session, vm)) {
> + xenapiSessionErrorHandler(dom->conn, VIR_ERR_INTERNAL_ERROR, NULL);
> + xen_vm_set_free(vms);
> + return -1;
> + }
> + xen_vm_set_free(vms);
> + return 0;
> +}
> +
> +
> +/*
> +* xenapiDomainGetOSType
> +*
> +*
> +* Returns OS version on success or NULL in case of error
> +*/
> +static char *
> +xenapiDomainGetOSType (virDomainPtr dom)
> +{
> + /* vm.get_os-version */
> + int i;
> + xen_vm vm;
> + char *os_version=NULL;
> + xen_vm_record *record;
> + xen_string_string_map *result;
> + char uuid[VIR_UUID_STRING_BUFLEN];
> + xen_session *session = ((struct _xenapiPrivate *)(dom->conn->privateData))->session;
> + virUUIDFormat(dom->uuid,uuid);
> + if (xen_vm_get_by_uuid(session, &vm, uuid)) {
> + xen_vm_get_record(session, &record, vm);
> + if (record) {
> + xen_vm_guest_metrics_get_os_version(session, &result, record->guest_metrics->u.handle);
> + if (result) {
> + for (i=0; i<(result->size); i++) {
> + if (STREQ(result->contents[i].key, "distro")) {
> + if (STREQ(result->contents[i].val, "windows")) {
Is distro != windows a good indicator for paravirtualization mode? How
do you detect the case when you have a non-windows system in HVM mode?
> + os_version = strdup("hvm");
OOM check missing.
> + } else {
> + os_version = strdup("xen");
OOM check missing.
> + }
> + }
> + }
> + xen_string_string_map_free(result);
> + } else
> + xenapiSessionErrorHandler(dom->conn, VIR_ERR_NO_OS, NULL);
> + xen_vm_record_free(record);
> + } else
> + xenapiSessionErrorHandler(dom->conn, VIR_ERR_NO_DOMAIN, NULL);
> + xen_vm_free(vm);
> + }
> + else
> + xenapiSessionErrorHandler(dom->conn, VIR_ERR_NO_DOMAIN, NULL);
> + if ( os_version == NULL ) {
> + os_version = strdup("unknown");
OOM check missing.
> + }
> + return os_version;
> +}
> +
> +/*
> +* 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;
> + xen_vm_get_by_name_label(session, &vms, dom->name);
> + if (vms != NULL && vms->size!=0) {
Maybe you should unify the way you check for a valid vm set, because
you do it both ways around.
> + /* vm.get_memory_static_max */
> + 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);
> + } else {
> + xenapiSessionErrorHandler(dom->conn, VIR_ERR_NO_DOMAIN, NULL);
> + return 0;
> + }
> +}
> +
> +
> +/*
> +* xenapiDomainGetInfo:
> +*
> +* Fills a structure with domain information
> +* Returns 0 on success or -1 in case of error
> +*/
> +static int
> +xenapiDomainGetInfo (virDomainPtr dom, virDomainInfoPtr info)
> +{
> + int64_t maxmem=0,memory=0,vcpu=0;
> + xen_vm vm;
> + xen_vm_record *record;
> + xen_vm_set *vms;
> + xen_session *session = ((struct _xenapiPrivate *)(dom->conn->privateData))->session;
> + info->cpuTime = 0; /* CPU time is not advertised */
> + if (xen_vm_get_by_name_label(session, &vms, dom->name)) {
You forget to check for a valid vm set.
> + vm = vms->contents[0];
> + xen_vm_get_memory_static_max(session, &maxmem, vm);
> + info->maxMem = (maxmem/1024);
> + enum xen_vm_power_state state = XEN_VM_POWER_STATE_UNKNOWN;
> + xen_vm_get_power_state(session, &state, vm);
> + info->state = mapPowerState(state);
> + xen_vm_get_record(session, &record, vm);
> + if (record!=NULL) {
> + xen_vm_metrics_get_memory_actual(session, &memory, record->metrics->u.handle);
> + info->memory = (memory/1024);
> + xen_vm_record_free(record);
> + }
> + xen_vm_get_vcpus_max(session, &vcpu, vm);
> + info->nrVirtCpu = vcpu;
> + xen_vm_set_free(vms);
> + return 0;
> + }
> + xenapiSessionErrorHandler(dom->conn, VIR_ERR_NO_DOMAIN, NULL);
> + return -1;
> +}
> +
> +
> +
> +/*
> +* xenapiDomainPinVcpu
> +*
> +* Dynamically change the real CPUs which can be allocated to a virtual CPU
> +* Returns 0 on success or -1 in case of error
> +*/
> +static int
> +xenapiDomainPinVcpu (virDomainPtr dom, unsigned int vcpu,
> + unsigned char *cpumap, int maplen)
> +{
> + char *value;
> + xen_vm vm;
> + xen_vm_set *vms;
> + xen_session *session = ((struct _xenapiPrivate *)(dom->conn->privateData))->session;
> + xen_vm_get_by_name_label(session, &vms, dom->name);
> + if (vms!=NULL && vms->size!=0) {
> + vm = vms->contents[0];
> + value = mapDomainPinVcpu(vcpu, cpumap, maplen);
value leaks.
> + xen_vm_remove_from_vcpus_params(session, vm, (char *)"mask");
> + if (xen_vm_add_to_vcpus_params(session, vm, (char *)"mask", value)) {
> + xen_vm_set_free(vms);
> + return 0;
> + }
> + xenapiSessionErrorHandler(dom->conn, VIR_ERR_INTERNAL_ERROR, NULL);
> + xen_vm_set_free(vms);
> + }
> + xenapiSessionErrorHandler(dom->conn, VIR_ERR_NO_DOMAIN, NULL);
> + return -1;
> +}
> +
> +/*
> +* 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) return -1;
You call should call virReportOOMError in case of an allcoation error.
> + if (virDomainGetInfo(dom,domInfo)==0) {
> + nvcpus = domInfo->nrVirtCpu;
> + VIR_FREE(domInfo);
> + } else {
> + xenapiSessionErrorHandler(dom->conn, VIR_ERR_INTERNAL_ERROR, "Couldn't fetch Domain Information");
> + return -1;
> + }
> + if ( virNodeGetInfo(dom->conn,&nodeInfo)==0)
> + 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)) {
> + xenapiSessionErrorHandler(dom->conn, VIR_ERR_NO_DOMAIN, NULL);
> + return -1;
> + }
Check for a valid vm set is missing.
> + vm = vms->contents[0];
> + if (!xen_vm_get_vcpus_params(session, &vcpu_params, vm)) {
> + 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")) {
> + mask = strdup(vcpu_params->contents[i].val);
Break the for look here an check for OOM error here.
> + }
> + }
> + 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);
> + }
mask leaks.
> + return i;
> +}
> +
> +/*
> +* xenapiDomainGetMaxVcpus
> +*
> +*
> +* Returns maximum number of Vcpus on success or -1 in case of error
> +*/
> +static int
> +xenapiDomainGetMaxVcpus (virDomainPtr dom)
> +{
> + xen_vm vm;
> + xen_vm_set *vms;
> + int64_t maxvcpu=0;
> + enum xen_vm_power_state state;
> + xen_session *session = ((struct _xenapiPrivate *)(dom->conn->privateData))->session;
> + if (xen_vm_get_by_name_label(session, &vms, dom->name)) {
Check for a valid vm set is missing.
> + vm = vms->contents[0];
> + xen_vm_get_power_state(session, &state, vm);
> + if(state == XEN_VM_POWER_STATE_RUNNING) {
> + xen_vm_get_vcpus_max(session, &maxvcpu, vm);
> + } else {
> + maxvcpu = xenapiGetMaxVcpus(dom->conn, NULL);
> + }
> + xen_vm_set_free(vms);
> + return (int)maxvcpu;
> + }
> + xenapiSessionErrorHandler(dom->conn, VIR_ERR_INTERNAL_ERROR, NULL);
> + return -1;
> +}
> +
> +/*
> +* 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)
> +{
> + xen_vm vm=NULL;
> + xen_vm_set *vms;
> + xen_string_string_map *result=NULL;
> + xen_session *session = ((struct _xenapiPrivate *)(dom->conn->privateData))->session;
> + struct _virDomainDef * defPtr = NULL;
Use virDomainDefPtr.
> + if (VIR_ALLOC(defPtr)<0)
virReportOOMError missing.
> + return NULL;
> +
> + xen_vm_get_by_name_label(session, &vms, dom->name);
> + if (vms==NULL || vms->size==0) {
> + xenapiSessionErrorHandler(dom->conn, VIR_ERR_NO_DOMAIN, NULL);
> + return NULL;
> + }
> +
> + vm = vms->contents[0];
> + defPtr->virtType = VIR_DOMAIN_VIRT_XEN;
> + defPtr->id = dom->id;
> + strcpy((char *)defPtr->uuid,(char *)dom->uuid);
You cannot copy a raw UUID using strcpy, becaue it isn't a string an
can contain 0 values. But strcpy will stop at the first 0 value.
Use memcpy(defPtr->uuid, dom->uuid, VIR_UUID_BUFLEN) instead.
> + defPtr->name = strdup(dom->name);
OOM check missing.
> + char *boot_policy=NULL;
> + xen_vm_get_hvm_boot_policy(session, &boot_policy, vm);
> + if (STREQ(boot_policy,"BIOS order")) {
> + defPtr->os.type = strdup("hvm");
OOM check missing.
> + 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;
> + }
> + }
> + xen_string_string_map_free(result);
> + }
> + VIR_FREE(boot_policy);
> + } else {
> + defPtr->os.type = strdup("linux");
Use xen instead of linux.
> + defPtr->os.loader = strdup("pygrub");
OOM checks missing.
> +
> + char *value=NULL;
> + xen_vm_get_pv_kernel(session, &value, vm);
> + if (!STREQ(value,"")) {
> + defPtr->os.kernel = strdup(value);
OOM check missing.
> + VIR_FREE(value);
> + }
> + xen_vm_get_pv_ramdisk(session, &value, vm);
> + if (!STREQ(value,"")) {
> + defPtr->os.initrd = strdup(value);
OOM check missing.
> + VIR_FREE(value);
> + }
> + xen_vm_get_pv_args(session, &value, vm);
> + if (!STREQ(value,"")) {
> + defPtr->os.cmdline = strdup(value);
OOM check missing.
> + VIR_FREE(value);
> + }
> + VIR_FREE(boot_policy);
> + defPtr->os.bootloader = strdup("pygrub");
OOM check missing.
> + }
> + char *val=NULL;
> + xen_vm_get_pv_bootloader_args(session, &val, vm);
> + if (!STREQ(val,"")) {
> + defPtr->os.bootloaderArgs = strdup(val);
OOM check missing.
Also use STRNEQ instead of !STREQ.
> + 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<<0);
Use VIR_DOMAIN_FEATURE_ACPI instead of 0.
> + else if (STREQ(result->contents[i].key,"apic"))
> + defPtr->features = defPtr->features | (1<<1);
Use VIR_DOMAIN_FEATURE_APIC instead of 1.
> + else if (STREQ(result->contents[i].key,"pae"))
> + defPtr->features = defPtr->features | (1<<2);
Use VIR_DOMAIN_FEATURE_PAE instead of 2.
> + }
> + }
> + 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) {
virReportOOMError missing.
> + return NULL;
> + }
> + for (i=0; i<vif_set->size; i++) {
> + if (VIR_ALLOC(defPtr->nets[i])<0) {
virReportOOMError missing.
> + return NULL;
> + }
> + 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 = strdup(bridge);
> + VIR_FREE(bridge);
Why strdup'ing and freeing bridge? Why not assign it directly?
> + }
> + 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);
> + return virDomainDefFormat(defPtr,0);
You're leaking defPtr here. Free it using virDomainDefFree.
> +}
> +
> +/*
> +* 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;
> + usenames = strdup(record->name_label);
OOM check missing.
> + 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);
> + return -1;
> + }
> + }
> + doms=j;
> + xen_vm_set_free(result);
> + return doms;
> + }
> + xenapiSessionErrorHandler(conn, VIR_ERR_NO_DEVICE, NULL);
You're abusing VIR_ERR_NO_DEVICE here.
> + return -1;
> +}
> +
> +/*
> +* xenapiNumOfDefinedDomains
> +*
> +* Provides the number of defined but inactive domains
> +* Returns number of domains found on success or -1 in case of error
> +*/
> +static int
> +xenapiNumOfDefinedDomains (virConnectPtr conn)
> +{
> + xen_vm_set *result;
> + xen_vm_record *record;
> + int DomNum=0,i;
> + xen_session *session = ((struct _xenapiPrivate *)(conn->privateData))->session;
> + xen_vm_get_all(session, &result);
> + if ( result != NULL) {
> + for ( i=0; i< (result->size); i++ ) {
> + xen_vm_get_record(session, &record, result->contents[i]);
> + if (record==NULL && !session->ok) {
> + xenapiSessionErrorHandler(conn, VIR_ERR_INTERNAL_ERROR, NULL);
> + xen_vm_set_free(result);
> + return -1;
> + }
> + if (record->is_a_template == 0)
> + DomNum++;
> + xen_vm_record_free(record);
> + }
> + xen_vm_set_free(result);
> + return DomNum;
> + }
> + xenapiSessionErrorHandler(conn, VIR_ERR_NO_DEVICE, NULL);
You're abusing VIR_ERR_NO_DEVICE here.
> + return -1;
> +}
> +
> +/*
> +* xenapiDomainCreate
> +*
> +* starts a VM
> +* Return 0 on success or -1 in case of error
> +*/
> +static int
> +xenapiDomainCreate (virDomainPtr dom)
> +{
> + xen_vm_set *vms;
> + xen_vm vm;
> + xen_session *session = ((struct _xenapiPrivate *)(dom->conn->privateData))->session;
> + xen_vm_get_by_name_label(session, &vms, dom->name);
> + if (vms!=NULL && vms->size!=0) {
> + vm = vms->contents[0];
> + if (!xen_vm_start(session, vm, false, false)) {
> + xenapiSessionErrorHandler(dom->conn, VIR_ERR_INTERNAL_ERROR, NULL);
> + xen_vm_set_free(vms);
> + return -1;
> + }
> + xen_vm_set_free(vms);
> + } else {
You're potentially leaking vms here, because if vms is not NULL but
vms->size is then you goto the else branch and vms leaks.
I just noticed this pattern here, but this affects probably several functions.
> + xenapiSessionErrorHandler(dom->conn, VIR_ERR_NO_DOMAIN, NULL);
> + return -1;
> + }
> + return 0;
> +}
> +
> +/*
> +* xenapiDomainDefineXML
> +*
> +* Defines a domain from the given XML but does not start it
> +* Returns 0 on success or -1 in case of error
> +*/
> +static virDomainPtr
> +xenapiDomainDefineXML (virConnectPtr conn, const char *xml)
> +{
> + xen_vm_record *record=NULL;
> + xen_vm vm=NULL;
> + virDomainPtr domP=NULL;
> + virCapsPtr caps = getCapsObject();
> + if (!caps)
> + return NULL;
> + virDomainDefPtr defPtr = virDomainDefParseString(caps, xml, 0);
> + if (!defPtr)
> + return NULL;
> + if(createVMRecordFromXml( conn, defPtr, &record, &vm)!=0) {
> + if (!(((struct _xenapiPrivate *)(conn->privateData))->session->ok))
> + xenapiSessionErrorHandler(conn, VIR_ERR_INTERNAL_ERROR, NULL);
> + else
> + xenapiSessionErrorHandler(conn, VIR_ERR_INTERNAL_ERROR, "Couldn't get VM information from XML");
You're leaking defPtr here. Free it using virDomainDefFree.
> + return NULL;
> + }
> + if (record!=NULL) {
> + unsigned char raw_uuid[VIR_UUID_BUFLEN];
> + virUUIDParse(record->uuid,raw_uuid);
> + domP = virGetDomain(conn, record->name_label, raw_uuid);
> + if (domP==NULL && !(((struct _xenapiPrivate *)(conn->privateData))->session->ok))
> + xenapiSessionErrorHandler(conn, VIR_ERR_NO_DOMAIN, NULL);
> + xen_vm_record_free(record);
> + }
> + else if (vm!=NULL)
> + xen_vm_free(vm);
You're leaking defPtr here. Free it using virDomainDefFree.
> + return domP;
> +}
> +
> +/*
> +* xenapiDomainUndefine
> +*
> +* destroys a domain
> +* Return 0 on success or -1 in case of error
> +*/
> +static int
> +xenapiDomainUndefine (virDomainPtr dom)
> +{
> + struct xen_vm_set *vms;
> + xen_vm vm;
> + xen_session *session = ((struct _xenapiPrivate *)(dom->conn->privateData))->session;
> + if (!xen_vm_get_by_name_label(session, &vms, dom->name)) {
> + xenapiSessionErrorHandler(dom->conn, VIR_ERR_NO_DOMAIN, NULL);
> + return -1;
> + }
Again, missing check for a valid vm set.
> + vm = vms->contents[0];
> + if (!xen_vm_destroy(session, vm)) {
> + xenapiSessionErrorHandler(dom->conn, VIR_ERR_INTERNAL_ERROR, NULL);
> + xen_vm_set_free(vms);
> + return -1;
> + }
> + xen_vm_set_free(vms);
> + return 0;
> +}
> +
> +/*
> +* xenapiDomainGetAutostart
> +*
> +* Provides a boolean value indicating whether the domain configured
> +* to be automatically started when the host machine boots
> +* Return 0 on success or -1 in case of error
> +*/
> +static int
> +xenapiDomainGetAutostart (virDomainPtr dom, int *autostart)
> +{
> + int i,flag=0;
> + xen_vm_set *vms;
> + xen_vm vm;
> + xen_string_string_map *result;
> + xen_session *session = ((struct _xenapiPrivate *)(dom->conn->privateData))->session;
> + if(!xen_vm_get_by_name_label(session, &vms, dom->name)) {
> + xenapiSessionErrorHandler(dom->conn, VIR_ERR_NO_DOMAIN, NULL);
> + return -1;
> + }
Missing check for a valid vm set.
> + vm = vms->contents[0];
> + if (!xen_vm_get_other_config(session, &result, vm)) {
> + xenapiSessionErrorHandler(dom->conn, VIR_ERR_INTERNAL_ERROR, NULL);
> + xen_vm_set_free(vms);
> + return -1;
> + }
> + for (i=0; i < result->size; i++) {
> + if (STREQ(result->contents[i].key, "auto_poweron")) {
> + flag=1;
> + if (STREQ(result->contents[i].val, "true"))
> + *autostart = 1;
> + else
> + *autostart = 0;
> + }
> + }
> + xen_vm_set_free(vms);
> + xen_string_string_map_free(result);
> + if (flag==0) return -1;
> + return 0;
> +}
> +
> +/*
> +* xenapiDomainSetAutostart
> +*
> +* Configure the domain to be automatically started when the host machine boots
> +* Return 0 on success or -1 in case of error
> +*/
> +static int
> +xenapiDomainSetAutostart (virDomainPtr dom, int autostart)
> +{
> + xen_vm_set *vms;
> + xen_vm vm;
> + char *value;
> + xen_session *session = ((struct _xenapiPrivate *)(dom->conn->privateData))->session;
> + if (!xen_vm_get_by_name_label(session, &vms, dom->name)) {
> + xenapiSessionErrorHandler(dom->conn, VIR_ERR_NO_DOMAIN, NULL);
> + return -1;
> + }
Missing check for a valid vm set.
> + vm = vms->contents[0];
> + xen_vm_remove_from_other_config(session, vm, (char *)"auto_poweron");
> + if (autostart==1)
> + value = (char *)"true";
> + else
> + value = (char *)"false";
> + if (!xen_vm_add_to_other_config(session, vm, (char *)"auto_poweron", value)) {
> + xenapiSessionErrorHandler(dom->conn, VIR_ERR_INTERNAL_ERROR, NULL);
> + xen_vm_set_free(vms);
> + return -1;
> + }
> + xen_vm_set_free(vms);
> + return 0;
> +}
> +
> +static char *
> +xenapiDomainGetSchedulerType (virDomainPtr dom ATTRIBUTE_UNUSED, int *nparams)
> +{
> + *nparams = 0;
> + return (char *)"credit";
Don't return a const string here. The result is suppoed to be
allocated. You need to strdup it and don't forget the OOM check.
> +}
> +
> +
> +/*
> +* xenapiNodeGetCellsFreeMemory
> +*
> +*
> +* Returns the number of entries filled in freeMems, or -1 in case of error.
> +*/
> +static int
> +xenapiNodeGetCellsFreeMemory (virConnectPtr conn, unsigned long long *freeMems ATTRIBUTE_UNUSED,
> + int startCell, int maxCells)
Remove ATTRIBUTE_UNUSED from freeMems.
> +{
> + if (maxCells >1 && startCell >0) {
> + xenapiSessionErrorHandler(conn, VIR_ERR_NO_SUPPORT, "");
> + return -1;
> + } else {
> + freeMems[0] = xenapiNodeGetFreeMemory(conn);
> + 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;
> + #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, 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, 1);
> + curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 1);
You should set CURLOPT_SSL_VERIFYHOST to 2, with 1 the common name
field is ignored.
Or just don't change the SSL config. libcurl defaults to secure settings.
> + CURLcode result = curl_easy_perform(curl);
> + curl_easy_cleanup(curl);
> + return result;
> +}
> +
> 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-02-26 15:15:55.000000000 +0000
> @@ -0,0 +1,44 @@
> +/*
> + * xenapi_driver_private.h: Xen API driver's private header file.
> + * Copyright (C) 2009 Citrix Ltd.
> + * Sharadha Prabhakar <sharadha.prabhakar@xxxxxxxxxx>
> + */
> +
> +
> +#ifndef __VIR_XENAPI_H__
> +#define __VIR_XENAPI_H__
> +
> +#include <xen/api/xen_common.h>
> +#include <libxml/tree.h>
> +
> +#define PRINT_XML
> +#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;
> +};
> +
> +#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-02-26 15:49:24.000000000 +0000
> @@ -0,0 +1,433 @@
> +/*
> + * xenapi_utils.c: Xen API driver -- utils parts.
> + * Copyright (C) 2009 Citrix Ltd.
> + * Sharadha Prabhakar <sharadha.prabhakar@xxxxxxxxxx>
> + */
> +
> +
> +/* converts bitmap to string of the form '1,2...' */
> +char *
> +mapDomainPinVcpu(unsigned int vcpu, unsigned char *cpumap, int maplen)
> +{
> + char buf[VIR_UUID_BUFLEN], mapstr[sizeof(cpumap_t) * 64];
> + char *ret=NULL;
> + int i, j;
> + mapstr[0] = 0;
> + for (i = 0; i < maplen; i++) {
> + for (j = 0; j < 8; j++) {
> + if (cpumap[i] & (1 << j)) {
> + snprintf(buf, sizeof(buf), "%d,", (8 * i) + j);
> + strcat(mapstr, buf);
Use the virBuffer API instea of snprintf and strcat.
> + }
> + }
> + }
> + mapstr[strlen(mapstr) - 1] = 0;
> + snprintf(buf, sizeof(buf), "%d", vcpu);
> + ret = strdup(mapstr);
Use virAsprintf instead of snprintf and strdup.
> + return ret;
> +}
> +
> +
> +/* 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)
> + return -1;
> + (*strings)->size = sz;
> + (*strings)->contents[sz-1].key = strdup(key);
> + (*strings)->contents[sz-1].val = strdup(val);
OOM check missing.
> + 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);
> + }
> +}
> +
> +
> +/* Create a VM record from the XML description */
> +int
> +createVMRecordFromXml (virConnectPtr conn, virDomainDefPtr def,
> + xen_vm_record **record, xen_vm *vm)
> +{
> + *record = xen_vm_record_alloc();
> + (*record)->name_label = strdup(def->name);
OOM check missing.
As mentioned before, you're forgot to copy the UUID here.
> + if (STREQ(def->os.type,"hvm")) {
> + (*record)->hvm_boot_policy = strdup("BIOS order");
OOM check missing.
> + 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")) {
> + (*record)->pv_bootloader = strdup("pygrub");
OOM check missing.
> + (*record)->pv_kernel = def->os.kernel;
> + (*record)->pv_ramdisk = def->os.initrd;
> + (*record)->pv_args = def->os.cmdline;
> + (*record)->hvm_boot_params = xen_string_string_map_alloc(0);
> + }
> + if (def->os.bootloaderArgs)
> + (*record)->pv_bootloader_args = def->os.bootloaderArgs;
> +
> + 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<<0))
> + allocStringMap(&strings,(char *)"acpi",(char *)"true");
> + if (def->features & (1<<1))
> + allocStringMap(&strings,(char *)"apci",(char *)"true");
> + if (def->features & (1<<2))
> + allocStringMap(&strings,(char *)"pae",(char *)"true");
> + }
Use the virDomainFeature enum values here too.
> + 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)
> + bridge = strdup(def->nets[i]->data.bridge.brname);
OOM check missing.
> + if (def->nets[i]->mac) {
> + char macStr[VIR_MAC_STRING_BUFLEN];
> + virFormatMacAddr(def->nets[i]->mac, macStr);
> + mac = strdup(macStr);
OOM check missing.
> + }
> + if (mac!=NULL && bridge!=NULL) {
> + char device[NETWORK_DEVID_SIZE]="\0";
> + sprintf(device,"%d",device_number);
> + createVifNetwork(conn, *vm, device, bridge, mac);
> + VIR_FREE(bridge);
> + device_number++;
> + }
You need to free bridge here in the case mac is NULL but bridge is not.
> + }
> + }
> + return 0;
> +}
> +
Matthias
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list