Cole Robinson wrote:

The attached patch implements the following operations for the qemu driver:

virDomainGetMaxMemory virDomainSetMaxMemory virDomainSetMemory

A few questions/comments:

1) I changed maxmem and memory in the qemu_vm_def struct to unsigned long to match the public api for memory values. Seems to work, but not sure if there are any undesirable side effects to this.

2) Should SetMaxMem be able to be called on a running guest? This code allows it, since maxmem is basically a metavalue that doesn't directly affect a guest.

3) Should maxmem be able to be set lower than the currently allocated mem? This code does not allow this. If this changed, would also need to take into account how we would handle this if we can change the maxmem while the guest is running. After rethinking, we probably should be able to do this, but I haven't changed the code.

Thanks, Cole

In testing this with virt-manager I found a bug: I was using comparing maxmem against the wrong memory value. Fixed this and updated the parameter names to be a bit more clear. Seems to work as excepted now. Thanks, Cole diff --git a/src/qemu_conf.c b/src/qemu_conf.c index a196bb8..f6ae06b 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -1650,7 +1650,7 @@ int qemudBuildCommandLine(virConnectPtr conn, (vm->def->graphicsType == QEMUD_GRAPHICS_SDL ? 0 : 1)) + /* graphics */ (vm->migrateFrom[0] ? 3 : 0); /* migrateFrom */ - snprintf(memory, sizeof(memory), "%d", vm->def->memory/1024); + snprintf(memory, sizeof(memory), "%lu", vm->def->memory/1024); snprintf(vcpus, sizeof(vcpus), "%d", vm->def->vcpus); if (!(*argv = malloc(sizeof(**argv) * (len+1)))) @@ -2820,9 +2820,9 @@ char *qemudGenerateXML(virConnectPtr conn, virUUIDFormat(uuid, uuidstr); if (virBufferVSprintf(buf, " <uuid>%s</uuid>\n", uuidstr) < 0) goto no_memory; - if (virBufferVSprintf(buf, " <memory>%d</memory>\n", def->maxmem) < 0) + if (virBufferVSprintf(buf, " <memory>%lu</memory>\n", def->maxmem) < 0) goto no_memory; - if (virBufferVSprintf(buf, " <currentMemory>%d</currentMemory>\n", def->memory) < 0) + if (virBufferVSprintf(buf, " <currentMemory>%lu</currentMemory>\n", def->memory) < 0) goto no_memory; if (virBufferVSprintf(buf, " <vcpu>%d</vcpu>\n", def->vcpus) < 0) goto no_memory; diff --git a/src/qemu_conf.h b/src/qemu_conf.h index 12aa6ae..c1aae75 100644 --- a/src/qemu_conf.h +++ b/src/qemu_conf.h @@ -193,8 +193,8 @@ struct qemud_vm_def { unsigned char uuid[VIR_UUID_BUFLEN]; char name[QEMUD_MAX_NAME_LEN]; - int memory; - int maxmem; + unsigned long memory; + unsigned long maxmem; int vcpus; int noReboot; diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 2b4c2a6..af5fc40 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -1765,6 +1765,66 @@ static char *qemudDomainGetOSType(virDomainPtr dom) { return type; } +/* Returns max memory in kb, 0 if error */ +static unsigned long qemudDomainGetMaxMemory(virDomainPtr dom) { + struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; + struct qemud_vm *vm = qemudFindVMByUUID(driver, dom->uuid); + + if (!vm) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, + _("no domain with matching uuid '%s'"), dom->uuid); + return 0; + } + + return vm->def->maxmem; +} + +static int qemudDomainSetMaxMemory(virDomainPtr dom, unsigned long newmax) { + struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; + struct qemud_vm *vm = qemudFindVMByUUID(driver, dom->uuid); + + if (!vm) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, + _("no domain with matching uuid '%s'"), dom->uuid); + return -1; + } + + if (newmax < vm->def->memory) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_ARG, + _("cannot set max memory lower than current memory")); + return -1; + } + + vm->def->maxmem = newmax; + return 0; +} + +static int qemudDomainSetMemory(virDomainPtr dom, unsigned long newmem) { + struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; + struct qemud_vm *vm = qemudFindVMByUUID(driver, dom->uuid); + + if (!vm) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, + _("no domain with matching uuid '%s'"), dom->uuid); + return -1; + } + + if (qemudIsActiveVM(vm)) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR, + _("cannot set memory of an active domain")); + return -1; + } + + if (newmem > vm->def->maxmem) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_ARG, + _("cannot set memory higher than max memory")); + return -1; + } + + vm->def->memory = newmem; + return 0; +} + static int qemudDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; @@ -2886,9 +2946,9 @@ static virDriver qemuDriver = { NULL, /* domainReboot */ qemudDomainDestroy, /* domainDestroy */ qemudDomainGetOSType, /* domainGetOSType */ - NULL, /* domainGetMaxMemory */ - NULL, /* domainSetMaxMemory */ - NULL, /* domainSetMemory */ + qemudDomainGetMaxMemory, /* domainGetMaxMemory */ + qemudDomainSetMaxMemory, /* domainSetMaxMemory */ + qemudDomainSetMemory, /* domainSetMemory */ qemudDomainGetInfo, /* domainGetInfo */ qemudDomainSave, /* domainSave */ qemudDomainRestore, /* domainRestore */