[libvirt] [PATCH 2/4] qemu: Make SetVcpu command hotplug only

Similar to the Set*Mem commands, this implementation was bogus and misleading. Make it clear this is a hotplug only operation, and that the hotplug piece isn't even implemented. Also drop the overkill maxvcpus validation: we don't perform this check at XML define time so clearly no one is missing it, and there is always the risk that our info will be out of date, possibly preventing legitimate CPU values. --- src/qemu/qemu_driver.c | 42 +++++++----------------------------------- 1 files changed, 7 insertions(+), 35 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 56a450c..ef1f638 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4100,12 +4100,11 @@ cleanup: } -static int qemudDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus) { +static int qemudDomainSetVcpus(virDomainPtr dom, + ATTRIBUTE_UNUSED unsigned int nvcpus) { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; - int max; int ret = -1; - const char *type; qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -4119,41 +4118,14 @@ static int qemudDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus) { goto cleanup; } - if (qemuDomainObjBeginJob(vm) < 0) + if (!virDomainObjIsActive(vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); goto cleanup; - - if (virDomainObjIsActive(vm)) { - qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("cannot change vcpu count of an active domain")); - goto endjob; - } - - if (!(type = virDomainVirtTypeToString(vm->def->virtType))) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("unknown virt type in domain definition '%d'"), - vm->def->virtType); - goto endjob; } - if ((max = qemudGetMaxVCPUs(NULL, type)) < 0) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("could not determine max vcpus for the domain")); - goto endjob; - } - - if (nvcpus > max) { - qemuReportError(VIR_ERR_INVALID_ARG, - _("requested vcpus is greater than max allowable" - " vcpus for the domain: %d > %d"), nvcpus, max); - goto endjob; - } - - vm->def->vcpus = nvcpus; - ret = 0; - -endjob: - if (qemuDomainObjEndJob(vm) == 0) - vm = NULL; + qemuReportError(VIR_ERR_NO_SUPPORT, + "%s", _("cpu hotplug not yet supported")); cleanup: if (vm) -- 1.6.5.2

On Fri, Feb 12, 2010 at 10:32:15AM -0500, Cole Robinson wrote:
Similar to the Set*Mem commands, this implementation was bogus and misleading. Make it clear this is a hotplug only operation, and that the hotplug piece isn't even implemented.
Also drop the overkill maxvcpus validation: we don't perform this check at XML define time so clearly no one is missing it, and there is always the risk that our info will be out of date, possibly preventing legitimate CPU values. --- src/qemu/qemu_driver.c | 42 +++++++----------------------------------- 1 files changed, 7 insertions(+), 35 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 56a450c..ef1f638 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4100,12 +4100,11 @@ cleanup: }
-static int qemudDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus) { +static int qemudDomainSetVcpus(virDomainPtr dom, + ATTRIBUTE_UNUSED unsigned int nvcpus) { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; - int max; int ret = -1; - const char *type;
qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -4119,41 +4118,14 @@ static int qemudDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus) { goto cleanup; }
- if (qemuDomainObjBeginJob(vm) < 0) + if (!virDomainObjIsActive(vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); goto cleanup; - - if (virDomainObjIsActive(vm)) { - qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("cannot change vcpu count of an active domain")); - goto endjob; - } - - if (!(type = virDomainVirtTypeToString(vm->def->virtType))) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("unknown virt type in domain definition '%d'"), - vm->def->virtType); - goto endjob; }
- if ((max = qemudGetMaxVCPUs(NULL, type)) < 0) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("could not determine max vcpus for the domain")); - goto endjob; - } - - if (nvcpus > max) { - qemuReportError(VIR_ERR_INVALID_ARG, - _("requested vcpus is greater than max allowable" - " vcpus for the domain: %d > %d"), nvcpus, max); - goto endjob; - } - - vm->def->vcpus = nvcpus; - ret = 0; - -endjob: - if (qemuDomainObjEndJob(vm) == 0) - vm = NULL; + qemuReportError(VIR_ERR_NO_SUPPORT, + "%s", _("cpu hotplug not yet supported"));
cleanup: if (vm)
ACK 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 02/15/2010 06:09 AM, Daniel P. Berrange wrote:
On Fri, Feb 12, 2010 at 10:32:15AM -0500, Cole Robinson wrote:
Similar to the Set*Mem commands, this implementation was bogus and misleading. Make it clear this is a hotplug only operation, and that the hotplug piece isn't even implemented.
Also drop the overkill maxvcpus validation: we don't perform this check at XML define time so clearly no one is missing it, and there is always the risk that our info will be out of date, possibly preventing legitimate CPU values. --- src/qemu/qemu_driver.c | 42 +++++++----------------------------------- 1 files changed, 7 insertions(+), 35 deletions(-)
ACK
Daniel
Thanks, pushed now. - Cole
participants (2)
-
Cole Robinson
-
Daniel P. Berrange