[libvirt] [PATCH v4 0/9] Introduce display of IOThreads Information

v3: http://www.redhat.com/archives/libvir-list/2015-February/msg00593.html Changes since v3: (from code reviews) - Updated to use 1.2.14 not 1.2.13 - Change struct from virDomainIOThreadsInfo to virDomainIOThreadInfo - Remove the resources and nresources from virDomainIOThreadInfo and of course all associated code as well as changing the remote struct from iothreads_* to iothread_ - Create separate routines for virDomainGetIOThreadsInfo, virDomainGetIOThreadPin, and virDomainPinIOThread... and of course all the accompanying code - Use EnterMonitor instead of EnterMonitorAsync Still need to generate a v3 of the python code, but I'll wait for this to be reviewed first. John Ferlan (9): Implement public API for virDomainGetIOThreadsInfo remote: Implement the remote plumbing for virDomainGetIOThreadsInfo qemu: Implement the qemu driver fetch for IOThreads virsh: Add 'iothreadsinfo' command Implement public API for getting/setting specific IOThread pinning remote: Plumbing for getting/setting specific IOThread pinning domain: Introduce virDomainIOThreadsPin{Add|Del} qemu: Add support to get/set specific IOThread pinning data virsh: Add iothreadpin command daemon/remote.c | 123 ++++++++- include/libvirt/libvirt-domain.h | 41 ++- src/conf/domain_conf.c | 64 +++++ src/conf/domain_conf.h | 10 + src/driver-hypervisor.h | 24 +- src/libvirt-domain.c | 227 ++++++++++++++++- src/libvirt_private.syms | 2 + src/libvirt_public.syms | 8 + src/qemu/qemu_driver.c | 526 +++++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 143 ++++++++++- src/remote/remote_protocol.x | 61 ++++- src/remote_protocol-structs | 43 ++++ src/rpc/gendispatch.pl | 2 + tools/virsh-domain.c | 216 ++++++++++++++++ tools/virsh.pod | 38 +++ 15 files changed, 1521 insertions(+), 7 deletions(-) -- 2.1.0

