[libvirt] PATCH: Switch openvz driver to domain APIs
This patch is the minimum effort port of the OpenVZ driver over to the new generic domain XML routines. It basically removes all the existing config format stuff from openvz_conf, defines a set of openvz capabilities and implements the DumpXML method too. This is enough to get all the core libvirt functions working. The biggest flaw I see currently is that the openvz driver doesn't load the existing device config for networks or filesystems of existing VMs, so you can't see that info in the XML dump Daniel diff -r e270be59a80f src/openvz_conf.c --- a/src/openvz_conf.c Wed Aug 27 17:03:25 2008 +0100 +++ b/src/openvz_conf.c Wed Aug 27 17:26:08 2008 +0100 @@ -40,39 +40,29 @@ #include <limits.h> #include <errno.h> #include <string.h> - -#include <libxml/parser.h> -#include <libxml/tree.h> -#include <libxml/xpath.h> -#include <libxml/uri.h> - -#include "internal.h" - -#include "openvz_driver.h" +#include <sys/utsname.h> + #include "openvz_conf.h" #include "uuid.h" #include "buf.h" #include "memory.h" #include "util.h" -#include "xml.h" -#include "domain_conf.h" static char *openvzLocateConfDir(void); -static struct openvz_vm_def *openvzParseXML(virConnectPtr conn, xmlDocPtr xml); static int openvzGetVPSUUID(int vpsid, char *uuidstr); -static int openvzSetUUID(int vpsid); static int openvzLocateConfFile(int vpsid, char *conffile, int maxlen); +static int openvzAssignUUIDs(void); void openvzError (virConnectPtr conn, virErrorNumber code, const char *fmt, ...) { va_list args; - char errorMessage[OPENVZ_MAX_ERROR_LEN]; + char errorMessage[1024]; const char *errmsg; if (fmt) { va_start(args, fmt); - vsnprintf(errorMessage, OPENVZ_MAX_ERROR_LEN-1, fmt, args); + vsnprintf(errorMessage, sizeof(errorMessage)-1, fmt, args); va_end(args); } else { errorMessage[0] = '\0'; @@ -84,46 +74,6 @@ openvzError (virConnectPtr conn, virErro errmsg, errorMessage); } -struct openvz_vm -*openvzFindVMByID(const struct openvz_driver *driver, int id) { - struct openvz_vm *vm = driver->vms; - - while (vm) { - if (vm->vpsid == id) - return vm; - vm = vm->next; - } - - return NULL; -} - -struct openvz_vm -*openvzFindVMByUUID(const struct openvz_driver *driver, - const unsigned char *uuid) { - struct openvz_vm *vm = driver->vms; - - while (vm) { - if (!memcmp(vm->vmdef->uuid, uuid, VIR_UUID_BUFLEN)) - return vm; - vm = vm->next; - } - - return NULL; -} - -struct openvz_vm -*openvzFindVMByName(const struct openvz_driver *driver, - const char *name) { - struct openvz_vm *vm = driver->vms; - - while (vm) { - if (STREQ(vm->vmdef->name, name)) - return vm; - vm = vm->next; - } - - return NULL; -} int strtoI(const char *str) @@ -135,6 +85,43 @@ strtoI(const char *str) return val; } + +virCapsPtr openvzCapsInit(void) +{ + struct utsname utsname; + virCapsPtr caps; + virCapsGuestPtr guest; + + uname(&utsname); + + if ((caps = virCapabilitiesNew(utsname.machine, + 0, 0)) == NULL) + goto no_memory; + + if ((guest = virCapabilitiesAddGuest(caps, + "exe", + utsname.machine, + sizeof(int) == 4 ? 32 : 8, + NULL, + NULL, + 0, + NULL)) == NULL) + goto no_memory; + + if (virCapabilitiesAddGuestDomain(guest, + "openvz", + NULL, + NULL, + 0, + NULL) == NULL) + goto no_memory; + return caps; + +no_memory: + virCapabilitiesFree(caps); + return NULL; +} + /* function checks MAC address is empty return 0 - empty @@ -164,438 +151,105 @@ char *openvzMacToString(const unsigned c return strdup(str); } -void -openvzRemoveInactiveVM(struct openvz_driver *driver, struct openvz_vm *vm) -{ - driver->num_inactive--; - openvzFreeVM(driver, vm, 1); -} - -/* Free all memory associated with a openvz_vm_def structure */ -void -openvzFreeVMDef(struct openvz_vm_def *def) -{ - if (def) { - virDomainNetDefFree(def->net); - } -} - -/* Free all memory associated with a openvz_vm structure - * @checkCallee == 0 then openvzFreeDriver() is callee else some other function - */ -void -openvzFreeVM(struct openvz_driver *driver, struct openvz_vm *vm, - int checkCallee) -{ - struct openvz_vm *vms; - - if (!vm && !driver) - return; - vms = driver->vms; - if (checkCallee) { - if (vms == vm) - driver->vms = vm->next; - else { - while (vms) { - struct openvz_vm *prev = vms; - - vms = vms->next; - if (vms == vm) { - prev->next = vms->next; - break; - } - } - } - } - if (vms) { - openvzFreeVMDef(vm->vmdef); - VIR_FREE(vm); - } -} - /* Free all memory associated with a openvz_driver structure */ void openvzFreeDriver(struct openvz_driver *driver) { - struct openvz_vm *next; - + virDomainObjPtr dom; + if (!driver) return; - if (driver->vms) - for(next = driver->vms->next; driver->vms; driver->vms = next) - openvzFreeVM(driver, driver->vms, 0); - VIR_FREE(driver); -} - -struct openvz_vm * -openvzAssignVMDef(virConnectPtr conn, - struct openvz_driver *driver, struct openvz_vm_def *def) -{ - struct openvz_vm *vm = NULL; - - if (!driver || !def) - return NULL; - - if ((vm = openvzFindVMByName(driver, def->name))) { - if (!openvzIsActiveVM(vm)) { - openvzFreeVMDef(vm->vmdef); - vm->vmdef = def; - } - else - { - openvzLog(OPENVZ_ERR, - _("Error already an active OPENVZ VM having id '%s'"), - def->name); - openvzFreeVMDef(def); - return NULL; /* can't redefine an active domain */ - } - - return vm; - } - - if (VIR_ALLOC(vm) < 0) { - openvzFreeVMDef(def); - openvzError(conn, VIR_ERR_NO_MEMORY, _("vm")); - return NULL; - } - - vm->vpsid = -1; /* -1 needed for to represent inactiveness of domain before 'start' */ - vm->status = VIR_DOMAIN_SHUTOFF; - vm->vmdef = def; - vm->next = driver->vms; - - driver->vms = vm; - driver->num_inactive++; - - return vm; -} - -struct openvz_vm_def -*openvzParseVMDef(virConnectPtr conn, - const char *xmlStr, const char *displayName) -{ - xmlDocPtr xml; - struct openvz_vm_def *def = NULL; - - xml = xmlReadDoc(BAD_CAST xmlStr, displayName ? displayName : "domain.xml", NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | XML_PARSE_NOERROR | XML_PARSE_NOWARNING); - if (!xml) { - openvzError(conn, VIR_ERR_XML_ERROR, NULL); - return NULL; - } - - def = openvzParseXML(conn, xml); - xmlFreeDoc(xml); - - return def; -} - -/* Parse filesystem section -Sample: -<filesystem type="template"> - <source name="fedora-core-5-i386"/> - <quota type="size" max="10000"/> - <quota type="inodes" max="100"/> -</filesystem> -*/ -static int openvzParseDomainFS(virConnectPtr conn, - struct openvz_fs_def *fs, - xmlXPathContextPtr ctxt) -{ - xmlNodePtr cur, obj; - char *type = NULL; - int n; - xmlNodePtr *nodes = NULL; - - - if ((n = virXPathNodeSet(conn, "/domain/devices/filesystem", - ctxt, &nodes)) < 0) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, - _("missing filesystem tag")); - goto error; - } - - if (n > 1) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, - _("There should be only one filesystem tag")); - goto error; - } - - obj = nodes[0]; - - /*check template type*/ - type = virXMLPropString(obj, "type"); - if (type == NULL) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, - _("missing type attribute")); - goto error; - } - - if (STRNEQ(type, "template")) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, - _("Unknown type attribute %s"), type); - goto error; - } - VIR_FREE(type); - - cur = obj->children; - while(cur != NULL) - { - if (cur->type == XML_ELEMENT_NODE) { - if (xmlStrEqual(cur->name, BAD_CAST "source")) { - char * name = virXMLPropString(cur, "name"); - - if (name != NULL) { - strncpy(fs->tmpl, name,sizeof(fs->tmpl)); - fs->tmpl[sizeof(fs->tmpl) - 1] = '\0'; - } - VIR_FREE(name); - } else if (xmlStrEqual(cur->name, BAD_CAST "quota")) { - char * qtype = virXMLPropString(cur, "type"); - char * max = virXMLPropString(cur, "max"); - - if (qtype != NULL && STREQ(qtype, "size") && max != NULL) - fs->disksize = strtoI(max); - else if (qtype != NULL && STREQ(qtype, "inodes") && max != NULL) - fs->diskinodes = strtoI(max); - VIR_FREE(qtype); - VIR_FREE(max); - } - } - cur = cur->next; - } - VIR_FREE(nodes); - - return 0; - - error: - VIR_FREE(nodes); - VIR_FREE(type); - - return -1; -} - - -/* - * Parses a libvirt XML definition of a guest, and populates the - * the openvz_vm struct with matching data about the guests config - */ -static struct openvz_vm_def -*openvzParseXML(virConnectPtr conn, - xmlDocPtr xml) { - xmlNodePtr root = NULL; - char *prop = NULL; - xmlXPathContextPtr ctxt = NULL; - xmlXPathObjectPtr obj = NULL; - struct openvz_vm_def *def = NULL; - xmlNodePtr *nodes = NULL; - int i, n; - - if (VIR_ALLOC(def) < 0) { - openvzError(conn, VIR_ERR_NO_MEMORY, _("xmlXPathContext")); - return NULL; - } - - /* Prepare parser / xpath context */ - root = xmlDocGetRootElement(xml); - if ((root == NULL) || (!xmlStrEqual(root->name, BAD_CAST "domain"))) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, _("incorrect root element")); - goto bail_out; - } - - ctxt = xmlXPathNewContext(xml); - if (ctxt == NULL) { - openvzError(conn, VIR_ERR_NO_MEMORY, _("xmlXPathContext")); - goto bail_out; - } - ctxt->node = root; - - /* Find out what type of OPENVZ virtualization to use */ - if (!(prop = virXMLPropString(root, "type"))) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, _("missing domain type attribute")); - goto bail_out; - } - - if (STRNEQ(prop, "openvz")){ - openvzError(conn, VIR_ERR_INTERNAL_ERROR, _("invalid domain type attribute")); - goto bail_out; - } - VIR_FREE(prop); - - /* Extract domain name */ - obj = xmlXPathEval(BAD_CAST "string(/domain/name[1])", ctxt); - if ((obj == NULL) || (obj->type != XPATH_STRING) || - (obj->stringval == NULL) || (obj->stringval[0] == 0)) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, _("invalid domain name")); - goto bail_out; - } - - /* rejecting VPS ID <= OPENVZ_RSRV_VM_LIMIT for they are reserved */ - if (strtoI((const char *) obj->stringval) <= OPENVZ_RSRV_VM_LIMIT) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, - _("VPS ID Error (must be an integer greater than 100")); - goto bail_out; - } - strncpy(def->name, (const char *) obj->stringval, OPENVZ_NAME_MAX); - xmlXPathFreeObject(obj); - obj = NULL; - - /* Extract domain uuid */ - prop = virXPathString(conn, "string(./uuid[1])", ctxt); - if (!prop) { - int err; - if ((err = virUUIDGenerate(def->uuid))) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, - _("Failed to generate UUID: %s"), - strerror(err)); - goto bail_out; - } - } else { - if (virUUIDParse(prop, def->uuid) < 0) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, - _("malformed uuid element")); - goto bail_out; - } - VIR_FREE(prop); - } - - /* extract virtual CPUs */ - if (virXPathULong(conn, "string(./vcpu[1])", ctxt, &def->vcpus) < 0) - def->vcpus = 0; //use default CPUs count - - /* Extract filesystem info */ - if (openvzParseDomainFS(conn, &(def->fs), ctxt)) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, - _("malformed filesystem tag")); - goto bail_out; - } - - /* analysis of the network devices */ - if ((n = virXPathNodeSet(conn, "/domain/devices/interface", - ctxt, &nodes)) < 0) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot extract network devices")); - goto bail_out; - } - - for (i = n - 1 ; i >= 0 ; i--) { - virDomainNetDefPtr net = virDomainNetDefParseXML(conn, - nodes[i]); - if (!net) - goto bail_out; - - net->next = def->net; - def->net = net; - } - VIR_FREE(nodes); - - xmlXPathFreeContext(ctxt); - return def; - - bail_out: - VIR_FREE(prop); - xmlXPathFreeObject(obj); - xmlXPathFreeContext(ctxt); - openvzFreeVMDef(def); - - return NULL; -} - -struct openvz_vm * -openvzGetVPSInfo(virConnectPtr conn) { + + dom = driver->domains; + while (dom) { + virDomainObjPtr tmp = dom->next; + virDomainObjFree(dom); + dom = tmp; + } + + virCapabilitiesFree(driver->caps); +} + + + +int openvzLoadDomains(struct openvz_driver *driver) { FILE *fp; int veid, ret; char status[16]; char uuidstr[VIR_UUID_STRING_BUFLEN]; - struct openvz_vm *vm; - struct openvz_vm **pnext; - struct openvz_driver *driver; - struct openvz_vm_def *vmdef; - char temp[124]; - - vm = NULL; - driver = conn->privateData; - driver->num_active = 0; - driver->num_inactive = 0; - - if((fp = popen(VZLIST " -a -ovpsid,status -H 2>/dev/null", "r")) == NULL) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, _("popen failed")); - return NULL; - } - pnext = &vm; + virDomainObjPtr dom = NULL, prev = NULL; + + if (openvzAssignUUIDs() < 0) + return -1; + + if ((fp = popen(VZLIST " -a -ovpsid,status -H 2>/dev/null", "r")) == NULL) { + openvzError(NULL, VIR_ERR_INTERNAL_ERROR, _("popen failed")); + return -1; + } + while(!feof(fp)) { - if (VIR_ALLOC(*pnext) < 0) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, _("calloc failed")); - goto error; - } - - if(!vm) - vm = *pnext; - if (fscanf(fp, "%d %s\n", &veid, status) != 2) { if (feof(fp)) break; - - openvzError(conn, VIR_ERR_INTERNAL_ERROR, - _("Failed to parse vzlist output")); - goto error; - } - if(STRNEQ(status, "stopped")) { - (*pnext)->status = VIR_DOMAIN_RUNNING; - driver->num_active ++; - (*pnext)->vpsid = veid; - } - else { - (*pnext)->status = VIR_DOMAIN_SHUTOFF; - driver->num_inactive ++; - /* - * inactive domains don't have their ID set in libvirt, - * thought this doesn't make sense for OpenVZ - */ - (*pnext)->vpsid = -1; - } - - if (VIR_ALLOC(vmdef) < 0) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, _("calloc failed")); - goto error; - } - - snprintf(vmdef->name, OPENVZ_NAME_MAX, "%i", veid); + + openvzError(NULL, VIR_ERR_INTERNAL_ERROR, + _("Failed to parse vzlist output")); + goto cleanup; + } + + if (VIR_ALLOC(dom) < 0 || + VIR_ALLOC(dom->def) < 0) + goto no_memory; + + if (STRNEQ(status, "stopped")) + dom->state = VIR_DOMAIN_RUNNING; + else + dom->state = VIR_DOMAIN_SHUTOFF; + + dom->pid = veid; + dom->def->id = dom->state == VIR_DOMAIN_SHUTOFF ? -1 : veid; + + if (asprintf(&dom->def->name, "%i", veid) < 0) { + dom->def->name = NULL; + goto no_memory; + } + openvzGetVPSUUID(veid, uuidstr); - ret = virUUIDParse(uuidstr, vmdef->uuid); - - if(ret == -1) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, - _("UUID in config file malformed")); - VIR_FREE(vmdef); - goto error; - } - - /*get VCPU*/ - ret = openvzReadConfigParam(veid, "CPUS", temp, sizeof(temp)); - if (ret < 0) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, - _("Cound not read config for container %d"), veid); - goto error; - } else if (ret > 0) { - vmdef->vcpus = strtoI(temp); - } - - - (*pnext)->vmdef = vmdef; - pnext = &(*pnext)->next; - } - return vm; -error: - while (vm != NULL) { - struct openvz_vm *next; - - next = vm->next; - VIR_FREE(vm->vmdef); - VIR_FREE(vm); - vm = next; - } - return NULL; + ret = virUUIDParse(uuidstr, dom->def->uuid); + + if (ret == -1) { + openvzError(NULL, VIR_ERR_INTERNAL_ERROR, + _("UUID in config file malformed")); + goto cleanup; + } + + dom->def->vcpus = 1; + if (!(dom->def->os.type = strdup("exe"))) + goto no_memory; + if (!(dom->def->os.init = strdup("/sbin/init"))) + goto no_memory; + + /* XXX load rest of VM config data .... */ + + if (prev) { + prev->next = dom; + } else { + driver->domains = dom; + } + prev = dom; + } + + fclose(fp); + + return 0; + + no_memory: + openvzError(NULL, VIR_ERR_NO_MEMORY, NULL); + + cleanup: + fclose(fp); + virDomainObjFree(dom); + return -1; } /* @@ -797,7 +451,7 @@ openvzSetUUID(int vpsid){ * */ -int openvzAssignUUIDs(void) +static int openvzAssignUUIDs(void) { DIR *dp; struct dirent *dent; diff -r e270be59a80f src/openvz_conf.h --- a/src/openvz_conf.h Wed Aug 27 17:03:25 2008 +0100 +++ b/src/openvz_conf.h Wed Aug 27 17:03:45 2008 +0100 @@ -28,94 +28,35 @@ #ifndef OPENVZ_CONF_H #define OPENVZ_CONF_H -#include "openvz_driver.h" +#include "internal.h" #include "domain_conf.h" enum { OPENVZ_WARN, OPENVZ_ERR }; -#define OPENVZ_NAME_MAX 8 -#define OPENVZ_TMPL_MAX 256 -#define OPENVZ_UNAME_MAX 32 -#define OPENVZ_IP_MAX 16 -#define OPENVZ_HOSTNAME_MAX 256 -#define OPENVZ_PROFILE_MAX 256 -#define OPENVZ_MAX_ERROR_LEN 1024 -#define OPENVZ_MAX_XML_LEN 4096 -#define OPENVZ_MAX_QUOTA 8 -#define OPENVZ_MAX_XPathEval_LEN 256 -#define OPENVZ_RSRV_VM_LIMIT 100 +#define openvzLog(level, msg...) { if(level == OPENVZ_WARN) \ + fprintf(stderr, "\nWARNING: ");\ + else \ + fprintf(stderr, "\nERROR: ");\ + fprintf(stderr, "\n\t");\ + fprintf(stderr, msg);\ + fprintf(stderr, "\n"); } -enum openvz_quota{ - VM_LEVEL = 0, - USER_LEVEL = 1, +/* OpenVZ commands - Replace with wrapper scripts later? */ +#define VZLIST "vzlist" +#define VZCTL "vzctl" + +struct openvz_driver { + virCapsPtr caps; + virDomainObjPtr domains; }; - -/* TODO Add more properties here */ -struct vps_props { - int kmemsize; /* currently held */ - int kmemsize_m; /* max held */ - int kmemsize_b; /* barrier */ - int kmemsize_l; /* limit */ - int kmemsize_f; /* fail count */ - -}; - -struct openvz_fs_def { - char tmpl[OPENVZ_TMPL_MAX]; - long int disksize, diskinodes; -}; - -struct openvz_vm_def { - char name[OPENVZ_NAME_MAX]; - unsigned char uuid[VIR_UUID_BUFLEN]; - char profile[OPENVZ_PROFILE_MAX]; - unsigned long vcpus; - struct openvz_fs_def fs; - virDomainNetDefPtr net; -}; - -struct ovz_quota { - enum openvz_quota type; - unsigned int size; - char uname[OPENVZ_UNAME_MAX]; - struct ovz_quota *next; -}; - -struct openvz_vm { - int vpsid; - int status; - struct openvz_vm_def *vmdef; - struct openvz_vm *next; -}; - -static inline int -openvzIsActiveVM(struct openvz_vm *vm) -{ - return vm->vpsid != -1; -} void openvzError (virConnectPtr conn, virErrorNumber code, const char *fmt, ...) ATTRIBUTE_FORMAT(printf, 3, 4); int openvz_readline(int fd, char *ptr, int maxlen); int openvzReadConfigParam(int vpsid ,const char * param, char *value, int maxlen); -struct openvz_vm *openvzFindVMByID(const struct openvz_driver *driver, int id); -struct openvz_vm *openvzFindVMByUUID(const struct openvz_driver *driver, - const unsigned char *uuid); - -struct openvz_vm *openvzFindVMByName(const struct openvz_driver *driver, const char *name); -struct openvz_vm_def *openvzParseVMDef(virConnectPtr conn, const char *xmlStr, - const char *displayName); - -struct openvz_vm *openvzAssignVMDef(virConnectPtr conn, struct openvz_driver *driver, - struct openvz_vm_def *def); - -struct openvz_vm *openvzGetVPSInfo(virConnectPtr conn); -void openvzGenerateUUID(unsigned char *uuid); -int openvzAssignUUIDs(void); -void openvzRemoveInactiveVM(struct openvz_driver *driver, struct openvz_vm *vm); +virCapsPtr openvzCapsInit(void); +int openvzLoadDomains(struct openvz_driver *driver); void openvzFreeDriver(struct openvz_driver *driver); -void openvzFreeVM(struct openvz_driver *driver, struct openvz_vm *vm, int checkCallee); -void openvzFreeVMDef(struct openvz_vm_def *def); int strtoI(const char *str); int openvzCheckEmptyMac(const unsigned char *mac); char *openvzMacToString(const unsigned char *mac); diff -r e270be59a80f src/openvz_driver.c --- a/src/openvz_driver.c Wed Aug 27 17:03:25 2008 +0100 +++ b/src/openvz_driver.c Wed Aug 27 17:29:40 2008 +0100 @@ -60,33 +60,6 @@ #define CMDBUF_LEN 1488 #define CMDOP_LEN 288 -static virDomainPtr openvzDomainLookupByID(virConnectPtr conn, int id); -static char *openvzGetOSType(virDomainPtr dom); -static int openvzGetNodeInfo(virConnectPtr conn, virNodeInfoPtr nodeinfo); -static virDomainPtr openvzDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid); -static virDomainPtr openvzDomainLookupByName(virConnectPtr conn, const char *name); -static int openvzDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info); -static int openvzDomainShutdown(virDomainPtr dom); -static int openvzDomainReboot(virDomainPtr dom, unsigned int flags); -static int openvzDomainCreate(virDomainPtr dom); -static virDrvOpenStatus openvzOpen(virConnectPtr conn, - xmlURIPtr uri, - virConnectAuthPtr auth ATTRIBUTE_UNUSED, - int flags ATTRIBUTE_UNUSED); -static int openvzClose(virConnectPtr conn); -static const char *openvzGetType(virConnectPtr conn ATTRIBUTE_UNUSED); -static int openvzListDomains(virConnectPtr conn, int *ids, int nids); -static int openvzNumDomains(virConnectPtr conn); -static int openvzListDefinedDomains(virConnectPtr conn, char **const names, int nnames); -static int openvzNumDefinedDomains(virConnectPtr conn); - -static virDomainPtr openvzDomainDefineXML(virConnectPtr conn, const char *xml); -static virDomainPtr openvzDomainCreateLinux(virConnectPtr conn, const char *xml, - unsigned int flags ATTRIBUTE_UNUSED); - -static int openvzDomainUndefine(virDomainPtr dom); -static void cmdExecFree(const char *cmdExec[]); - static int openvzGetProcessInfo(unsigned long long *cpuTime, int vpsid); static int openvzGetMaxVCPUs(virConnectPtr conn, const char *type); static int openvzDomainGetMaxVcpus(virDomainPtr dom); @@ -110,7 +83,7 @@ static int openvzDomainDefineCmd(virConn static int openvzDomainDefineCmd(virConnectPtr conn, const char *args[], int maxarg, - struct openvz_vm_def *vmdef) + virDomainDefPtr vmdef) { int narg; @@ -143,15 +116,29 @@ static int openvzDomainDefineCmd(virConn ADD_ARG_LIT("--quiet"); ADD_ARG_LIT("create"); ADD_ARG_LIT(vmdef->name); - - if ((vmdef->fs.tmpl && *(vmdef->fs.tmpl))) { + + if (vmdef->fss) { + if (vmdef->fss->type != VIR_DOMAIN_FS_TYPE_TEMPLATE) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("only filesystem templates are supported")); + return -1; + } + + if (vmdef->fss->next) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("only one filesystem supported")); + return -1; + } + ADD_ARG_LIT("--ostemplate"); - ADD_ARG_LIT(vmdef->fs.tmpl); - } + ADD_ARG_LIT(vmdef->fss->src); + } +#if 0 if ((vmdef->profile && *(vmdef->profile))) { ADD_ARG_LIT("--config"); ADD_ARG_LIT(vmdef->profile); } +#endif ADD_ARG(NULL); return 0; @@ -165,39 +152,43 @@ static int openvzDomainDefineCmd(virConn static virDomainPtr openvzDomainLookupByID(virConnectPtr conn, - int id) { + int id) { struct openvz_driver *driver = (struct openvz_driver *)conn->privateData; - struct openvz_vm *vm; + virDomainObjPtr vm; virDomainPtr dom; - vm = openvzFindVMByID(driver, id); + vm = virDomainFindByID(driver->domains, id); if (!vm) { openvzError(conn, VIR_ERR_NO_DOMAIN, NULL); return NULL; } - dom = virGetDomain(conn, vm->vmdef->name, vm->vmdef->uuid); - if (!dom) { - openvzError(conn, VIR_ERR_NO_MEMORY, _("virDomainPtr")); - return NULL; - } - - dom->id = vm->vpsid; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid); + if (!dom) + return NULL; + + dom->id = vm->def->id; return dom; } -static char *openvzGetOSType(virDomainPtr dom ATTRIBUTE_UNUSED) -{ - /* OpenVZ runs on Linux and runs only Linux */ - return strdup("linux"); +static char *openvzGetOSType(virDomainPtr dom) +{ + struct openvz_driver *driver = (struct openvz_driver *)dom->conn->privateData; + virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + char *ret = strdup(vm->def->os.type); + + if (!ret) + openvzError(dom->conn, VIR_ERR_NO_MEMORY, NULL); + + return ret; } static virDomainPtr openvzDomainLookupByUUID(virConnectPtr conn, - const unsigned char *uuid) { + const unsigned char *uuid) { struct openvz_driver *driver = (struct openvz_driver *)conn->privateData; - struct openvz_vm *vm = openvzFindVMByUUID(driver, uuid); + virDomainObjPtr vm = virDomainFindByUUID(driver->domains, uuid); virDomainPtr dom; if (!vm) { @@ -205,20 +196,18 @@ static virDomainPtr openvzDomainLookupBy return NULL; } - dom = virGetDomain(conn, vm->vmdef->name, vm->vmdef->uuid); - if (!dom) { - openvzError(conn, VIR_ERR_NO_MEMORY, _("virDomainPtr")); - return NULL; - } - - dom->id = vm->vpsid; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid); + if (!dom) + return NULL; + + dom->id = vm->def->id; return dom; } static virDomainPtr openvzDomainLookupByName(virConnectPtr conn, const char *name) { struct openvz_driver *driver = (struct openvz_driver *)conn->privateData; - struct openvz_vm *vm = openvzFindVMByName(driver, name); + virDomainObjPtr vm = virDomainFindByName(driver->domains, name); virDomainPtr dom; if (!vm) { @@ -226,74 +215,83 @@ static virDomainPtr openvzDomainLookupBy return NULL; } - dom = virGetDomain(conn, vm->vmdef->name, vm->vmdef->uuid); - if (!dom) { - openvzError(conn, VIR_ERR_NO_MEMORY, _("virDomainPtr")); - return NULL; - } - - dom->id = vm->vpsid; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid); + if (!dom) + return NULL; + + dom->id = vm->def->id; return dom; } static int openvzDomainGetInfo(virDomainPtr dom, - virDomainInfoPtr info) { + virDomainInfoPtr info) { struct openvz_driver *driver = (struct openvz_driver *)dom->conn->privateData; - struct openvz_vm *vm = openvzFindVMByUUID(driver, dom->uuid); + virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); if (!vm) { openvzError(dom->conn, VIR_ERR_INVALID_DOMAIN, - _("no domain with matching uuid")); - return -1; - } - - info->state = vm->status; - - if (!openvzIsActiveVM(vm)) { + _("no domain with matching uuid")); + return -1; + } + + info->state = vm->state; + + if (!virDomainIsActive(vm)) { info->cpuTime = 0; } else { if (openvzGetProcessInfo(&(info->cpuTime), dom->id) < 0) { openvzError(dom->conn, VIR_ERR_OPERATION_FAILED, - _("cannot read cputime for domain %d"), dom->id); + _("cannot read cputime for domain %d"), dom->id); return -1; } } /* TODO These need to be calculated differently for OpenVZ */ //info->cpuTime = - //info->maxMem = vm->def->maxmem; - //info->memory = vm->def->memory; - info->nrVirtCpu = vm->vmdef->vcpus; - return 0; -} + info->maxMem = vm->def->maxmem; + info->memory = vm->def->memory; + info->nrVirtCpu = vm->def->vcpus; + return 0; +} + + +static char *openvzDomainDumpXML(virDomainPtr dom, int flags) { + struct openvz_driver *driver = (struct openvz_driver *)dom->conn->privateData; + virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + + if (!vm) { + openvzError(dom->conn, VIR_ERR_INVALID_DOMAIN, + _("no domain with matching uuid")); + return NULL; + } + + return virDomainDefFormat(dom->conn, vm->def, flags); +} + + static int openvzDomainShutdown(virDomainPtr dom) { struct openvz_driver *driver = (struct openvz_driver *)dom->conn->privateData; - struct openvz_vm *vm = openvzFindVMByUUID(driver, dom->uuid); - const char *prog[] = {VZCTL, "--quiet", "stop", vm->vmdef->name, NULL}; + virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + const char *prog[] = {VZCTL, "--quiet", "stop", vm->def->name, NULL}; if (!vm) { openvzError(dom->conn, VIR_ERR_INVALID_DOMAIN, - _("no domain with matching id")); - return -1; - } - - if (vm->status != VIR_DOMAIN_RUNNING) { - openvzError(dom->conn, VIR_ERR_OPERATION_DENIED, - _("domain is not in running state")); - return -1; - } - - if (virRun(dom->conn, prog, NULL) < 0) { + _("no domain with matching uuid")); + return -1; + } + + if (vm->state != VIR_DOMAIN_RUNNING) { openvzError(dom->conn, VIR_ERR_INTERNAL_ERROR, - _("Could not exec %s"), VZCTL); - return -1; - } - - vm->vpsid = -1; - vm->status = VIR_DOMAIN_SHUTOFF; - ovz_driver.num_inactive ++; - ovz_driver.num_active --; + _("domain is not in running state")); + return -1; + } + + if (virRun(dom->conn, prog, NULL) < 0) + return -1; + + vm->def->id = -1; + vm->state = VIR_DOMAIN_SHUTOFF; return 0; } @@ -301,26 +299,23 @@ static int openvzDomainReboot(virDomainP static int openvzDomainReboot(virDomainPtr dom, unsigned int flags ATTRIBUTE_UNUSED) { struct openvz_driver *driver = (struct openvz_driver *)dom->conn->privateData; - struct openvz_vm *vm = openvzFindVMByUUID(driver, dom->uuid); - const char *prog[] = {VZCTL, "--quiet", "restart", vm->vmdef->name, NULL}; + virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + const char *prog[] = {VZCTL, "--quiet", "restart", vm->def->name, NULL}; if (!vm) { openvzError(dom->conn, VIR_ERR_INVALID_DOMAIN, - _("no domain with matching id")); - return -1; - } - - if (vm->status != VIR_DOMAIN_RUNNING) { - openvzError(dom->conn, VIR_ERR_OPERATION_DENIED, - _("domain is not in running state")); - return -1; - } - - if (virRun(dom->conn, prog, NULL) < 0) { + _("no domain with matching uuid")); + return -1; + } + + if (vm->state != VIR_DOMAIN_RUNNING) { openvzError(dom->conn, VIR_ERR_INTERNAL_ERROR, - _("Could not exec %s"), VZCTL); - return -1; - } + _("domain is not in running state")); + return -1; + } + + if (virRun(dom->conn, prog, NULL) < 0) + return -1; return 0; } @@ -421,24 +416,31 @@ openvzDomainDefineXML(virConnectPtr conn openvzDomainDefineXML(virConnectPtr conn, const char *xml) { struct openvz_driver *driver = (struct openvz_driver *) conn->privateData; - struct openvz_vm_def *vmdef = NULL; - struct openvz_vm *vm = NULL; + virDomainDefPtr vmdef = NULL; + virDomainObjPtr vm = NULL; virDomainPtr dom = NULL; const char *prog[OPENVZ_MAX_ARG]; prog[0] = NULL; - if ((vmdef = openvzParseVMDef(conn, xml, NULL)) == NULL) - return NULL; - - vm = openvzFindVMByName(driver, vmdef->name); + if ((vmdef = virDomainDefParseString(conn, driver->caps, xml)) == NULL) + return NULL; + + if (vmdef->os.init == NULL && + !(vmdef->os.init = strdup("/sbin/init"))) { + virDomainDefFree(vmdef); + return NULL; + } + + vm = virDomainFindByName(driver->domains, vmdef->name); if (vm) { + virDomainDefFree(vmdef); openvzLog(OPENVZ_ERR, _("Already an OPENVZ VM active with the id '%s'"), vmdef->name); return NULL; } - if (!(vm = openvzAssignVMDef(conn, driver, vmdef))) { - openvzFreeVMDef(vmdef); - openvzLog(OPENVZ_ERR, "%s", _("Error creating OPENVZ VM")); + if (!(vm = virDomainAssignDef(conn, &driver->domains, vmdef))) { + virDomainDefFree(vmdef); + return NULL; } if (openvzDomainDefineCmd(conn, prog, OPENVZ_MAX_ARG, vmdef) < 0) { @@ -461,11 +463,11 @@ openvzDomainDefineXML(virConnectPtr conn goto exit; } - dom = virGetDomain(conn, vm->vmdef->name, vm->vmdef->uuid); + dom = virGetDomain(conn, vm->def->name, vm->def->uuid); if (dom) - dom->id = vm->vpsid; - - if (openvzDomainSetNetwork(conn, vmdef->name, vmdef->net) < 0) { + dom->id = -1; + + if (openvzDomainSetNetwork(conn, vmdef->name, vmdef->nets) < 0) { openvzError(conn, VIR_ERR_INTERNAL_ERROR, _("Could not configure network")); goto exit; @@ -488,27 +490,33 @@ openvzDomainCreateLinux(virConnectPtr co openvzDomainCreateLinux(virConnectPtr conn, const char *xml, unsigned int flags ATTRIBUTE_UNUSED) { - struct openvz_vm_def *vmdef = NULL; - struct openvz_vm *vm = NULL; + virDomainDefPtr vmdef = NULL; + virDomainObjPtr vm = NULL; virDomainPtr dom = NULL; struct openvz_driver *driver = (struct openvz_driver *) conn->privateData; const char *progstart[] = {VZCTL, "--quiet", "start", NULL, NULL}; const char *progcreate[OPENVZ_MAX_ARG]; progcreate[0] = NULL; - if (!(vmdef = openvzParseVMDef(conn, xml, NULL))) - return NULL; - - vm = openvzFindVMByName(driver, vmdef->name); + if ((vmdef = virDomainDefParseString(conn, driver->caps, xml)) == NULL) + return NULL; + + if (vmdef->os.init == NULL && + !(vmdef->os.init = strdup("/sbin/init"))) { + virDomainDefFree(vmdef); + return NULL; + } + + vm = virDomainFindByName(driver->domains, vmdef->name); if (vm) { - openvzFreeVMDef(vmdef); + virDomainDefFree(vmdef); openvzLog(OPENVZ_ERR, _("Already an OPENVZ VM defined with the id '%d'"), strtoI(vmdef->name)); return NULL; } - if (!(vm = openvzAssignVMDef(conn, driver, vmdef))) { - openvzLog(OPENVZ_ERR, "%s", _("Error creating OPENVZ VM")); + if (!(vm = virDomainAssignDef(conn, &driver->domains, vmdef))) { + virDomainDefFree(vmdef); return NULL; } @@ -530,7 +538,7 @@ openvzDomainCreateLinux(virConnectPtr co goto exit; } - if (openvzDomainSetNetwork(conn, vmdef->name, vmdef->net) < 0) { + if (openvzDomainSetNetwork(conn, vmdef->name, vmdef->nets) < 0) { openvzError(conn, VIR_ERR_INTERNAL_ERROR, _("Could not configure network")); goto exit; @@ -544,20 +552,19 @@ openvzDomainCreateLinux(virConnectPtr co goto exit; } - vm->vpsid = strtoI(vmdef->name); - vm->status = VIR_DOMAIN_RUNNING; - ovz_driver.num_inactive--; - ovz_driver.num_active++; - - dom = virGetDomain(conn, vm->vmdef->name, vm->vmdef->uuid); + vm->pid = strtoI(vmdef->name); + vm->def->id = vm->pid; + vm->state = VIR_DOMAIN_RUNNING; + + dom = virGetDomain(conn, vm->def->name, vm->def->uuid); if (dom) - dom->id = vm->vpsid; + dom->id = vm->def->id; if (vmdef->vcpus > 0) { if (openvzDomainSetVcpus(dom, vmdef->vcpus) < 0) { openvzError(conn, VIR_ERR_INTERNAL_ERROR, - _("Could not set number of virtual cpu")); - goto exit; + _("Could not set number of virtual cpu")); + goto exit; } } @@ -570,8 +577,8 @@ openvzDomainCreate(virDomainPtr dom) openvzDomainCreate(virDomainPtr dom) { struct openvz_driver *driver = (struct openvz_driver *)dom->conn->privateData; - struct openvz_vm *vm = openvzFindVMByName(driver, dom->name); - const char *prog[] = {VZCTL, "--quiet", "start", vm->vmdef->name, NULL }; + virDomainObjPtr vm = virDomainFindByName(driver->domains, dom->name); + const char *prog[] = {VZCTL, "--quiet", "start", vm->def->name, NULL }; if (!vm) { openvzError(dom->conn, VIR_ERR_INVALID_DOMAIN, @@ -579,7 +586,7 @@ openvzDomainCreate(virDomainPtr dom) return -1; } - if (vm->status != VIR_DOMAIN_SHUTOFF) { + if (vm->state != VIR_DOMAIN_SHUTOFF) { openvzError(dom->conn, VIR_ERR_OPERATION_DENIED, _("domain is not in shutoff state")); return -1; @@ -591,10 +598,9 @@ openvzDomainCreate(virDomainPtr dom) return -1; } - vm->vpsid = strtoI(vm->vmdef->name); - vm->status = VIR_DOMAIN_RUNNING; - ovz_driver.num_inactive --; - ovz_driver.num_active ++; + vm->pid = strtoI(vm->def->name); + vm->def->id = vm->pid; + vm->state = VIR_DOMAIN_RUNNING; return 0; } @@ -604,15 +610,15 @@ openvzDomainUndefine(virDomainPtr dom) { virConnectPtr conn= dom->conn; struct openvz_driver *driver = (struct openvz_driver *) conn->privateData; - struct openvz_vm *vm = openvzFindVMByUUID(driver, dom->uuid); - const char *prog[] = { VZCTL, "--quiet", "destroy", vm->vmdef->name, NULL }; + virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + const char *prog[] = { VZCTL, "--quiet", "destroy", vm->def->name, NULL }; if (!vm) { openvzError(conn, VIR_ERR_INVALID_DOMAIN, _("no domain with matching uuid")); return -1; } - if (openvzIsActiveVM(vm)) { + if (virDomainIsActive(vm)) { openvzError(conn, VIR_ERR_INTERNAL_ERROR, _("cannot delete active domain")); return -1; } @@ -623,7 +629,8 @@ openvzDomainUndefine(virDomainPtr dom) return -1; } - openvzRemoveInactiveVM(driver, vm); + virDomainRemoveInactive(&driver->domains, vm); + return 0; } @@ -632,8 +639,8 @@ openvzDomainSetAutostart(virDomainPtr do { virConnectPtr conn= dom->conn; struct openvz_driver *driver = (struct openvz_driver *) conn->privateData; - struct openvz_vm *vm = openvzFindVMByUUID(driver, dom->uuid); - const char *prog[] = { VZCTL, "--quiet", "set", vm->vmdef->name, + virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + const char *prog[] = { VZCTL, "--quiet", "set", vm->def->name, "--onboot", autostart ? "yes" : "no", "--save", NULL }; @@ -655,7 +662,7 @@ openvzDomainGetAutostart(virDomainPtr do { virConnectPtr conn= dom->conn; struct openvz_driver *driver = (struct openvz_driver *) conn->privateData; - struct openvz_vm *vm = openvzFindVMByUUID(driver, dom->uuid); + virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); char value[1024]; if (!vm) { @@ -663,7 +670,7 @@ openvzDomainGetAutostart(virDomainPtr do return -1; } - if (openvzReadConfigParam(strtoI(vm->vmdef->name), "ONBOOT", value, sizeof(value)) < 0) { + if (openvzReadConfigParam(strtoI(vm->def->name), "ONBOOT", value, sizeof(value)) < 0) { openvzError(conn, VIR_ERR_INTERNAL_ERROR, _("Could not read container config")); return -1; } @@ -692,32 +699,32 @@ static int openvzDomainSetVcpus(virDomai static int openvzDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus) { virConnectPtr conn= dom->conn; struct openvz_driver *driver = (struct openvz_driver *) conn->privateData; - struct openvz_vm *vm = openvzFindVMByUUID(driver, dom->uuid); + virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); char str_vcpus[32]; - const char *prog[] = { VZCTL, "--quiet", "set", vm->vmdef->name, + const char *prog[] = { VZCTL, "--quiet", "set", vm->def->name, "--cpus", str_vcpus, "--save", NULL }; snprintf(str_vcpus, 31, "%d", nvcpus); str_vcpus[31] = '\0'; if (nvcpus <= 0) { openvzError(conn, VIR_ERR_INTERNAL_ERROR, - _("VCPUs should be >= 1")); + _("VCPUs should be >= 1")); return -1; } if (!vm) { openvzError(conn, VIR_ERR_INVALID_DOMAIN, - _("no domain with matching uuid")); + _("no domain with matching uuid")); return -1; } if (virRun(conn, prog, NULL) < 0) { openvzError(conn, VIR_ERR_INTERNAL_ERROR, - _("Could not exec %s"), VZCTL); - return -1; - } - - vm->vmdef->vcpus = nvcpus; + _("Could not exec %s"), VZCTL); + return -1; + } + + vm->def->vcpus = nvcpus; return 0; } @@ -735,51 +742,48 @@ static virDrvOpenStatus openvzOpen(virCo virConnectAuthPtr auth ATTRIBUTE_UNUSED, int flags ATTRIBUTE_UNUSED) { - struct openvz_vm *vms; - + struct openvz_driver *driver; /*Just check if the user is root. Nothing really to open for OpenVZ */ - if (getuid()) { // OpenVZ tools can only be used by r00t - return VIR_DRV_OPEN_DECLINED; - } else { - if (uri == NULL || uri->scheme == NULL || uri->path == NULL) - return VIR_DRV_OPEN_DECLINED; - if (STRNEQ (uri->scheme, "openvz")) - return VIR_DRV_OPEN_DECLINED; - if (STRNEQ (uri->path, "/system")) - return VIR_DRV_OPEN_DECLINED; - } + if (getuid()) { // OpenVZ tools can only be used by r00t + return VIR_DRV_OPEN_DECLINED; + } else { + if (uri == NULL || uri->scheme == NULL || uri->path == NULL) + return VIR_DRV_OPEN_DECLINED; + if (STRNEQ (uri->scheme, "openvz")) + return VIR_DRV_OPEN_DECLINED; + if (STRNEQ (uri->path, "/system")) + return VIR_DRV_OPEN_DECLINED; + } /* See if we are running an OpenVZ enabled kernel */ - if(access("/proc/vz/veinfo", F_OK) == -1 || - access("/proc/user_beancounters", F_OK) == -1) { - return VIR_DRV_OPEN_DECLINED; - } - - conn->privateData = &ovz_driver; - - openvzAssignUUIDs(); - - vms = openvzGetVPSInfo(conn); - ovz_driver.vms = vms; - - return VIR_DRV_OPEN_SUCCESS; + if(access("/proc/vz/veinfo", F_OK) == -1 || + access("/proc/user_beancounters", F_OK) == -1) { + return VIR_DRV_OPEN_DECLINED; + } + + if (VIR_ALLOC(driver) < 0) { + openvzError(conn, VIR_ERR_NO_MEMORY, NULL); + return VIR_DRV_OPEN_ERROR; + } + + if (!(driver->caps = openvzCapsInit())) + goto cleanup; + + if (openvzLoadDomains(driver) < 0) + goto cleanup; + + conn->privateData = driver; + + return VIR_DRV_OPEN_SUCCESS; + +cleanup: + openvzFreeDriver(driver); + return VIR_DRV_OPEN_ERROR; }; static int openvzClose(virConnectPtr conn) { - struct openvz_driver *driver = (struct openvz_driver *)conn->privateData; - struct openvz_vm *vm = driver->vms; - - while(vm) { - openvzFreeVMDef(vm->vmdef); - vm = vm->next; - } - vm = driver->vms; - while (vm) { - struct openvz_vm *prev = vm; - vm = vm->next; - free(prev); - } - + + openvzFreeDriver(driver); conn->privateData = NULL; return 0; @@ -792,6 +796,12 @@ static int openvzGetNodeInfo(virConnectP static int openvzGetNodeInfo(virConnectPtr conn, virNodeInfoPtr nodeinfo) { return virNodeInfoPopulate(conn, nodeinfo); +} + +static char *openvzGetCapabilities(virConnectPtr conn) { + struct openvz_driver *driver = (struct openvz_driver *)conn->privateData; + + return virCapabilitiesFormatXML(driver->caps); } static int openvzListDomains(virConnectPtr conn, int *ids, int nids) { @@ -829,14 +839,22 @@ static int openvzListDomains(virConnectP } static int openvzNumDomains(virConnectPtr conn ATTRIBUTE_UNUSED) { - return ovz_driver.num_active; + struct openvz_driver *driver = conn->privateData; + int nactive = 0; + virDomainObjPtr vm = driver->domains; + while (vm) { + if (virDomainIsActive(vm)) + nactive++; + vm = vm->next; + } + return nactive; } static int openvzListDefinedDomains(virConnectPtr conn, - char **const names, int nnames) { + char **const names, int nnames) { int got = 0; int veid, pid, outfd = -1, errfd = -1, ret; - char vpsname[OPENVZ_NAME_MAX]; + char vpsname[32]; char buf[32]; char *endptr; const char *cmd[] = {VZLIST, "-ovpsid", "-H", "-S", NULL}; @@ -858,7 +876,7 @@ static int openvzListDefinedDomains(virC _("Could not parse VPS ID %s"), buf); continue; } - sprintf(vpsname, "%d", veid); + snprintf(vpsname, sizeof(vpsname), "%d", veid); names[got] = strdup(vpsname); got ++; } @@ -911,8 +929,16 @@ Version: 2.2 return 0; } -static int openvzNumDefinedDomains(virConnectPtr conn ATTRIBUTE_UNUSED) { - return ovz_driver.num_inactive; +static int openvzNumDefinedDomains(virConnectPtr conn) { + struct openvz_driver *driver = (struct openvz_driver *) conn->privateData; + int ninactive = 0; + virDomainObjPtr vm = driver->domains; + while (vm) { + if (!virDomainIsActive(vm)) + ninactive++; + vm = vm->next; + } + return ninactive; } static virDriver openvzDriver = { @@ -929,7 +955,7 @@ static virDriver openvzDriver = { NULL, /* uri */ openvzGetMaxVCPUs, /* getMaxVcpus */ openvzGetNodeInfo, /* nodeGetInfo */ - NULL, /* getCapabilities */ + openvzGetCapabilities, /* getCapabilities */ openvzListDomains, /* listDomains */ openvzNumDomains, /* numOfDomains */ openvzDomainCreateLinux, /* domainCreateLinux */ @@ -953,7 +979,7 @@ static virDriver openvzDriver = { NULL, /* domainPinVcpu */ NULL, /* domainGetVcpus */ openvzDomainGetMaxVcpus, /* domainGetMaxVcpus */ - NULL, /* domainDumpXML */ + openvzDomainDumpXML, /* domainDumpXML */ openvzListDefinedDomains, /* listDomains */ openvzNumDefinedDomains, /* numOfDomains */ openvzDomainCreate, /* domainCreate */ diff -r e270be59a80f src/openvz_driver.h --- a/src/openvz_driver.h Wed Aug 27 17:03:25 2008 +0100 +++ b/src/openvz_driver.h Wed Aug 27 17:03:45 2008 +0100 @@ -31,24 +31,6 @@ #include "internal.h" -/* OpenVZ commands - Replace with wrapper scripts later? */ -#define VZLIST "vzlist" -#define VZCTL "vzctl" - -struct openvz_driver { - struct openvz_vm *vms; - int num_active; - int num_inactive; -}; - int openvzRegister(void); -#define openvzLog(level, msg...) { if(level == OPENVZ_WARN) \ - fprintf(stderr, "\nWARNING: ");\ - else \ - fprintf(stderr, "\nERROR: ");\ - fprintf(stderr, "\n\t");\ - fprintf(stderr, msg);\ - fprintf(stderr, "\n"); } - #endif -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
The biggest flaw I see currently is that the openvz driver doesn't load the existing device config for networks or filesystems of existing VMs, so you can't see that info in the XML dump Yes. Good note. I will implement it.
- /*get VCPU*/ - ret = openvzReadConfigParam(veid, "CPUS", temp, sizeof(temp)); - if (ret < 0) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, - _("Cound not read config for container %d"), veid); - goto error; - } else if (ret > 0) { - vmdef->vcpus = strtoI(temp); - } Why did you remove loading VCPUs and always set = 1 ? + + dom->def->vcpus = 1;
On Thu, Aug 28, 2008 at 12:06:08PM +0400, Evgeniy Sokolov wrote:
The biggest flaw I see currently is that the openvz driver doesn't load the existing device config for networks or filesystems of existing VMs, so you can't see that info in the XML dump Yes. Good note. I will implement it.
- /*get VCPU*/ - ret = openvzReadConfigParam(veid, "CPUS", temp, sizeof(temp)); - if (ret < 0) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, - _("Cound not read config for container %d"), veid); - goto error; - } else if (ret > 0) { - vmdef->vcpus = strtoI(temp); - } Why did you remove loading VCPUs and always set = 1 ?
Stupid mistake during a manual merge. Here's an updated patch which fixes that mistake Daniel diff -r e270be59a80f src/openvz_conf.c --- a/src/openvz_conf.c Wed Aug 27 17:03:25 2008 +0100 +++ b/src/openvz_conf.c Thu Aug 28 09:49:14 2008 +0100 @@ -40,39 +40,29 @@ #include <limits.h> #include <errno.h> #include <string.h> - -#include <libxml/parser.h> -#include <libxml/tree.h> -#include <libxml/xpath.h> -#include <libxml/uri.h> - -#include "internal.h" - -#include "openvz_driver.h" +#include <sys/utsname.h> + #include "openvz_conf.h" #include "uuid.h" #include "buf.h" #include "memory.h" #include "util.h" -#include "xml.h" -#include "domain_conf.h" static char *openvzLocateConfDir(void); -static struct openvz_vm_def *openvzParseXML(virConnectPtr conn, xmlDocPtr xml); static int openvzGetVPSUUID(int vpsid, char *uuidstr); -static int openvzSetUUID(int vpsid); static int openvzLocateConfFile(int vpsid, char *conffile, int maxlen); +static int openvzAssignUUIDs(void); void openvzError (virConnectPtr conn, virErrorNumber code, const char *fmt, ...) { va_list args; - char errorMessage[OPENVZ_MAX_ERROR_LEN]; + char errorMessage[1024]; const char *errmsg; if (fmt) { va_start(args, fmt); - vsnprintf(errorMessage, OPENVZ_MAX_ERROR_LEN-1, fmt, args); + vsnprintf(errorMessage, sizeof(errorMessage)-1, fmt, args); va_end(args); } else { errorMessage[0] = '\0'; @@ -84,46 +74,6 @@ openvzError (virConnectPtr conn, virErro errmsg, errorMessage); } -struct openvz_vm -*openvzFindVMByID(const struct openvz_driver *driver, int id) { - struct openvz_vm *vm = driver->vms; - - while (vm) { - if (vm->vpsid == id) - return vm; - vm = vm->next; - } - - return NULL; -} - -struct openvz_vm -*openvzFindVMByUUID(const struct openvz_driver *driver, - const unsigned char *uuid) { - struct openvz_vm *vm = driver->vms; - - while (vm) { - if (!memcmp(vm->vmdef->uuid, uuid, VIR_UUID_BUFLEN)) - return vm; - vm = vm->next; - } - - return NULL; -} - -struct openvz_vm -*openvzFindVMByName(const struct openvz_driver *driver, - const char *name) { - struct openvz_vm *vm = driver->vms; - - while (vm) { - if (STREQ(vm->vmdef->name, name)) - return vm; - vm = vm->next; - } - - return NULL; -} int strtoI(const char *str) @@ -135,6 +85,43 @@ strtoI(const char *str) return val; } + +virCapsPtr openvzCapsInit(void) +{ + struct utsname utsname; + virCapsPtr caps; + virCapsGuestPtr guest; + + uname(&utsname); + + if ((caps = virCapabilitiesNew(utsname.machine, + 0, 0)) == NULL) + goto no_memory; + + if ((guest = virCapabilitiesAddGuest(caps, + "exe", + utsname.machine, + sizeof(int) == 4 ? 32 : 8, + NULL, + NULL, + 0, + NULL)) == NULL) + goto no_memory; + + if (virCapabilitiesAddGuestDomain(guest, + "openvz", + NULL, + NULL, + 0, + NULL) == NULL) + goto no_memory; + return caps; + +no_memory: + virCapabilitiesFree(caps); + return NULL; +} + /* function checks MAC address is empty return 0 - empty @@ -164,438 +151,117 @@ char *openvzMacToString(const unsigned c return strdup(str); } -void -openvzRemoveInactiveVM(struct openvz_driver *driver, struct openvz_vm *vm) -{ - driver->num_inactive--; - openvzFreeVM(driver, vm, 1); -} - -/* Free all memory associated with a openvz_vm_def structure */ -void -openvzFreeVMDef(struct openvz_vm_def *def) -{ - if (def) { - virDomainNetDefFree(def->net); - } -} - -/* Free all memory associated with a openvz_vm structure - * @checkCallee == 0 then openvzFreeDriver() is callee else some other function - */ -void -openvzFreeVM(struct openvz_driver *driver, struct openvz_vm *vm, - int checkCallee) -{ - struct openvz_vm *vms; - - if (!vm && !driver) - return; - vms = driver->vms; - if (checkCallee) { - if (vms == vm) - driver->vms = vm->next; - else { - while (vms) { - struct openvz_vm *prev = vms; - - vms = vms->next; - if (vms == vm) { - prev->next = vms->next; - break; - } - } - } - } - if (vms) { - openvzFreeVMDef(vm->vmdef); - VIR_FREE(vm); - } -} - /* Free all memory associated with a openvz_driver structure */ void openvzFreeDriver(struct openvz_driver *driver) { - struct openvz_vm *next; - + virDomainObjPtr dom; + if (!driver) return; - if (driver->vms) - for(next = driver->vms->next; driver->vms; driver->vms = next) - openvzFreeVM(driver, driver->vms, 0); - VIR_FREE(driver); -} - -struct openvz_vm * -openvzAssignVMDef(virConnectPtr conn, - struct openvz_driver *driver, struct openvz_vm_def *def) -{ - struct openvz_vm *vm = NULL; - - if (!driver || !def) - return NULL; - - if ((vm = openvzFindVMByName(driver, def->name))) { - if (!openvzIsActiveVM(vm)) { - openvzFreeVMDef(vm->vmdef); - vm->vmdef = def; - } - else - { - openvzLog(OPENVZ_ERR, - _("Error already an active OPENVZ VM having id '%s'"), - def->name); - openvzFreeVMDef(def); - return NULL; /* can't redefine an active domain */ - } - - return vm; - } - - if (VIR_ALLOC(vm) < 0) { - openvzFreeVMDef(def); - openvzError(conn, VIR_ERR_NO_MEMORY, _("vm")); - return NULL; - } - - vm->vpsid = -1; /* -1 needed for to represent inactiveness of domain before 'start' */ - vm->status = VIR_DOMAIN_SHUTOFF; - vm->vmdef = def; - vm->next = driver->vms; - - driver->vms = vm; - driver->num_inactive++; - - return vm; -} - -struct openvz_vm_def -*openvzParseVMDef(virConnectPtr conn, - const char *xmlStr, const char *displayName) -{ - xmlDocPtr xml; - struct openvz_vm_def *def = NULL; - - xml = xmlReadDoc(BAD_CAST xmlStr, displayName ? displayName : "domain.xml", NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | XML_PARSE_NOERROR | XML_PARSE_NOWARNING); - if (!xml) { - openvzError(conn, VIR_ERR_XML_ERROR, NULL); - return NULL; - } - - def = openvzParseXML(conn, xml); - xmlFreeDoc(xml); - - return def; -} - -/* Parse filesystem section -Sample: -<filesystem type="template"> - <source name="fedora-core-5-i386"/> - <quota type="size" max="10000"/> - <quota type="inodes" max="100"/> -</filesystem> -*/ -static int openvzParseDomainFS(virConnectPtr conn, - struct openvz_fs_def *fs, - xmlXPathContextPtr ctxt) -{ - xmlNodePtr cur, obj; - char *type = NULL; - int n; - xmlNodePtr *nodes = NULL; - - - if ((n = virXPathNodeSet(conn, "/domain/devices/filesystem", - ctxt, &nodes)) < 0) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, - _("missing filesystem tag")); - goto error; - } - - if (n > 1) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, - _("There should be only one filesystem tag")); - goto error; - } - - obj = nodes[0]; - - /*check template type*/ - type = virXMLPropString(obj, "type"); - if (type == NULL) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, - _("missing type attribute")); - goto error; - } - - if (STRNEQ(type, "template")) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, - _("Unknown type attribute %s"), type); - goto error; - } - VIR_FREE(type); - - cur = obj->children; - while(cur != NULL) - { - if (cur->type == XML_ELEMENT_NODE) { - if (xmlStrEqual(cur->name, BAD_CAST "source")) { - char * name = virXMLPropString(cur, "name"); - - if (name != NULL) { - strncpy(fs->tmpl, name,sizeof(fs->tmpl)); - fs->tmpl[sizeof(fs->tmpl) - 1] = '\0'; - } - VIR_FREE(name); - } else if (xmlStrEqual(cur->name, BAD_CAST "quota")) { - char * qtype = virXMLPropString(cur, "type"); - char * max = virXMLPropString(cur, "max"); - - if (qtype != NULL && STREQ(qtype, "size") && max != NULL) - fs->disksize = strtoI(max); - else if (qtype != NULL && STREQ(qtype, "inodes") && max != NULL) - fs->diskinodes = strtoI(max); - VIR_FREE(qtype); - VIR_FREE(max); - } - } - cur = cur->next; - } - VIR_FREE(nodes); - - return 0; - - error: - VIR_FREE(nodes); - VIR_FREE(type); - - return -1; -} - - -/* - * Parses a libvirt XML definition of a guest, and populates the - * the openvz_vm struct with matching data about the guests config - */ -static struct openvz_vm_def -*openvzParseXML(virConnectPtr conn, - xmlDocPtr xml) { - xmlNodePtr root = NULL; - char *prop = NULL; - xmlXPathContextPtr ctxt = NULL; - xmlXPathObjectPtr obj = NULL; - struct openvz_vm_def *def = NULL; - xmlNodePtr *nodes = NULL; - int i, n; - - if (VIR_ALLOC(def) < 0) { - openvzError(conn, VIR_ERR_NO_MEMORY, _("xmlXPathContext")); - return NULL; - } - - /* Prepare parser / xpath context */ - root = xmlDocGetRootElement(xml); - if ((root == NULL) || (!xmlStrEqual(root->name, BAD_CAST "domain"))) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, _("incorrect root element")); - goto bail_out; - } - - ctxt = xmlXPathNewContext(xml); - if (ctxt == NULL) { - openvzError(conn, VIR_ERR_NO_MEMORY, _("xmlXPathContext")); - goto bail_out; - } - ctxt->node = root; - - /* Find out what type of OPENVZ virtualization to use */ - if (!(prop = virXMLPropString(root, "type"))) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, _("missing domain type attribute")); - goto bail_out; - } - - if (STRNEQ(prop, "openvz")){ - openvzError(conn, VIR_ERR_INTERNAL_ERROR, _("invalid domain type attribute")); - goto bail_out; - } - VIR_FREE(prop); - - /* Extract domain name */ - obj = xmlXPathEval(BAD_CAST "string(/domain/name[1])", ctxt); - if ((obj == NULL) || (obj->type != XPATH_STRING) || - (obj->stringval == NULL) || (obj->stringval[0] == 0)) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, _("invalid domain name")); - goto bail_out; - } - - /* rejecting VPS ID <= OPENVZ_RSRV_VM_LIMIT for they are reserved */ - if (strtoI((const char *) obj->stringval) <= OPENVZ_RSRV_VM_LIMIT) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, - _("VPS ID Error (must be an integer greater than 100")); - goto bail_out; - } - strncpy(def->name, (const char *) obj->stringval, OPENVZ_NAME_MAX); - xmlXPathFreeObject(obj); - obj = NULL; - - /* Extract domain uuid */ - prop = virXPathString(conn, "string(./uuid[1])", ctxt); - if (!prop) { - int err; - if ((err = virUUIDGenerate(def->uuid))) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, - _("Failed to generate UUID: %s"), - strerror(err)); - goto bail_out; - } - } else { - if (virUUIDParse(prop, def->uuid) < 0) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, - _("malformed uuid element")); - goto bail_out; - } - VIR_FREE(prop); - } - - /* extract virtual CPUs */ - if (virXPathULong(conn, "string(./vcpu[1])", ctxt, &def->vcpus) < 0) - def->vcpus = 0; //use default CPUs count - - /* Extract filesystem info */ - if (openvzParseDomainFS(conn, &(def->fs), ctxt)) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, - _("malformed filesystem tag")); - goto bail_out; - } - - /* analysis of the network devices */ - if ((n = virXPathNodeSet(conn, "/domain/devices/interface", - ctxt, &nodes)) < 0) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot extract network devices")); - goto bail_out; - } - - for (i = n - 1 ; i >= 0 ; i--) { - virDomainNetDefPtr net = virDomainNetDefParseXML(conn, - nodes[i]); - if (!net) - goto bail_out; - - net->next = def->net; - def->net = net; - } - VIR_FREE(nodes); - - xmlXPathFreeContext(ctxt); - return def; - - bail_out: - VIR_FREE(prop); - xmlXPathFreeObject(obj); - xmlXPathFreeContext(ctxt); - openvzFreeVMDef(def); - - return NULL; -} - -struct openvz_vm * -openvzGetVPSInfo(virConnectPtr conn) { + + dom = driver->domains; + while (dom) { + virDomainObjPtr tmp = dom->next; + virDomainObjFree(dom); + dom = tmp; + } + + virCapabilitiesFree(driver->caps); +} + + + +int openvzLoadDomains(struct openvz_driver *driver) { FILE *fp; int veid, ret; char status[16]; char uuidstr[VIR_UUID_STRING_BUFLEN]; - struct openvz_vm *vm; - struct openvz_vm **pnext; - struct openvz_driver *driver; - struct openvz_vm_def *vmdef; - char temp[124]; - - vm = NULL; - driver = conn->privateData; - driver->num_active = 0; - driver->num_inactive = 0; - - if((fp = popen(VZLIST " -a -ovpsid,status -H 2>/dev/null", "r")) == NULL) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, _("popen failed")); - return NULL; - } - pnext = &vm; + virDomainObjPtr dom = NULL, prev = NULL; + char temp[50]; + + if (openvzAssignUUIDs() < 0) + return -1; + + if ((fp = popen(VZLIST " -a -ovpsid,status -H 2>/dev/null", "r")) == NULL) { + openvzError(NULL, VIR_ERR_INTERNAL_ERROR, _("popen failed")); + return -1; + } + while(!feof(fp)) { - if (VIR_ALLOC(*pnext) < 0) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, _("calloc failed")); - goto error; - } - - if(!vm) - vm = *pnext; - if (fscanf(fp, "%d %s\n", &veid, status) != 2) { if (feof(fp)) break; - - openvzError(conn, VIR_ERR_INTERNAL_ERROR, - _("Failed to parse vzlist output")); - goto error; - } - if(STRNEQ(status, "stopped")) { - (*pnext)->status = VIR_DOMAIN_RUNNING; - driver->num_active ++; - (*pnext)->vpsid = veid; - } - else { - (*pnext)->status = VIR_DOMAIN_SHUTOFF; - driver->num_inactive ++; - /* - * inactive domains don't have their ID set in libvirt, - * thought this doesn't make sense for OpenVZ - */ - (*pnext)->vpsid = -1; - } - - if (VIR_ALLOC(vmdef) < 0) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, _("calloc failed")); - goto error; - } - - snprintf(vmdef->name, OPENVZ_NAME_MAX, "%i", veid); + + openvzError(NULL, VIR_ERR_INTERNAL_ERROR, + _("Failed to parse vzlist output")); + goto cleanup; + } + + if (VIR_ALLOC(dom) < 0 || + VIR_ALLOC(dom->def) < 0) + goto no_memory; + + if (STRNEQ(status, "stopped")) + dom->state = VIR_DOMAIN_RUNNING; + else + dom->state = VIR_DOMAIN_SHUTOFF; + + dom->pid = veid; + dom->def->id = dom->state == VIR_DOMAIN_SHUTOFF ? -1 : veid; + + if (asprintf(&dom->def->name, "%i", veid) < 0) { + dom->def->name = NULL; + goto no_memory; + } + openvzGetVPSUUID(veid, uuidstr); - ret = virUUIDParse(uuidstr, vmdef->uuid); - - if(ret == -1) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, - _("UUID in config file malformed")); - VIR_FREE(vmdef); - goto error; - } - - /*get VCPU*/ + ret = virUUIDParse(uuidstr, dom->def->uuid); + + if (ret == -1) { + openvzError(NULL, VIR_ERR_INTERNAL_ERROR, + _("UUID in config file malformed")); + goto cleanup; + } + + if (!(dom->def->os.type = strdup("exe"))) + goto no_memory; + if (!(dom->def->os.init = strdup("/sbin/init"))) + goto no_memory; + ret = openvzReadConfigParam(veid, "CPUS", temp, sizeof(temp)); if (ret < 0) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, - _("Cound not read config for container %d"), veid); - goto error; + openvzError(NULL, VIR_ERR_INTERNAL_ERROR, + _("Cound not read config for container %d"), + veid); + goto cleanup; } else if (ret > 0) { - vmdef->vcpus = strtoI(temp); - } - - - (*pnext)->vmdef = vmdef; - pnext = &(*pnext)->next; - } - return vm; -error: - while (vm != NULL) { - struct openvz_vm *next; - - next = vm->next; - VIR_FREE(vm->vmdef); - VIR_FREE(vm); - vm = next; - } - return NULL; + dom->def->vcpus = strtoI(temp); + } else { + dom->def->vcpus = 1; + } + + /* XXX load rest of VM config data .... */ + + if (prev) { + prev->next = dom; + } else { + driver->domains = dom; + } + prev = dom; + } + + fclose(fp); + + return 0; + + no_memory: + openvzError(NULL, VIR_ERR_NO_MEMORY, NULL); + + cleanup: + fclose(fp); + virDomainObjFree(dom); + return -1; } /* @@ -797,7 +463,7 @@ openvzSetUUID(int vpsid){ * */ -int openvzAssignUUIDs(void) +static int openvzAssignUUIDs(void) { DIR *dp; struct dirent *dent; diff -r e270be59a80f src/openvz_conf.h --- a/src/openvz_conf.h Wed Aug 27 17:03:25 2008 +0100 +++ b/src/openvz_conf.h Wed Aug 27 17:03:45 2008 +0100 @@ -28,94 +28,35 @@ #ifndef OPENVZ_CONF_H #define OPENVZ_CONF_H -#include "openvz_driver.h" +#include "internal.h" #include "domain_conf.h" enum { OPENVZ_WARN, OPENVZ_ERR }; -#define OPENVZ_NAME_MAX 8 -#define OPENVZ_TMPL_MAX 256 -#define OPENVZ_UNAME_MAX 32 -#define OPENVZ_IP_MAX 16 -#define OPENVZ_HOSTNAME_MAX 256 -#define OPENVZ_PROFILE_MAX 256 -#define OPENVZ_MAX_ERROR_LEN 1024 -#define OPENVZ_MAX_XML_LEN 4096 -#define OPENVZ_MAX_QUOTA 8 -#define OPENVZ_MAX_XPathEval_LEN 256 -#define OPENVZ_RSRV_VM_LIMIT 100 +#define openvzLog(level, msg...) { if(level == OPENVZ_WARN) \ + fprintf(stderr, "\nWARNING: ");\ + else \ + fprintf(stderr, "\nERROR: ");\ + fprintf(stderr, "\n\t");\ + fprintf(stderr, msg);\ + fprintf(stderr, "\n"); } -enum openvz_quota{ - VM_LEVEL = 0, - USER_LEVEL = 1, +/* OpenVZ commands - Replace with wrapper scripts later? */ +#define VZLIST "vzlist" +#define VZCTL "vzctl" + +struct openvz_driver { + virCapsPtr caps; + virDomainObjPtr domains; }; - -/* TODO Add more properties here */ -struct vps_props { - int kmemsize; /* currently held */ - int kmemsize_m; /* max held */ - int kmemsize_b; /* barrier */ - int kmemsize_l; /* limit */ - int kmemsize_f; /* fail count */ - -}; - -struct openvz_fs_def { - char tmpl[OPENVZ_TMPL_MAX]; - long int disksize, diskinodes; -}; - -struct openvz_vm_def { - char name[OPENVZ_NAME_MAX]; - unsigned char uuid[VIR_UUID_BUFLEN]; - char profile[OPENVZ_PROFILE_MAX]; - unsigned long vcpus; - struct openvz_fs_def fs; - virDomainNetDefPtr net; -}; - -struct ovz_quota { - enum openvz_quota type; - unsigned int size; - char uname[OPENVZ_UNAME_MAX]; - struct ovz_quota *next; -}; - -struct openvz_vm { - int vpsid; - int status; - struct openvz_vm_def *vmdef; - struct openvz_vm *next; -}; - -static inline int -openvzIsActiveVM(struct openvz_vm *vm) -{ - return vm->vpsid != -1; -} void openvzError (virConnectPtr conn, virErrorNumber code, const char *fmt, ...) ATTRIBUTE_FORMAT(printf, 3, 4); int openvz_readline(int fd, char *ptr, int maxlen); int openvzReadConfigParam(int vpsid ,const char * param, char *value, int maxlen); -struct openvz_vm *openvzFindVMByID(const struct openvz_driver *driver, int id); -struct openvz_vm *openvzFindVMByUUID(const struct openvz_driver *driver, - const unsigned char *uuid); - -struct openvz_vm *openvzFindVMByName(const struct openvz_driver *driver, const char *name); -struct openvz_vm_def *openvzParseVMDef(virConnectPtr conn, const char *xmlStr, - const char *displayName); - -struct openvz_vm *openvzAssignVMDef(virConnectPtr conn, struct openvz_driver *driver, - struct openvz_vm_def *def); - -struct openvz_vm *openvzGetVPSInfo(virConnectPtr conn); -void openvzGenerateUUID(unsigned char *uuid); -int openvzAssignUUIDs(void); -void openvzRemoveInactiveVM(struct openvz_driver *driver, struct openvz_vm *vm); +virCapsPtr openvzCapsInit(void); +int openvzLoadDomains(struct openvz_driver *driver); void openvzFreeDriver(struct openvz_driver *driver); -void openvzFreeVM(struct openvz_driver *driver, struct openvz_vm *vm, int checkCallee); -void openvzFreeVMDef(struct openvz_vm_def *def); int strtoI(const char *str); int openvzCheckEmptyMac(const unsigned char *mac); char *openvzMacToString(const unsigned char *mac); diff -r e270be59a80f src/openvz_driver.c --- a/src/openvz_driver.c Wed Aug 27 17:03:25 2008 +0100 +++ b/src/openvz_driver.c Thu Aug 28 09:50:23 2008 +0100 @@ -60,33 +60,6 @@ #define CMDBUF_LEN 1488 #define CMDOP_LEN 288 -static virDomainPtr openvzDomainLookupByID(virConnectPtr conn, int id); -static char *openvzGetOSType(virDomainPtr dom); -static int openvzGetNodeInfo(virConnectPtr conn, virNodeInfoPtr nodeinfo); -static virDomainPtr openvzDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid); -static virDomainPtr openvzDomainLookupByName(virConnectPtr conn, const char *name); -static int openvzDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info); -static int openvzDomainShutdown(virDomainPtr dom); -static int openvzDomainReboot(virDomainPtr dom, unsigned int flags); -static int openvzDomainCreate(virDomainPtr dom); -static virDrvOpenStatus openvzOpen(virConnectPtr conn, - xmlURIPtr uri, - virConnectAuthPtr auth ATTRIBUTE_UNUSED, - int flags ATTRIBUTE_UNUSED); -static int openvzClose(virConnectPtr conn); -static const char *openvzGetType(virConnectPtr conn ATTRIBUTE_UNUSED); -static int openvzListDomains(virConnectPtr conn, int *ids, int nids); -static int openvzNumDomains(virConnectPtr conn); -static int openvzListDefinedDomains(virConnectPtr conn, char **const names, int nnames); -static int openvzNumDefinedDomains(virConnectPtr conn); - -static virDomainPtr openvzDomainDefineXML(virConnectPtr conn, const char *xml); -static virDomainPtr openvzDomainCreateLinux(virConnectPtr conn, const char *xml, - unsigned int flags ATTRIBUTE_UNUSED); - -static int openvzDomainUndefine(virDomainPtr dom); -static void cmdExecFree(const char *cmdExec[]); - static int openvzGetProcessInfo(unsigned long long *cpuTime, int vpsid); static int openvzGetMaxVCPUs(virConnectPtr conn, const char *type); static int openvzDomainGetMaxVcpus(virDomainPtr dom); @@ -110,7 +83,7 @@ static int openvzDomainDefineCmd(virConn static int openvzDomainDefineCmd(virConnectPtr conn, const char *args[], int maxarg, - struct openvz_vm_def *vmdef) + virDomainDefPtr vmdef) { int narg; @@ -143,15 +116,29 @@ static int openvzDomainDefineCmd(virConn ADD_ARG_LIT("--quiet"); ADD_ARG_LIT("create"); ADD_ARG_LIT(vmdef->name); - - if ((vmdef->fs.tmpl && *(vmdef->fs.tmpl))) { + + if (vmdef->fss) { + if (vmdef->fss->type != VIR_DOMAIN_FS_TYPE_TEMPLATE) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("only filesystem templates are supported")); + return -1; + } + + if (vmdef->fss->next) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("only one filesystem supported")); + return -1; + } + ADD_ARG_LIT("--ostemplate"); - ADD_ARG_LIT(vmdef->fs.tmpl); - } + ADD_ARG_LIT(vmdef->fss->src); + } +#if 0 if ((vmdef->profile && *(vmdef->profile))) { ADD_ARG_LIT("--config"); ADD_ARG_LIT(vmdef->profile); } +#endif ADD_ARG(NULL); return 0; @@ -165,39 +152,43 @@ static int openvzDomainDefineCmd(virConn static virDomainPtr openvzDomainLookupByID(virConnectPtr conn, - int id) { + int id) { struct openvz_driver *driver = (struct openvz_driver *)conn->privateData; - struct openvz_vm *vm; + virDomainObjPtr vm; virDomainPtr dom; - vm = openvzFindVMByID(driver, id); + vm = virDomainFindByID(driver->domains, id); if (!vm) { openvzError(conn, VIR_ERR_NO_DOMAIN, NULL); return NULL; } - dom = virGetDomain(conn, vm->vmdef->name, vm->vmdef->uuid); - if (!dom) { - openvzError(conn, VIR_ERR_NO_MEMORY, _("virDomainPtr")); - return NULL; - } - - dom->id = vm->vpsid; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid); + if (!dom) + return NULL; + + dom->id = vm->def->id; return dom; } -static char *openvzGetOSType(virDomainPtr dom ATTRIBUTE_UNUSED) -{ - /* OpenVZ runs on Linux and runs only Linux */ - return strdup("linux"); +static char *openvzGetOSType(virDomainPtr dom) +{ + struct openvz_driver *driver = (struct openvz_driver *)dom->conn->privateData; + virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + char *ret = strdup(vm->def->os.type); + + if (!ret) + openvzError(dom->conn, VIR_ERR_NO_MEMORY, NULL); + + return ret; } static virDomainPtr openvzDomainLookupByUUID(virConnectPtr conn, - const unsigned char *uuid) { + const unsigned char *uuid) { struct openvz_driver *driver = (struct openvz_driver *)conn->privateData; - struct openvz_vm *vm = openvzFindVMByUUID(driver, uuid); + virDomainObjPtr vm = virDomainFindByUUID(driver->domains, uuid); virDomainPtr dom; if (!vm) { @@ -205,20 +196,18 @@ static virDomainPtr openvzDomainLookupBy return NULL; } - dom = virGetDomain(conn, vm->vmdef->name, vm->vmdef->uuid); - if (!dom) { - openvzError(conn, VIR_ERR_NO_MEMORY, _("virDomainPtr")); - return NULL; - } - - dom->id = vm->vpsid; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid); + if (!dom) + return NULL; + + dom->id = vm->def->id; return dom; } static virDomainPtr openvzDomainLookupByName(virConnectPtr conn, const char *name) { struct openvz_driver *driver = (struct openvz_driver *)conn->privateData; - struct openvz_vm *vm = openvzFindVMByName(driver, name); + virDomainObjPtr vm = virDomainFindByName(driver->domains, name); virDomainPtr dom; if (!vm) { @@ -226,74 +215,81 @@ static virDomainPtr openvzDomainLookupBy return NULL; } - dom = virGetDomain(conn, vm->vmdef->name, vm->vmdef->uuid); - if (!dom) { - openvzError(conn, VIR_ERR_NO_MEMORY, _("virDomainPtr")); - return NULL; - } - - dom->id = vm->vpsid; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid); + if (!dom) + return NULL; + + dom->id = vm->def->id; return dom; } static int openvzDomainGetInfo(virDomainPtr dom, - virDomainInfoPtr info) { + virDomainInfoPtr info) { struct openvz_driver *driver = (struct openvz_driver *)dom->conn->privateData; - struct openvz_vm *vm = openvzFindVMByUUID(driver, dom->uuid); + virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); if (!vm) { openvzError(dom->conn, VIR_ERR_INVALID_DOMAIN, - _("no domain with matching uuid")); - return -1; - } - - info->state = vm->status; - - if (!openvzIsActiveVM(vm)) { + _("no domain with matching uuid")); + return -1; + } + + info->state = vm->state; + + if (!virDomainIsActive(vm)) { info->cpuTime = 0; } else { if (openvzGetProcessInfo(&(info->cpuTime), dom->id) < 0) { openvzError(dom->conn, VIR_ERR_OPERATION_FAILED, - _("cannot read cputime for domain %d"), dom->id); + _("cannot read cputime for domain %d"), dom->id); return -1; } } - /* TODO These need to be calculated differently for OpenVZ */ - //info->cpuTime = - //info->maxMem = vm->def->maxmem; - //info->memory = vm->def->memory; - info->nrVirtCpu = vm->vmdef->vcpus; - return 0; -} + info->maxMem = vm->def->maxmem; + info->memory = vm->def->memory; + info->nrVirtCpu = vm->def->vcpus; + return 0; +} + + +static char *openvzDomainDumpXML(virDomainPtr dom, int flags) { + struct openvz_driver *driver = (struct openvz_driver *)dom->conn->privateData; + virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + + if (!vm) { + openvzError(dom->conn, VIR_ERR_INVALID_DOMAIN, + _("no domain with matching uuid")); + return NULL; + } + + return virDomainDefFormat(dom->conn, vm->def, flags); +} + + static int openvzDomainShutdown(virDomainPtr dom) { struct openvz_driver *driver = (struct openvz_driver *)dom->conn->privateData; - struct openvz_vm *vm = openvzFindVMByUUID(driver, dom->uuid); - const char *prog[] = {VZCTL, "--quiet", "stop", vm->vmdef->name, NULL}; + virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + const char *prog[] = {VZCTL, "--quiet", "stop", vm->def->name, NULL}; if (!vm) { openvzError(dom->conn, VIR_ERR_INVALID_DOMAIN, - _("no domain with matching id")); - return -1; - } - - if (vm->status != VIR_DOMAIN_RUNNING) { - openvzError(dom->conn, VIR_ERR_OPERATION_DENIED, - _("domain is not in running state")); - return -1; - } - - if (virRun(dom->conn, prog, NULL) < 0) { + _("no domain with matching uuid")); + return -1; + } + + if (vm->state != VIR_DOMAIN_RUNNING) { openvzError(dom->conn, VIR_ERR_INTERNAL_ERROR, - _("Could not exec %s"), VZCTL); - return -1; - } - - vm->vpsid = -1; - vm->status = VIR_DOMAIN_SHUTOFF; - ovz_driver.num_inactive ++; - ovz_driver.num_active --; + _("domain is not in running state")); + return -1; + } + + if (virRun(dom->conn, prog, NULL) < 0) + return -1; + + vm->def->id = -1; + vm->state = VIR_DOMAIN_SHUTOFF; return 0; } @@ -301,26 +297,23 @@ static int openvzDomainReboot(virDomainP static int openvzDomainReboot(virDomainPtr dom, unsigned int flags ATTRIBUTE_UNUSED) { struct openvz_driver *driver = (struct openvz_driver *)dom->conn->privateData; - struct openvz_vm *vm = openvzFindVMByUUID(driver, dom->uuid); - const char *prog[] = {VZCTL, "--quiet", "restart", vm->vmdef->name, NULL}; + virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + const char *prog[] = {VZCTL, "--quiet", "restart", vm->def->name, NULL}; if (!vm) { openvzError(dom->conn, VIR_ERR_INVALID_DOMAIN, - _("no domain with matching id")); - return -1; - } - - if (vm->status != VIR_DOMAIN_RUNNING) { - openvzError(dom->conn, VIR_ERR_OPERATION_DENIED, - _("domain is not in running state")); - return -1; - } - - if (virRun(dom->conn, prog, NULL) < 0) { + _("no domain with matching uuid")); + return -1; + } + + if (vm->state != VIR_DOMAIN_RUNNING) { openvzError(dom->conn, VIR_ERR_INTERNAL_ERROR, - _("Could not exec %s"), VZCTL); - return -1; - } + _("domain is not in running state")); + return -1; + } + + if (virRun(dom->conn, prog, NULL) < 0) + return -1; return 0; } @@ -421,24 +414,31 @@ openvzDomainDefineXML(virConnectPtr conn openvzDomainDefineXML(virConnectPtr conn, const char *xml) { struct openvz_driver *driver = (struct openvz_driver *) conn->privateData; - struct openvz_vm_def *vmdef = NULL; - struct openvz_vm *vm = NULL; + virDomainDefPtr vmdef = NULL; + virDomainObjPtr vm = NULL; virDomainPtr dom = NULL; const char *prog[OPENVZ_MAX_ARG]; prog[0] = NULL; - if ((vmdef = openvzParseVMDef(conn, xml, NULL)) == NULL) - return NULL; - - vm = openvzFindVMByName(driver, vmdef->name); + if ((vmdef = virDomainDefParseString(conn, driver->caps, xml)) == NULL) + return NULL; + + if (vmdef->os.init == NULL && + !(vmdef->os.init = strdup("/sbin/init"))) { + virDomainDefFree(vmdef); + return NULL; + } + + vm = virDomainFindByName(driver->domains, vmdef->name); if (vm) { + virDomainDefFree(vmdef); openvzLog(OPENVZ_ERR, _("Already an OPENVZ VM active with the id '%s'"), vmdef->name); return NULL; } - if (!(vm = openvzAssignVMDef(conn, driver, vmdef))) { - openvzFreeVMDef(vmdef); - openvzLog(OPENVZ_ERR, "%s", _("Error creating OPENVZ VM")); + if (!(vm = virDomainAssignDef(conn, &driver->domains, vmdef))) { + virDomainDefFree(vmdef); + return NULL; } if (openvzDomainDefineCmd(conn, prog, OPENVZ_MAX_ARG, vmdef) < 0) { @@ -461,11 +461,11 @@ openvzDomainDefineXML(virConnectPtr conn goto exit; } - dom = virGetDomain(conn, vm->vmdef->name, vm->vmdef->uuid); + dom = virGetDomain(conn, vm->def->name, vm->def->uuid); if (dom) - dom->id = vm->vpsid; - - if (openvzDomainSetNetwork(conn, vmdef->name, vmdef->net) < 0) { + dom->id = -1; + + if (openvzDomainSetNetwork(conn, vmdef->name, vmdef->nets) < 0) { openvzError(conn, VIR_ERR_INTERNAL_ERROR, _("Could not configure network")); goto exit; @@ -488,27 +488,33 @@ openvzDomainCreateLinux(virConnectPtr co openvzDomainCreateLinux(virConnectPtr conn, const char *xml, unsigned int flags ATTRIBUTE_UNUSED) { - struct openvz_vm_def *vmdef = NULL; - struct openvz_vm *vm = NULL; + virDomainDefPtr vmdef = NULL; + virDomainObjPtr vm = NULL; virDomainPtr dom = NULL; struct openvz_driver *driver = (struct openvz_driver *) conn->privateData; const char *progstart[] = {VZCTL, "--quiet", "start", NULL, NULL}; const char *progcreate[OPENVZ_MAX_ARG]; progcreate[0] = NULL; - if (!(vmdef = openvzParseVMDef(conn, xml, NULL))) - return NULL; - - vm = openvzFindVMByName(driver, vmdef->name); + if ((vmdef = virDomainDefParseString(conn, driver->caps, xml)) == NULL) + return NULL; + + if (vmdef->os.init == NULL && + !(vmdef->os.init = strdup("/sbin/init"))) { + virDomainDefFree(vmdef); + return NULL; + } + + vm = virDomainFindByName(driver->domains, vmdef->name); if (vm) { - openvzFreeVMDef(vmdef); + virDomainDefFree(vmdef); openvzLog(OPENVZ_ERR, _("Already an OPENVZ VM defined with the id '%d'"), strtoI(vmdef->name)); return NULL; } - if (!(vm = openvzAssignVMDef(conn, driver, vmdef))) { - openvzLog(OPENVZ_ERR, "%s", _("Error creating OPENVZ VM")); + if (!(vm = virDomainAssignDef(conn, &driver->domains, vmdef))) { + virDomainDefFree(vmdef); return NULL; } @@ -530,7 +536,7 @@ openvzDomainCreateLinux(virConnectPtr co goto exit; } - if (openvzDomainSetNetwork(conn, vmdef->name, vmdef->net) < 0) { + if (openvzDomainSetNetwork(conn, vmdef->name, vmdef->nets) < 0) { openvzError(conn, VIR_ERR_INTERNAL_ERROR, _("Could not configure network")); goto exit; @@ -544,20 +550,19 @@ openvzDomainCreateLinux(virConnectPtr co goto exit; } - vm->vpsid = strtoI(vmdef->name); - vm->status = VIR_DOMAIN_RUNNING; - ovz_driver.num_inactive--; - ovz_driver.num_active++; - - dom = virGetDomain(conn, vm->vmdef->name, vm->vmdef->uuid); + vm->pid = strtoI(vmdef->name); + vm->def->id = vm->pid; + vm->state = VIR_DOMAIN_RUNNING; + + dom = virGetDomain(conn, vm->def->name, vm->def->uuid); if (dom) - dom->id = vm->vpsid; + dom->id = vm->def->id; if (vmdef->vcpus > 0) { if (openvzDomainSetVcpus(dom, vmdef->vcpus) < 0) { openvzError(conn, VIR_ERR_INTERNAL_ERROR, - _("Could not set number of virtual cpu")); - goto exit; + _("Could not set number of virtual cpu")); + goto exit; } } @@ -570,8 +575,8 @@ openvzDomainCreate(virDomainPtr dom) openvzDomainCreate(virDomainPtr dom) { struct openvz_driver *driver = (struct openvz_driver *)dom->conn->privateData; - struct openvz_vm *vm = openvzFindVMByName(driver, dom->name); - const char *prog[] = {VZCTL, "--quiet", "start", vm->vmdef->name, NULL }; + virDomainObjPtr vm = virDomainFindByName(driver->domains, dom->name); + const char *prog[] = {VZCTL, "--quiet", "start", vm->def->name, NULL }; if (!vm) { openvzError(dom->conn, VIR_ERR_INVALID_DOMAIN, @@ -579,7 +584,7 @@ openvzDomainCreate(virDomainPtr dom) return -1; } - if (vm->status != VIR_DOMAIN_SHUTOFF) { + if (vm->state != VIR_DOMAIN_SHUTOFF) { openvzError(dom->conn, VIR_ERR_OPERATION_DENIED, _("domain is not in shutoff state")); return -1; @@ -591,10 +596,9 @@ openvzDomainCreate(virDomainPtr dom) return -1; } - vm->vpsid = strtoI(vm->vmdef->name); - vm->status = VIR_DOMAIN_RUNNING; - ovz_driver.num_inactive --; - ovz_driver.num_active ++; + vm->pid = strtoI(vm->def->name); + vm->def->id = vm->pid; + vm->state = VIR_DOMAIN_RUNNING; return 0; } @@ -604,15 +608,15 @@ openvzDomainUndefine(virDomainPtr dom) { virConnectPtr conn= dom->conn; struct openvz_driver *driver = (struct openvz_driver *) conn->privateData; - struct openvz_vm *vm = openvzFindVMByUUID(driver, dom->uuid); - const char *prog[] = { VZCTL, "--quiet", "destroy", vm->vmdef->name, NULL }; + virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + const char *prog[] = { VZCTL, "--quiet", "destroy", vm->def->name, NULL }; if (!vm) { openvzError(conn, VIR_ERR_INVALID_DOMAIN, _("no domain with matching uuid")); return -1; } - if (openvzIsActiveVM(vm)) { + if (virDomainIsActive(vm)) { openvzError(conn, VIR_ERR_INTERNAL_ERROR, _("cannot delete active domain")); return -1; } @@ -623,7 +627,8 @@ openvzDomainUndefine(virDomainPtr dom) return -1; } - openvzRemoveInactiveVM(driver, vm); + virDomainRemoveInactive(&driver->domains, vm); + return 0; } @@ -632,8 +637,8 @@ openvzDomainSetAutostart(virDomainPtr do { virConnectPtr conn= dom->conn; struct openvz_driver *driver = (struct openvz_driver *) conn->privateData; - struct openvz_vm *vm = openvzFindVMByUUID(driver, dom->uuid); - const char *prog[] = { VZCTL, "--quiet", "set", vm->vmdef->name, + virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + const char *prog[] = { VZCTL, "--quiet", "set", vm->def->name, "--onboot", autostart ? "yes" : "no", "--save", NULL }; @@ -655,7 +660,7 @@ openvzDomainGetAutostart(virDomainPtr do { virConnectPtr conn= dom->conn; struct openvz_driver *driver = (struct openvz_driver *) conn->privateData; - struct openvz_vm *vm = openvzFindVMByUUID(driver, dom->uuid); + virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); char value[1024]; if (!vm) { @@ -663,7 +668,7 @@ openvzDomainGetAutostart(virDomainPtr do return -1; } - if (openvzReadConfigParam(strtoI(vm->vmdef->name), "ONBOOT", value, sizeof(value)) < 0) { + if (openvzReadConfigParam(strtoI(vm->def->name), "ONBOOT", value, sizeof(value)) < 0) { openvzError(conn, VIR_ERR_INTERNAL_ERROR, _("Could not read container config")); return -1; } @@ -692,32 +697,32 @@ static int openvzDomainSetVcpus(virDomai static int openvzDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus) { virConnectPtr conn= dom->conn; struct openvz_driver *driver = (struct openvz_driver *) conn->privateData; - struct openvz_vm *vm = openvzFindVMByUUID(driver, dom->uuid); + virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); char str_vcpus[32]; - const char *prog[] = { VZCTL, "--quiet", "set", vm->vmdef->name, + const char *prog[] = { VZCTL, "--quiet", "set", vm->def->name, "--cpus", str_vcpus, "--save", NULL }; snprintf(str_vcpus, 31, "%d", nvcpus); str_vcpus[31] = '\0'; if (nvcpus <= 0) { openvzError(conn, VIR_ERR_INTERNAL_ERROR, - _("VCPUs should be >= 1")); + _("VCPUs should be >= 1")); return -1; } if (!vm) { openvzError(conn, VIR_ERR_INVALID_DOMAIN, - _("no domain with matching uuid")); + _("no domain with matching uuid")); return -1; } if (virRun(conn, prog, NULL) < 0) { openvzError(conn, VIR_ERR_INTERNAL_ERROR, - _("Could not exec %s"), VZCTL); - return -1; - } - - vm->vmdef->vcpus = nvcpus; + _("Could not exec %s"), VZCTL); + return -1; + } + + vm->def->vcpus = nvcpus; return 0; } @@ -735,51 +740,48 @@ static virDrvOpenStatus openvzOpen(virCo virConnectAuthPtr auth ATTRIBUTE_UNUSED, int flags ATTRIBUTE_UNUSED) { - struct openvz_vm *vms; - + struct openvz_driver *driver; /*Just check if the user is root. Nothing really to open for OpenVZ */ - if (getuid()) { // OpenVZ tools can only be used by r00t - return VIR_DRV_OPEN_DECLINED; - } else { - if (uri == NULL || uri->scheme == NULL || uri->path == NULL) - return VIR_DRV_OPEN_DECLINED; - if (STRNEQ (uri->scheme, "openvz")) - return VIR_DRV_OPEN_DECLINED; - if (STRNEQ (uri->path, "/system")) - return VIR_DRV_OPEN_DECLINED; - } + if (getuid()) { // OpenVZ tools can only be used by r00t + return VIR_DRV_OPEN_DECLINED; + } else { + if (uri == NULL || uri->scheme == NULL || uri->path == NULL) + return VIR_DRV_OPEN_DECLINED; + if (STRNEQ (uri->scheme, "openvz")) + return VIR_DRV_OPEN_DECLINED; + if (STRNEQ (uri->path, "/system")) + return VIR_DRV_OPEN_DECLINED; + } /* See if we are running an OpenVZ enabled kernel */ - if(access("/proc/vz/veinfo", F_OK) == -1 || - access("/proc/user_beancounters", F_OK) == -1) { - return VIR_DRV_OPEN_DECLINED; - } - - conn->privateData = &ovz_driver; - - openvzAssignUUIDs(); - - vms = openvzGetVPSInfo(conn); - ovz_driver.vms = vms; - - return VIR_DRV_OPEN_SUCCESS; + if(access("/proc/vz/veinfo", F_OK) == -1 || + access("/proc/user_beancounters", F_OK) == -1) { + return VIR_DRV_OPEN_DECLINED; + } + + if (VIR_ALLOC(driver) < 0) { + openvzError(conn, VIR_ERR_NO_MEMORY, NULL); + return VIR_DRV_OPEN_ERROR; + } + + if (!(driver->caps = openvzCapsInit())) + goto cleanup; + + if (openvzLoadDomains(driver) < 0) + goto cleanup; + + conn->privateData = driver; + + return VIR_DRV_OPEN_SUCCESS; + +cleanup: + openvzFreeDriver(driver); + return VIR_DRV_OPEN_ERROR; }; static int openvzClose(virConnectPtr conn) { - struct openvz_driver *driver = (struct openvz_driver *)conn->privateData; - struct openvz_vm *vm = driver->vms; - - while(vm) { - openvzFreeVMDef(vm->vmdef); - vm = vm->next; - } - vm = driver->vms; - while (vm) { - struct openvz_vm *prev = vm; - vm = vm->next; - free(prev); - } - + + openvzFreeDriver(driver); conn->privateData = NULL; return 0; @@ -792,6 +794,12 @@ static int openvzGetNodeInfo(virConnectP static int openvzGetNodeInfo(virConnectPtr conn, virNodeInfoPtr nodeinfo) { return virNodeInfoPopulate(conn, nodeinfo); +} + +static char *openvzGetCapabilities(virConnectPtr conn) { + struct openvz_driver *driver = (struct openvz_driver *)conn->privateData; + + return virCapabilitiesFormatXML(driver->caps); } static int openvzListDomains(virConnectPtr conn, int *ids, int nids) { @@ -829,14 +837,22 @@ static int openvzListDomains(virConnectP } static int openvzNumDomains(virConnectPtr conn ATTRIBUTE_UNUSED) { - return ovz_driver.num_active; + struct openvz_driver *driver = conn->privateData; + int nactive = 0; + virDomainObjPtr vm = driver->domains; + while (vm) { + if (virDomainIsActive(vm)) + nactive++; + vm = vm->next; + } + return nactive; } static int openvzListDefinedDomains(virConnectPtr conn, - char **const names, int nnames) { + char **const names, int nnames) { int got = 0; int veid, pid, outfd = -1, errfd = -1, ret; - char vpsname[OPENVZ_NAME_MAX]; + char vpsname[32]; char buf[32]; char *endptr; const char *cmd[] = {VZLIST, "-ovpsid", "-H", "-S", NULL}; @@ -858,7 +874,7 @@ static int openvzListDefinedDomains(virC _("Could not parse VPS ID %s"), buf); continue; } - sprintf(vpsname, "%d", veid); + snprintf(vpsname, sizeof(vpsname), "%d", veid); names[got] = strdup(vpsname); got ++; } @@ -911,8 +927,16 @@ Version: 2.2 return 0; } -static int openvzNumDefinedDomains(virConnectPtr conn ATTRIBUTE_UNUSED) { - return ovz_driver.num_inactive; +static int openvzNumDefinedDomains(virConnectPtr conn) { + struct openvz_driver *driver = (struct openvz_driver *) conn->privateData; + int ninactive = 0; + virDomainObjPtr vm = driver->domains; + while (vm) { + if (!virDomainIsActive(vm)) + ninactive++; + vm = vm->next; + } + return ninactive; } static virDriver openvzDriver = { @@ -929,7 +953,7 @@ static virDriver openvzDriver = { NULL, /* uri */ openvzGetMaxVCPUs, /* getMaxVcpus */ openvzGetNodeInfo, /* nodeGetInfo */ - NULL, /* getCapabilities */ + openvzGetCapabilities, /* getCapabilities */ openvzListDomains, /* listDomains */ openvzNumDomains, /* numOfDomains */ openvzDomainCreateLinux, /* domainCreateLinux */ @@ -953,7 +977,7 @@ static virDriver openvzDriver = { NULL, /* domainPinVcpu */ NULL, /* domainGetVcpus */ openvzDomainGetMaxVcpus, /* domainGetMaxVcpus */ - NULL, /* domainDumpXML */ + openvzDomainDumpXML, /* domainDumpXML */ openvzListDefinedDomains, /* listDomains */ openvzNumDefinedDomains, /* numOfDomains */ openvzDomainCreate, /* domainCreate */ diff -r e270be59a80f src/openvz_driver.h --- a/src/openvz_driver.h Wed Aug 27 17:03:25 2008 +0100 +++ b/src/openvz_driver.h Wed Aug 27 17:03:45 2008 +0100 @@ -31,24 +31,6 @@ #include "internal.h" -/* OpenVZ commands - Replace with wrapper scripts later? */ -#define VZLIST "vzlist" -#define VZCTL "vzctl" - -struct openvz_driver { - struct openvz_vm *vms; - int num_active; - int num_inactive; -}; - int openvzRegister(void); -#define openvzLog(level, msg...) { if(level == OPENVZ_WARN) \ - fprintf(stderr, "\nWARNING: ");\ - else \ - fprintf(stderr, "\nERROR: ");\ - fprintf(stderr, "\n\t");\ - fprintf(stderr, msg);\ - fprintf(stderr, "\n"); } - #endif -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Thu, Aug 28, 2008 at 12:06:08PM +0400, Evgeniy Sokolov wrote:
The biggest flaw I see currently is that the openvz driver doesn't load the existing device config for networks or filesystems of existing VMs, so you can't see that info in the XML dump Yes. Good note. I will implement it. - /*get VCPU*/ - ret = openvzReadConfigParam(veid, "CPUS", temp, sizeof(temp)); - if (ret < 0) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, - _("Cound not read config for container %d"), veid); - goto error; - } else if (ret > 0) { - vmdef->vcpus = strtoI(temp); - } Why did you remove loading VCPUs and always set = 1 ?
Stupid mistake during a manual merge. Here's an updated patch which fixes that mistake
The patch looks good now.
Daniel
On Fri, Aug 29, 2008 at 12:54:18PM +0400, Evgeniy Sokolov wrote:
On Thu, Aug 28, 2008 at 12:06:08PM +0400, Evgeniy Sokolov wrote:
The biggest flaw I see currently is that the openvz driver doesn't load the existing device config for networks or filesystems of existing VMs, so you can't see that info in the XML dump Yes. Good note. I will implement it. - /*get VCPU*/ - ret = openvzReadConfigParam(veid, "CPUS", temp, sizeof(temp)); - if (ret < 0) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, - _("Cound not read config for container %d"), veid); - goto error; - } else if (ret > 0) { - vmdef->vcpus = strtoI(temp); - } Why did you remove loading VCPUs and always set = 1 ?
Stupid mistake during a manual merge. Here's an updated patch which fixes that mistake
The patch looks good now.
Thanks - i've committed this patch now, so now all our drivers are using the same XML processing APIs ! Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Fri, Sep 05, 2008 at 03:20:32PM +0100, Daniel P. Berrange wrote:
On Fri, Aug 29, 2008 at 12:54:18PM +0400, Evgeniy Sokolov wrote:
On Thu, Aug 28, 2008 at 12:06:08PM +0400, Evgeniy Sokolov wrote:
The biggest flaw I see currently is that the openvz driver doesn't load the existing device config for networks or filesystems of existing VMs, so you can't see that info in the XML dump Yes. Good note. I will implement it. - /*get VCPU*/ - ret = openvzReadConfigParam(veid, "CPUS", temp, sizeof(temp)); - if (ret < 0) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, - _("Cound not read config for container %d"), veid); - goto error; - } else if (ret > 0) { - vmdef->vcpus = strtoI(temp); - } Why did you remove loading VCPUs and always set = 1 ?
Stupid mistake during a manual merge. Here's an updated patch which fixes that mistake
The patch looks good now.
Thanks - i've committed this patch now, so now all our drivers are using the same XML processing APIs !
Excellent !!! Kudos for this, this is a huge long term improvement ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
"Daniel P. Berrange" <berrange@redhat.com> wrote:
Here's an updated patch ... diff -r e270be59a80f src/openvz_conf.c ... + if (VIR_ALLOC(dom) < 0 || + VIR_ALLOC(dom->def) < 0) + goto no_memory; + + if (STRNEQ(status, "stopped")) + dom->state = VIR_DOMAIN_RUNNING; + else + dom->state = VIR_DOMAIN_SHUTOFF;
I prefer to avoid negatives, if (STREQ(status, "stopped")) dom->state = VIR_DOMAIN_SHUTOFF; else dom->state = VIR_DOMAIN_RUNNING; And sometimes it's worthwhile to avoid the duplication of assignment+LHS. So, this seems slightly more readable/maintainable, since dom->state appears only once, and the two enum values are even closer to each other: dom->state = (STREQ(status, "stopped") ? VIR_DOMAIN_SHUTOFF : VIR_DOMAIN_RUNNING);
+ + dom->pid = veid; + dom->def->id = dom->state == VIR_DOMAIN_SHUTOFF ? -1 : veid; + ... diff -r e270be59a80f src/openvz_driver.c --- a/src/openvz_driver.c Wed Aug 27 17:03:25 2008 +0100 +++ b/src/openvz_driver.c Thu Aug 28 09:50:23 2008 +0100 ... +static char *openvzGetOSType(virDomainPtr dom) +{ + struct openvz_driver *driver = (struct openvz_driver *)dom->conn->privateData; + virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid);
handle a NULL vm
+ char *ret = strdup(vm->def->os.type); + + if (!ret) + openvzError(dom->conn, VIR_ERR_NO_MEMORY, NULL); + + return ret; } ... static int openvzDomainShutdown(virDomainPtr dom) { struct openvz_driver *driver = (struct openvz_driver *)dom->conn->privateData; - struct openvz_vm *vm = openvzFindVMByUUID(driver, dom->uuid); - const char *prog[] = {VZCTL, "--quiet", "stop", vm->vmdef->name, NULL}; + virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + const char *prog[] = {VZCTL, "--quiet", "stop", vm->def->name, NULL};
I know it's nothing new, but the above use of "vm" can dereference NULL.
if (!vm) { openvzError(dom->conn, VIR_ERR_INVALID_DOMAIN, - _("no domain with matching id")); - return -1; - }
...
@@ -301,26 +297,23 @@ static int openvzDomainReboot(virDomainP static int openvzDomainReboot(virDomainPtr dom, unsigned int flags ATTRIBUTE_UNUSED) { struct openvz_driver *driver = (struct openvz_driver *)dom->conn->privateData; - struct openvz_vm *vm = openvzFindVMByUUID(driver, dom->uuid); - const char *prog[] = {VZCTL, "--quiet", "restart", vm->vmdef->name, NULL}; + virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + const char *prog[] = {VZCTL, "--quiet", "restart", vm->def->name, NULL};
same here
if (!vm) { openvzError(dom->conn, VIR_ERR_INVALID_DOMAIN, - _("no domain with matching id")); - return -1; - }
...
@@ -570,8 +575,8 @@ openvzDomainCreate(virDomainPtr dom) openvzDomainCreate(virDomainPtr dom) { struct openvz_driver *driver = (struct openvz_driver *)dom->conn->privateData; - struct openvz_vm *vm = openvzFindVMByName(driver, dom->name); - const char *prog[] = {VZCTL, "--quiet", "start", vm->vmdef->name, NULL }; + virDomainObjPtr vm = virDomainFindByName(driver->domains, dom->name); + const char *prog[] = {VZCTL, "--quiet", "start", vm->def->name, NULL };
if (!vm) { openvzError(dom->conn, VIR_ERR_INVALID_DOMAIN, @@ -579,7 +584,7 @@ openvzDomainCreate(virDomainPtr dom) return -1; }
- if (vm->status != VIR_DOMAIN_SHUTOFF) { + if (vm->state != VIR_DOMAIN_SHUTOFF) { openvzError(dom->conn, VIR_ERR_OPERATION_DENIED, _("domain is not in shutoff state")); return -1; @@ -591,10 +596,9 @@ openvzDomainCreate(virDomainPtr dom) return -1; }
- vm->vpsid = strtoI(vm->vmdef->name); - vm->status = VIR_DOMAIN_RUNNING; - ovz_driver.num_inactive --; - ovz_driver.num_active ++; + vm->pid = strtoI(vm->def->name); + vm->def->id = vm->pid; + vm->state = VIR_DOMAIN_RUNNING;
return 0; } @@ -604,15 +608,15 @@ openvzDomainUndefine(virDomainPtr dom) { virConnectPtr conn= dom->conn; struct openvz_driver *driver = (struct openvz_driver *) conn->privateData; - struct openvz_vm *vm = openvzFindVMByUUID(driver, dom->uuid); - const char *prog[] = { VZCTL, "--quiet", "destroy", vm->vmdef->name, NULL }; + virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + const char *prog[] = { VZCTL, "--quiet", "destroy", vm->def->name, NULL };
Same here: handle vm == NULL
if (!vm) { openvzError(conn, VIR_ERR_INVALID_DOMAIN, _("no domain with matching uuid")); return -1; }
- if (openvzIsActiveVM(vm)) { + if (virDomainIsActive(vm)) { openvzError(conn, VIR_ERR_INTERNAL_ERROR, _("cannot delete active domain")); return -1; } @@ -623,7 +627,8 @@ openvzDomainUndefine(virDomainPtr dom) return -1; }
- openvzRemoveInactiveVM(driver, vm); + virDomainRemoveInactive(&driver->domains, vm); + return 0; }
@@ -632,8 +637,8 @@ openvzDomainSetAutostart(virDomainPtr do { virConnectPtr conn= dom->conn; struct openvz_driver *driver = (struct openvz_driver *) conn->privateData; - struct openvz_vm *vm = openvzFindVMByUUID(driver, dom->uuid); - const char *prog[] = { VZCTL, "--quiet", "set", vm->vmdef->name, + virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + const char *prog[] = { VZCTL, "--quiet", "set", vm->def->name,
And again.
"--onboot", autostart ? "yes" : "no", "--save", NULL };
@@ -655,7 +660,7 @@ openvzDomainGetAutostart(virDomainPtr do { virConnectPtr conn= dom->conn; struct openvz_driver *driver = (struct openvz_driver *) conn->privateData; - struct openvz_vm *vm = openvzFindVMByUUID(driver, dom->uuid); + virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); char value[1024];
if (!vm) { @@ -663,7 +668,7 @@ openvzDomainGetAutostart(virDomainPtr do return -1; }
- if (openvzReadConfigParam(strtoI(vm->vmdef->name), "ONBOOT", value, sizeof(value)) < 0) { + if (openvzReadConfigParam(strtoI(vm->def->name), "ONBOOT", value, sizeof(value)) < 0) { openvzError(conn, VIR_ERR_INTERNAL_ERROR, _("Could not read container config")); return -1; } @@ -692,32 +697,32 @@ static int openvzDomainSetVcpus(virDomai static int openvzDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus) { virConnectPtr conn= dom->conn; struct openvz_driver *driver = (struct openvz_driver *) conn->privateData; - struct openvz_vm *vm = openvzFindVMByUUID(driver, dom->uuid); + virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); char str_vcpus[32]; - const char *prog[] = { VZCTL, "--quiet", "set", vm->vmdef->name, + const char *prog[] = { VZCTL, "--quiet", "set", vm->def->name,
and again
@@ -735,51 +740,48 @@ static virDrvOpenStatus openvzOpen(virCo virConnectAuthPtr auth ATTRIBUTE_UNUSED, int flags ATTRIBUTE_UNUSED) { - struct openvz_vm *vms; - + struct openvz_driver *driver; /*Just check if the user is root. Nothing really to open for OpenVZ */ - if (getuid()) { // OpenVZ tools can only be used by r00t - return VIR_DRV_OPEN_DECLINED; - } else { - if (uri == NULL || uri->scheme == NULL || uri->path == NULL) - return VIR_DRV_OPEN_DECLINED; - if (STRNEQ (uri->scheme, "openvz")) - return VIR_DRV_OPEN_DECLINED; - if (STRNEQ (uri->path, "/system")) - return VIR_DRV_OPEN_DECLINED; - } + if (getuid()) { // OpenVZ tools can only be used by r00t
If you use geteuid, this will work also when EUID==0, but UID!=0.
+ return VIR_DRV_OPEN_DECLINED; + } else { + if (uri == NULL || uri->scheme == NULL || uri->path == NULL) + return VIR_DRV_OPEN_DECLINED; + if (STRNEQ (uri->scheme, "openvz")) + return VIR_DRV_OPEN_DECLINED; + if (STRNEQ (uri->path, "/system")) + return VIR_DRV_OPEN_DECLINED;
How about the following equivalent code, instead. Then, readers don't have to visually compare the three return statements in order to understand that they're all returning the same value: if (uri == NULL || uri->scheme == NULL || uri->path == NULL || STRNEQ (uri->scheme, "openvz") || STRNEQ (uri->path, "/system")) return VIR_DRV_OPEN_DECLINED; ...
+ if(access("/proc/vz/veinfo", F_OK) == -1 || + access("/proc/user_beancounters", F_OK) == -1) { + return VIR_DRV_OPEN_DECLINED; + } + + if (VIR_ALLOC(driver) < 0) { + openvzError(conn, VIR_ERR_NO_MEMORY, NULL); + return VIR_DRV_OPEN_ERROR; + } + + if (!(driver->caps = openvzCapsInit())) + goto cleanup;
At first I thought that taking the above "goto cleanup" was a bug, since openvzFreeDriver dereferences driver->vms, which looked like it would be uninitialized in the just-malloc'd buffer. Actually, it's ok, because VIR_ALLOC performs a calloc-like memset-'\0' of the returned buffer.
+ if (openvzLoadDomains(driver) < 0) + goto cleanup; _("Could not parse VPS ID %s"), buf); continue; } - sprintf(vpsname, "%d", veid); + snprintf(vpsname, sizeof(vpsname), "%d", veid); names[got] = strdup(vpsname);
Not yours (it's in context after all), but we must not put NULL in the "names" array, since virsh.c's cmdList function sorts the strings in that list. Hence we must detect failed strdup here.
got ++; }
...
On Fri, Aug 29, 2008 at 05:14:12PM +0200, Jim Meyering wrote:
"Daniel P. Berrange" <berrange@redhat.com> wrote:
Here's an updated patch ... diff -r e270be59a80f src/openvz_conf.c ... + if (VIR_ALLOC(dom) < 0 || + VIR_ALLOC(dom->def) < 0) + goto no_memory; + + if (STRNEQ(status, "stopped")) + dom->state = VIR_DOMAIN_RUNNING; + else + dom->state = VIR_DOMAIN_SHUTOFF;
I prefer to avoid negatives,
if (STREQ(status, "stopped")) dom->state = VIR_DOMAIN_SHUTOFF; else dom->state = VIR_DOMAIN_RUNNING;
I made this made.
+ + dom->pid = veid; + dom->def->id = dom->state == VIR_DOMAIN_SHUTOFF ? -1 : veid; + ... diff -r e270be59a80f src/openvz_driver.c --- a/src/openvz_driver.c Wed Aug 27 17:03:25 2008 +0100 +++ b/src/openvz_driver.c Thu Aug 28 09:50:23 2008 +0100 ... +static char *openvzGetOSType(virDomainPtr dom) +{ + struct openvz_driver *driver = (struct openvz_driver *)dom->conn->privateData; + virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid);
handle a NULL vm
And this change.
static int openvzDomainShutdown(virDomainPtr dom) { struct openvz_driver *driver = (struct openvz_driver *)dom->conn->privateData; - struct openvz_vm *vm = openvzFindVMByUUID(driver, dom->uuid); - const char *prog[] = {VZCTL, "--quiet", "stop", vm->vmdef->name, NULL}; + virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + const char *prog[] = {VZCTL, "--quiet", "stop", vm->def->name, NULL};
I know it's nothing new, but the above use of "vm" can dereference NULL.
Every 'virDomainPtr' object is /required/ to have a non-NULL 'def' field. Only the 'newDef' field is alllowed to be NULL.
+ return VIR_DRV_OPEN_DECLINED; + } else { + if (uri == NULL || uri->scheme == NULL || uri->path == NULL) + return VIR_DRV_OPEN_DECLINED; + if (STRNEQ (uri->scheme, "openvz")) + return VIR_DRV_OPEN_DECLINED; + if (STRNEQ (uri->path, "/system")) + return VIR_DRV_OPEN_DECLINED;
How about the following equivalent code, instead. Then, readers don't have to visually compare the three return statements in order to understand that they're all returning the same value:
if (uri == NULL || uri->scheme == NULL || uri->path == NULL || STRNEQ (uri->scheme, "openvz") || STRNEQ (uri->path, "/system")) return VIR_DRV_OPEN_DECLINED;
I made that change to.
+ if (openvzLoadDomains(driver) < 0) + goto cleanup; _("Could not parse VPS ID %s"), buf); continue; } - sprintf(vpsname, "%d", veid); + snprintf(vpsname, sizeof(vpsname), "%d", veid); names[got] = strdup(vpsname);
Not yours (it's in context after all), but we must not put NULL in the "names" array, since virsh.c's cmdList function sorts the strings in that list. Hence we must detect failed strdup here.
I put in a check & cleanup for OOM there. Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
"Daniel P. Berrange" <berrange@redhat.com> wrote:
static int openvzDomainShutdown(virDomainPtr dom) { struct openvz_driver *driver = (struct openvz_driver *)dom->conn->privateData; - struct openvz_vm *vm = openvzFindVMByUUID(driver, dom->uuid); - const char *prog[] = {VZCTL, "--quiet", "stop", vm->vmdef->name, NULL}; + virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + const char *prog[] = {VZCTL, "--quiet", "stop", vm->def->name, NULL};
I know it's nothing new, but the above use of "vm" can dereference NULL.
Every 'virDomainPtr' object is /required/ to have a non-NULL 'def' field. Only the 'newDef' field is alllowed to be NULL.
Yes, but "vm" itself will be NULL if virDomainFindByUUID fails.
On Fri, Sep 05, 2008 at 04:31:06PM +0200, Jim Meyering wrote:
"Daniel P. Berrange" <berrange@redhat.com> wrote:
static int openvzDomainShutdown(virDomainPtr dom) { struct openvz_driver *driver = (struct openvz_driver *)dom->conn->privateData; - struct openvz_vm *vm = openvzFindVMByUUID(driver, dom->uuid); - const char *prog[] = {VZCTL, "--quiet", "stop", vm->vmdef->name, NULL}; + virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + const char *prog[] = {VZCTL, "--quiet", "stop", vm->def->name, NULL};
I know it's nothing new, but the above use of "vm" can dereference NULL.
Every 'virDomainPtr' object is /required/ to have a non-NULL 'def' field. Only the 'newDef' field is alllowed to be NULL.
Yes, but "vm" itself will be NULL if virDomainFindByUUID fails.
Oh, you meant the first pointer deference, not the second Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Thu, Aug 28, 2008 at 09:53:57AM +0100, Daniel P. Berrange wrote:
On Thu, Aug 28, 2008 at 12:06:08PM +0400, Evgeniy Sokolov wrote:
The biggest flaw I see currently is that the openvz driver doesn't load the existing device config for networks or filesystems of existing VMs, so you can't see that info in the XML dump Yes. Good note. I will implement it.
- /*get VCPU*/ - ret = openvzReadConfigParam(veid, "CPUS", temp, sizeof(temp)); - if (ret < 0) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, - _("Cound not read config for container %d"), veid); - goto error; - } else if (ret > 0) { - vmdef->vcpus = strtoI(temp); - } Why did you remove loading VCPUs and always set = 1 ?
Stupid mistake during a manual merge. Here's an updated patch which fixes that mistake
Fine by me, if Evgeniy is okay with it, let's commit this once Jim's comments are taken into account. I just wishes patch could learn about function boundaries instead of cherry picking pieces left and right to generate the new output, +1 Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (4)
-
Daniel P. Berrange -
Daniel Veillard -
Evgeniy Sokolov -
Jim Meyering