[libvirt] [PATCH] Support VCPU hotplug in QEMU guests

[Daniel P. Berrange]

On Tue, Feb 16, 2010 at 03:55:26PM +0100, Daniel Veillard wrote:
> On Tue, Feb 16, 2010 at 12:12:31PM +0000, Daniel P. Berrange wrote:
> > QEMU has a monitor command 'set_cpu' which allows a specific
> > CPU to be toggled between online& offline state. libvirt CPU
> > hotplug does not work in terms of individual indexes CPUs.
> > Thus to support this, we iteratively toggle the online state
> > when the total number of vCPUs is adjusted via libvirt
> > 
> > NB, currently untested since QEMU segvs when running this!
> > 
> > * src/qemu/qemu_driver.c: Toggle online state for CPUs when
> >   doing hotplug
> > * src/qemu/qemu_monitor.c, src/qemu/qemu_monitor.h,
> >   src/qemu/qemu_monitor_json.c, src/qemu/qemu_monitor_json.h,
> >   src/qemu/qemu_monitor_text.c, src/qemu/qemu_monitor_text.h: Add
> >   monitor API for toggling a CPU's online status via 'set_cpu
> 
> > +static int qemudDomainHotplugVcpus(virDomainObjPtr vm, unsigned int nvcpus)
> > +{
> > +    qemuDomainObjPrivatePtr priv = vm->privateData;
> > +    int i, rc;
> > +    int ret = -1;
> > +
> > +    /* We need different branches here, because we want to offline
> > +     * in reverse order to onlining, so any partial fail leaves us in a
> > +     * reasonably sensible state */
> > +    if (nvcpus > vm->def->vcpus) {
> > +        for (i = vm->def->vcpus ; i < nvcpus ; i++) {
> > +            /* Online new CPU */
> > +            rc = qemuMonitorSetCPU(priv->mon, i, 1);
> > +            if (rc == 0)
> > +                goto unsupported;
> > +            if (rc < 0)
> > +                goto cleanup;
> > +
> > +            vm->def->vcpus++;
> > +        }
> > +    } else {
> > +        for (i = vm->def->vcpus - 1 ; i >= nvcpus ; i--) {
> > +            /* Offline old CPU */
> > +            rc = qemuMonitorSetCPU(priv->mon, i, 0);
> > +            if (rc == 0)
> > +                goto unsupported;
> > +            if (rc < 0)
> > +                goto cleanup;
> > +
> > +            vm->def->vcpus--;
> > +        }
> > +    }
> > +
> > +    ret = 0;
> > +
> > +cleanup:
> > +    return ret;
> > +
> > +unsupported:
> > +    qemudReportError(NULL, NULL, NULL, VIR_ERR_OPERATION_INVALID, "%s",
> > +                     _("cannot change vcpu count of an active domain"));
> > +    goto cleanup;
> > +}
> 
>   Hum, seems QEMu would allow CPU 0 and 2 to be online and CPU 1 to be
>   offline for example but our code really assumes all CPUs from 0 to
>   vm->def->vcpus are always online (and other offline). I hope this
>   restriction we impose won't be a problem later.

Yes, that is correct. With the current libvirt API / XML description,
this is the best that we can do. In the future we might find we have
to extend our format to allow exact specification of which individual
CPUs are online. Currently I don't think this matters, but if we add
support for NUMA topology inside the guest, we might need finer 
control. eg, you want to unplug a specific CPU from a specific guest
NUMA cell.

> 
> [...]
> > +int qemuMonitorJSONSetCPU(qemuMonitorPtr mon,
> > +                          int cpu, int online)
> > +{
> > +    int ret;
> > +    virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("balloon",
> > +                                                     "U:cpu", (unsigned long long)cpu,
> > +                                                     "s:state", online ? "online" : "offline",
> > +                                                     NULL);
> > +    virJSONValuePtr reply = NULL;
> > +    if (!cmd)
> > +        return -1;
> > +
> > +    ret = qemuMonitorJSONCommand(mon, cmd, &reply);
> > +
> > +    if (ret == 0) {
> > +        /* XXX See if CPU soft-failed due to lack of ACPI */
> > +#if 0
> > +        if (qemuMonitorJSONHasError(reply, "DeviceNotActive") ||
> > +            qemuMonitorJSONHasError(reply, "KVMMissingCap"))
> > +            goto cleanup;
> > +#endif
> 
>   Hum ... why can't we activate this now ?

QEMU doesn't actually return those errors in this case. This was a cut+paste
from teh balloon driver that I left in to remind me to investigate that later

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 :|