Re: [libvirt] [PATCH 3/4] Addition of XenAPI support to libvirt

On Fri, Feb 19, 2010 at 11:23:32AM +0000, Sharadha Prabhakar (3P) wrote: The xenapiSessionErrorHandler function should really be declared as a macro that automatically adds FILE/FUNCTION/LINE, rather than requiring all callers todo this. For example, the QEMU driver defines a simple macro for error reporting that looks like this: #define qemuReportError(code, fmt...) \ virReportErrorHelper(NULL, VIR_FROM_QEMU, code, __FILE__, \ __FUNCTION__, __LINE__, fmt) Also, as of last week, we no longer recommend passing 'conn' into error handlers. We've not updated all existing code to follow this scheme, but you can just leave it out of your new code. Don't encode the password in the URI, since this is very insecure - the URIs get logged everywhere. The 'virConnectAuthPtr auth' parameter passed to this method provides a way for your driver to prompt for a password. If you want a simple example of how to use this parameter for collecting a password have a look at the esxUtil_RequestPassword() method in the src/esx/esx_util.c method - I expect it does exactly what you will need. The libvirt coding standards don't allow the use of malloc(), realloc() or free() since this APIs are very error-prone. Take a look at the HACKING file for the 'Low level memory management' section which describes our replacements VIR_ALLOC/REALLOC/FREE. In this particular case though, since you're building up a string, just call virAsprintf(), eg virAsprintf(&url, "https://"%s/", conn->uri->sever) Same note about malloc() as above - in this case you'll want VIR_ALLOC(privP) This should really be the Xen hypervisor version And this should return the XenAPI library version These days, we don't let the drivers format or parse any XML directly. Instead there are standard APIs that drivers can call out to, which are all under the src/conf/ directory. In this case you'll want the capabilities.h file, which lets you construct a virCapsPtr object. Once populatd, you can serialize it to XML format using virCapabilitiesFormatXML(). Also, the domain type should be 'xen' rather than 'xenapi', since this refers to the hypervisor type, not the API type. Rather than repeatedly casting conn->privateData at every usage, the common style in libvirt drivers is to declare it as the first variable at entry to the function, eg struct _xenapiPrivate *priv = conn->privateData; then just refer to priv->session where needed during the function/ > + ids[i] = (int)(t0 & 0xffffffff); > + } > + list = result->size; > + xen_vm_set_free(result); > + return list; > + } > + return -1; As with the capabilities API, we have standard API for parsing the domain XML format too. So this code would just do something like virDomainDefPtr def = virDomainDefParseString(caps, xmlDesc, flags); then pass 'def' into your createVMRecordFromXml() method instead of the raw XML file. Once you've using the 'virDomainDefPtr' object you'll not need this since you can just directly use def->uuid which is already in raw format. > + if (vm!=NULL) { > + if (xen_vm_start(((struct _xenapiPrivate *)(conn->privateData))->session, > + vm, false, false)) { > + domP = virGetDomain(conn, record->name_label, raw_uuid); > + xen_vm_free(vm); > + } > + else > + xenapiSessionErrorHandler(conn, VIR_ERR_INTERNAL_ERROR, NULL, __FILE__, __FUNCTION__, __LINE__); > + } > + xen_vm_record_free(record); > + } > + else > + xenapiSessionErrorHandler(conn, VIR_ERR_INTERNAL_ERROR, NULL, __FILE__, __FUNCTION__, __LINE__); > + return domP; > + > + > +/* > +* 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]; > + virUUIDFormat(dom->uuid,uuid); > + if (xen_vm_get_by_uuid(((struct _xenapiPrivate *)(dom->conn->privateData))->session, > + &vm, uuid)) { > + xen_vm_get_record(((struct _xenapiPrivate *)(dom->conn->privateData))->session, > + &record, vm); > + if (record!=NULL) { > + xen_vm_guest_metrics_get_os_version(((struct _xenapiPrivate *)(dom->conn->privateData))->session, &result, > + record->guest_metrics->u.handle); > + if (result != NULL) { > + for (i=0; i<(result->size); i++) { > + if (STREQ(result->contents[i].key, "distro")) { > + if (STREQ(result->contents[i].val, "windows")) { > + os_version = strdup(result->contents[i].val); > + } else { > + os_version = strdup("linux"); > + } > + } > + } > + xen_string_string_map_free(result); > + } else > + xenapiSessionErrorHandler(dom->conn, VIR_ERR_NO_OS, NULL, __FILE__, __FUNCTION__, __LINE__); > + xen_vm_record_free(record); > + } else > + xenapiSessionErrorHandler(dom->conn, VIR_ERR_NO_DOMAIN, NULL, __FILE__, __FUNCTION__, __LINE__); > + xen_vm_free(vm); > + } > + else > + xenapiSessionErrorHandler(dom->conn, VIR_ERR_NO_DOMAIN, NULL, __FILE__, __FUNCTION__, __LINE__); > + if ( os_version == NULL ) { > + os_version = strdup("unknown"); > + } > + return os_version; > +} This libvirt method / API actually has a rather misleading name. It is not wanting the guest operating system name, but rather the guest ABI. Which in case of Xen is either 'xen' (paravirt) or 'hvm' (fullvirt). For any method not supported by XenAPI, simply leave it out of your driver altogether - put a NULL in the virDriver struct entry at the end of your file. libvirt will then automatically report a VIR_ERR_NO_SUPPORT error. The 'to' parameter shouldn't be unused here. The guest state should be saved to this named file. If XenAPI doesn't support saving to an explicit named file, then just leave out this libvirt driver API. In the future we will add a new save/restore API that doesn't require a named file. > +{ > + int ret_code = -1; > + ret_code = xenapiDomainSuspend(dom); > + return ret_code; > +/* > +* xenapiDomainRestore > +* > +* Resumes a VM > +* Return 0 on success or -1 in case of error > +*/ > +static int > +xenapiDomainRestore (virConnectPtr conn, const char *from ATTRIBUTE_UNUSED) Same comment here as with save - it must restore state from this named file. Otherwise leave out this method. > +{ > + /* resume from : NI */ > + xen_vm_set *result=NULL; > + xen_host host=NULL; > + xen_vm_record *record = NULL; > + unsigned char raw_uuid[RAW_UUID_BUFLEN]; > + virDomainPtr domP=NULL; > + int ret_code=-1; > + > + xen_session_get_this_host(((struct _xenapiPrivate *)(conn->privateData))->session, &host, > + ((struct _xenapiPrivate *)(conn->privateData))->session); > + if ( host!=NULL ) { > + xen_host_get_resident_vms(((struct _xenapiPrivate *)(conn->privateData))->session, &result, host); > + if ( result != NULL ) { > + xen_vm_get_record(((struct _xenapiPrivate *)(conn->privateData))->session, > + &record, result->contents[0]); > + if (record!=NULL) { > + virUUIDParse(record->uuid,raw_uuid); > + domP = virGetDomain(conn, record->name_label, raw_uuid); > + if (domP!=NULL) > + ret_code = xenapiDomainResume(domP); > + else > + xenapiSessionErrorHandler(conn, VIR_ERR_NO_DOMAIN, NULL, __FILE__, __FUNCTION__, __LINE__); > + xen_vm_record_free(record); > + } > + xen_vm_set_free(result); > + } > + xen_host_free(host); > + } > + if (!(((struct _xenapiPrivate *)(conn->privateData))->session->ok)) > + xenapiSessionErrorHandler(conn, VIR_ERR_NO_DOMAIN, NULL, __FILE__, __FUNCTION__, __LINE__); > + return ret_code; > +static int > +xenapiDomainCoreDump (virDomainPtr dom, const char *to ATTRIBUTE_UNUSED, int flags ATTRIBUTE_UNUSED) > +{ > + xenapiSessionErrorHandler(dom->conn, VIR_ERR_NO_SUPPORT, NULL, __FILE__, __FUNCTION__, __LINE__); > + 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) > +{ > + virBuffer buf = VIR_BUFFER_INITIALIZER; > + char uuid_string[VIR_UUID_STRING_BUFLEN]; > + xen_vm vm=NULL; > + xen_vm_set *vms; > + xen_string_string_map *result=NULL; > + > + xen_vm_get_by_name_label(((struct _xenapiPrivate *)(dom->conn->privateData))->session, > + &vms, dom->name); > + if (vms==NULL || vms->size==0) { > + xenapiSessionErrorHandler(dom->conn, VIR_ERR_NO_DOMAIN, NULL, __FILE__, __FUNCTION__, __LINE__); > + return NULL; > + } > + > + vm = vms->contents[0]; > + virBufferAddLit(&buf, "<domain type='xenapi'>\n"); > + virBufferEscapeString(&buf, " <name>%s</name>\n", "testVM"); > + virUUIDFormat(dom->uuid,uuid_string); > + virBufferEscapeString(&buf, " <uuid>%s</uuid>\n",uuid_string); As mentioned earlier, this method shouldn't be writing out XML directly. What you want todo is build a virDomainDefPtr object with all the data. Then just call virDomainDefFormat() to generate the XML document. > + > + > + virBufferAddLit(&buf, " <os>\n"); > + char *boot_policy=NULL; > + xen_vm_get_hvm_boot_policy(((struct _xenapiPrivate *)(dom->conn->privateData))->session, > + &boot_policy, vm); > + if (STREQ(boot_policy,"BIOS order")) { > + virBufferAddLit(&buf, " <type>hvm</type>\n"); > + xen_vm_get_hvm_boot_params(((struct _xenapiPrivate *)(dom->conn->privateData))->session, > + &result, vm); > + if (result!=NULL) { > + int i; > + for (i=0; i<(result->size); i++) { > + if (STREQ(result->contents[i].key,"order")) { > + int j=0; > + while(result->contents[i].val[j]!='\0') { > + virBufferEscapeString(&buf, " <boot dev='%s' />\n", > + mapXmlBootOrder(result->contents[i].val[j])); > + j++; > + } > + } > + } > + xen_string_string_map_free(result); > + } > + VIR_FREE(boot_policy); > + } else { > + virBufferAddLit(&buf, " <type>linux</type>\n"); > + virBufferAddLit(&buf, " <loader>pygrub</loader>\n"); > + char *value=NULL; > + xen_vm_get_pv_kernel(((struct _xenapiPrivate *)(dom->conn->privateData))->session, > + &value, vm); > + if (!STREQ(value,"")) { > + virBufferEscapeString(&buf," <kernel>%s</kernel>\n",value); > + VIR_FREE(value); > + } > + xen_vm_get_pv_ramdisk(((struct _xenapiPrivate *)(dom->conn->privateData))->session, > + &value, vm); > + if (!STREQ(value,"")) { > + virBufferEscapeString(&buf," <initrd>%s</initrd>\n",value); > + VIR_FREE(value); > + } > + xen_vm_get_pv_args(((struct _xenapiPrivate *)(dom->conn->privateData))->session, > + &value, vm); > + if (!STREQ(value,"")) { > + virBufferEscapeString(&buf," <cmdline>%s</cmdline>\n",value); > + VIR_FREE(value); > + } > + VIR_FREE(boot_policy); > + } > + virBufferAddLit(&buf, " </os>\n"); > + virBufferAddLit(&buf, " <bootloader>pygrub</bootloader>\n"); > + char *val=NULL; > + xen_vm_get_pv_bootloader_args(((struct _xenapiPrivate *)(dom->conn->privateData))->session, > + &val, vm); > + if (!STREQ(val,"")) { > + virBufferEscapeString(&buf," <bootloader_args>%s</bootloader_args>\n",val); > + VIR_FREE(val); > + } > + unsigned long memory=0; > + memory = xenapiDomainGetMaxMemory(dom); > + virBufferVSprintf(&buf," <memory>%lu</memory>\n",memory); > + int64_t dynamic_mem=0; > + if (xen_vm_get_memory_dynamic_max(((struct _xenapiPrivate *)(dom->conn->privateData))->session, > + &dynamic_mem, vm)) { > + virBufferVSprintf(&buf," <currentmemory>%lld</currentmemory>\n",(dynamic_mem/1024)); > + } else { > + virBufferVSprintf(&buf," <currentmemory>%lu</currentmemory>\n",memory); > + } > + virBufferVSprintf(&buf," <vcpu>%d</vcpu>\n",xenapiDomainGetMaxVcpus(dom)); > + enum xen_on_normal_exit action; > + if (xen_vm_get_actions_after_shutdown(((struct _xenapiPrivate *)(dom->conn->privateData))->session, > + &action, vm)) { > + virBufferEscapeString(&buf," <on_poweroff>%s</on_poweroff>\n",xen_on_normal_exit_to_string(action)); > + } > + if (xen_vm_get_actions_after_reboot(((struct _xenapiPrivate *)(dom->conn->privateData))->session, > + &action, vm)) { > + virBufferEscapeString(&buf," <on_reboot>%s</on_reboot>\n",xen_on_normal_exit_to_string(action)); > + } > + enum xen_on_crash_behaviour crash; > + if (xen_vm_get_actions_after_crash(((struct _xenapiPrivate *)(dom->conn->privateData))->session, &crash, vm)) { > + virBufferEscapeString(&buf," <on_crash>%s</on_crash>\n",xen_on_crash_behaviour_to_string(crash)); > + } > + xen_vm_get_platform(((struct _xenapiPrivate *)(dom->conn->privateData))->session, > + &result, vm); > + if (result!=NULL) { > + int i; > + virBufferAddLit(&buf, " <features>\n"); > + for(i=0; i< (result->size); i++) { > + if (STREQ(result->contents[i].val,"true")) { > + virBufferVSprintf(&buf," <%s/>\n",result->contents[i].key); > + } > + } > + virBufferAddLit(&buf, " </features>\n"); > + xen_string_string_map_free(result); > + } > + struct xen_vif_set *vif_set=NULL; > + xen_vm_get_vifs(((struct _xenapiPrivate *)(dom->conn->privateData))->session, > + &vif_set, vm); > + if (vif_set) { > + int i; > + xen_vif vif; > + xen_vif_record *vif_rec=NULL; > + xen_network network; > + char *bridge=NULL; > + for (i=0; i<vif_set->size; i++) { > + virBufferAddLit(&buf, " <interface type='bridge'>\n"); > + vif = vif_set->contents[i]; > + xen_vif_get_network(((struct _xenapiPrivate *)(dom->conn->privateData))->session, > + &network, vif); > + if (network!=NULL) { > + xen_network_get_bridge(((struct _xenapiPrivate *)(dom->conn->privateData))->session, > + &bridge, network); > + if (bridge!=NULL) { > + virBufferEscapeString(&buf," <source bridge='%s' />\n",bridge); > + VIR_FREE(bridge); > + } > + xen_network_free(network); > + } > + xen_vif_get_record(((struct _xenapiPrivate *)(dom->conn->privateData))->session, > + &vif_rec, vif); > + if (vif_rec!=NULL) { > + virBufferEscapeString(&buf," <mac address='%s' />\n", vif_rec->mac); > + xen_vif_record_free(vif_rec); > + } > + virBufferAddLit(&buf, " </interface>\n"); > + } > + xen_vif_set_free(vif_set); > + } > + virBufferAddLit(&buf, "</domain>"); > + if (vms) xen_vm_set_free(vms); > + return virBufferContentAndReset(&buf); > +static char * > +xenapiDomainXMLFromNative(virConnectPtr conn, > + const char *format ATTRIBUTE_UNUSED, > + const char *config ATTRIBUTE_UNUSED, > + unsigned int flags ATTRIBUTE_UNUSED) > +{ > + xenapiSessionErrorHandler(conn, VIR_ERR_NO_SUPPORT, "", __FILE__, __FUNCTION__, __LINE__); > + return NULL; > + > +static char * > +xenapiDomainXMLToNative(virConnectPtr conn, > + const char *format ATTRIBUTE_UNUSED, > + const char *xmlData ATTRIBUTE_UNUSED, > + unsigned int flags ATTRIBUTE_UNUSED) > +{ > + xenapiSessionErrorHandler(conn, VIR_ERR_NO_SUPPORT, "", __FILE__, __FUNCTION__, __LINE__); > + return NULL; > +} Just leave these out & libvirt automatically report VIR_ERR_NO_SUPPORT You shouldn't disable SSL verification here. It should be on by default. For consistency with other libvirt drivers, you may wish to allow your xenapi:/// uri to have an optional 'no_verify=1' parameter to turn off SSL verification. It should always be verifying by default though. > + 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-18 15:37:49.000000000 +0000 > @@ -0,0 +1,44 @@ > +/* > + * xenapi.c: Xen API driver. > + * Copyright (C) 2009 Citrix Ltd. > + * Sharadha Prabhakar <sharadha.prabhakar(a)citrix.com&gt; > + */ > + > + > +#ifndef __VIR_XENAPI_H__ > +#define __VIR_XENAPI_H__ > + > +#include <xen/api/xen_common.h> > +#include <libxml/tree.h> > + > +//#define PRINT_XML > +#define RAW_UUID_BUFLEN (16) This one already exists as VIR_UUID_BUFLEN I think those two can be left out & just declared static in the source file, since they're not used externally. Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|