Add virDomainGetIOThreadInfo in order to return a list of virDomainIOThreadInfoPtr structures which list the IOThread ID and the CPU Affinity map for each IOThread for the domain. For an active domain, the live data will be returned, while for an inactive domain, the config data will be returned. The API supports either the --live or --config flag, but not both. Also added virDomainIOThreadsInfoFree in order to free the cpumap and the IOThreadInfo structure. Signed-off-by: John Ferlan <jferlan@redhat.com> --- include/libvirt/libvirt-domain.h | 22 +++++++++++- src/driver-hypervisor.h | 8 ++++- src/libvirt-domain.c | 75 +++++++++++++++++++++++++++++++++++++++- src/libvirt_public.syms | 6 ++++ 4 files changed, 108 insertions(+), 3 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index a9d3efd..9487b80 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -4,7 +4,7 @@ * Description: Provides APIs for the management of domains * Author: Daniel Veillard <veillard@redhat.com> * - * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006-2015 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -1591,6 +1591,26 @@ int virDomainGetEmulatorPinInfo (virDomainPtr domain, unsigned int flags); /** + * virIOThreadInfo: + * + * The data structure for information about all IOThreads in a domain + */ +typedef struct _virDomainIOThreadInfo virDomainIOThreadInfo; +typedef virDomainIOThreadInfo *virDomainIOThreadInfoPtr; +struct _virDomainIOThreadInfo { + unsigned int iothread_id; /* IOThread ID */ + unsigned char *cpumap; /* CPU map for thread. A pointer to an */ + /* array of real CPUs (in 8-bit bytes) */ + int cpumaplen; /* cpumap size */ +}; + +void virDomainIOThreadsInfoFree(virDomainIOThreadInfoPtr info); + +int virDomainGetIOThreadsInfo(virDomainPtr domain, + virDomainIOThreadInfoPtr **info, + unsigned int flags); + +/** * VIR_USE_CPU: * @cpumap: pointer to a bit map of real CPUs (in 8-bit bytes) (IN/OUT) * @cpu: the physical CPU number diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index a1198ad..c2b4cd8 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -1,7 +1,7 @@ /* * driver-hypervisor.h: entry points for hypervisor drivers * - * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006-2015 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -381,6 +381,11 @@ typedef int (*virDrvDomainGetMaxVcpus)(virDomainPtr domain); typedef int +(*virDrvDomainGetIOThreadsInfo)(virDomainPtr domain, + virDomainIOThreadInfoPtr **info, + unsigned int flags); + +typedef int (*virDrvDomainGetSecurityLabel)(virDomainPtr domain, virSecurityLabelPtr seclabel); @@ -1254,6 +1259,7 @@ struct _virHypervisorDriver { virDrvDomainGetEmulatorPinInfo domainGetEmulatorPinInfo; virDrvDomainGetVcpus domainGetVcpus; virDrvDomainGetMaxVcpus domainGetMaxVcpus; + virDrvDomainGetIOThreadsInfo domainGetIOThreadsInfo; virDrvDomainGetSecurityLabel domainGetSecurityLabel; virDrvDomainGetSecurityLabelList domainGetSecurityLabelList; virDrvNodeGetSecurityModel nodeGetSecurityModel; diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 89d1eab..27f6abe 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -1,7 +1,7 @@ /* * libvirt-domain.c: entry points for virDomainPtr APIs * - * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006-2015 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -7891,6 +7891,79 @@ virDomainGetMaxVcpus(virDomainPtr domain) /** + * virDomainGetIOThreadsInfo: + * @dom: a domain object + * @info: pointer to an array of virDomainIOThreadInfo structures (OUT) + * @flags: bitwise-OR of virDomainModificationImpact + * Must not be VIR_DOMAIN_AFFECT_LIVE and + * VIR_DOMAIN_AFFECT_CONFIG concurrently. + * + * Fetch IOThreads of an active domain including the cpumap information to + * determine on which CPU the IOThread has affinity to run. + * + * Returns the number of IOThreads or -1 in case of error. + * On success, the array of information is stored into @info. The caller is + * responsible for calling virDomainIOThreadsInfoFree() on each array element, + * then calling free() on @info. On error, @info is set to NULL. + */ +int +virDomainGetIOThreadsInfo(virDomainPtr dom, + virDomainIOThreadInfoPtr **info, + unsigned int flags) +{ + VIR_DOMAIN_DEBUG(dom, "info=%p flags=%x", info, flags); + + virResetLastError(); + + virCheckDomainReturn(dom, -1); + virCheckReadOnlyGoto(dom->conn->flags, error); + virCheckNonNullArgGoto(info, error); + *info = NULL; + + /* At most one of these two flags should be set. */ + if ((flags & VIR_DOMAIN_AFFECT_LIVE) && + (flags & VIR_DOMAIN_AFFECT_CONFIG)) { + virReportInvalidArg(flags, + _("flags 'affect live' and 'affect config' in %s " + "are mutually exclusive"), + __FUNCTION__); + goto error; + } + + if (dom->conn->driver->domainGetIOThreadsInfo) { + int ret; + ret = dom->conn->driver->domainGetIOThreadsInfo(dom, info, flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(dom->conn); + return -1; +} + + +/** + * virDomainIOThreadsInfoFree: + * @info: pointer to a virDomainIOThreadInfo object + * + * Frees the memory used by @info. + */ +void +virDomainIOThreadsInfoFree(virDomainIOThreadInfoPtr info) +{ + if (!info) + return; + + VIR_FREE(info->cpumap); + VIR_FREE(info); +} + + +/** * virDomainGetSecurityLabel: * @domain: a domain object * @seclabel: pointer to a virSecurityLabel structure diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 4ea5cff..34852c1 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -695,4 +695,10 @@ LIBVIRT_1.2.12 { virDomainDefineXMLFlags; } LIBVIRT_1.2.11; +LIBVIRT_1.2.14 { + global: + virDomainIOThreadsInfoFree; + virDomainGetIOThreadsInfo; +} LIBVIRT_1.2.12; + # .... define new API here using predicted next version number .... -- 2.1.0

On Thu, Mar 05, 2015 at 09:02:58PM -0500, John Ferlan wrote:
Add virDomainGetIOThreadInfo in order to return a list of virDomainIOThreadInfoPtr structures which list the IOThread ID and the CPU Affinity map for each IOThread for the domain.
For an active domain, the live data will be returned, while for an inactive domain, the config data will be returned.
The API supports either the --live or --config flag, but not both.
Also added virDomainIOThreadsInfoFree in order to free the cpumap and the IOThreadInfo structure.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- include/libvirt/libvirt-domain.h | 22 +++++++++++- src/driver-hypervisor.h | 8 ++++- src/libvirt-domain.c | 75 +++++++++++++++++++++++++++++++++++++++- src/libvirt_public.syms | 6 ++++ 4 files changed, 108 insertions(+), 3 deletions(-)
ACK Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

Implement the remote plumbing for virDomainGetIOThreadsInfo Signed-off-by: John Ferlan <jferlan@redhat.com> --- daemon/remote.c | 74 +++++++++++++++++++++++++++++++++++++++- src/remote/remote_driver.c | 81 +++++++++++++++++++++++++++++++++++++++++++- src/remote/remote_protocol.x | 28 +++++++++++++-- src/remote_protocol-structs | 19 +++++++++++ src/rpc/gendispatch.pl | 1 + 5 files changed, 199 insertions(+), 4 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index d657a09..ff64eeb 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1,7 +1,7 @@ /* * remote.c: handlers for RPC method calls * - * Copyright (C) 2007-2014 Red Hat, Inc. + * Copyright (C) 2007-2015 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -2269,6 +2269,78 @@ remoteDispatchDomainGetVcpus(virNetServerPtr server ATTRIBUTE_UNUSED, } static int +remoteDispatchDomainGetIOThreadsInfo(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_get_iothreads_info_args *args, + remote_domain_get_iothreads_info_ret *ret) +{ + int rv = -1; + size_t i; + struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); + virDomainIOThreadInfoPtr *info = NULL; + virDomainPtr dom = NULL; + remote_domain_iothread_info *dst; + int ninfo = 0; + + if (!priv->conn) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (!(dom = get_nonnull_domain(priv->conn, args->dom))) + goto cleanup; + + if ((ninfo = virDomainGetIOThreadsInfo(dom, &info, args->flags)) < 0) + goto cleanup; + + if (ninfo > REMOTE_IOTHREADS_INFO_MAX) { + virReportError(VIR_ERR_RPC, + _("Too many IOThreads in info: %d for limit %d"), + ninfo, REMOTE_IOTHREADS_INFO_MAX); + goto cleanup; + } + + if (ninfo) { + if (VIR_ALLOC_N(ret->info.info_val, ninfo) < 0) + goto cleanup; + + ret->info.info_len = ninfo; + + for (i = 0; i < ninfo; i++) { + dst = &ret->info.info_val[i]; + dst->iothread_id = info[i]->iothread_id; + + /* No need to allocate/copy the cpumap if we make the reasonable + * assumption that unsigned char and char are the same size. + */ + dst->cpumap.cpumap_len = info[i]->cpumaplen; + dst->cpumap.cpumap_val = (char *)info[i]->cpumap; + info[i]->cpumap = NULL; + } + } else { + ret->info.info_len = 0; + ret->info.info_val = NULL; + } + + ret->ret = ninfo; + + rv = 0; + + cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + virObjectUnref(dom); + if (ninfo >= 0) + for (i = 0; i < ninfo; i++) + virDomainIOThreadsInfoFree(info[i]); + VIR_FREE(info); + + return rv; +} + +static int remoteDispatchDomainMigratePrepare(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, virNetMessagePtr msg ATTRIBUTE_UNUSED, diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 76c1d0c..42dab9d 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2,7 +2,7 @@ * remote_driver.c: driver to provide access to libvirtd running * on a remote machine * - * Copyright (C) 2007-2014 Red Hat, Inc. + * Copyright (C) 2007-2015 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -2316,6 +2316,84 @@ remoteDomainGetVcpus(virDomainPtr domain, } static int +remoteDomainGetIOThreadsInfo(virDomainPtr dom, + virDomainIOThreadInfoPtr **info, + unsigned int flags) +{ + int rv = -1; + size_t i; + struct private_data *priv = dom->conn->privateData; + remote_domain_get_iothreads_info_args args; + remote_domain_get_iothreads_info_ret ret; + remote_domain_iothread_info *src; + virDomainIOThreadInfoPtr *info_ret = NULL; + + remoteDriverLock(priv); + + make_nonnull_domain(&args.dom, dom); + + args.flags = flags; + + memset(&ret, 0, sizeof(ret)); + + if (call(dom->conn, priv, 0, REMOTE_PROC_DOMAIN_GET_IOTHREADS_INFO, + (xdrproc_t)xdr_remote_domain_get_iothreads_info_args, + (char *)&args, + (xdrproc_t)xdr_remote_domain_get_iothreads_info_ret, + (char *)&ret) == -1) + goto done; + + if (ret.info.info_len > REMOTE_IOTHREADS_INFO_MAX) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Too many IOThreads in info: %d for limit %d"), + ret.info.info_len, REMOTE_IOTHREADS_INFO_MAX); + goto cleanup; + } + + if (info) { + if (!ret.info.info_len) { + *info = NULL; + rv = ret.ret; + goto cleanup; + } + + if (VIR_ALLOC_N(info_ret, ret.info.info_len) < 0) + goto cleanup; + + for (i = 0; i < ret.info.info_len; i++) { + src = &ret.info.info_val[i]; + + if (VIR_ALLOC(info_ret[i]) < 0) + goto cleanup; + + info_ret[i]->iothread_id = src->iothread_id; + if (VIR_ALLOC_N(info_ret[i]->cpumap, src->cpumap.cpumap_len) < 0) + goto cleanup; + memcpy(info_ret[i]->cpumap, src->cpumap.cpumap_val, + src->cpumap.cpumap_len); + info_ret[i]->cpumaplen = src->cpumap.cpumap_len; + } + *info = info_ret; + info_ret = NULL; + } + + rv = ret.ret; + + cleanup: + if (info_ret) { + for (i = 0; i < ret.info.info_len; i++) + virDomainIOThreadsInfoFree(info_ret[i]); + VIR_FREE(info_ret); + } + xdr_free((xdrproc_t)xdr_remote_domain_get_iothreads_info_ret, + (char *) &ret); + + done: + remoteDriverUnlock(priv); + return rv; +} + +static int remoteDomainGetSecurityLabel(virDomainPtr domain, virSecurityLabelPtr seclabel) { remote_domain_get_security_label_args args; @@ -8027,6 +8105,7 @@ static virHypervisorDriver hypervisor_driver = { .domainGetEmulatorPinInfo = remoteDomainGetEmulatorPinInfo, /* 0.10.0 */ .domainGetVcpus = remoteDomainGetVcpus, /* 0.3.0 */ .domainGetMaxVcpus = remoteDomainGetMaxVcpus, /* 0.3.0 */ + .domainGetIOThreadsInfo = remoteDomainGetIOThreadsInfo, /* 1.2.14 */ .domainGetSecurityLabel = remoteDomainGetSecurityLabel, /* 0.6.1 */ .domainGetSecurityLabelList = remoteDomainGetSecurityLabelList, /* 0.10.0 */ .nodeGetSecurityModel = remoteNodeGetSecurityModel, /* 0.6.1 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index c8162a5..4ea535e 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -3,7 +3,7 @@ * remote_internal driver and libvirtd. This protocol is * internal and may change at any time. * - * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006-2015 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -85,6 +85,9 @@ const REMOTE_VCPUINFO_MAX = 16384; /* Upper limit on cpumaps (bytes) passed to virDomainGetVcpus. */ const REMOTE_CPUMAPS_MAX = 8388608; +/* Upper limit on number of info fields returned by virDomainGetIOThreads. */ +const REMOTE_IOTHREADS_INFO_MAX = 16384; + /* Upper limit on migrate cookie. */ const REMOTE_MIGRATE_COOKIE_MAX = 4194304; @@ -1181,6 +1184,21 @@ struct remote_domain_get_max_vcpus_ret { int num; }; +struct remote_domain_iothread_info { + unsigned int iothread_id; + opaque cpumap<REMOTE_CPUMAP_MAX>; +}; + +struct remote_domain_get_iothreads_info_args { + remote_nonnull_domain dom; + unsigned int flags; +}; + +struct remote_domain_get_iothreads_info_ret { + remote_domain_iothread_info info<REMOTE_IOTHREADS_INFO_MAX>; + unsigned int ret; +}; + struct remote_domain_get_security_label_args { remote_nonnull_domain dom; }; @@ -5569,5 +5587,11 @@ enum remote_procedure { * @acl: domain:write * @acl: domain:save */ - REMOTE_PROC_DOMAIN_DEFINE_XML_FLAGS = 350 + REMOTE_PROC_DOMAIN_DEFINE_XML_FLAGS = 350, + + /** + * @generate: none + * @acl: domain:read + */ + REMOTE_PROC_DOMAIN_GET_IOTHREADS_INFO = 351 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index df6eaf3..907bcd4 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -807,6 +807,24 @@ struct remote_domain_get_max_vcpus_args { struct remote_domain_get_max_vcpus_ret { int num; }; +struct remote_domain_iothread_info { + u_int iothread_id; + struct { + u_int cpumap_len; + char * cpumap_val; + } cpumap; +}; +struct remote_domain_get_iothreads_info_args { + remote_nonnull_domain dom; + u_int flags; +}; +struct remote_domain_get_iothreads_info_ret { + struct { + u_int info_len; + remote_domain_iothread_info * info_val; + } info; + u_int ret; +}; struct remote_domain_get_security_label_args { remote_nonnull_domain dom; }; @@ -2963,4 +2981,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_EVENT_CALLBACK_AGENT_LIFECYCLE = 348, REMOTE_PROC_DOMAIN_GET_FSINFO = 349, REMOTE_PROC_DOMAIN_DEFINE_XML_FLAGS = 350, + REMOTE_PROC_DOMAIN_GET_IOTHREADS_INFO = 351, }; diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index 0dc167a..78cb415 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -67,6 +67,7 @@ sub fixup_name { $name =~ s/Fsfreeze$/FSFreeze/; $name =~ s/Fsthaw$/FSThaw/; $name =~ s/Fsinfo$/FSInfo/; + $name =~ s/Iothreads$/IOThreads/; $name =~ s/Scsi/SCSI/; $name =~ s/Wwn$/WWN/; $name =~ s/Dhcp$/DHCP/; -- 2.1.0

On Thu, Mar 05, 2015 at 09:02:59PM -0500, John Ferlan wrote:
Implement the remote plumbing for virDomainGetIOThreadsInfo
Signed-off-by: John Ferlan <jferlan@redhat.com> --- daemon/remote.c | 74 +++++++++++++++++++++++++++++++++++++++- src/remote/remote_driver.c | 81 +++++++++++++++++++++++++++++++++++++++++++- src/remote/remote_protocol.x | 28 +++++++++++++-- src/remote_protocol-structs | 19 +++++++++++ src/rpc/gendispatch.pl | 1 + 5 files changed, 199 insertions(+), 4 deletions(-)
ACK Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

Depending on the flags passed, either attempt to return the active/live IOThread data for the domain or the config data. The active/live path will call into the Monitor in order to get the IOThread data and then correlate the thread_id's returned from the monitor to the currently running system/threads in order to ascertain the affinity for each iothread_id. The config path will map each of the configured IOThreads and return any configured iothreadspin data Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_driver.c | 224 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 224 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ffa4e19..d8a761d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5557,6 +5557,229 @@ qemuDomainGetMaxVcpus(virDomainPtr dom) VIR_DOMAIN_VCPU_MAXIMUM)); } +static int +qemuDomainGetIOThreadsLive(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainIOThreadInfoPtr **info) +{ + qemuDomainObjPrivatePtr priv; + qemuMonitorIOThreadsInfoPtr *iothreads = NULL; + virDomainIOThreadInfoPtr *info_ret = NULL; + int niothreads = 0; + int maxcpu, hostcpus, maplen; + size_t i; + int ret = -1; + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) + goto cleanup; + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("cannot list IOThreads for an inactive domain")); + goto endjob; + } + + priv = vm->privateData; + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("IOThreads not supported with this binary")); + goto endjob; + } + + qemuDomainObjEnterMonitor(driver, vm); + niothreads = qemuMonitorGetIOThreads(priv->mon, &iothreads); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto endjob; + if (niothreads < 0) + goto endjob; + + /* Nothing to do */ + if (niothreads == 0) { + ret = 0; + goto endjob; + } + + if ((hostcpus = nodeGetCPUCount()) < 0) + goto endjob; + + maplen = VIR_CPU_MAPLEN(hostcpus); + maxcpu = maplen * 8; + if (maxcpu > hostcpus) + maxcpu = hostcpus; + + if (VIR_ALLOC_N(info_ret, niothreads) < 0) + goto endjob; + + for (i = 0; i < niothreads; i++) { + virBitmapPtr map = NULL; + unsigned char *tmpmap = NULL; + int tmpmaplen = 0; + + if (VIR_ALLOC(info_ret[i]) < 0) + goto endjob; + + if (virStrToLong_ui(iothreads[i]->name + strlen("iothread"), NULL, 10, + &info_ret[i]->iothread_id) < 0) + goto endjob; + + if (VIR_ALLOC_N(info_ret[i]->cpumap, maplen) < 0) + goto endjob; + + if (virProcessGetAffinity(iothreads[i]->thread_id, &map, maxcpu) < 0) + goto endjob; + + virBitmapToData(map, &tmpmap, &tmpmaplen); + if (tmpmaplen > maplen) + tmpmaplen = maplen; + memcpy(info_ret[i]->cpumap, tmpmap, tmpmaplen); + info_ret[i]->cpumaplen = tmpmaplen; + + VIR_FREE(tmpmap); + virBitmapFree(map); + } + + *info = info_ret; + info_ret = NULL; + ret = niothreads; + + endjob: + qemuDomainObjEndJob(driver, vm); + + cleanup: + if (info_ret) { + for (i = 0; i < niothreads; i++) + virDomainIOThreadsInfoFree(info_ret[i]); + VIR_FREE(info_ret); + } + if (iothreads) { + for (i = 0; i < niothreads; i++) + qemuMonitorIOThreadsInfoFree(iothreads[i]); + VIR_FREE(iothreads); + } + + return ret; +} + +static int +qemuDomainGetIOThreadsConfig(virDomainDefPtr targetDef, + virDomainIOThreadInfoPtr **info) +{ + virDomainIOThreadInfoPtr *info_ret = NULL; + virDomainVcpuPinDefPtr *iothreadspin_list; + virBitmapPtr cpumask = NULL; + unsigned char *cpumap; + int maxcpu, hostcpus, maplen; + size_t i, pcpu; + bool pinned; + int ret = -1; + + if (targetDef->iothreads == 0) + return 0; + + if ((hostcpus = nodeGetCPUCount()) < 0) + goto cleanup; + + maplen = VIR_CPU_MAPLEN(hostcpus); + maxcpu = maplen * 8; + if (maxcpu > hostcpus) + maxcpu = hostcpus; + + if (VIR_ALLOC_N(info_ret, targetDef->iothreads) < 0) + goto cleanup; + + for (i = 0; i < targetDef->iothreads; i++) { + if (VIR_ALLOC(info_ret[i]) < 0) + goto cleanup; + + /* IOThreads being counting at 1 */ + info_ret[i]->iothread_id = i + 1; + + if (VIR_ALLOC_N(info_ret[i]->cpumap, maplen) < 0) + goto cleanup; + + /* Initialize the cpumap */ + info_ret[i]->cpumaplen = maplen; + memset(info_ret[i]->cpumap, 0xff, maplen); + if (maxcpu % 8) + info_ret[i]->cpumap[maplen - 1] &= (1 << maxcpu % 8) - 1; + } + + /* If iothreadspin setting exists, there are unused physical cpus */ + iothreadspin_list = targetDef->cputune.iothreadspin; + for (i = 0; i < targetDef->cputune.niothreadspin; i++) { + /* vcpuid is the iothread_id... + * iothread_id is the index into info_ret + 1, so we can + * assume that the info_ret index we want is vcpuid - 1 + */ + cpumap = info_ret[iothreadspin_list[i]->vcpuid - 1]->cpumap; + cpumask = iothreadspin_list[i]->cpumask; + + for (pcpu = 0; pcpu < maxcpu; pcpu++) { + if (virBitmapGetBit(cpumask, pcpu, &pinned) < 0) + goto cleanup; + if (!pinned) + VIR_UNUSE_CPU(cpumap, pcpu); + } + } + + *info = info_ret; + info_ret = NULL; + ret = targetDef->iothreads; + + cleanup: + if (info_ret) { + for (i = 0; i < targetDef->iothreads; i++) + virDomainIOThreadsInfoFree(info_ret[i]); + VIR_FREE(info_ret); + } + + return ret; +} + +static int +qemuDomainGetIOThreadsInfo(virDomainPtr dom, + virDomainIOThreadInfoPtr **info, + unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + virDomainObjPtr vm; + virCapsPtr caps = NULL; + virDomainDefPtr targetDef = NULL; + int ret = -1; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + + if (!(vm = qemuDomObjFromDomain(dom))) + goto cleanup; + + if (virDomainGetIOThreadsInfoEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + if (!(caps = virQEMUDriverGetCapabilities(driver, false))) + goto cleanup; + + if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt, vm, &flags, + &targetDef) < 0) + goto cleanup; + + if (flags & VIR_DOMAIN_AFFECT_LIVE) + targetDef = vm->def; + + /* Coverity didn't realize that targetDef must be set if we got here. */ + sa_assert(targetDef); + + if (flags & VIR_DOMAIN_AFFECT_LIVE) + ret = qemuDomainGetIOThreadsLive(driver, vm, info); + else + ret = qemuDomainGetIOThreadsConfig(targetDef, info); + + cleanup: + qemuDomObjEndAPI(&vm); + virObjectUnref(caps); + return ret; +} + static int qemuDomainGetSecurityLabel(virDomainPtr dom, virSecurityLabelPtr seclabel) { virQEMUDriverPtr driver = dom->conn->privateData; @@ -19108,6 +19331,7 @@ static virHypervisorDriver qemuHypervisorDriver = { .domainGetEmulatorPinInfo = qemuDomainGetEmulatorPinInfo, /* 0.10.0 */ .domainGetVcpus = qemuDomainGetVcpus, /* 0.4.4 */ .domainGetMaxVcpus = qemuDomainGetMaxVcpus, /* 0.4.4 */ + .domainGetIOThreadsInfo = qemuDomainGetIOThreadsInfo, /* 1.2.14 */ .domainGetSecurityLabel = qemuDomainGetSecurityLabel, /* 0.6.1 */ .domainGetSecurityLabelList = qemuDomainGetSecurityLabelList, /* 0.10.0 */ .nodeGetSecurityModel = qemuNodeGetSecurityModel, /* 0.6.1 */ -- 2.1.0

On Thu, Mar 05, 2015 at 09:03:00PM -0500, John Ferlan wrote:
Depending on the flags passed, either attempt to return the active/live IOThread data for the domain or the config data.
The active/live path will call into the Monitor in order to get the IOThread data and then correlate the thread_id's returned from the monitor to the currently running system/threads in order to ascertain the affinity for each iothread_id.
The config path will map each of the configured IOThreads and return any configured iothreadspin data
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_driver.c | 224 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 224 insertions(+)
ACK Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Thu, Mar 05, 2015 at 09:03:00PM -0500, John Ferlan wrote:
Depending on the flags passed, either attempt to return the active/live IOThread data for the domain or the config data.
The active/live path will call into the Monitor in order to get the IOThread data and then correlate the thread_id's returned from the monitor to the currently running system/threads in order to ascertain the affinity for each iothread_id.
The config path will map each of the configured IOThreads and return any configured iothreadspin data
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_driver.c | 224 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 224 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ffa4e19..d8a761d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5557,6 +5557,229 @@ qemuDomainGetMaxVcpus(virDomainPtr dom) VIR_DOMAIN_VCPU_MAXIMUM)); }
+static int +qemuDomainGetIOThreadsLive(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainIOThreadInfoPtr **info) +{ + qemuDomainObjPrivatePtr priv; + qemuMonitorIOThreadsInfoPtr *iothreads = NULL; + virDomainIOThreadInfoPtr *info_ret = NULL; + int niothreads = 0; + int maxcpu, hostcpus, maplen; + size_t i; + int ret = -1; + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) + goto cleanup; + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("cannot list IOThreads for an inactive domain")); + goto endjob; + } + + priv = vm->privateData; + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("IOThreads not supported with this binary")); + goto endjob; + } + + qemuDomainObjEnterMonitor(driver, vm); + niothreads = qemuMonitorGetIOThreads(priv->mon, &iothreads); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto endjob; + if (niothreads < 0) + goto endjob; + + /* Nothing to do */ + if (niothreads == 0) { + ret = 0; + goto endjob; + } + + if ((hostcpus = nodeGetCPUCount()) < 0) + goto endjob; + + maplen = VIR_CPU_MAPLEN(hostcpus);
The maplen is not needed. Just pass 'hostcpus' to 'virProcessGetAffinity' and it will generate a virBitmap of the right size, then virBitmapToData computes the correct maplen.
+ maxcpu = maplen * 8; + if (maxcpu > hostcpus)
These two lines are redundant. If maplen * 8 < hostcpus, then VIR_CPU_MAPLEN is flawed, because the map does not hold at least hostcpus bits. If maplen * 8 >= hostcpus, the value of hostcpus is used.
+ maxcpu = hostcpus;
+ + if (VIR_ALLOC_N(info_ret, niothreads) < 0) + goto endjob; + + for (i = 0; i < niothreads; i++) { + virBitmapPtr map = NULL; + unsigned char *tmpmap = NULL; + int tmpmaplen = 0; + + if (VIR_ALLOC(info_ret[i]) < 0) + goto endjob; + + if (virStrToLong_ui(iothreads[i]->name + strlen("iothread"), NULL, 10, + &info_ret[i]->iothread_id) < 0) + goto endjob; + + if (VIR_ALLOC_N(info_ret[i]->cpumap, maplen) < 0) + goto endjob; + + if (virProcessGetAffinity(iothreads[i]->thread_id, &map, maxcpu) < 0) + goto endjob; + + virBitmapToData(map, &tmpmap, &tmpmaplen);
+ if (tmpmaplen > maplen) + tmpmaplen = maplen;
This is equivalent to: if (VIR_CPU_MAPLEN(hostcpus) > VIR_CPU_MAPLEN(hostcpus))
+ memcpy(info_ret[i]->cpumap, tmpmap, tmpmaplen); + info_ret[i]->cpumaplen = tmpmaplen; + + VIR_FREE(tmpmap); + virBitmapFree(map); + } + + *info = info_ret; + info_ret = NULL; + ret = niothreads; + + endjob: + qemuDomainObjEndJob(driver, vm); + + cleanup: + if (info_ret) { + for (i = 0; i < niothreads; i++) + virDomainIOThreadsInfoFree(info_ret[i]); + VIR_FREE(info_ret); + } + if (iothreads) { + for (i = 0; i < niothreads; i++) + qemuMonitorIOThreadsInfoFree(iothreads[i]); + VIR_FREE(iothreads); + } + + return ret; +} + +static int +qemuDomainGetIOThreadsConfig(virDomainDefPtr targetDef, + virDomainIOThreadInfoPtr **info) +{ + virDomainIOThreadInfoPtr *info_ret = NULL; + virDomainVcpuPinDefPtr *iothreadspin_list; + virBitmapPtr cpumask = NULL; + unsigned char *cpumap; + int maxcpu, hostcpus, maplen; + size_t i, pcpu; + bool pinned; + int ret = -1; + + if (targetDef->iothreads == 0) + return 0; + + if ((hostcpus = nodeGetCPUCount()) < 0) + goto cleanup; + + maplen = VIR_CPU_MAPLEN(hostcpus); + maxcpu = maplen * 8; + if (maxcpu > hostcpus) + maxcpu = hostcpus;
Same redunancy here.
+ + if (VIR_ALLOC_N(info_ret, targetDef->iothreads) < 0) + goto cleanup; + + for (i = 0; i < targetDef->iothreads; i++) { + if (VIR_ALLOC(info_ret[i]) < 0) + goto cleanup; + + /* IOThreads being counting at 1 */ + info_ret[i]->iothread_id = i + 1; +
As I mentioned in my reply to v3: https://www.redhat.com/archives/libvir-list/2015-March/msg00249.html this really would look readable with virBitmapToData. I meant something like this: pininfo = virDomainVcpuPinFindByVcpu(targetDef->cputune.iothreadspin,); if (!pininfo) { bitmap = virBitmapNew(hostcpus); virBitmapSetAllBits(bitmap); pininfo = bitmap; } virBitmapToData(pininfo, info[i]->cpumap, info[i]->cpumaplen)
+ if (VIR_ALLOC_N(info_ret[i]->cpumap, maplen) < 0) + goto cleanup; + + /* Initialize the cpumap */ + info_ret[i]->cpumaplen = maplen; + memset(info_ret[i]->cpumap, 0xff, maplen); + if (maxcpu % 8) + info_ret[i]->cpumap[maplen - 1] &= (1 << maxcpu % 8) - 1;
+ } + + /* If iothreadspin setting exists, there are unused physical cpus */ + iothreadspin_list = targetDef->cputune.iothreadspin; + for (i = 0; i < targetDef->cputune.niothreadspin; i++) { + /* vcpuid is the iothread_id... + * iothread_id is the index into info_ret + 1, so we can + * assume that the info_ret index we want is vcpuid - 1 + */ + cpumap = info_ret[iothreadspin_list[i]->vcpuid - 1]->cpumap; + cpumask = iothreadspin_list[i]->cpumask; + + for (pcpu = 0; pcpu < maxcpu; pcpu++) { + if (virBitmapGetBit(cpumask, pcpu, &pinned) < 0) + goto cleanup; + if (!pinned) + VIR_UNUSE_CPU(cpumap, pcpu); + } + }
This is essentially an open-coded version of virBitmapToData. Jan

On 03/06/2015 07:39 AM, Ján Tomko wrote:
On Thu, Mar 05, 2015 at 09:03:00PM -0500, John Ferlan wrote:
Depending on the flags passed, either attempt to return the active/live IOThread data for the domain or the config data.
The active/live path will call into the Monitor in order to get the IOThread data and then correlate the thread_id's returned from the monitor to the currently running system/threads in order to ascertain the affinity for each iothread_id.
The config path will map each of the configured IOThreads and return any configured iothreadspin data
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_driver.c | 224 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 224 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ffa4e19..d8a761d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5557,6 +5557,229 @@ qemuDomainGetMaxVcpus(virDomainPtr dom) VIR_DOMAIN_VCPU_MAXIMUM)); }
+static int +qemuDomainGetIOThreadsLive(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainIOThreadInfoPtr **info) +{ + qemuDomainObjPrivatePtr priv; + qemuMonitorIOThreadsInfoPtr *iothreads = NULL; + virDomainIOThreadInfoPtr *info_ret = NULL; + int niothreads = 0; + int maxcpu, hostcpus, maplen; + size_t i; + int ret = -1; + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) + goto cleanup; + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("cannot list IOThreads for an inactive domain")); + goto endjob; + } + + priv = vm->privateData; + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("IOThreads not supported with this binary")); + goto endjob; + } + + qemuDomainObjEnterMonitor(driver, vm); + niothreads = qemuMonitorGetIOThreads(priv->mon, &iothreads); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto endjob; + if (niothreads < 0) + goto endjob; + + /* Nothing to do */ + if (niothreads == 0) { + ret = 0; + goto endjob; + } + + if ((hostcpus = nodeGetCPUCount()) < 0) + goto endjob; + + maplen = VIR_CPU_MAPLEN(hostcpus);
The maplen is not needed. Just pass 'hostcpus' to 'virProcessGetAffinity' and it will generate a virBitmap of the right size, then virBitmapToData computes the correct maplen.
+ maxcpu = maplen * 8; + if (maxcpu > hostcpus)
These two lines are redundant. If maplen * 8 < hostcpus, then VIR_CPU_MAPLEN is flawed, because the map does not hold at least hostcpus bits. If maplen * 8 >= hostcpus, the value of hostcpus is used.
Hmm... I'll have to go back and look again as this was pretty much following the VcpuPin or EmulatorPin examples with a touch of GetCPUMap since this code returns the cpumap rather than expecting one on input.
+ maxcpu = hostcpus;
+ + if (VIR_ALLOC_N(info_ret, niothreads) < 0) + goto endjob; + + for (i = 0; i < niothreads; i++) { + virBitmapPtr map = NULL; + unsigned char *tmpmap = NULL; + int tmpmaplen = 0; + + if (VIR_ALLOC(info_ret[i]) < 0) + goto endjob; + + if (virStrToLong_ui(iothreads[i]->name + strlen("iothread"), NULL, 10, + &info_ret[i]->iothread_id) < 0) + goto endjob; + + if (VIR_ALLOC_N(info_ret[i]->cpumap, maplen) < 0) + goto endjob; + + if (virProcessGetAffinity(iothreads[i]->thread_id, &map, maxcpu) < 0) + goto endjob; + + virBitmapToData(map, &tmpmap, &tmpmaplen);
+ if (tmpmaplen > maplen) + tmpmaplen = maplen;
This is equivalent to: if (VIR_CPU_MAPLEN(hostcpus) > VIR_CPU_MAPLEN(hostcpus))
+ memcpy(info_ret[i]->cpumap, tmpmap, tmpmaplen); + info_ret[i]->cpumaplen = tmpmaplen; + + VIR_FREE(tmpmap); + virBitmapFree(map); + } + + *info = info_ret; + info_ret = NULL; + ret = niothreads; + + endjob: + qemuDomainObjEndJob(driver, vm); + + cleanup: + if (info_ret) { + for (i = 0; i < niothreads; i++) + virDomainIOThreadsInfoFree(info_ret[i]); + VIR_FREE(info_ret); + } + if (iothreads) { + for (i = 0; i < niothreads; i++) + qemuMonitorIOThreadsInfoFree(iothreads[i]); + VIR_FREE(iothreads); + } + + return ret; +} + +static int +qemuDomainGetIOThreadsConfig(virDomainDefPtr targetDef, + virDomainIOThreadInfoPtr **info) +{ + virDomainIOThreadInfoPtr *info_ret = NULL; + virDomainVcpuPinDefPtr *iothreadspin_list; + virBitmapPtr cpumask = NULL; + unsigned char *cpumap; + int maxcpu, hostcpus, maplen; + size_t i, pcpu; + bool pinned; + int ret = -1; + + if (targetDef->iothreads == 0) + return 0; + + if ((hostcpus = nodeGetCPUCount()) < 0) + goto cleanup; + + maplen = VIR_CPU_MAPLEN(hostcpus); + maxcpu = maplen * 8; + if (maxcpu > hostcpus) + maxcpu = hostcpus;
Same redunancy here.
+ + if (VIR_ALLOC_N(info_ret, targetDef->iothreads) < 0) + goto cleanup; + + for (i = 0; i < targetDef->iothreads; i++) { + if (VIR_ALLOC(info_ret[i]) < 0) + goto cleanup; + + /* IOThreads being counting at 1 */ + info_ret[i]->iothread_id = i + 1; +
As I mentioned in my reply to v3: https://www.redhat.com/archives/libvir-list/2015-March/msg00249.html this really would look readable with virBitmapToData. I meant something like this:
pininfo = virDomainVcpuPinFindByVcpu(targetDef->cputune.iothreadspin,); if (!pininfo) { bitmap = virBitmapNew(hostcpus); virBitmapSetAllBits(bitmap); pininfo = bitmap; } virBitmapToData(pininfo, info[i]->cpumap, info[i]->cpumaplen)
I'll revisit, but again I was keeping in line with other examples.
+ if (VIR_ALLOC_N(info_ret[i]->cpumap, maplen) < 0) + goto cleanup; + + /* Initialize the cpumap */ + info_ret[i]->cpumaplen = maplen; + memset(info_ret[i]->cpumap, 0xff, maplen); + if (maxcpu % 8) + info_ret[i]->cpumap[maplen - 1] &= (1 << maxcpu % 8) - 1;
+ } + + /* If iothreadspin setting exists, there are unused physical cpus */ + iothreadspin_list = targetDef->cputune.iothreadspin; + for (i = 0; i < targetDef->cputune.niothreadspin; i++) { + /* vcpuid is the iothread_id... + * iothread_id is the index into info_ret + 1, so we can + * assume that the info_ret index we want is vcpuid - 1 + */ + cpumap = info_ret[iothreadspin_list[i]->vcpuid - 1]->cpumap; + cpumask = iothreadspin_list[i]->cpumask; + + for (pcpu = 0; pcpu < maxcpu; pcpu++) { + if (virBitmapGetBit(cpumask, pcpu, &pinned) < 0) + goto cleanup; + if (!pinned) + VIR_UNUSE_CPU(cpumap, pcpu); + } + }
This is essentially an open-coded version of virBitmapToData.
I'll address these in an update John

On Fri, Mar 06, 2015 at 08:07:56AM -0500, John Ferlan wrote:
The maplen is not needed. Just pass 'hostcpus' to 'virProcessGetAffinity' and it will generate a virBitmap of the right size, then virBitmapToData computes the correct maplen.
+ maxcpu = maplen * 8; + if (maxcpu > hostcpus)
These two lines are redundant. If maplen * 8 < hostcpus, then VIR_CPU_MAPLEN is flawed, because the map does not hold at least hostcpus bits. If maplen * 8 >= hostcpus, the value of hostcpus is used.
Hmm... I'll have to go back and look again as this was pretty much following the VcpuPin or EmulatorPin examples with a touch of GetCPUMap since this code returns the cpumap rather than expecting one on input.
Those are terrible examples long overdue for a cleanup. Jan

Add the 'iothreadsinfo' command to display IOThread Info data. Allow for [--live] or [--config] options in order to display live or config data for an active domain. $ virsh iothreadsinfo --help NAME iothreadsinfo - view domain IOThreads SYNOPSIS iothreadsinfo <domain> [--config] [--live] [--current] DESCRIPTION Returns basic information about the domain IOThreads. OPTIONS [--domain] <string> domain name, id or uuid --config affect next boot --live affect running domain --current affect current domain An active domain may return: $ virsh iothreads $dom IOThread ID CPU Affinity --------------------------------------------------- 1 2 2 3 3 0 $ echo $? 0 For domains which don't have IOThreads the following is returned: $ virsh iothreads $dom No IOThreads found for the domain $ echo $? 0 For domains which are not running the following is returned: $ virsh iothreads $dom --live error: Unable to get domain IOThreads information error: Requested operation is not valid: domain is not running $ echo $? 1 Editing a domains configuration and modifying the iothreadpin data for thread 3 from nothing provided to setting a cpuset of '0-1' and then displaying using --config would display: $ virsh iothreads f18iothr --config IOThread ID CPU Affinity ---------------------------- 1 2 2 3 3 0-1 $ Signed-off-by: John Ferlan <jferlan@redhat.com> --- tools/virsh-domain.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 12 +++++++ 2 files changed, 106 insertions(+) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 55c269c..0cce57b 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6790,6 +6790,94 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd) } /* + * "iothreadsinfo" command + */ +static const vshCmdInfo info_iothreads[] = { + {.name = "help", + .data = N_("view domain IOThreads") + }, + {.name = "desc", + .data = N_("Returns basic information about the domain IOThreads.") + }, + {.name = NULL} +}; +static const vshCmdOptDef opts_iothreads[] = { + {.name = "domain", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("domain name, id or uuid") + }, + {.name = "config", + .type = VSH_OT_BOOL, + .help = N_("affect next boot") + }, + {.name = "live", + .type = VSH_OT_BOOL, + .help = N_("affect running domain") + }, + {.name = "current", + .type = VSH_OT_BOOL, + .help = N_("affect current domain") + }, + {.name = NULL} +}; + +static bool +cmdIOThreadsInfo(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom; + bool config = vshCommandOptBool(cmd, "config"); + bool live = vshCommandOptBool(cmd, "live"); + bool current = vshCommandOptBool(cmd, "current"); + int niothreads = 0; + virDomainIOThreadInfoPtr *info; + size_t i; + int maxcpu; + unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; + + VSH_EXCLUSIVE_OPTIONS_VAR(current, live); + VSH_EXCLUSIVE_OPTIONS_VAR(current, config); + + if (config) + flags |= VIR_DOMAIN_AFFECT_CONFIG; + if (live) + flags |= VIR_DOMAIN_AFFECT_LIVE; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if ((maxcpu = vshNodeGetCPUCount(ctl->conn)) < 0) + goto cleanup; + + if ((niothreads = virDomainGetIOThreadsInfo(dom, &info, flags)) < 0) { + vshError(ctl, _("Unable to get domain IOThreads information")); + goto cleanup; + } + + if (niothreads == 0) { + vshPrintExtra(ctl, _("No IOThreads found for the domain")); + goto cleanup; + } + + vshPrintExtra(ctl, " %-15s %-15s\n", + _("IOThread ID"), _("CPU Affinity")); + vshPrintExtra(ctl, "---------------------------------------------------\n"); + for (i = 0; i < niothreads; i++) { + + vshPrint(ctl, " %-15u ", info[i]->iothread_id); + ignore_value(vshPrintPinInfo(info[i]->cpumap, info[i]->cpumaplen, + maxcpu, 0)); + vshPrint(ctl, "\n"); + virDomainIOThreadsInfoFree(info[i]); + } + VIR_FREE(info); + + cleanup: + virDomainFree(dom); + return niothreads >= 0; +} + +/* * "cpu-compare" command */ static const vshCmdInfo info_cpu_compare[] = { @@ -12702,6 +12790,12 @@ const vshCmdDef domManagementCmds[] = { .info = info_inject_nmi, .flags = 0 }, + {.name = "iothreadsinfo", + .handler = cmdIOThreadsInfo, + .opts = opts_iothreads, + .info = info_iothreads, + .flags = 0 + }, {.name = "send-key", .handler = cmdSendKey, .opts = opts_send_key, diff --git a/tools/virsh.pod b/tools/virsh.pod index 1f71c91..f19601e 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1361,6 +1361,18 @@ If I<--timeout> is specified, the command gives up waiting for events after I<seconds> have elapsed. With I<--loop>, the command prints all events until a timeout or interrupt key. +=item B<iothreadsinfo> I<domain> [[I<--live>] [I<--config>] | [I<--current>]] + +Display basic domain IOThreads information including the IOThread ID and +the CPU Affinity for each IOThread. + +If I<--live> is specified, get the IOThreads data from the running guest. If +the guest is not running, an error is returned. +If I<--config> is specified, get the IOThreads data from the next boot of +a persistent guest. +If I<--current> is specified or I<--live> and I<--config> are not specified, +then get the IOThread data based on the current guest state. + =item B<managedsave> I<domain> [I<--bypass-cache>] [{I<--running> | I<--paused>}] [I<--verbose>] -- 2.1.0

On Thu, Mar 05, 2015 at 09:03:01PM -0500, John Ferlan wrote:
Add the 'iothreadsinfo' command to display IOThread Info data. Allow for [--live] or [--config] options in order to display live or config data for an active domain.
$ virsh iothreadsinfo --help NAME iothreadsinfo - view domain IOThreads
SYNOPSIS iothreadsinfo <domain> [--config] [--live] [--current]
DESCRIPTION Returns basic information about the domain IOThreads.
OPTIONS [--domain] <string> domain name, id or uuid --config affect next boot --live affect running domain --current affect current domain
An active domain may return:
$ virsh iothreads $dom IOThread ID CPU Affinity --------------------------------------------------- 1 2 2 3 3 0
$ echo $? 0
For domains which don't have IOThreads the following is returned:
$ virsh iothreads $dom No IOThreads found for the domain
$ echo $? 0
For domains which are not running the following is returned:
$ virsh iothreads $dom --live error: Unable to get domain IOThreads information error: Requested operation is not valid: domain is not running
$ echo $? 1
Editing a domains configuration and modifying the iothreadpin data for thread 3 from nothing provided to setting a cpuset of '0-1' and then displaying using --config would display:
$ virsh iothreads f18iothr --config IOThread ID CPU Affinity ---------------------------- 1 2 2 3 3 0-1
$
Signed-off-by: John Ferlan <jferlan@redhat.com> --- tools/virsh-domain.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 12 +++++++ 2 files changed, 106 insertions(+)
ACK Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

Add virDomainGetIOThreadPin to fetch the pinned CPU affinity map for one IOThread. Add virDomainPinIOThread to allow setting the CPU affinity for a specific IOThread. Signed-off-by: John Ferlan <jferlan@redhat.com> --- include/libvirt/libvirt-domain.h | 10 +++ src/driver-hypervisor.h | 16 +++++ src/libvirt-domain.c | 152 +++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 2 + 4 files changed, 180 insertions(+) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 9487b80..fc35cd2 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1609,6 +1609,16 @@ void virDomainIOThreadsInfoFree(virDomainIOThreadInfoPtr info); int virDomainGetIOThreadsInfo(virDomainPtr domain, virDomainIOThreadInfoPtr **info, unsigned int flags); +int virDomainGetIOThreadPin (virDomainPtr domain, + unsigned int iothread_id, + unsigned char *cpumap, + int maplen, + unsigned int flags); +int virDomainPinIOThread(virDomainPtr domain, + unsigned int iothread_id, + unsigned char *cpumap, + int maplen, + unsigned int flags); /** * VIR_USE_CPU: diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index c2b4cd8..d62ce4b 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -386,6 +386,20 @@ typedef int unsigned int flags); typedef int +(*virDrvDomainGetIOThreadPin)(virDomainPtr domain, + unsigned int iothread_id, + unsigned char *cpumap, + int maplen, + unsigned int flags); + +typedef int +(*virDrvDomainPinIOThread)(virDomainPtr domain, + unsigned int iothread_id, + unsigned char *cpumap, + int maplen, + unsigned int flags); + +typedef int (*virDrvDomainGetSecurityLabel)(virDomainPtr domain, virSecurityLabelPtr seclabel); @@ -1260,6 +1274,8 @@ struct _virHypervisorDriver { virDrvDomainGetVcpus domainGetVcpus; virDrvDomainGetMaxVcpus domainGetMaxVcpus; virDrvDomainGetIOThreadsInfo domainGetIOThreadsInfo; + virDrvDomainGetIOThreadPin domainGetIOThreadPin; + virDrvDomainPinIOThread domainPinIOThread; virDrvDomainGetSecurityLabel domainGetSecurityLabel; virDrvDomainGetSecurityLabelList domainGetSecurityLabelList; virDrvNodeGetSecurityModel nodeGetSecurityModel; diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 27f6abe..ee3659a 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -7964,6 +7964,158 @@ virDomainIOThreadsInfoFree(virDomainIOThreadInfoPtr info) /** + * virDomainGetIOThreadPin: + * @domain: pointer to domain object, or NULL for Domain0 + * @iothread_id: An IOThread ID of which to get the pin information + * @cpumap: pointer to a bit map of real CPUs for the IOThread of this + * domain (in 8-bit bytes) (OUT) + * The memory allocated to cpumap must be maplen bytes + * Must not be NULL. + * @maplen: the number of bytes in the cpumap, from 1 up to size of CPU map. + * Must be positive. + * @flags: bitwise-OR of virDomainModificationImpact + * Must not be VIR_DOMAIN_AFFECT_LIVE and + * VIR_DOMAIN_AFFECT_CONFIG concurrently. + * + * Query the CPU affinity setting of an IOThread ID of the domain, store + * it in cpumap. + * + * Returns 1 in case of success, + * 0 in case of no IOThread threads are pinned to pcpus, + * -1 in case of failure. + */ +int +virDomainGetIOThreadPin(virDomainPtr domain, + unsigned int iothread_id, + unsigned char *cpumap, + int maplen, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "iothread_id=%u cpumap=%p, maplen=%d, flags=%x", + iothread_id, cpumap, maplen, flags); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + conn = domain->conn; + + if ((unsigned short) iothread_id != iothread_id) { + virReportError(VIR_ERR_OVERFLOW, _("input too large: %u"), + iothread_id); + goto error; + } + virCheckPositiveArgGoto(iothread_id, error); + virCheckNonNullArgGoto(cpumap, error); + virCheckPositiveArgGoto(maplen, error); + + /* At most one of these two flags should be set. */ + if ((flags & VIR_DOMAIN_AFFECT_LIVE) && + (flags & VIR_DOMAIN_AFFECT_CONFIG)) { + virReportInvalidArg(flags, + _("flags 'affect live' and 'affect config' in %s " + "are mutually exclusive"), + __FUNCTION__); + goto error; + } + conn = domain->conn; + + if (conn->driver->domainGetIOThreadPin) { + int ret; + ret = conn->driver->domainGetIOThreadPin(domain, iothread_id, + cpumap, maplen, flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(domain->conn); + return -1; +} + + +/** + * virDomainPinIOThread: + * @domain: a domain object + * @iothread_id: either the thread_id to modify or a count of IOThreads + * to be added or removed from the domain depending on the @flags setting + * @cpumap: pointer to a bit map of real CPUs (in 8-bit bytes) (IN) + * Each bit set to 1 means that corresponding CPU is usable. + * Bytes are stored in little-endian order: CPU0-7, 8-15... + * In each byte, lowest CPU number is least significant bit. + * @maplen: number of bytes in cpumap, from 1 up to size of CPU map in + * underlying virtualization system (Xen...). + * If maplen < size, missing bytes are set to zero. + * If maplen > size, failure code is returned. + * @flags: bitwise-OR of virDomainModificationImpact + * + * Dynamically change the real CPUs which can be allocated to an IOThread. + * This function may require privileged access to the hypervisor. + * + * @flags may include VIR_DOMAIN_AFFECT_LIVE or VIR_DOMAIN_AFFECT_CONFIG. + * Both flags may be set. + * If VIR_DOMAIN_AFFECT_LIVE is set, the change affects a running domain + * and may fail if domain is not alive. + * If VIR_DOMAIN_AFFECT_CONFIG is set, the change affects persistent state, + * and will fail for transient domains. If neither flag is specified (that is, + * @flags is VIR_DOMAIN_AFFECT_CURRENT), then an inactive domain modifies + * persistent setup, while an active domain is hypervisor-dependent on whether + * just live or both live and persistent state is changed. + * Not all hypervisors can support all flag combinations. + * + * See also virDomainGetIOThreadsInfo for querying this information. + * + * Returns 0 in case of success, -1 in case of failure. + */ +int +virDomainPinIOThread(virDomainPtr domain, + unsigned int iothread_id, + unsigned char *cpumap, + int maplen, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "iothread_id=%u, cpumap=%p, maplen=%d", + iothread_id, cpumap, maplen); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + conn = domain->conn; + + virCheckReadOnlyGoto(conn->flags, error); + if ((unsigned short) iothread_id != iothread_id) { + virReportError(VIR_ERR_OVERFLOW, _("input too large: %u"), + iothread_id); + goto error; + } + virCheckPositiveArgGoto(iothread_id, error); + virCheckNonNullArgGoto(cpumap, error); + virCheckPositiveArgGoto(maplen, error); + + if (conn->driver->domainPinIOThread) { + int ret; + ret = conn->driver->domainPinIOThread(domain, iothread_id, + cpumap, maplen, flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(domain->conn); + return -1; +} + + +/** * virDomainGetSecurityLabel: * @domain: a domain object * @seclabel: pointer to a virSecurityLabel structure diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 34852c1..e966bdd 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -699,6 +699,8 @@ LIBVIRT_1.2.14 { global: virDomainIOThreadsInfoFree; virDomainGetIOThreadsInfo; + virDomainGetIOThreadPin; + virDomainPinIOThread; } LIBVIRT_1.2.12; # .... define new API here using predicted next version number .... -- 2.1.0

On Thu, Mar 05, 2015 at 09:03:02PM -0500, John Ferlan wrote:
Add virDomainGetIOThreadPin to fetch the pinned CPU affinity map for one IOThread.
Add virDomainPinIOThread to allow setting the CPU affinity for a specific IOThread.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- include/libvirt/libvirt-domain.h | 10 +++ src/driver-hypervisor.h | 16 +++++ src/libvirt-domain.c | 152 +++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 2 + 4 files changed, 180 insertions(+)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 9487b80..fc35cd2 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1609,6 +1609,16 @@ void virDomainIOThreadsInfoFree(virDomainIOThreadInfoPtr info); int virDomainGetIOThreadsInfo(virDomainPtr domain, virDomainIOThreadInfoPtr **info, unsigned int flags); +int virDomainGetIOThreadPin (virDomainPtr domain, + unsigned int iothread_id, + unsigned char *cpumap, + int maplen, + unsigned int flags);
Isn't the GetIOThreadsInfo method & struct already reporting the pinning info for all current IO threads ? I'm not sure what the difference is between what GetIOThreadsInfo and GetIOThreadPin is ? +struct _virDomainIOThreadInfo { + unsigned int iothread_id; /* IOThread ID */ + unsigned char *cpumap; /* CPU map for thread. A pointer to an */ + /* array of real CPUs (in 8-bit bytes) */ + int cpumaplen; /* cpumap size */ +}; These fields match all the parameters in GetIOThreadPin, so it seems adding a GetIOThreadPin is redundant,
+int virDomainPinIOThread(virDomainPtr domain, + unsigned int iothread_id, + unsigned char *cpumap, + int maplen, + unsigned int flags);
This is fine though
+/** + * virDomainPinIOThread: + * @domain: a domain object + * @iothread_id: either the thread_id to modify or a count of IOThreads + * to be added or removed from the domain depending on the @flags setting
This can be updated now.
+ * @cpumap: pointer to a bit map of real CPUs (in 8-bit bytes) (IN) + * Each bit set to 1 means that corresponding CPU is usable. + * Bytes are stored in little-endian order: CPU0-7, 8-15... + * In each byte, lowest CPU number is least significant bit. + * @maplen: number of bytes in cpumap, from 1 up to size of CPU map in + * underlying virtualization system (Xen...). + * If maplen < size, missing bytes are set to zero. + * If maplen > size, failure code is returned. + * @flags: bitwise-OR of virDomainModificationImpact + * + * Dynamically change the real CPUs which can be allocated to an IOThread. + * This function may require privileged access to the hypervisor. + * + * @flags may include VIR_DOMAIN_AFFECT_LIVE or VIR_DOMAIN_AFFECT_CONFIG. + * Both flags may be set. + * If VIR_DOMAIN_AFFECT_LIVE is set, the change affects a running domain + * and may fail if domain is not alive. + * If VIR_DOMAIN_AFFECT_CONFIG is set, the change affects persistent state, + * and will fail for transient domains. If neither flag is specified (that is, + * @flags is VIR_DOMAIN_AFFECT_CURRENT), then an inactive domain modifies + * persistent setup, while an active domain is hypervisor-dependent on whether + * just live or both live and persistent state is changed. + * Not all hypervisors can support all flag combinations. + * + * See also virDomainGetIOThreadsInfo for querying this information. + * + * Returns 0 in case of success, -1 in case of failure. + */ +int +virDomainPinIOThread(virDomainPtr domain, + unsigned int iothread_id, + unsigned char *cpumap, + int maplen, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "iothread_id=%u, cpumap=%p, maplen=%d", + iothread_id, cpumap, maplen); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + conn = domain->conn; + + virCheckReadOnlyGoto(conn->flags, error); + if ((unsigned short) iothread_id != iothread_id) { + virReportError(VIR_ERR_OVERFLOW, _("input too large: %u"), + iothread_id); + goto error; + } + virCheckPositiveArgGoto(iothread_id, error); + virCheckNonNullArgGoto(cpumap, error); + virCheckPositiveArgGoto(maplen, error); + + if (conn->driver->domainPinIOThread) { + int ret; + ret = conn->driver->domainPinIOThread(domain, iothread_id, + cpumap, maplen, flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(domain->conn); + return -1; +}
Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 03/06/2015 04:46 AM, Daniel P. Berrange wrote:
On Thu, Mar 05, 2015 at 09:03:02PM -0500, John Ferlan wrote:
Add virDomainGetIOThreadPin to fetch the pinned CPU affinity map for one IOThread.
Add virDomainPinIOThread to allow setting the CPU affinity for a specific IOThread.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- include/libvirt/libvirt-domain.h | 10 +++ src/driver-hypervisor.h | 16 +++++ src/libvirt-domain.c | 152 +++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 2 + 4 files changed, 180 insertions(+)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 9487b80..fc35cd2 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1609,6 +1609,16 @@ void virDomainIOThreadsInfoFree(virDomainIOThreadInfoPtr info); int virDomainGetIOThreadsInfo(virDomainPtr domain, virDomainIOThreadInfoPtr **info, unsigned int flags); +int virDomainGetIOThreadPin (virDomainPtr domain, + unsigned int iothread_id, + unsigned char *cpumap, + int maplen, + unsigned int flags);
Isn't the GetIOThreadsInfo method & struct already reporting the pinning info for all current IO threads ? I'm not sure what the difference is between what GetIOThreadsInfo and GetIOThreadPin is ?
+struct _virDomainIOThreadInfo { + unsigned int iothread_id; /* IOThread ID */ + unsigned char *cpumap; /* CPU map for thread. A pointer to an */ + /* array of real CPUs (in 8-bit bytes) */ + int cpumaplen; /* cpumap size */ +};
These fields match all the parameters in GetIOThreadPin, so it seems adding a GetIOThreadPin is redundant,
Yes - although based on reviews from yesterday it wasn't totally clear if having a GetIOThreadPin in addition to GetIOThreadsInfo was requested, so I added it to be "safe", but can remove it. There is a GetVcpus and a GetVcpuPinInfo and the IOThreads code mirrors that. Tks - John
+int virDomainPinIOThread(virDomainPtr domain, + unsigned int iothread_id, + unsigned char *cpumap, + int maplen, + unsigned int flags);
This is fine though
+/** + * virDomainPinIOThread: + * @domain: a domain object + * @iothread_id: either the thread_id to modify or a count of IOThreads + * to be added or removed from the domain depending on the @flags setting
This can be updated now.
+ * @cpumap: pointer to a bit map of real CPUs (in 8-bit bytes) (IN) + * Each bit set to 1 means that corresponding CPU is usable. + * Bytes are stored in little-endian order: CPU0-7, 8-15... + * In each byte, lowest CPU number is least significant bit. + * @maplen: number of bytes in cpumap, from 1 up to size of CPU map in + * underlying virtualization system (Xen...). + * If maplen < size, missing bytes are set to zero. + * If maplen > size, failure code is returned. + * @flags: bitwise-OR of virDomainModificationImpact + * + * Dynamically change the real CPUs which can be allocated to an IOThread. + * This function may require privileged access to the hypervisor. + * + * @flags may include VIR_DOMAIN_AFFECT_LIVE or VIR_DOMAIN_AFFECT_CONFIG. + * Both flags may be set. + * If VIR_DOMAIN_AFFECT_LIVE is set, the change affects a running domain + * and may fail if domain is not alive. + * If VIR_DOMAIN_AFFECT_CONFIG is set, the change affects persistent state, + * and will fail for transient domains. If neither flag is specified (that is, + * @flags is VIR_DOMAIN_AFFECT_CURRENT), then an inactive domain modifies + * persistent setup, while an active domain is hypervisor-dependent on whether + * just live or both live and persistent state is changed. + * Not all hypervisors can support all flag combinations. + * + * See also virDomainGetIOThreadsInfo for querying this information. + * + * Returns 0 in case of success, -1 in case of failure. + */ +int +virDomainPinIOThread(virDomainPtr domain, + unsigned int iothread_id, + unsigned char *cpumap, + int maplen, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "iothread_id=%u, cpumap=%p, maplen=%d", + iothread_id, cpumap, maplen); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + conn = domain->conn; + + virCheckReadOnlyGoto(conn->flags, error); + if ((unsigned short) iothread_id != iothread_id) { + virReportError(VIR_ERR_OVERFLOW, _("input too large: %u"), + iothread_id); + goto error; + } + virCheckPositiveArgGoto(iothread_id, error); + virCheckNonNullArgGoto(cpumap, error); + virCheckPositiveArgGoto(maplen, error); + + if (conn->driver->domainPinIOThread) { + int ret; + ret = conn->driver->domainPinIOThread(domain, iothread_id, + cpumap, maplen, flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(domain->conn); + return -1; +}
Regards, Daniel

On Fri, Mar 06, 2015 at 06:06:49AM -0500, John Ferlan wrote:
On 03/06/2015 04:46 AM, Daniel P. Berrange wrote:
On Thu, Mar 05, 2015 at 09:03:02PM -0500, John Ferlan wrote:
Add virDomainGetIOThreadPin to fetch the pinned CPU affinity map for one IOThread.
Add virDomainPinIOThread to allow setting the CPU affinity for a specific IOThread.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- include/libvirt/libvirt-domain.h | 10 +++ src/driver-hypervisor.h | 16 +++++ src/libvirt-domain.c | 152 +++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 2 + 4 files changed, 180 insertions(+)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 9487b80..fc35cd2 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1609,6 +1609,16 @@ void virDomainIOThreadsInfoFree(virDomainIOThreadInfoPtr info); int virDomainGetIOThreadsInfo(virDomainPtr domain, virDomainIOThreadInfoPtr **info, unsigned int flags); +int virDomainGetIOThreadPin (virDomainPtr domain, + unsigned int iothread_id, + unsigned char *cpumap, + int maplen, + unsigned int flags);
Isn't the GetIOThreadsInfo method & struct already reporting the pinning info for all current IO threads ? I'm not sure what the difference is between what GetIOThreadsInfo and GetIOThreadPin is ?
+struct _virDomainIOThreadInfo { + unsigned int iothread_id; /* IOThread ID */ + unsigned char *cpumap; /* CPU map for thread. A pointer to an */ + /* array of real CPUs (in 8-bit bytes) */ + int cpumaplen; /* cpumap size */ +};
These fields match all the parameters in GetIOThreadPin, so it seems adding a GetIOThreadPin is redundant,
Yes - although based on reviews from yesterday it wasn't totally clear if having a GetIOThreadPin in addition to GetIOThreadsInfo was requested, so I added it to be "safe", but can remove it.
There is a GetVcpus and a GetVcpuPinInfo and the IOThreads code mirrors that.
Ah that's more of a historical accident :-) We added GetVcpus first to simply return the number of VCPUs and later added GetVcpuPinInfo to return pinning info. If we were doing it fresh, we'd only add the GetVcpuPinInfo since it offers the superset of features. If you repost without GetIOThreadPin, I'd ack the rest. BTW, no need to repost the first 3 I already acked. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

Yes - although based on reviews from yesterday it wasn't totally clear if having a GetIOThreadPin in addition to GetIOThreadsInfo was requested, so I added it to be "safe", but can remove it.
There is a GetVcpus and a GetVcpuPinInfo and the IOThreads code mirrors that.
Ah that's more of a historical accident :-) We added GetVcpus first to simply return the number of VCPUs and later added GetVcpuPinInfo to return pinning info. If we were doing it fresh, we'd only add the GetVcpuPinInfo since it offers the superset of features.
If you repost without GetIOThreadPin, I'd ack the rest.
BTW, no need to repost the first 3 I already acked.
OK - I'll push 1-4 since they're ACK'd (I have a related libvirt-python patches ready too), then repost the last 5. Is this something that'll need adjustments in the perl bindings? (not that I know perl all that well) Tks - John

On Fri, Mar 06, 2015 at 06:53:20AM -0500, John Ferlan wrote:
Yes - although based on reviews from yesterday it wasn't totally clear if having a GetIOThreadPin in addition to GetIOThreadsInfo was requested, so I added it to be "safe", but can remove it.
There is a GetVcpus and a GetVcpuPinInfo and the IOThreads code mirrors that.
Ah that's more of a historical accident :-) We added GetVcpus first to simply return the number of VCPUs and later added GetVcpuPinInfo to return pinning info. If we were doing it fresh, we'd only add the GetVcpuPinInfo since it offers the superset of features.
If you repost without GetIOThreadPin, I'd ack the rest.
BTW, no need to repost the first 3 I already acked.
OK - I'll push 1-4 since they're ACK'd (I have a related libvirt-python patches ready too), then repost the last 5.
Is this something that'll need adjustments in the perl bindings? (not that I know perl all that well)
Yep, every new API requires work in the perl bindings. Most people ignore it, so I end up doing the work myself, but if you fancy trying todo it I certainly wouldn't complain :-) Just let me know either way. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

Implement the remote plumbing to get an IOThread's pinned CPU data or to allow pinning an IOThread to a specific CPU set Signed-off-by: John Ferlan <jferlan@redhat.com> --- daemon/remote.c | 49 ++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 62 ++++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 35 ++++++++++++++++++++++++- src/remote_protocol-structs | 24 +++++++++++++++++ src/rpc/gendispatch.pl | 1 + 5 files changed, 170 insertions(+), 1 deletion(-) diff --git a/daemon/remote.c b/daemon/remote.c index ff64eeb..8907b7e 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -2341,6 +2341,55 @@ remoteDispatchDomainGetIOThreadsInfo(virNetServerPtr server ATTRIBUTE_UNUSED, } static int +remoteDispatchDomainGetIOThreadPin(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_get_iothread_pin_args *args, + remote_domain_get_iothread_pin_ret *ret) +{ + virDomainPtr dom = NULL; + unsigned char *cpumap = NULL; + int r; + int rv = -1; + struct daemonClientPrivate *priv = + virNetServerClientGetPrivateData(client); + + if (!priv->conn) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (!(dom = get_nonnull_domain(priv->conn, args->dom))) + goto cleanup; + + /* Allocate buffers to take the results */ + if (args->maplen > 0 && + VIR_ALLOC_N(cpumap, args->maplen) < 0) + goto cleanup; + + if ((r = virDomainGetIOThreadPin(dom, + args->iothread_id, + cpumap, + args->maplen, + args->flags)) < 0) + goto cleanup; + + ret->ret = r; + ret->cpumap.cpumap_len = args->maplen; + ret->cpumap.cpumap_val = (char *) cpumap; + cpumap = NULL; + + rv = 0; + + cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + VIR_FREE(cpumap); + virObjectUnref(dom); + return rv; +} +static int remoteDispatchDomainMigratePrepare(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, virNetMessagePtr msg ATTRIBUTE_UNUSED, diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 42dab9d..fa99a31 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2394,6 +2394,66 @@ remoteDomainGetIOThreadsInfo(virDomainPtr dom, } static int +remoteDomainGetIOThreadPin(virDomainPtr domain, + unsigned int iothread_id, + unsigned char *cpumap, + int maplen, + unsigned int flags) +{ + int rv = -1; + size_t i; + remote_domain_get_iothread_pin_args args; + remote_domain_get_iothread_pin_ret ret; + struct private_data *priv = domain->conn->privateData; + + remoteDriverLock(priv); + + /* There is only one cpumap for each IOThread thread */ + if (maplen > REMOTE_CPUMAPS_MAX) { + virReportError(VIR_ERR_RPC, + _("IOThread map buffer length exceeds maximum: %d > %d"), + maplen, REMOTE_CPUMAPS_MAX); + goto done; + } + + make_nonnull_domain(&args.dom, domain); + args.iothread_id = iothread_id; + args.maplen = maplen; + args.flags = flags; + + memset(&ret, 0, sizeof(ret)); + + if (call(domain->conn, priv, 0, REMOTE_PROC_DOMAIN_GET_IOTHREAD_PIN, + (xdrproc_t) xdr_remote_domain_get_iothread_pin_args, + (char *) &args, + (xdrproc_t) xdr_remote_domain_get_iothread_pin_ret, + (char *) &ret) == -1) + goto done; + + if (ret.cpumap.cpumap_len > maplen) { + virReportError(VIR_ERR_RPC, + _("host reports map buffer length exceeds maximum: %d > %d"), + ret.cpumap.cpumap_len, maplen); + goto cleanup; + } + + memset(cpumap, 0, maplen); + + for (i = 0; i < ret.cpumap.cpumap_len; ++i) + cpumap[i] = ret.cpumap.cpumap_val[i]; + + rv = ret.ret; + + cleanup: + xdr_free((xdrproc_t) xdr_remote_domain_get_iothread_pin_ret, + (char *) &ret); + + done: + remoteDriverUnlock(priv); + return rv; +} + +static int remoteDomainGetSecurityLabel(virDomainPtr domain, virSecurityLabelPtr seclabel) { remote_domain_get_security_label_args args; @@ -8106,6 +8166,8 @@ static virHypervisorDriver hypervisor_driver = { .domainGetVcpus = remoteDomainGetVcpus, /* 0.3.0 */ .domainGetMaxVcpus = remoteDomainGetMaxVcpus, /* 0.3.0 */ .domainGetIOThreadsInfo = remoteDomainGetIOThreadsInfo, /* 1.2.14 */ + .domainGetIOThreadPin = remoteDomainGetIOThreadPin, /* 1.2.14 */ + .domainPinIOThread = remoteDomainPinIOThread, /* 1.2.14 */ .domainGetSecurityLabel = remoteDomainGetSecurityLabel, /* 0.6.1 */ .domainGetSecurityLabelList = remoteDomainGetSecurityLabelList, /* 0.10.0 */ .nodeGetSecurityModel = remoteNodeGetSecurityModel, /* 0.6.1 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 4ea535e..dc916f3 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -1199,6 +1199,25 @@ struct remote_domain_get_iothreads_info_ret { unsigned int ret; }; +struct remote_domain_get_iothread_pin_args { + remote_nonnull_domain dom; + unsigned int iothread_id; + int maplen; + unsigned int flags; +}; + +struct remote_domain_get_iothread_pin_ret { + opaque cpumap<REMOTE_CPUMAPS_MAX>; + int ret; +}; + +struct remote_domain_pin_iothread_args { + remote_nonnull_domain dom; + unsigned int iothreads_id; + opaque cpumap<REMOTE_CPUMAP_MAX>; /* (unsigned char *) */ + unsigned int flags; +}; + struct remote_domain_get_security_label_args { remote_nonnull_domain dom; }; @@ -5593,5 +5612,19 @@ enum remote_procedure { * @generate: none * @acl: domain:read */ - REMOTE_PROC_DOMAIN_GET_IOTHREADS_INFO = 351 + REMOTE_PROC_DOMAIN_GET_IOTHREADS_INFO = 351, + + /** + * @generate: none + * @acl: domain:read + */ + REMOTE_PROC_DOMAIN_GET_IOTHREAD_PIN = 352, + + /** + * @generate: both + * @acl: domain:write + * @acl: domain:save:!VIR_DOMAIN_AFFECT_CONFIG|VIR_DOMAIN_AFFECT_LIVE + * @acl: domain:save:VIR_DOMAIN_AFFECT_CONFIG + */ + REMOTE_PROC_DOMAIN_PIN_IOTHREAD = 353 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 907bcd4..4fa710c 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -825,6 +825,28 @@ struct remote_domain_get_iothreads_info_ret { } info; u_int ret; }; +struct remote_domain_get_iothread_pin_args { + remote_nonnull_domain dom; + u_int iothread_id; + int maplen; + u_int flags; +}; +struct remote_domain_get_iothread_pin_ret { + struct { + u_int cpumap_len; + char * cpumap_val; + } cpumap; + int ret; +}; +struct remote_domain_pin_iothread_args { + remote_nonnull_domain dom; + u_int iothreads_id; + struct { + u_int cpumap_len; + char * cpumap_val; + } cpumap; + u_int flags; +}; struct remote_domain_get_security_label_args { remote_nonnull_domain dom; }; @@ -2982,4 +3004,6 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_GET_FSINFO = 349, REMOTE_PROC_DOMAIN_DEFINE_XML_FLAGS = 350, REMOTE_PROC_DOMAIN_GET_IOTHREADS_INFO = 351, + REMOTE_PROC_DOMAIN_GET_IOTHREAD_PIN = 352, + REMOTE_PROC_DOMAIN_PIN_IOTHREAD = 353, }; diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index 78cb415..8b488eb 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -68,6 +68,7 @@ sub fixup_name { $name =~ s/Fsthaw$/FSThaw/; $name =~ s/Fsinfo$/FSInfo/; $name =~ s/Iothreads$/IOThreads/; + $name =~ s/Iothread$/IOThread/; $name =~ s/Scsi/SCSI/; $name =~ s/Wwn$/WWN/; $name =~ s/Dhcp$/DHCP/; -- 2.1.0

https://bugzilla.redhat.com/show_bug.cgi?id=1135491 More or less a virtual copy of the existing virDomainVcpuPin{Add|Del} API's. NB: The IOThreads implementation "reused" the virDomainVcpuPinDefPtr since it provided everything necessary - an "id" and a "map" for each thread id configured. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/domain_conf.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 10 ++++++++ src/libvirt_private.syms | 2 ++ 3 files changed, 76 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6da02b0..fc4e8bd 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16708,6 +16708,70 @@ virDomainEmulatorPinDel(virDomainDefPtr def) return 0; } +int +virDomainIOThreadsPinAdd(virDomainVcpuPinDefPtr **iothreadspin_list, + size_t *niothreadspin, + unsigned char *cpumap, + int maplen, + int iothread_val) +{ + /* IOThreads share the virDomainVcpuPinDefPtr */ + virDomainVcpuPinDefPtr iothreadpin = NULL; + + if (!iothreadspin_list) + return -1; + + iothreadpin = virDomainVcpuPinFindByVcpu(*iothreadspin_list, + *niothreadspin, + iothread_val); + if (iothreadpin) { + iothreadpin->vcpuid = iothread_val; + virBitmapFree(iothreadpin->cpumask); + iothreadpin->cpumask = virBitmapNewData(cpumap, maplen); + if (!iothreadpin->cpumask) + return -1; + + return 0; + } + + /* No existing iothreadpin matches iothread_val, adding a new one */ + + if (VIR_ALLOC(iothreadpin) < 0) + goto error; + + iothreadpin->vcpuid = iothread_val; + iothreadpin->cpumask = virBitmapNewData(cpumap, maplen); + if (!iothreadpin->cpumask) + goto error; + + if (VIR_APPEND_ELEMENT(*iothreadspin_list, *niothreadspin, iothreadpin) < 0) + goto error; + + return 0; + + error: + virDomainVcpuPinDefFree(iothreadpin); + return -1; +} + +void +virDomainIOThreadsPinDel(virDomainDefPtr def, + int iothread_val) +{ + size_t i; + /* IOThreads share the virDomainVcpuPinDefPtr */ + virDomainVcpuPinDefPtr *iothreadspin_list = def->cputune.iothreadspin; + + for (i = 0; i < def->cputune.niothreadspin; i++) { + if (iothreadspin_list[i]->vcpuid == iothread_val) { + virBitmapFree(iothreadspin_list[i]->cpumask); + VIR_DELETE_ELEMENT(def->cputune.iothreadspin, i, + def->cputune.niothreadspin); + return; + } + } +} + static int virDomainEventActionDefFormat(virBufferPtr buf, int type, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 36bb418..17342a4 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2563,6 +2563,16 @@ int virDomainEmulatorPinAdd(virDomainDefPtr def, int virDomainEmulatorPinDel(virDomainDefPtr def); +/* IOThreads share the virDomainVcpuPinDefPtr */ +int virDomainIOThreadsPinAdd(virDomainVcpuPinDefPtr **iothreadspin_list, + size_t *niothreads, + unsigned char *cpumap, + int maplen, + int iothread_val); + +void virDomainIOThreadsPinDel(virDomainDefPtr def, + int iothread_val); + void virDomainRNGDefFree(virDomainRNGDefPtr def); bool virDomainDiskDefDstDuplicates(virDomainDefPtr def); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c810cf7..baacdc2 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -308,6 +308,8 @@ virDomainHubTypeToString; virDomainHypervTypeFromString; virDomainHypervTypeToString; virDomainInputDefFree; +virDomainIOThreadsPinAdd; +virDomainIOThreadsPinDel; virDomainLeaseDefFree; virDomainLeaseIndex; virDomainLeaseInsert; -- 2.1.0

Add qemuDomainGetIOThreadPin to handle getting the CPU Affinity for one specific IOThread Add qemuDomainPinIOThread to handle setting the cpuset for a specific IOThread Signed-off-by: John Ferlan <jferlan@redhat.com> --- include/libvirt/libvirt-domain.h | 9 ++ src/qemu/qemu_driver.c | 302 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 311 insertions(+) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index fc35cd2..e81389a 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -3223,6 +3223,15 @@ typedef void (*virConnectDomainEventDeviceRemovedCallback)(virConnectPtr conn, # define VIR_DOMAIN_TUNABLE_CPU_EMULATORPIN "cputune.emulatorpin" /** + * VIR_DOMAIN_TUNABLE_CPU_IOTHREADSPIN: + * + * Macro represents formatted pinning for one IOThread specified by id which is + * appended to the parameter name, for example "cputune.iothreadpin1", + * as VIR_TYPED_PARAM_STRING. + */ +# define VIR_DOMAIN_TUNABLE_CPU_IOTHREADSPIN "cputune.iothreadpin%u" + +/** * VIR_DOMAIN_TUNABLE_CPU_CPU_SHARES: * * Macro represents proportional weight of the scheduler used on the diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d8a761d..5c8040e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5780,6 +5780,306 @@ qemuDomainGetIOThreadsInfo(virDomainPtr dom, return ret; } +static int +qemuDomainGetIOThreadPin(virDomainPtr dom, + unsigned int iothread_id, + unsigned char *cpumap, + int maplen, + unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + virDomainObjPtr vm = NULL; + virDomainDefPtr targetDef = NULL; + int ret = -1; + virCapsPtr caps = NULL; + int maxcpu, hostcpus, pcpu; + virBitmapPtr cpumask = NULL; + bool pinned; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + + if (!(vm = qemuDomObjFromDomain(dom))) + goto cleanup; + + if (virDomainGetIOThreadPinEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + if (!(caps = virQEMUDriverGetCapabilities(driver, false))) + goto cleanup; + + if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt, + vm, &flags, &targetDef) < 0) + goto cleanup; + + if (flags & VIR_DOMAIN_AFFECT_LIVE) + targetDef = vm->def; + + /* Coverity didn't realize that targetDef must be set if we got here. */ + sa_assert(targetDef); + + if ((hostcpus = nodeGetCPUCount()) < 0) + goto cleanup; + + maxcpu = maplen * 8; + if (maxcpu > hostcpus) + maxcpu = hostcpus; + + /* initialize cpumap */ + memset(cpumap, 0xff, maplen); + if (maxcpu % 8) + cpumap[maplen - 1] &= (1 << maxcpu % 8) - 1; + + if (targetDef->cputune.iothreadspin) { + if (iothread_id > targetDef->iothreads) { + virReportError(VIR_ERR_INVALID_ARG, + _("iothread value out of range %d > %d"), + iothread_id, targetDef->iothreads); + goto cleanup; + } + + cpumask = targetDef->cputune.iothreadspin[iothread_id - 1]->cpumask; + } else if (targetDef->cpumask) { + cpumask = targetDef->cpumask; + } else { + ret = 0; + goto cleanup; + } + + for (pcpu = 0; pcpu < maxcpu; pcpu++) { + if (virBitmapGetBit(cpumask, pcpu, &pinned) < 0) + goto cleanup; + if (!pinned) + VIR_UNUSE_CPU(cpumap, pcpu); + } + ret = 1; + + cleanup: + qemuDomObjEndAPI(&vm); + virObjectUnref(caps); + return ret; +} + +static int +qemuDomainPinIOThread(virDomainPtr dom, + unsigned int iothread_id, + unsigned char *cpumap, + int maplen, + unsigned int flags) +{ + int ret = -1; + virQEMUDriverPtr driver = dom->conn->privateData; + virQEMUDriverConfigPtr cfg = NULL; + virDomainObjPtr vm; + virCapsPtr caps = NULL; + virDomainDefPtr persistentDef = NULL; + virBitmapPtr pcpumap = NULL; + bool doReset = false; + qemuDomainObjPrivatePtr priv; + virDomainVcpuPinDefPtr *newIOThreadsPin = NULL; + size_t newIOThreadsPinNum = 0; + virCgroupPtr cgroup_iothread = NULL; + virObjectEventPtr event = NULL; + char paramField[VIR_TYPED_PARAM_FIELD_LENGTH] = ""; + char *str = NULL; + virTypedParameterPtr eventParams = NULL; + int eventNparams = 0; + int eventMaxparams = 0; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + + cfg = virQEMUDriverGetConfig(driver); + + if (!(vm = qemuDomObjFromDomain(dom))) + goto cleanup; + priv = vm->privateData; + + if (virDomainPinIOThreadEnsureACL(dom->conn, vm->def, flags) < 0) + goto cleanup; + + if (!(caps = virQEMUDriverGetCapabilities(driver, false))) + goto cleanup; + + if (vm->def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("Changing affinity for IOThread dynamically is " + "not allowed when CPU placement is 'auto'")); + goto cleanup; + } + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + goto cleanup; + + if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt, vm, &flags, + &persistentDef) < 0) + goto endjob; + + if (flags & VIR_DOMAIN_AFFECT_LIVE) + persistentDef = vm->def; + + /* Coverity didn't realize that targetDef must be set if we got here. */ + sa_assert(persistentDef); + + if (!(pcpumap = virBitmapNewData(cpumap, maplen))) + goto endjob; + + if (virBitmapIsAllClear(pcpumap)) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("Empty iothread cpumap list for pinning")); + goto endjob; + } + + /* pinning to all physical cpus means resetting, + * so check if we can reset setting. + */ + if (virBitmapIsAllSet(pcpumap)) + doReset = true; + + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + + if (priv->iothreadpids == NULL) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("IOThread affinity is not supported")); + goto endjob; + } + + if (iothread_id > priv->niothreadpids) { + virReportError(VIR_ERR_INVALID_ARG, + _("iothread value out of range %d > %d"), + iothread_id, priv->niothreadpids); + goto endjob; + } + + if (vm->def->cputune.iothreadspin) { + /* The VcpuPinDefCopy works for IOThreads too */ + newIOThreadsPin = + virDomainVcpuPinDefCopy(vm->def->cputune.iothreadspin, + vm->def->cputune.niothreadspin); + if (!newIOThreadsPin) + goto endjob; + + newIOThreadsPinNum = vm->def->cputune.niothreadspin; + } else { + if (VIR_ALLOC(newIOThreadsPin) < 0) + goto endjob; + newIOThreadsPinNum = 0; + } + + if (virDomainIOThreadsPinAdd(&newIOThreadsPin, &newIOThreadsPinNum, + cpumap, maplen, iothread_id) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to update iothreadspin")); + virDomainVcpuPinDefArrayFree(newIOThreadsPin, newIOThreadsPinNum); + goto endjob; + } + + /* Configure the corresponding cpuset cgroup before set affinity. */ + if (virCgroupHasController(priv->cgroup, + VIR_CGROUP_CONTROLLER_CPUSET)) { + if (virCgroupNewIOThread(priv->cgroup, iothread_id, + false, &cgroup_iothread) < 0) + goto endjob; + if (qemuSetupCgroupIOThreadsPin(cgroup_iothread, + newIOThreadsPin, + newIOThreadsPinNum, + iothread_id) < 0) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("failed to set cpuset.cpus in cgroup" + " for iothread %d"), iothread_id); + goto endjob; + } + } else { + if (virProcessSetAffinity(priv->iothreadpids[iothread_id - 1], + pcpumap) < 0) { + virReportError(VIR_ERR_SYSTEM_ERROR, + _("failed to set cpu affinity for IOThread %d"), + iothread_id); + goto endjob; + } + } + + if (doReset) { + virDomainIOThreadsPinDel(vm->def, iothread_id); + } else { + if (vm->def->cputune.iothreadspin) + virDomainVcpuPinDefArrayFree(vm->def->cputune.iothreadspin, + vm->def->cputune.niothreadspin); + + vm->def->cputune.iothreadspin = newIOThreadsPin; + vm->def->cputune.niothreadspin = newIOThreadsPinNum; + newIOThreadsPin = NULL; + } + + if (newIOThreadsPin) + virDomainVcpuPinDefArrayFree(newIOThreadsPin, newIOThreadsPinNum); + + if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) + goto endjob; + + if (snprintf(paramField, VIR_TYPED_PARAM_FIELD_LENGTH, + VIR_DOMAIN_TUNABLE_CPU_IOTHREADSPIN, iothread_id) < 0) { + goto endjob; + } + + str = virBitmapFormat(pcpumap); + if (virTypedParamsAddString(&eventParams, &eventNparams, + &eventMaxparams, paramField, str) < 0) + goto endjob; + + event = virDomainEventTunableNewFromDom(dom, eventParams, eventNparams); + } + + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + if (iothread_id > persistentDef->iothreads) { + virReportError(VIR_ERR_INVALID_ARG, + _("iothread value out of range %d > %d"), + iothread_id, persistentDef->iothreads); + goto endjob; + } + + if (doReset) { + virDomainIOThreadsPinDel(persistentDef, iothread_id); + } else { + if (!persistentDef->cputune.iothreadspin) { + if (VIR_ALLOC(persistentDef->cputune.iothreadspin) < 0) + goto endjob; + persistentDef->cputune.niothreadspin = 0; + } + if (virDomainIOThreadsPinAdd(&persistentDef->cputune.iothreadspin, + &persistentDef->cputune.niothreadspin, + cpumap, + maplen, + iothread_id) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to update or add iothreadspin xml " + "of a persistent domain")); + goto endjob; + } + } + + ret = virDomainSaveConfig(cfg->configDir, persistentDef); + goto endjob; + } + + ret = 0; + + endjob: + qemuDomainObjEndJob(driver, vm); + + cleanup: + if (cgroup_iothread) + virCgroupFree(&cgroup_iothread); + if (event) + qemuDomainEventQueue(driver, event); + VIR_FREE(str); + virBitmapFree(pcpumap); + qemuDomObjEndAPI(&vm); + virObjectUnref(caps); + virObjectUnref(cfg); + return ret; +} + static int qemuDomainGetSecurityLabel(virDomainPtr dom, virSecurityLabelPtr seclabel) { virQEMUDriverPtr driver = dom->conn->privateData; @@ -19332,6 +19632,8 @@ static virHypervisorDriver qemuHypervisorDriver = { .domainGetVcpus = qemuDomainGetVcpus, /* 0.4.4 */ .domainGetMaxVcpus = qemuDomainGetMaxVcpus, /* 0.4.4 */ .domainGetIOThreadsInfo = qemuDomainGetIOThreadsInfo, /* 1.2.14 */ + .domainGetIOThreadPin = qemuDomainGetIOThreadPin, /* 1.2.14 */ + .domainPinIOThread = qemuDomainPinIOThread, /* 1.2.14 */ .domainGetSecurityLabel = qemuDomainGetSecurityLabel, /* 0.6.1 */ .domainGetSecurityLabelList = qemuDomainGetSecurityLabelList, /* 0.10.0 */ .nodeGetSecurityModel = qemuNodeGetSecurityModel, /* 0.6.1 */ -- 2.1.0

https://bugzilla.redhat.com/show_bug.cgi?id=1135491 $ virsh iothread --help NAME iothreadpin - control or query domain IOThread affinity SYNOPSIS iothreadpin <domain> <iothread> [--cpulist <string>] [--config] [--live] [--current] DESCRIPTION Pin domain IOThreads to host physical CPUs. OPTIONS [--domain] <string> domain name, id or uuid [--iothread] <number> IOThread ID number --cpulist <string> host cpu number(s) to set, or omit option to query --config affect next boot --live affect running domain --current affect current domain Unlike the iothreadsinfo, allow the get of CPU pin information for a single IOThread as well as adding support to change the pinned CPUs $ virsh iothreadpin $dom 3 IOThread ID CPU Affinity --------------------------------------------------- 3 0-1 $ virsh iothreadpin $dom 3 0-2 Then view the change $ virsh iothreadpin $dom 1 IOThread ID CPU Affinity --------------------------------------------------- 3 0-2 If an invalid value is supplied, an error will be displayed $ virsh iothreads f18iothr 4 3 error: invalid argument: iothread value out of range 4 > 3 $ Signed-off-by: John Ferlan <jferlan@redhat.com> --- tools/virsh-domain.c | 122 +++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 26 +++++++++++ 2 files changed, 148 insertions(+) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 0cce57b..dcd7a26 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6878,6 +6878,122 @@ cmdIOThreadsInfo(vshControl *ctl, const vshCmd *cmd) } /* + * "iothreadpin" command + */ +static const vshCmdInfo info_iothreadpin[] = { + {.name = "help", + .data = N_("control or query domain IOThread affinity") + }, + {.name = "desc", + .data = N_("Pin domain IOThreads to host physical CPUs.") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_iothreadpin[] = { + {.name = "domain", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("domain name, id or uuid") + }, + {.name = "iothread", + .type = VSH_OT_INT, + .flags = VSH_OFLAG_REQ, + .help = N_("IOThread ID number") + }, + {.name = "cpulist", + .type = VSH_OT_STRING, + .flags = VSH_OFLAG_EMPTY_OK, + .help = N_("host cpu number(s) to set, or omit option to query") + }, + {.name = "config", + .type = VSH_OT_BOOL, + .help = N_("affect next boot") + }, + {.name = "live", + .type = VSH_OT_BOOL, + .help = N_("affect running domain") + }, + {.name = "current", + .type = VSH_OT_BOOL, + .help = N_("affect current domain") + }, + {.name = NULL} +}; + +static bool +cmdIOThreadPin(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom; + const char *cpulist = NULL; + bool config = vshCommandOptBool(cmd, "config"); + bool live = vshCommandOptBool(cmd, "live"); + bool current = vshCommandOptBool(cmd, "current"); + unsigned int iothread_id = 0; + int maxcpu; + bool ret = false; + unsigned char *cpumap = NULL; + size_t cpumaplen; + unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; + + VSH_EXCLUSIVE_OPTIONS_VAR(current, live); + VSH_EXCLUSIVE_OPTIONS_VAR(current, config); + + if (config) + flags |= VIR_DOMAIN_AFFECT_CONFIG; + if (live) + flags |= VIR_DOMAIN_AFFECT_LIVE; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (vshCommandOptStringReq(ctl, cmd, "cpulist", &cpulist) < 0) + return false; + + if (!cpulist) + VSH_EXCLUSIVE_OPTIONS_VAR(live, config); + + if (vshCommandOptUInt(cmd, "iothread", &iothread_id) < 0) { + vshError(ctl, "%s", _("iothreadpin: Invalid IOThread number.")); + goto cleanup; + } + + if ((maxcpu = vshNodeGetCPUCount(ctl->conn)) < 0) + goto cleanup; + cpumaplen = VIR_CPU_MAPLEN(maxcpu); + + /* Query mode: show CPU affinity information then exit. */ + if (!cpulist) { + cpumap = vshMalloc(ctl, cpumaplen); + if (virDomainGetIOThreadPin(dom, iothread_id, + cpumap, cpumaplen, flags) < 0) + goto cleanup; + + vshPrintExtra(ctl, " %-15s %-15s\n", + _("IOThread ID"), _("CPU Affinity")); + vshPrintExtra(ctl, "---------------------------------------------------\n"); + vshPrint(ctl, " %-15u ", iothread_id); + ignore_value(vshPrintPinInfo(cpumap, cpumaplen, maxcpu, 0)); + vshPrint(ctl, "\n"); + } else { + /* Pin mode: pinning specified vcpu to specified physical cpus*/ + if (!(cpumap = vshParseCPUList(ctl, cpulist, maxcpu, cpumaplen))) + goto cleanup; + + if (virDomainPinIOThread(dom, iothread_id, + cpumap, cpumaplen, flags) != 0) + goto cleanup; + + } + ret = true; + + cleanup: + VIR_FREE(cpumap); + virDomainFree(dom); + return ret; +} + +/* * "cpu-compare" command */ static const vshCmdInfo info_cpu_compare[] = { @@ -12796,6 +12912,12 @@ const vshCmdDef domManagementCmds[] = { .info = info_iothreads, .flags = 0 }, + {.name = "iothreadpin", + .handler = cmdIOThreadPin, + .opts = opts_iothreadpin, + .info = info_iothreadpin, + .flags = 0 + }, {.name = "send-key", .handler = cmdSendKey, .opts = opts_send_key, diff --git a/tools/virsh.pod b/tools/virsh.pod index f19601e..9289895 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1373,6 +1373,32 @@ a persistent guest. If I<--current> is specified or I<--live> and I<--config> are not specified, then get the IOThread data based on the current guest state. +=item B<iothreadpin> I<domain> I<iothread> [I<cpulist>] +[[I<--live>] [I<--config>] | [I<--current>]] + +Query or change the pinning of a domain IOThread to host physical CPUs. To +pin an I<iothread>, specify I<cpulist>; otherwise, specify just the +I<iothread> to query a specific IOThread. In order to retrieve a list +of all IOThreads, use B<iothreadsinfo>. + +I<cpulist> is a list of physical CPU numbers. Its syntax is a comma +separated list and a special markup using '-' and '^' (ex. '0-4', '0-3,^2') can +also be allowed. The '-' denotes the range and the '^' denotes exclusive. +If you want to reset iothreadpin setting, that is, to pin an I<iothread> +to all physical cpus, simply specify 'r' as a cpulist. + +If I<--live> is specified, affect a running guest. If the guest is not running, +an error is returned. +If I<--config> is specified, affect the next boot of a persistent guest. +If I<--current> is specified or I<--live> and I<--config> are not specified, +affect the current guest state. +Both I<--live> and I<--config> flags may be given if I<cpulist> is present, +but I<--current> is exclusive. +If no flag is specified, behavior is different depending on hypervisor. + +B<Note>: The expression is sequentially evaluated, so "0-15,^8" is +identical to "9-14,0-7,15" but not identical to "^8,0-15". + =item B<managedsave> I<domain> [I<--bypass-cache>] [{I<--running> | I<--paused>}] [I<--verbose>] -- 2.1.0
participants (3)
-
Daniel P. Berrange
-
John Ferlan
-
Ján Tomko