Markus Groß wrote:
> ---
> src/libxl/libxl_driver.c | 295 +++++++++++++++++++++++++++++++++++++++++++++-
> 1 files changed, 290 insertions(+), 5 deletions(-)
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 7ee3930..49fc90f 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -27,6 +27,7 @@
> #include <config.h>
>
> #include <sys/utsname.h>
> +#include <math.h>
> #include <libxl.h>
>
> #include "internal.h"
> @@ -1207,6 +1208,290 @@ libxlDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info)
> return ret;
> }
>
> +static int
> +libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
> + unsigned int flags)
> +{
> + libxlDriverPrivatePtr driver = dom->conn->privateData;
> + libxlDomainObjPrivatePtr priv;
> + virDomainDefPtr def;
> + virDomainObjPtr vm;
> + libxl_cpumap map;
> + uint8_t *bitmask = NULL;
> + unsigned int maplen;
> + unsigned int i, pos;
> + int max;
> + int ret = -1;
> +
> + virCheckFlags(VIR_DOMAIN_VCPU_LIVE |
> + VIR_DOMAIN_VCPU_CONFIG |
> + VIR_DOMAIN_VCPU_MAXIMUM, -1);
> +
> + /* At least one of LIVE or CONFIG must be set. MAXIMUM cannot be
> + * mixed with LIVE. */
> + if ((flags & (VIR_DOMAIN_VCPU_LIVE | VIR_DOMAIN_VCPU_CONFIG)) == 0 ||
> + (flags & (VIR_DOMAIN_VCPU_MAXIMUM | VIR_DOMAIN_VCPU_LIVE)) ==
> + (VIR_DOMAIN_VCPU_MAXIMUM | VIR_DOMAIN_VCPU_LIVE)) {
> + libxlError(VIR_ERR_INVALID_ARG,
> + _("invalid flag combination: (0x%x)"), flags);
> + return -1;
> + }
>
I think we should ensure nvcpus != 0
> +
> + libxlDriverLock(driver);
> + vm = virDomainFindByUUID(&driver->domains, dom->uuid);
> + libxlDriverUnlock(driver);
> +
> + if (!vm) {
> + libxlError(VIR_ERR_NO_DOMAIN, "%s", _("no domain with matching uuid"));
> + goto cleanup;
> + }
> +
> + if (!virDomainObjIsActive(vm) && (flags & VIR_DOMAIN_VCPU_LIVE)) {
> + libxlError(VIR_ERR_OPERATION_INVALID, "%s",
> + _("cannot set vcpus on an inactive domain"));
> + goto cleanup;
> + }
> +
> + if (!vm->persistent && (flags & VIR_DOMAIN_VCPU_CONFIG)) {
> + libxlError(VIR_ERR_OPERATION_INVALID, "%s",
> + _("cannot change persistent config of a transient domain"));
> + goto cleanup;
> + }
> +
> + if ((max = libxlGetMaxVcpus(dom->conn, NULL)) < 0) {
> + libxlError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("could not determine max vcpus for the domain"));
> + goto cleanup;
> + }
> +
> + if (!(flags & VIR_DOMAIN_VCPU_MAXIMUM) && vm->def->maxvcpus < max) {
> + max = vm->def->maxvcpus;
> + }
> +
> + if (nvcpus > max) {
> + libxlError(VIR_ERR_INVALID_ARG,
> + _("requested vcpus is greater than max allowable"
> + " vcpus for the domain: %d > %d"), nvcpus, max);
> + goto cleanup;
> + }
> +
> + priv = vm->privateData;
> +
> + if (!(def = virDomainObjGetPersistentDef(driver->caps, vm)))
> + goto cleanup;
> +
>
[...]
> + maplen = (unsigned int) ceil((double) nvcpus / 8);
> + if (VIR_ALLOC_N(bitmask, maplen) < 0) {
> + virReportOOMError();
> + goto cleanup;
> + }
> +
> + memset(bitmask, 0, maplen);
> + for (i = 0; i < nvcpus; ++i) {
> + pos = (unsigned int) floor((double) i / 8);
> + bitmask[pos] |= 1 << (i % 8);
> + }
> +
> + map.size = maplen;
> + map.map = bitmask;
>
Hmm, could this be simplified to
if (libxl_cpumap_alloc(&priv->ctx, &map)) {
virReportOOMError()
goto cleanup;
}
for (i = 0; i < nvcpus; i++)
libxl_cpumap_set(&map, i);
> +
> + switch (flags) {
> + case VIR_DOMAIN_VCPU_MAXIMUM | VIR_DOMAIN_VCPU_CONFIG:
> + def->maxvcpus = nvcpus;
> + if (nvcpus < def->vcpus)
> + def->vcpus = nvcpus;
> + ret = 0;
> + break;
> +
> + case VIR_DOMAIN_VCPU_CONFIG:
> + def->vcpus = nvcpus;
> + break;
> +
> + case VIR_DOMAIN_VCPU_LIVE:
> + if (libxl_set_vcpuonline(&priv->ctx, dom->id, &map) != 0) {
> + libxlError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed to set vcpus for domain '%d'"
> + " with libxenlight"), dom->id);
> + goto cleanup;
> + }
> + break;
> +
> + case VIR_DOMAIN_VCPU_LIVE | VIR_DOMAIN_VCPU_CONFIG:
> + if (libxl_set_vcpuonline(&priv->ctx, dom->id, &map) != 0) {
> + libxlError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed to set vcpus for domain '%d'"
> + " with libxenlight"), dom->id);
> + goto cleanup;
> + }
> + def->vcpus = nvcpus;
> + break;
> + }
> +
> + ret = 0;
> +
> + if (flags & VIR_DOMAIN_VCPU_CONFIG)
> + ret = virDomainSaveConfig(driver->configDir, def);
> +
> +cleanup:
> + VIR_FREE(bitmask);
> + if (vm)
> + virDomainObjUnlock(vm);
> + return ret;
> +}
> +
> +static int
> +libxlDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus)
> +{
> + return libxlDomainSetVcpusFlags(dom, nvcpus, VIR_DOMAIN_VCPU_LIVE);
> +}
> +
> +static int
> +libxlDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags)
> +{
> + libxlDriverPrivatePtr driver = dom->conn->privateData;
> + virDomainObjPtr vm;
> + virDomainDefPtr def;
> + int ret = -1;
> +
> + virCheckFlags(VIR_DOMAIN_VCPU_LIVE |
> + VIR_DOMAIN_VCPU_CONFIG |
> + VIR_DOMAIN_VCPU_MAXIMUM, -1);
>
Documentation for this function says that it is an error to set both
VIR_DOMAIN_VCPU_LIVE and VIR_DOMAIN_VCPU_CONFIG, so we should check for it.
> +
> + libxlDriverLock(driver);
> + vm = virDomainFindByUUID(&driver->domains, dom->uuid);
> + libxlDriverUnlock(driver);
> +
> + if (!vm) {
> + libxlError(VIR_ERR_NO_DOMAIN, "%s", _("no domain with matching uuid"));
> + goto cleanup;
> + }
> +
> + if (flags & VIR_DOMAIN_VCPU_LIVE) {
> + if (!virDomainObjIsActive(vm)) {
> + libxlError(VIR_ERR_OPERATION_INVALID,
> + "%s", _("Domain is not running"));
> + goto cleanup;
> + }
> + def = vm->def;
> + } else {
> + def = vm->newDef ? vm->newDef : vm->def;
> + }
> +
> + ret = (flags & VIR_DOMAIN_VCPU_MAXIMUM) ? def->maxvcpus : def->vcpus;
> +
> +cleanup:
> + if (vm)
> + virDomainObjUnlock(vm);
> + return ret;
> +}
> +
> +static int
> +libxlDomainPinVcpu(virDomainPtr dom, unsigned int vcpu, unsigned char *cpumap,
> + int maplen)
> +{
> + libxlDriverPrivatePtr driver = dom->conn->privateData;
> + libxlDomainObjPrivatePtr priv;
> + virDomainObjPtr vm;
> + int ret = -1;
> + libxl_cpumap map;
> +
> + libxlDriverLock(driver);
> + vm = virDomainFindByUUID(&driver->domains, dom->uuid);
> + libxlDriverUnlock(driver);
> +
> + if (!vm) {
> + libxlError(VIR_ERR_NO_DOMAIN, "%s", _("no domain with matching uuid"));
> + goto cleanup;
> + }
> +
> + if (!virDomainObjIsActive(vm)) {
> + libxlError(VIR_ERR_OPERATION_INVALID, "%s",
> + _("cannot pin vcpus on an inactive domain"));
> + goto cleanup;
> + }
> +
> + priv = vm->privateData;
> +
> + map.size = maplen;
> + map.map = cpumap;
> + if (libxl_set_vcpuaffinity(&priv->ctx, dom->id, vcpu, &map) != 0) {
> + libxlError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed to pin vcpu '%d' with libxenlight"), vcpu);
> + goto cleanup;
> + }
> + ret = 0;
> +
> +cleanup:
> + if (vm)
> + virDomainObjUnlock(vm);
> + return ret;
> +}
> +
> +
> +static int
> +libxlDomainGetVcpus(virDomainPtr dom, virVcpuInfoPtr info, int maxinfo,
> + unsigned char *cpumaps, int maplen)
> +{
> + libxlDriverPrivatePtr driver = dom->conn->privateData;
> + libxlDomainObjPrivatePtr priv;
> + virDomainObjPtr vm;
> + int ret = -1;
> +
>
Extra newline.
> + libxl_vcpuinfo *vcpuinfo;
> + int maxcpu, hostcpus;
> +
> + libxlDriverLock(driver);
> + vm = virDomainFindByUUID(&driver->domains, dom->uuid);
> + libxlDriverUnlock(driver);
> +
> + if (!vm) {
> + libxlError(VIR_ERR_NO_DOMAIN, "%s", _("no domain with matching uuid"));
> + goto cleanup;
> + }
> +
> + if (!virDomainObjIsActive(vm)) {
> + libxlError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not running"));
> + goto cleanup;
> + }
> +
> + priv = vm->privateData;
> + if ((vcpuinfo = libxl_list_vcpu(&priv->ctx, dom->id, &maxcpu,
> + &hostcpus)) == NULL) {
> + libxlError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed to list vcpus for domain '%d' with libxenlight"),
> + dom->id);
> + goto cleanup;
> + }
> +
> + memset(cpumaps, 0, maplen * maxinfo);
>
Documentation for this function says cpumaps can be NULL.
> + unsigned int i = 0;
>
I thought I recalled variable declaration only at start of a block but
don't see anything about it in HACKING. Perhaps someone else can clarify.
Regards,
Jim
> + for (i = 0; i < maxcpu && i < maxinfo; ++i) {
> + info[i].number = vcpuinfo[i].vcpuid;
> + info[i].cpu = vcpuinfo[i].cpu;
> + info[i].cpuTime = vcpuinfo[i].vcpu_time;
> + if (vcpuinfo[i].running)
> + info[i].state = VIR_VCPU_RUNNING;
> + else if (vcpuinfo[i].blocked)
> + info[i].state = VIR_VCPU_BLOCKED;
> + else
> + info[i].state = VIR_VCPU_OFFLINE;
> +
> + unsigned char *cpumap = VIR_GET_CPUMAP(cpumaps, maplen, i);
> + memcpy(cpumap, vcpuinfo[i].cpumap.map,
> + MIN(maplen, vcpuinfo[i].cpumap.size));
> +
> + libxl_vcpuinfo_destroy(&vcpuinfo[i]);
> + }
> + VIR_FREE(vcpuinfo);
> +
> + ret = maxinfo;
> +
> +cleanup:
> + if (vm)
> + virDomainObjUnlock(vm);
> + return ret;
> +}
> +
> static char *
> libxlDomainDumpXML(virDomainPtr dom, int flags)
> {
> @@ -1561,11 +1846,11 @@ static virDriver libxlDriver = {
> NULL, /* domainSave */
> NULL, /* domainRestore */
> NULL, /* domainCoreDump */
> - NULL, /* domainSetVcpus */
> - NULL, /* domainSetVcpusFlags */
> - NULL, /* domainGetVcpusFlags */
> - NULL, /* domainPinVcpu */
> - NULL, /* domainGetVcpus */
> + libxlDomainSetVcpus, /* domainSetVcpus */
> + libxlDomainSetVcpusFlags, /* domainSetVcpusFlags */
> + libxlDomainGetVcpusFlags, /* domainGetVcpusFlags */
> + libxlDomainPinVcpu, /* domainPinVcpu */
> + libxlDomainGetVcpus, /* domainGetVcpus */
> NULL, /* domainGetMaxVcpus */
> NULL, /* domainGetSecurityLabel */
> NULL, /* nodeGetSecurityModel */
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list