Re: [libvirt] [RFC PATCH] Cannot set persistent config mem/vcpu above live maxmem/maxvcpu

On 01/23/2014 08:25 PM, Geoff Franks wrote: Thanks. You may also want to wrap long lines, and/or use 'git send-email' to make the patches easier to use, but for a first time submission, this is enough to let us start review. I think I see what you are complaining about - in the old code, we don't reject newmem > persistent max, and so we can end up failing halfway through. But your new code isn't any better - even though you now check against both limits, you can still fail halfway through. I'd rather see us check both limits and reject the operation up front if we can't meet both limits, than to lower the checks down into the live/persistent branches but risk failure with live modified when the persistent branch finally detects failure. It also has the benefit of being a smaller diffstat: diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c index a7ef209..82d29c4 100644 --- i/src/qemu/qemu_driver.c +++ w/src/qemu/qemu_driver.c @@ -2242,8 +2242,16 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, } else { /* resize the current memory */ + unsigned long oldmax = 0; - if (newmem > vm->def->mem.max_balloon) { + if (flags & VIR_DOMAIN_AFFECT_LIVE) + oldmax = vm->def->mem.max_balloon; + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + if (!oldmax || oldmax > persistentDef->mem.max_balloon) + oldmax = persistentDef->mem.max_balloon; + } + + if (newmem > oldmax) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("cannot set memory higher than max memory")); goto endjob; Long line. Also, I think it suffers from the same problem - if both _LIVE and _CONFIG are given at once, then we want to cap things at the lower of the two limits, rather than using just the live max as the limit. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org