Re: [libvirt] [PATCH 3/5] setmem: implement the code to address the new API in the qemu driver

On 03/02/2011 01:13 AM, Taku Izumi wrote: Doesn't that fail for a transient domain? That is, shouldn't the persistentDef only be set when flags & VIR_DOMAIN_MEM_CONFIG is requested and validated? Merge conflict with my audit patches. Plus Daniel's request to rearrange the switch statement into if statements. Hmm, for other API additions, I have provided dummy wrappers for the new function anywhere the old function existed, that merely require the same flags as the old function would use. I'll probably propose that as a followup patch. Here's what I ended up with before pushing: diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c index 2f15144..7921d98 100644 --- i/src/qemu/qemu_driver.c +++ w/src/qemu/qemu_driver.c @@ -1568,12 +1568,22 @@ cleanup: return ret; } -static int qemudDomainSetMemory(virDomainPtr dom, unsigned long newmem) { +static int qemudDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, + unsigned int flags) { struct qemud_driver *driver = dom->conn->privateData; qemuDomainObjPrivatePtr priv; virDomainObjPtr vm; + virDomainDefPtr persistentDef = NULL; int ret = -1, r; + virCheckFlags(VIR_DOMAIN_MEM_LIVE | + VIR_DOMAIN_MEM_CONFIG, -1); + + if ((flags & (VIR_DOMAIN_MEM_LIVE | VIR_DOMAIN_MEM_CONFIG)) == 0) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("invalid flag combination: (0x%x)"), flags); + } + qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); qemuDriverUnlock(driver); @@ -1594,24 +1604,42 @@ static int qemudDomainSetMemory(virDomainPtr dom, unsigned long newmem) { if (qemuDomainObjBeginJob(vm) < 0) goto cleanup; - if (!virDomainObjIsActive(vm)) { + if (!virDomainObjIsActive(vm) && (flags & VIR_DOMAIN_MEM_LIVE)) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); goto endjob; } - priv = vm->privateData; - qemuDomainObjEnterMonitor(vm); - r = qemuMonitorSetBalloon(priv->mon, newmem); - qemuDomainObjExitMonitor(vm); - qemuAuditMemory(vm, vm->def->mem.cur_balloon, newmem, "update", r == 1); - if (r < 0) - goto endjob; + if (flags & VIR_DOMAIN_MEM_CONFIG) { + if (!vm->persistent) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("cannot change persistent config of a transient domain")); + goto endjob; + } + if (!(persistentDef = virDomainObjGetPersistentDef(driver->caps, vm))) + goto endjob; + } - /* Lack of balloon support is a fatal error */ - if (r == 0) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("cannot set memory of an active domain")); + if (flags & VIR_DOMAIN_MEM_LIVE) { + priv = vm->privateData; + qemuDomainObjEnterMonitor(vm); + r = qemuMonitorSetBalloon(priv->mon, newmem); + qemuDomainObjExitMonitor(vm); + qemuAuditMemory(vm, vm->def->mem.cur_balloon, newmem, "update", r == 1); + if (r < 0) + goto endjob; + + /* Lack of balloon support is a fatal error */ + if (r == 0) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("cannot set memory of an active domain")); + goto endjob; + } + } + + if (flags& VIR_DOMAIN_MEM_CONFIG) { + persistentDef->mem.cur_balloon = newmem; + ret = virDomainSaveConfig(driver->configDir, persistentDef); goto endjob; } @@ -1626,6 +1654,10 @@ cleanup: return ret; } +static int qemudDomainSetMemory(virDomainPtr dom, unsigned long newmem) { + return qemudDomainSetMemoryFlags(dom, newmem, VIR_DOMAIN_MEM_LIVE); +} + static int qemudDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info) { struct qemud_driver *driver = dom->conn->privateData; @@ -6854,7 +6886,7 @@ static virDriver qemuDriver = { qemudDomainGetMaxMemory, /* domainGetMaxMemory */ NULL, /* domainSetMaxMemory */ qemudDomainSetMemory, /* domainSetMemory */ - NULL, /* domainSetMemoryFlags */ + qemudDomainSetMemoryFlags, /* domainSetMemoryFlags */ qemudDomainGetInfo, /* domainGetInfo */ qemudDomainSave, /* domainSave */ qemudDomainRestore, /* domainRestore */ -- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org