[libvirt] [PATCH 0/7] iothread api followups
Looking at the proposed SetIOThread API, I noticed that the virsh command for printing the info is named 'iothreadsinfo'. This seemed counter-intuitive for me. Since the API has not been released yet, I propose a rename of the command to: iothreadinfo (patch 5) and the API for freeing one struct to: virDomainIOThreadInfoFree (patch 1) I don't feel as strongly about renaming the virDomainGetIOThreadsInfo API (patches 3, 4) or the internal APIs (patch 2). Looking at virVcpuInfoPtr (which might not be the best inspiration), I realized including the cpu time might be a good idea. Ján Tomko (7): Rename virDomainIOThreadsInfoFree to virDomainIOThreadInfoFree Rename qemuMonitorIOThreadsInfo* to qemuMonitorIOThreadInfo* Rename DomainGetIOThreadsInfo to DomainGetIOThreadInfo gendispatch: remove IOThreads from name fixups virsh: rename iothreadsinfo to iothreadinfo Do not use vshPrintPinInfo in iothreadinfo add cpu time to iothreadinfo daemon/remote.c | 15 +++++++------ include/libvirt/libvirt-domain.h | 5 +++-- src/driver-hypervisor.h | 4 ++-- src/libvirt-domain.c | 20 ++++++++--------- src/libvirt_public.syms | 4 ++-- src/qemu/qemu_driver.c | 24 ++++++++++++++------ src/qemu/qemu_monitor.c | 4 ++-- src/qemu/qemu_monitor.h | 10 ++++----- src/qemu/qemu_monitor_json.c | 8 +++---- src/qemu/qemu_monitor_json.h | 2 +- src/qemu/qemu_process.c | 4 ++-- src/remote/remote_driver.c | 23 ++++++++++---------- src/remote/remote_protocol.x | 11 +++++----- src/remote_protocol-structs | 7 +++--- src/rpc/gendispatch.pl | 1 - tests/qemumonitorjsontest.c | 4 ++-- tools/virsh-domain.c | 47 ++++++++++++++++++++++++++++------------ 17 files changed, 113 insertions(+), 80 deletions(-) -- 2.0.5
This function only frees the info for one thread. --- daemon/remote.c | 2 +- include/libvirt/libvirt-domain.h | 2 +- src/libvirt-domain.c | 6 +++--- src/libvirt_public.syms | 2 +- src/qemu/qemu_driver.c | 4 ++-- src/remote/remote_driver.c | 2 +- tools/virsh-domain.c | 2 +- 7 files changed, 10 insertions(+), 10 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index ea7ae94..be0febb 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -2334,7 +2334,7 @@ remoteDispatchDomainGetIOThreadsInfo(virNetServerPtr server ATTRIBUTE_UNUSED, virObjectUnref(dom); if (ninfo >= 0) for (i = 0; i < ninfo; i++) - virDomainIOThreadsInfoFree(info[i]); + virDomainIOThreadInfoFree(info[i]); VIR_FREE(info); return rv; diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 5d1d868..94b55b7 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1605,7 +1605,7 @@ struct _virDomainIOThreadInfo { int cpumaplen; /* cpumap size */ }; -void virDomainIOThreadsInfoFree(virDomainIOThreadInfoPtr info); +void virDomainIOThreadInfoFree(virDomainIOThreadInfoPtr info); int virDomainGetIOThreadsInfo(virDomainPtr domain, virDomainIOThreadInfoPtr **info, diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 0bd9274..73ab56d 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -7906,7 +7906,7 @@ virDomainGetMaxVcpus(virDomainPtr domain) * * 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, + * responsible for calling virDomainIOThreadInfoFree() on each array element, * then calling free() on @info. On error, @info is set to NULL. */ int @@ -7949,13 +7949,13 @@ virDomainGetIOThreadsInfo(virDomainPtr dom, /** - * virDomainIOThreadsInfoFree: + * virDomainIOThreadInfoFree: * @info: pointer to a virDomainIOThreadInfo object * * Frees the memory used by @info. */ void -virDomainIOThreadsInfoFree(virDomainIOThreadInfoPtr info) +virDomainIOThreadInfoFree(virDomainIOThreadInfoPtr info) { if (!info) return; diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index e4cf7ed..7163969 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -697,7 +697,7 @@ LIBVIRT_1.2.12 { LIBVIRT_1.2.14 { global: - virDomainIOThreadsInfoFree; + virDomainIOThreadInfoFree; virDomainGetIOThreadsInfo; virDomainPinIOThread; virDomainInterfaceAddresses; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2c55fb0..f5c234e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5788,7 +5788,7 @@ qemuDomainGetIOThreadsLive(virQEMUDriverPtr driver, cleanup: if (info_ret) { for (i = 0; i < niothreads; i++) - virDomainIOThreadsInfoFree(info_ret[i]); + virDomainIOThreadInfoFree(info_ret[i]); VIR_FREE(info_ret); } if (iothreads) { @@ -5859,7 +5859,7 @@ qemuDomainGetIOThreadsConfig(virDomainDefPtr targetDef, cleanup: if (info_ret) { for (i = 0; i < targetDef->iothreads; i++) - virDomainIOThreadsInfoFree(info_ret[i]); + virDomainIOThreadInfoFree(info_ret[i]); VIR_FREE(info_ret); } virBitmapFree(bitmap); diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index e69f235..7172ada 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2382,7 +2382,7 @@ remoteDomainGetIOThreadsInfo(virDomainPtr dom, cleanup: if (info_ret) { for (i = 0; i < ret.info.info_len; i++) - virDomainIOThreadsInfoFree(info_ret[i]); + virDomainIOThreadInfoFree(info_ret[i]); VIR_FREE(info_ret); } xdr_free((xdrproc_t)xdr_remote_domain_get_iothreads_info_ret, diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 1d8225c..0713b93 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6868,7 +6868,7 @@ cmdIOThreadsInfo(vshControl *ctl, const vshCmd *cmd) ignore_value(vshPrintPinInfo(info[i]->cpumap, info[i]->cpumaplen, maxcpu, 0)); vshPrint(ctl, "\n"); - virDomainIOThreadsInfoFree(info[i]); + virDomainIOThreadInfoFree(info[i]); } VIR_FREE(info); -- 2.0.5
On Wed, Mar 25, 2015 at 19:39:06 +0100, Ján Tomko wrote:
This function only frees the info for one thread. --- daemon/remote.c | 2 +- include/libvirt/libvirt-domain.h | 2 +- src/libvirt-domain.c | 6 +++--- src/libvirt_public.syms | 2 +- src/qemu/qemu_driver.c | 4 ++-- src/remote/remote_driver.c | 2 +- tools/virsh-domain.c | 2 +- 7 files changed, 10 insertions(+), 10 deletions(-)
ACK, Peter
It only deals with a single thread. --- src/qemu/qemu_driver.c | 4 ++-- src/qemu/qemu_monitor.c | 4 ++-- src/qemu/qemu_monitor.h | 10 +++++----- src/qemu/qemu_monitor_json.c | 8 ++++---- src/qemu/qemu_monitor_json.h | 2 +- src/qemu/qemu_process.c | 4 ++-- tests/qemumonitorjsontest.c | 4 ++-- 7 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f5c234e..b2597ce 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5715,7 +5715,7 @@ qemuDomainGetIOThreadsLive(virQEMUDriverPtr driver, virDomainIOThreadInfoPtr **info) { qemuDomainObjPrivatePtr priv; - qemuMonitorIOThreadsInfoPtr *iothreads = NULL; + qemuMonitorIOThreadInfoPtr *iothreads = NULL; virDomainIOThreadInfoPtr *info_ret = NULL; int niothreads = 0; int hostcpus; @@ -5793,7 +5793,7 @@ qemuDomainGetIOThreadsLive(virQEMUDriverPtr driver, } if (iothreads) { for (i = 0; i < niothreads; i++) - qemuMonitorIOThreadsInfoFree(iothreads[i]); + qemuMonitorIOThreadInfoFree(iothreads[i]); VIR_FREE(iothreads); } diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index a388945..5da0b8f 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4336,7 +4336,7 @@ qemuMonitorRTCResetReinjection(qemuMonitorPtr mon) */ int qemuMonitorGetIOThreads(qemuMonitorPtr mon, - qemuMonitorIOThreadsInfoPtr **iothreads) + qemuMonitorIOThreadInfoPtr **iothreads) { VIR_DEBUG("mon=%p iothreads=%p", mon, iothreads); @@ -4356,7 +4356,7 @@ qemuMonitorGetIOThreads(qemuMonitorPtr mon, return qemuMonitorJSONGetIOThreads(mon, iothreads); } -void qemuMonitorIOThreadsInfoFree(qemuMonitorIOThreadsInfoPtr iothread) +void qemuMonitorIOThreadInfoFree(qemuMonitorIOThreadInfoPtr iothread) { if (!iothread) return; diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index a663978..c7c3dab 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -872,17 +872,17 @@ int qemuMonitorGetGuestCPU(qemuMonitorPtr mon, int qemuMonitorRTCResetReinjection(qemuMonitorPtr mon); -typedef struct _qemuMonitorIOThreadsInfo qemuMonitorIOThreadsInfo; -typedef qemuMonitorIOThreadsInfo *qemuMonitorIOThreadsInfoPtr; +typedef struct _qemuMonitorIOThreadInfo qemuMonitorIOThreadInfo; +typedef qemuMonitorIOThreadInfo *qemuMonitorIOThreadInfoPtr; -struct _qemuMonitorIOThreadsInfo { +struct _qemuMonitorIOThreadInfo { char *name; int thread_id; }; int qemuMonitorGetIOThreads(qemuMonitorPtr mon, - qemuMonitorIOThreadsInfoPtr **iothreads); + qemuMonitorIOThreadInfoPtr **iothreads); -void qemuMonitorIOThreadsInfoFree(qemuMonitorIOThreadsInfoPtr iothread); +void qemuMonitorIOThreadInfoFree(qemuMonitorIOThreadInfoPtr iothread); typedef struct _qemuMonitorMemoryDeviceInfo qemuMonitorMemoryDeviceInfo; typedef qemuMonitorMemoryDeviceInfo *qemuMonitorMemoryDeviceInfoPtr; diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 67ce042..aa844ca 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -6411,13 +6411,13 @@ qemuMonitorJSONRTCResetReinjection(qemuMonitorPtr mon) */ int qemuMonitorJSONGetIOThreads(qemuMonitorPtr mon, - qemuMonitorIOThreadsInfoPtr **iothreads) + qemuMonitorIOThreadInfoPtr **iothreads) { int ret = -1; virJSONValuePtr cmd; virJSONValuePtr reply = NULL; virJSONValuePtr data; - qemuMonitorIOThreadsInfoPtr *infolist = NULL; + qemuMonitorIOThreadInfoPtr *infolist = NULL; int n = 0; size_t i; @@ -6455,7 +6455,7 @@ qemuMonitorJSONGetIOThreads(qemuMonitorPtr mon, for (i = 0; i < n; i++) { virJSONValuePtr child = virJSONValueArrayGet(data, i); const char *tmp; - qemuMonitorIOThreadsInfoPtr info; + qemuMonitorIOThreadInfoPtr info; if (VIR_ALLOC(info) < 0) goto cleanup; @@ -6486,7 +6486,7 @@ qemuMonitorJSONGetIOThreads(qemuMonitorPtr mon, cleanup: if (ret < 0 && infolist) { for (i = 0; i < n; i++) - qemuMonitorIOThreadsInfoFree(infolist[i]); + qemuMonitorIOThreadInfoFree(infolist[i]); VIR_FREE(infolist); } virJSONValueFree(cmd); diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 143ed03..1be3b98 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -467,7 +467,7 @@ int qemuMonitorJSONGetGuestCPU(qemuMonitorPtr mon, int qemuMonitorJSONRTCResetReinjection(qemuMonitorPtr mon); int qemuMonitorJSONGetIOThreads(qemuMonitorPtr mon, - qemuMonitorIOThreadsInfoPtr **iothreads) + qemuMonitorIOThreadInfoPtr **iothreads) ATTRIBUTE_NONNULL(2); int qemuMonitorJSONGetMemoryDeviceInfo(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 59eae1c..79f763e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2224,7 +2224,7 @@ qemuProcessDetectIOThreadPIDs(virQEMUDriverPtr driver, int asyncJob) { qemuDomainObjPrivatePtr priv = vm->privateData; - qemuMonitorIOThreadsInfoPtr *iothreads = NULL; + qemuMonitorIOThreadInfoPtr *iothreads = NULL; int niothreads = 0; int ret = -1; size_t i; @@ -2267,7 +2267,7 @@ qemuProcessDetectIOThreadPIDs(virQEMUDriverPtr driver, cleanup: if (iothreads) { for (i = 0; i < niothreads; i++) - qemuMonitorIOThreadsInfoFree(iothreads[i]); + qemuMonitorIOThreadInfoFree(iothreads[i]); VIR_FREE(iothreads); } return ret; diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 1380df1..f09176a 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -2241,7 +2241,7 @@ testQemuMonitorJSONGetIOThreads(const void *data) { virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data; qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt); - qemuMonitorIOThreadsInfoPtr *info; + qemuMonitorIOThreadInfoPtr *info; int ninfo = 0; int ret = -1; size_t i; @@ -2300,7 +2300,7 @@ testQemuMonitorJSONGetIOThreads(const void *data) cleanup: qemuMonitorTestFree(test); for (i = 0; i < ninfo; i++) - qemuMonitorIOThreadsInfoFree(info[i]); + qemuMonitorIOThreadInfoFree(info[i]); VIR_FREE(info); return ret; -- 2.0.5
On Wed, Mar 25, 2015 at 19:39:07 +0100, Ján Tomko wrote:
It only deals with a single thread. --- src/qemu/qemu_driver.c | 4 ++-- src/qemu/qemu_monitor.c | 4 ++-- src/qemu/qemu_monitor.h | 10 +++++----- src/qemu/qemu_monitor_json.c | 8 ++++---- src/qemu/qemu_monitor_json.h | 2 +- src/qemu/qemu_process.c | 4 ++-- tests/qemumonitorjsontest.c | 4 ++-- 7 files changed, 18 insertions(+), 18 deletions(-)
ACK, Peter
While it returns info about multiple threads, the version without the plural is easier to read. --- daemon/remote.c | 12 ++++++------ include/libvirt/libvirt-domain.h | 2 +- src/driver-hypervisor.h | 4 ++-- src/libvirt-domain.c | 14 +++++++------- src/libvirt_public.syms | 2 +- src/qemu/qemu_driver.c | 6 +++--- src/remote/remote_driver.c | 20 ++++++++++---------- src/remote/remote_protocol.x | 10 +++++----- src/remote_protocol-structs | 6 +++--- tools/virsh-domain.c | 2 +- 10 files changed, 39 insertions(+), 39 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index be0febb..2f4df48 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -2269,12 +2269,12 @@ remoteDispatchDomainGetVcpus(virNetServerPtr server ATTRIBUTE_UNUSED, } static int -remoteDispatchDomainGetIOThreadsInfo(virNetServerPtr server ATTRIBUTE_UNUSED, +remoteDispatchDomainGetIOThreadInfo(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) + remote_domain_get_iothread_info_args *args, + remote_domain_get_iothread_info_ret *ret) { int rv = -1; size_t i; @@ -2292,13 +2292,13 @@ remoteDispatchDomainGetIOThreadsInfo(virNetServerPtr server ATTRIBUTE_UNUSED, if (!(dom = get_nonnull_domain(priv->conn, args->dom))) goto cleanup; - if ((ninfo = virDomainGetIOThreadsInfo(dom, &info, args->flags)) < 0) + if ((ninfo = virDomainGetIOThreadInfo(dom, &info, args->flags)) < 0) goto cleanup; - if (ninfo > REMOTE_IOTHREADS_INFO_MAX) { + if (ninfo > REMOTE_IOTHREAD_INFO_MAX) { virReportError(VIR_ERR_RPC, _("Too many IOThreads in info: %d for limit %d"), - ninfo, REMOTE_IOTHREADS_INFO_MAX); + ninfo, REMOTE_IOTHREAD_INFO_MAX); goto cleanup; } diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 94b55b7..7be4219 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1607,7 +1607,7 @@ struct _virDomainIOThreadInfo { void virDomainIOThreadInfoFree(virDomainIOThreadInfoPtr info); -int virDomainGetIOThreadsInfo(virDomainPtr domain, +int virDomainGetIOThreadInfo(virDomainPtr domain, virDomainIOThreadInfoPtr **info, unsigned int flags); int virDomainPinIOThread(virDomainPtr domain, diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index 3f9bf02..9df593d 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -381,7 +381,7 @@ typedef int (*virDrvDomainGetMaxVcpus)(virDomainPtr domain); typedef int -(*virDrvDomainGetIOThreadsInfo)(virDomainPtr domain, +(*virDrvDomainGetIOThreadInfo)(virDomainPtr domain, virDomainIOThreadInfoPtr **info, unsigned int flags); @@ -1271,7 +1271,7 @@ struct _virHypervisorDriver { virDrvDomainGetEmulatorPinInfo domainGetEmulatorPinInfo; virDrvDomainGetVcpus domainGetVcpus; virDrvDomainGetMaxVcpus domainGetMaxVcpus; - virDrvDomainGetIOThreadsInfo domainGetIOThreadsInfo; + virDrvDomainGetIOThreadInfo domainGetIOThreadInfo; virDrvDomainPinIOThread domainPinIOThread; virDrvDomainGetSecurityLabel domainGetSecurityLabel; virDrvDomainGetSecurityLabelList domainGetSecurityLabelList; diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 73ab56d..f1608dc 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -7894,7 +7894,7 @@ virDomainGetMaxVcpus(virDomainPtr domain) /** - * virDomainGetIOThreadsInfo: + * virDomainGetIOThreadInfo: * @dom: a domain object * @info: pointer to an array of virDomainIOThreadInfo structures (OUT) * @flags: bitwise-OR of virDomainModificationImpact @@ -7910,9 +7910,9 @@ virDomainGetMaxVcpus(virDomainPtr domain) * then calling free() on @info. On error, @info is set to NULL. */ int -virDomainGetIOThreadsInfo(virDomainPtr dom, - virDomainIOThreadInfoPtr **info, - unsigned int flags) +virDomainGetIOThreadInfo(virDomainPtr dom, + virDomainIOThreadInfoPtr **info, + unsigned int flags) { VIR_DOMAIN_DEBUG(dom, "info=%p flags=%x", info, flags); @@ -7932,9 +7932,9 @@ virDomainGetIOThreadsInfo(virDomainPtr dom, goto error; } - if (dom->conn->driver->domainGetIOThreadsInfo) { + if (dom->conn->driver->domainGetIOThreadInfo) { int ret; - ret = dom->conn->driver->domainGetIOThreadsInfo(dom, info, flags); + ret = dom->conn->driver->domainGetIOThreadInfo(dom, info, flags); if (ret < 0) goto error; return ret; @@ -7993,7 +7993,7 @@ virDomainIOThreadInfoFree(virDomainIOThreadInfoPtr info) * 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. + * See also virDomainGetIOThreadInfo for querying this information. * * Returns 0 in case of success, -1 in case of failure. */ diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 7163969..28347c6 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -698,7 +698,7 @@ LIBVIRT_1.2.12 { LIBVIRT_1.2.14 { global: virDomainIOThreadInfoFree; - virDomainGetIOThreadsInfo; + virDomainGetIOThreadInfo; virDomainPinIOThread; virDomainInterfaceAddresses; virDomainInterfaceFree; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b2597ce..bef1223 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5868,7 +5868,7 @@ qemuDomainGetIOThreadsConfig(virDomainDefPtr targetDef, } static int -qemuDomainGetIOThreadsInfo(virDomainPtr dom, +qemuDomainGetIOThreadInfo(virDomainPtr dom, virDomainIOThreadInfoPtr **info, unsigned int flags) { @@ -5884,7 +5884,7 @@ qemuDomainGetIOThreadsInfo(virDomainPtr dom, if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - if (virDomainGetIOThreadsInfoEnsureACL(dom->conn, vm->def) < 0) + if (virDomainGetIOThreadInfoEnsureACL(dom->conn, vm->def) < 0) goto cleanup; if (!(caps = virQEMUDriverGetCapabilities(driver, false))) @@ -19824,7 +19824,7 @@ static virHypervisorDriver qemuHypervisorDriver = { .domainGetEmulatorPinInfo = qemuDomainGetEmulatorPinInfo, /* 0.10.0 */ .domainGetVcpus = qemuDomainGetVcpus, /* 0.4.4 */ .domainGetMaxVcpus = qemuDomainGetMaxVcpus, /* 0.4.4 */ - .domainGetIOThreadsInfo = qemuDomainGetIOThreadsInfo, /* 1.2.14 */ + .domainGetIOThreadInfo = qemuDomainGetIOThreadInfo, /* 1.2.14 */ .domainPinIOThread = qemuDomainPinIOThread, /* 1.2.14 */ .domainGetSecurityLabel = qemuDomainGetSecurityLabel, /* 0.6.1 */ .domainGetSecurityLabelList = qemuDomainGetSecurityLabelList, /* 0.10.0 */ diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 7172ada..8bd54e6 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2316,15 +2316,15 @@ remoteDomainGetVcpus(virDomainPtr domain, } static int -remoteDomainGetIOThreadsInfo(virDomainPtr dom, +remoteDomainGetIOThreadInfo(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_get_iothread_info_args args; + remote_domain_get_iothread_info_ret ret; remote_domain_iothread_info *src; virDomainIOThreadInfoPtr *info_ret = NULL; @@ -2336,17 +2336,17 @@ remoteDomainGetIOThreadsInfo(virDomainPtr dom, 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, + if (call(dom->conn, priv, 0, REMOTE_PROC_DOMAIN_GET_IOTHREAD_INFO, + (xdrproc_t)xdr_remote_domain_get_iothread_info_args, (char *)&args, - (xdrproc_t)xdr_remote_domain_get_iothreads_info_ret, + (xdrproc_t)xdr_remote_domain_get_iothread_info_ret, (char *)&ret) == -1) goto done; - if (ret.info.info_len > REMOTE_IOTHREADS_INFO_MAX) { + if (ret.info.info_len > REMOTE_IOTHREAD_INFO_MAX) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Too many IOThreads in info: %d for limit %d"), - ret.info.info_len, REMOTE_IOTHREADS_INFO_MAX); + ret.info.info_len, REMOTE_IOTHREAD_INFO_MAX); goto cleanup; } @@ -2385,7 +2385,7 @@ remoteDomainGetIOThreadsInfo(virDomainPtr dom, virDomainIOThreadInfoFree(info_ret[i]); VIR_FREE(info_ret); } - xdr_free((xdrproc_t)xdr_remote_domain_get_iothreads_info_ret, + xdr_free((xdrproc_t)xdr_remote_domain_get_iothread_info_ret, (char *) &ret); done: @@ -8208,7 +8208,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 */ + .domainGetIOThreadInfo = remoteDomainGetIOThreadInfo, /* 1.2.14 */ .domainPinIOThread = remoteDomainPinIOThread, /* 1.2.14 */ .domainGetSecurityLabel = remoteDomainGetSecurityLabel, /* 0.6.1 */ .domainGetSecurityLabelList = remoteDomainGetSecurityLabelList, /* 0.10.0 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index eb862e1..d90e6b5 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -86,7 +86,7 @@ const REMOTE_VCPUINFO_MAX = 16384; const REMOTE_CPUMAPS_MAX = 8388608; /* Upper limit on number of info fields returned by virDomainGetIOThreads. */ -const REMOTE_IOTHREADS_INFO_MAX = 16384; +const REMOTE_IOTHREAD_INFO_MAX = 16384; /* Upper limit on migrate cookie. */ const REMOTE_MIGRATE_COOKIE_MAX = 4194304; @@ -1195,13 +1195,13 @@ struct remote_domain_iothread_info { opaque cpumap<REMOTE_CPUMAP_MAX>; }; -struct remote_domain_get_iothreads_info_args { +struct remote_domain_get_iothread_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>; +struct remote_domain_get_iothread_info_ret { + remote_domain_iothread_info info<REMOTE_IOTHREAD_INFO_MAX>; unsigned int ret; }; @@ -5629,7 +5629,7 @@ enum remote_procedure { * @generate: none * @acl: domain:read */ - REMOTE_PROC_DOMAIN_GET_IOTHREADS_INFO = 351, + REMOTE_PROC_DOMAIN_GET_IOTHREAD_INFO = 351, /** * @generate: both diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index b3e2e40..e614f77 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -814,11 +814,11 @@ struct remote_domain_iothread_info { char * cpumap_val; } cpumap; }; -struct remote_domain_get_iothreads_info_args { +struct remote_domain_get_iothread_info_args { remote_nonnull_domain dom; u_int flags; }; -struct remote_domain_get_iothreads_info_ret { +struct remote_domain_get_iothread_info_ret { struct { u_int info_len; remote_domain_iothread_info * info_val; @@ -3014,7 +3014,7 @@ 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, + REMOTE_PROC_DOMAIN_GET_IOTHREAD_INFO = 351, REMOTE_PROC_DOMAIN_PIN_IOTHREAD = 352, REMOTE_PROC_DOMAIN_INTERFACE_ADDRESSES = 353, }; diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 0713b93..c8e0233 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6849,7 +6849,7 @@ cmdIOThreadsInfo(vshControl *ctl, const vshCmd *cmd) if ((maxcpu = vshNodeGetCPUCount(ctl->conn)) < 0) goto cleanup; - if ((niothreads = virDomainGetIOThreadsInfo(dom, &info, flags)) < 0) { + if ((niothreads = virDomainGetIOThreadInfo(dom, &info, flags)) < 0) { vshError(ctl, _("Unable to get domain IOThreads information")); goto cleanup; } -- 2.0.5
On Wed, Mar 25, 2015 at 19:39:08 +0100, Ján Tomko wrote:
While it returns info about multiple threads, the version without the plural is easier to read. --- daemon/remote.c | 12 ++++++------ include/libvirt/libvirt-domain.h | 2 +- src/driver-hypervisor.h | 4 ++-- src/libvirt-domain.c | 14 +++++++------- src/libvirt_public.syms | 2 +- src/qemu/qemu_driver.c | 6 +++--- src/remote/remote_driver.c | 20 ++++++++++---------- src/remote/remote_protocol.x | 10 +++++----- src/remote_protocol-structs | 6 +++--- tools/virsh-domain.c | 2 +- 10 files changed, 39 insertions(+), 39 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c index be0febb..2f4df48 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -2269,12 +2269,12 @@ remoteDispatchDomainGetVcpus(virNetServerPtr server ATTRIBUTE_UNUSED, }
static int -remoteDispatchDomainGetIOThreadsInfo(virNetServerPtr server ATTRIBUTE_UNUSED, +remoteDispatchDomainGetIOThreadInfo(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) + remote_domain_get_iothread_info_args *args, + remote_domain_get_iothread_info_ret *ret)
The whole block is misaligned now.
{ int rv = -1; size_t i; @@ -2292,13 +2292,13 @@ remoteDispatchDomainGetIOThreadsInfo(virNetServerPtr server ATTRIBUTE_UNUSED, if (!(dom = get_nonnull_domain(priv->conn, args->dom))) goto cleanup;
- if ((ninfo = virDomainGetIOThreadsInfo(dom, &info, args->flags)) < 0) + if ((ninfo = virDomainGetIOThreadInfo(dom, &info, args->flags)) < 0) goto cleanup;
- if (ninfo > REMOTE_IOTHREADS_INFO_MAX) { + if (ninfo > REMOTE_IOTHREAD_INFO_MAX) { virReportError(VIR_ERR_RPC, _("Too many IOThreads in info: %d for limit %d"), - ninfo, REMOTE_IOTHREADS_INFO_MAX); + ninfo, REMOTE_IOTHREAD_INFO_MAX); goto cleanup; }
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 94b55b7..7be4219 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1607,7 +1607,7 @@ struct _virDomainIOThreadInfo {
void virDomainIOThreadInfoFree(virDomainIOThreadInfoPtr info);
-int virDomainGetIOThreadsInfo(virDomainPtr domain, +int virDomainGetIOThreadInfo(virDomainPtr domain, virDomainIOThreadInfoPtr **info, unsigned int flags);
Misaligned ...
int virDomainPinIOThread(virDomainPtr domain, diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index 3f9bf02..9df593d 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -381,7 +381,7 @@ typedef int (*virDrvDomainGetMaxVcpus)(virDomainPtr domain);
typedef int -(*virDrvDomainGetIOThreadsInfo)(virDomainPtr domain, +(*virDrvDomainGetIOThreadInfo)(virDomainPtr domain, virDomainIOThreadInfoPtr **info, unsigned int flags);
Misaligned ...
@@ -1271,7 +1271,7 @@ struct _virHypervisorDriver { virDrvDomainGetEmulatorPinInfo domainGetEmulatorPinInfo; virDrvDomainGetVcpus domainGetVcpus; virDrvDomainGetMaxVcpus domainGetMaxVcpus; - virDrvDomainGetIOThreadsInfo domainGetIOThreadsInfo; + virDrvDomainGetIOThreadInfo domainGetIOThreadInfo; virDrvDomainPinIOThread domainPinIOThread; virDrvDomainGetSecurityLabel domainGetSecurityLabel; virDrvDomainGetSecurityLabelList domainGetSecurityLabelList; diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 73ab56d..f1608dc 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -7894,7 +7894,7 @@ virDomainGetMaxVcpus(virDomainPtr domain)
/** - * virDomainGetIOThreadsInfo: + * virDomainGetIOThreadInfo: * @dom: a domain object * @info: pointer to an array of virDomainIOThreadInfo structures (OUT) * @flags: bitwise-OR of virDomainModificationImpact @@ -7910,9 +7910,9 @@ virDomainGetMaxVcpus(virDomainPtr domain) * then calling free() on @info. On error, @info is set to NULL. */ int -virDomainGetIOThreadsInfo(virDomainPtr dom, - virDomainIOThreadInfoPtr **info, - unsigned int flags) +virDomainGetIOThreadInfo(virDomainPtr dom, + virDomainIOThreadInfoPtr **info, + unsigned int flags) { VIR_DOMAIN_DEBUG(dom, "info=%p flags=%x", info, flags);
@@ -7932,9 +7932,9 @@ virDomainGetIOThreadsInfo(virDomainPtr dom, goto error; }
- if (dom->conn->driver->domainGetIOThreadsInfo) { + if (dom->conn->driver->domainGetIOThreadInfo) { int ret; - ret = dom->conn->driver->domainGetIOThreadsInfo(dom, info, flags); + ret = dom->conn->driver->domainGetIOThreadInfo(dom, info, flags); if (ret < 0) goto error; return ret; @@ -7993,7 +7993,7 @@ virDomainIOThreadInfoFree(virDomainIOThreadInfoPtr info) * 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. + * See also virDomainGetIOThreadInfo for querying this information. * * Returns 0 in case of success, -1 in case of failure. */ diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 7163969..28347c6 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -698,7 +698,7 @@ LIBVIRT_1.2.12 { LIBVIRT_1.2.14 { global: virDomainIOThreadInfoFree; - virDomainGetIOThreadsInfo; + virDomainGetIOThreadInfo; virDomainPinIOThread; virDomainInterfaceAddresses; virDomainInterfaceFree; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b2597ce..bef1223 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5868,7 +5868,7 @@ qemuDomainGetIOThreadsConfig(virDomainDefPtr targetDef, }
static int -qemuDomainGetIOThreadsInfo(virDomainPtr dom, +qemuDomainGetIOThreadInfo(virDomainPtr dom, virDomainIOThreadInfoPtr **info, unsigned int flags)
Misaligned.
{ @@ -5884,7 +5884,7 @@ qemuDomainGetIOThreadsInfo(virDomainPtr dom, if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup;
- if (virDomainGetIOThreadsInfoEnsureACL(dom->conn, vm->def) < 0) + if (virDomainGetIOThreadInfoEnsureACL(dom->conn, vm->def) < 0) goto cleanup;
if (!(caps = virQEMUDriverGetCapabilities(driver, false))) @@ -19824,7 +19824,7 @@ static virHypervisorDriver qemuHypervisorDriver = { .domainGetEmulatorPinInfo = qemuDomainGetEmulatorPinInfo, /* 0.10.0 */ .domainGetVcpus = qemuDomainGetVcpus, /* 0.4.4 */ .domainGetMaxVcpus = qemuDomainGetMaxVcpus, /* 0.4.4 */ - .domainGetIOThreadsInfo = qemuDomainGetIOThreadsInfo, /* 1.2.14 */ + .domainGetIOThreadInfo = qemuDomainGetIOThreadInfo, /* 1.2.14 */ .domainPinIOThread = qemuDomainPinIOThread, /* 1.2.14 */ .domainGetSecurityLabel = qemuDomainGetSecurityLabel, /* 0.6.1 */ .domainGetSecurityLabelList = qemuDomainGetSecurityLabelList, /* 0.10.0 */ diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 7172ada..8bd54e6 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2316,15 +2316,15 @@ remoteDomainGetVcpus(virDomainPtr domain, }
static int -remoteDomainGetIOThreadsInfo(virDomainPtr dom, +remoteDomainGetIOThreadInfo(virDomainPtr dom, virDomainIOThreadInfoPtr **info, unsigned int flags)
Misaligned.
{ 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_get_iothread_info_args args; + remote_domain_get_iothread_info_ret ret; remote_domain_iothread_info *src; virDomainIOThreadInfoPtr *info_ret = NULL;
@@ -2336,17 +2336,17 @@ remoteDomainGetIOThreadsInfo(virDomainPtr dom,
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, + if (call(dom->conn, priv, 0, REMOTE_PROC_DOMAIN_GET_IOTHREAD_INFO, + (xdrproc_t)xdr_remote_domain_get_iothread_info_args, (char *)&args, - (xdrproc_t)xdr_remote_domain_get_iothreads_info_ret, + (xdrproc_t)xdr_remote_domain_get_iothread_info_ret, (char *)&ret) == -1) goto done;
- if (ret.info.info_len > REMOTE_IOTHREADS_INFO_MAX) { + if (ret.info.info_len > REMOTE_IOTHREAD_INFO_MAX) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Too many IOThreads in info: %d for limit %d"), - ret.info.info_len, REMOTE_IOTHREADS_INFO_MAX); + ret.info.info_len, REMOTE_IOTHREAD_INFO_MAX); goto cleanup; }
@@ -2385,7 +2385,7 @@ remoteDomainGetIOThreadsInfo(virDomainPtr dom, virDomainIOThreadInfoFree(info_ret[i]); VIR_FREE(info_ret); } - xdr_free((xdrproc_t)xdr_remote_domain_get_iothreads_info_ret, + xdr_free((xdrproc_t)xdr_remote_domain_get_iothread_info_ret, (char *) &ret);
done: @@ -8208,7 +8208,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 */ + .domainGetIOThreadInfo = remoteDomainGetIOThreadInfo, /* 1.2.14 */ .domainPinIOThread = remoteDomainPinIOThread, /* 1.2.14 */ .domainGetSecurityLabel = remoteDomainGetSecurityLabel, /* 0.6.1 */ .domainGetSecurityLabelList = remoteDomainGetSecurityLabelList, /* 0.10.0 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index eb862e1..d90e6b5 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -86,7 +86,7 @@ const REMOTE_VCPUINFO_MAX = 16384; const REMOTE_CPUMAPS_MAX = 8388608;
/* Upper limit on number of info fields returned by virDomainGetIOThreads. */ -const REMOTE_IOTHREADS_INFO_MAX = 16384; +const REMOTE_IOTHREAD_INFO_MAX = 16384;
/* Upper limit on migrate cookie. */ const REMOTE_MIGRATE_COOKIE_MAX = 4194304; @@ -1195,13 +1195,13 @@ struct remote_domain_iothread_info { opaque cpumap<REMOTE_CPUMAP_MAX>; };
-struct remote_domain_get_iothreads_info_args { +struct remote_domain_get_iothread_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>; +struct remote_domain_get_iothread_info_ret { + remote_domain_iothread_info info<REMOTE_IOTHREAD_INFO_MAX>; unsigned int ret; };
@@ -5629,7 +5629,7 @@ enum remote_procedure { * @generate: none * @acl: domain:read */ - REMOTE_PROC_DOMAIN_GET_IOTHREADS_INFO = 351, + REMOTE_PROC_DOMAIN_GET_IOTHREAD_INFO = 351,
/** * @generate: both diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index b3e2e40..e614f77 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -814,11 +814,11 @@ struct remote_domain_iothread_info { char * cpumap_val; } cpumap; }; -struct remote_domain_get_iothreads_info_args { +struct remote_domain_get_iothread_info_args { remote_nonnull_domain dom; u_int flags; }; -struct remote_domain_get_iothreads_info_ret { +struct remote_domain_get_iothread_info_ret { struct { u_int info_len; remote_domain_iothread_info * info_val; @@ -3014,7 +3014,7 @@ 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, + REMOTE_PROC_DOMAIN_GET_IOTHREAD_INFO = 351, REMOTE_PROC_DOMAIN_PIN_IOTHREAD = 352, REMOTE_PROC_DOMAIN_INTERFACE_ADDRESSES = 353, }; diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 0713b93..c8e0233 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6849,7 +6849,7 @@ cmdIOThreadsInfo(vshControl *ctl, const vshCmd *cmd) if ((maxcpu = vshNodeGetCPUCount(ctl->conn)) < 0) goto cleanup;
- if ((niothreads = virDomainGetIOThreadsInfo(dom, &info, flags)) < 0) { + if ((niothreads = virDomainGetIOThreadInfo(dom, &info, flags)) < 0) { vshError(ctl, _("Unable to get domain IOThreads information")); goto cleanup; }
The rest looks good. I slightly prefer this version so ACK if you fix all the misalignments. (Unless obviously somebody protests until you push it). Peter
-- 2.0.5
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
No more APIs have IOThreads in their name now. --- src/rpc/gendispatch.pl | 1 - 1 file changed, 1 deletion(-) diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index aa73d0c..b642d6e 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -67,7 +67,6 @@ sub fixup_name { $name =~ s/Fsfreeze$/FSFreeze/; $name =~ s/Fsthaw$/FSThaw/; $name =~ s/Fsinfo$/FSInfo/; - $name =~ s/Iothreads$/IOThreads/; $name =~ s/Iothread$/IOThread/; $name =~ s/Scsi/SCSI/; $name =~ s/Wwn$/WWN/; -- 2.0.5
On Wed, Mar 25, 2015 at 19:39:09 +0100, Ján Tomko wrote:
No more APIs have IOThreads in their name now. --- src/rpc/gendispatch.pl | 1 - 1 file changed, 1 deletion(-)
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index aa73d0c..b642d6e 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -67,7 +67,6 @@ sub fixup_name { $name =~ s/Fsfreeze$/FSFreeze/; $name =~ s/Fsthaw$/FSThaw/; $name =~ s/Fsinfo$/FSInfo/; - $name =~ s/Iothreads$/IOThreads/; $name =~ s/Iothread$/IOThread/; $name =~ s/Scsi/SCSI/; $name =~ s/Wwn$/WWN/;
Squash this to the previous patch. ACK Peter
On Thu, Mar 26, 2015 at 10:47:01AM +0100, Peter Krempa wrote:
On Wed, Mar 25, 2015 at 19:39:09 +0100, Ján Tomko wrote:
No more APIs have IOThreads in their name now. --- src/rpc/gendispatch.pl | 1 - 1 file changed, 1 deletion(-)
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index aa73d0c..b642d6e 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -67,7 +67,6 @@ sub fixup_name { $name =~ s/Fsfreeze$/FSFreeze/; $name =~ s/Fsthaw$/FSThaw/; $name =~ s/Fsinfo$/FSInfo/; - $name =~ s/Iothreads$/IOThreads/; $name =~ s/Iothread$/IOThread/; $name =~ s/Scsi/SCSI/; $name =~ s/Wwn$/WWN/;
Squash this to the previous patch.
ACK
Peter
I have squashed it in locally, fixed the alignment and sent patches for the python and perl bindings based on this rename (up to this patch). Jan
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
The plural seems unnecessary. --- tools/virsh-domain.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index c8e0233..afd92b1 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6792,7 +6792,7 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd) /* * "iothreadsinfo" command */ -static const vshCmdInfo info_iothreads[] = { +static const vshCmdInfo info_iothreadinfo[] = { {.name = "help", .data = N_("view domain IOThreads") }, @@ -6801,7 +6801,7 @@ static const vshCmdInfo info_iothreads[] = { }, {.name = NULL} }; -static const vshCmdOptDef opts_iothreads[] = { +static const vshCmdOptDef opts_iothreadinfo[] = { {.name = "domain", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, @@ -6823,7 +6823,7 @@ static const vshCmdOptDef opts_iothreads[] = { }; static bool -cmdIOThreadsInfo(vshControl *ctl, const vshCmd *cmd) +cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; bool config = vshCommandOptBool(cmd, "config"); @@ -12891,10 +12891,10 @@ const vshCmdDef domManagementCmds[] = { .info = info_inject_nmi, .flags = 0 }, - {.name = "iothreadsinfo", - .handler = cmdIOThreadsInfo, - .opts = opts_iothreads, - .info = info_iothreads, + {.name = "iothreadinfo", + .handler = cmdIOThreadInfo, + .opts = opts_iothreadinfo, + .info = info_iothreadinfo, .flags = 0 }, {.name = "iothreadpin", -- 2.0.5
On 03/25/2015 02:39 PM, Ján Tomko wrote:
The plural seems unnecessary. --- tools/virsh-domain.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
You would need to adjust virsh.pod too.
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index c8e0233..afd92b1 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6792,7 +6792,7 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd) /* * "iothreadsinfo" command */ -static const vshCmdInfo info_iothreads[] = { +static const vshCmdInfo info_iothreadinfo[] = { {.name = "help", .data = N_("view domain IOThreads") }, @@ -6801,7 +6801,7 @@ static const vshCmdInfo info_iothreads[] = { }, {.name = NULL} }; -static const vshCmdOptDef opts_iothreads[] = { +static const vshCmdOptDef opts_iothreadinfo[] = { {.name = "domain", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, @@ -6823,7 +6823,7 @@ static const vshCmdOptDef opts_iothreads[] = { };
static bool -cmdIOThreadsInfo(vshControl *ctl, const vshCmd *cmd) +cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; bool config = vshCommandOptBool(cmd, "config"); @@ -12891,10 +12891,10 @@ const vshCmdDef domManagementCmds[] = { .info = info_inject_nmi, .flags = 0 }, - {.name = "iothreadsinfo", - .handler = cmdIOThreadsInfo, - .opts = opts_iothreads, - .info = info_iothreads, + {.name = "iothreadinfo", + .handler = cmdIOThreadInfo, + .opts = opts_iothreadinfo, + .info = info_iothreadinfo, .flags = 0 }, {.name = "iothreadpin",
On Wed, Mar 25, 2015 at 17:22:26 -0400, John Ferlan wrote:
On 03/25/2015 02:39 PM, Ján Tomko wrote:
The plural seems unnecessary. --- tools/virsh-domain.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
You would need to adjust virsh.pod too.
ACK if you fix this ^^ Peter
On 03/25/2015 02:39 PM, Ján Tomko wrote:
The plural seems unnecessary. --- tools/virsh-domain.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index c8e0233..afd92b1 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6792,7 +6792,7 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd) /* * "iothreadsinfo" command
^^^ And this
*/ -static const vshCmdInfo info_iothreads[] = { +static const vshCmdInfo info_iothreadinfo[] = { {.name = "help", .data = N_("view domain IOThreads") }, @@ -6801,7 +6801,7 @@ static const vshCmdInfo info_iothreads[] = { }, {.name = NULL} }; -static const vshCmdOptDef opts_iothreads[] = { +static const vshCmdOptDef opts_iothreadinfo[] = { {.name = "domain", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, @@ -6823,7 +6823,7 @@ static const vshCmdOptDef opts_iothreads[] = { };
static bool -cmdIOThreadsInfo(vshControl *ctl, const vshCmd *cmd) +cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; bool config = vshCommandOptBool(cmd, "config"); @@ -12891,10 +12891,10 @@ const vshCmdDef domManagementCmds[] = { .info = info_inject_nmi, .flags = 0 }, - {.name = "iothreadsinfo", - .handler = cmdIOThreadsInfo, - .opts = opts_iothreads, - .info = info_iothreads, + {.name = "iothreadinfo", + .handler = cmdIOThreadInfo, + .opts = opts_iothreadinfo, + .info = info_iothreadinfo, .flags = 0 }, {.name = "iothreadpin",
Just format the bitmap via virBitmapFormat. --- tools/virsh-domain.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index afd92b1..cb9cb9d 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6834,6 +6834,7 @@ cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd) size_t i; int maxcpu; unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; + virBitmapPtr map = NULL; VSH_EXCLUSIVE_OPTIONS_VAR(current, live); VSH_EXCLUSIVE_OPTIONS_VAR(current, config); @@ -6863,16 +6864,24 @@ cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd) _("IOThread ID"), _("CPU Affinity")); vshPrintExtra(ctl, "---------------------------------------------------\n"); for (i = 0; i < niothreads; i++) { + char *mapstr = NULL; + virBitmapFree(map); + map = virBitmapNewData(info[i]->cpumap, info[i]->cpumaplen); + if (!map) + goto cleanup; + + if (!(mapstr = virBitmapFormat(map))) + goto cleanup; vshPrint(ctl, " %-15u ", info[i]->iothread_id); - ignore_value(vshPrintPinInfo(info[i]->cpumap, info[i]->cpumaplen, - maxcpu, 0)); + vshPrint(ctl, " %-15s ", mapstr); vshPrint(ctl, "\n"); virDomainIOThreadInfoFree(info[i]); } VIR_FREE(info); cleanup: + virBitmapFree(map); virDomainFree(dom); return niothreads >= 0; } -- 2.0.5
On Wed, Mar 25, 2015 at 19:39:11 +0100, Ján Tomko wrote:
Just format the bitmap via virBitmapFormat. --- tools/virsh-domain.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index afd92b1..cb9cb9d 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6834,6 +6834,7 @@ cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd) size_t i; int maxcpu; unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; + virBitmapPtr map = NULL;
VSH_EXCLUSIVE_OPTIONS_VAR(current, live); VSH_EXCLUSIVE_OPTIONS_VAR(current, config); @@ -6863,16 +6864,24 @@ cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd) _("IOThread ID"), _("CPU Affinity")); vshPrintExtra(ctl, "---------------------------------------------------\n"); for (i = 0; i < niothreads; i++) { + char *mapstr = NULL; + virBitmapFree(map); + map = virBitmapNewData(info[i]->cpumap, info[i]->cpumaplen); + if (!map) + goto cleanup; + + if (!(mapstr = virBitmapFormat(map))) + goto cleanup;
vshPrint(ctl, " %-15u ", info[i]->iothread_id); - ignore_value(vshPrintPinInfo(info[i]->cpumap, info[i]->cpumaplen, - maxcpu, 0)); + vshPrint(ctl, " %-15s ", mapstr); vshPrint(ctl, "\n"); virDomainIOThreadInfoFree(info[i]); } VIR_FREE(info);
cleanup: + virBitmapFree(map); virDomainFree(dom); return niothreads >= 0; }
Since vshPrintPinInfo produces the same output, how about we kill the reimplementation in vshPrintPinInfo and replace it with this code? And keep the use of vshPrintPinInfo here? Peter
On 03/26/2015 05:58 AM, Peter Krempa wrote:
On Wed, Mar 25, 2015 at 19:39:11 +0100, Ján Tomko wrote:
Just format the bitmap via virBitmapFormat. --- tools/virsh-domain.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index afd92b1..cb9cb9d 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6834,6 +6834,7 @@ cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd) size_t i; int maxcpu; unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; + virBitmapPtr map = NULL;
VSH_EXCLUSIVE_OPTIONS_VAR(current, live); VSH_EXCLUSIVE_OPTIONS_VAR(current, config); @@ -6863,16 +6864,24 @@ cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd) _("IOThread ID"), _("CPU Affinity")); vshPrintExtra(ctl, "---------------------------------------------------\n"); for (i = 0; i < niothreads; i++) { + char *mapstr = NULL; + virBitmapFree(map); + map = virBitmapNewData(info[i]->cpumap, info[i]->cpumaplen); + if (!map) + goto cleanup; + + if (!(mapstr = virBitmapFormat(map))) + goto cleanup;
vshPrint(ctl, " %-15u ", info[i]->iothread_id); - ignore_value(vshPrintPinInfo(info[i]->cpumap, info[i]->cpumaplen, - maxcpu, 0)); + vshPrint(ctl, " %-15s ", mapstr); vshPrint(ctl, "\n"); virDomainIOThreadInfoFree(info[i]); } VIR_FREE(info);
cleanup: + virBitmapFree(map); virDomainFree(dom); return niothreads >= 0; }
Since vshPrintPinInfo produces the same output, how about we kill the reimplementation in vshPrintPinInfo and replace it with this code? And keep the use of vshPrintPinInfo here?
So then this should also be done for cmdVcpuPin and cmdEmulatorPin since that was the model... John
On Thu, Mar 26, 2015 at 06:25:21 -0400, John Ferlan wrote:
On 03/26/2015 05:58 AM, Peter Krempa wrote:
On Wed, Mar 25, 2015 at 19:39:11 +0100, Ján Tomko wrote:
Just format the bitmap via virBitmapFormat. --- tools/virsh-domain.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index afd92b1..cb9cb9d 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6834,6 +6834,7 @@ cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd) size_t i; int maxcpu; unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; + virBitmapPtr map = NULL;
VSH_EXCLUSIVE_OPTIONS_VAR(current, live); VSH_EXCLUSIVE_OPTIONS_VAR(current, config); @@ -6863,16 +6864,24 @@ cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd) _("IOThread ID"), _("CPU Affinity")); vshPrintExtra(ctl, "---------------------------------------------------\n"); for (i = 0; i < niothreads; i++) { + char *mapstr = NULL; + virBitmapFree(map); + map = virBitmapNewData(info[i]->cpumap, info[i]->cpumaplen); + if (!map) + goto cleanup; + + if (!(mapstr = virBitmapFormat(map))) + goto cleanup;
vshPrint(ctl, " %-15u ", info[i]->iothread_id); - ignore_value(vshPrintPinInfo(info[i]->cpumap, info[i]->cpumaplen, - maxcpu, 0)); + vshPrint(ctl, " %-15s ", mapstr); vshPrint(ctl, "\n"); virDomainIOThreadInfoFree(info[i]); } VIR_FREE(info);
cleanup: + virBitmapFree(map); virDomainFree(dom); return niothreads >= 0; }
Since vshPrintPinInfo produces the same output, how about we kill the reimplementation in vshPrintPinInfo and replace it with this code? And keep the use of vshPrintPinInfo here?
So then this should also be done for cmdVcpuPin and cmdEmulatorPin since that was the model...
Or as I've said above, rewrite vshPrintPinInfo() with the internal code and leave it used in all places (here too). Peter
On 03/25/2015 02:39 PM, Ján Tomko wrote:
Just format the bitmap via virBitmapFormat. --- tools/virsh-domain.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index afd92b1..cb9cb9d 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6834,6 +6834,7 @@ cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd) size_t i; int maxcpu; unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; + virBitmapPtr map = NULL;
VSH_EXCLUSIVE_OPTIONS_VAR(current, live); VSH_EXCLUSIVE_OPTIONS_VAR(current, config); @@ -6863,16 +6864,24 @@ cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd) _("IOThread ID"), _("CPU Affinity")); vshPrintExtra(ctl, "---------------------------------------------------\n"); for (i = 0; i < niothreads; i++) { + char *mapstr = NULL;
Considering the other discussion about Set/Add/Del IOThread and since you're modifying the code anyway... How about adding a check for: if (info[i].iothread_id == 0) continue; That way we can "prepare" for a configuration that may have "holes" on the delete and won't have some future issue with a 1.2.14 virsh receiving something unexpected from a 1.2.15 daemon. John
+ virBitmapFree(map); + map = virBitmapNewData(info[i]->cpumap, info[i]->cpumaplen); + if (!map) + goto cleanup; + + if (!(mapstr = virBitmapFormat(map))) + goto cleanup;
vshPrint(ctl, " %-15u ", info[i]->iothread_id); - ignore_value(vshPrintPinInfo(info[i]->cpumap, info[i]->cpumaplen, - maxcpu, 0)); + vshPrint(ctl, " %-15s ", mapstr); vshPrint(ctl, "\n"); virDomainIOThreadInfoFree(info[i]); } VIR_FREE(info);
cleanup: + virBitmapFree(map); virDomainFree(dom); return niothreads >= 0; }
On Thu, Mar 26, 2015 at 08:50:24AM -0400, John Ferlan wrote:
On 03/25/2015 02:39 PM, Ján Tomko wrote:
Just format the bitmap via virBitmapFormat. --- tools/virsh-domain.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index afd92b1..cb9cb9d 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6834,6 +6834,7 @@ cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd) size_t i; int maxcpu; unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; + virBitmapPtr map = NULL;
VSH_EXCLUSIVE_OPTIONS_VAR(current, live); VSH_EXCLUSIVE_OPTIONS_VAR(current, config); @@ -6863,16 +6864,24 @@ cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd) _("IOThread ID"), _("CPU Affinity")); vshPrintExtra(ctl, "---------------------------------------------------\n"); for (i = 0; i < niothreads; i++) { + char *mapstr = NULL;
Considering the other discussion about Set/Add/Del IOThread and since you're modifying the code anyway...
How about adding a check for:
if (info[i].iothread_id == 0) continue;
That way we can "prepare" for a configuration that may have "holes" on the delete and won't have some future issue with a 1.2.14 virsh receiving something unexpected from a 1.2.15 daemon.
We would not need to include empty elements in the array - the holes would be apparent from the presence of a thread with id=1, then id=3. Otherwise the thread_id element would be redundant. Jan
Add cpuTime to virDomainIOThreadInfo, fill it out in the qemu driver and print it in virsh. --- daemon/remote.c | 1 + include/libvirt/libvirt-domain.h | 1 + src/qemu/qemu_driver.c | 10 ++++++++++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 1 + src/remote_protocol-structs | 1 + tools/virsh-domain.c | 18 ++++++++++++++---- 7 files changed, 29 insertions(+), 4 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 2f4df48..1f44ed5 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -2317,6 +2317,7 @@ remoteDispatchDomainGetIOThreadInfo(virNetServerPtr server ATTRIBUTE_UNUSED, */ dst->cpumap.cpumap_len = info[i]->cpumaplen; dst->cpumap.cpumap_val = (char *)info[i]->cpumap; + dst->cpu_time = info[i]->cpuTime; info[i]->cpumap = NULL; } } else { diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 7be4219..cca08ca 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1603,6 +1603,7 @@ struct _virDomainIOThreadInfo { unsigned char *cpumap; /* CPU map for thread. A pointer to an */ /* array of real CPUs (in 8-bit bytes) */ int cpumaplen; /* cpumap size */ + unsigned long long cpuTime; /* CPU time used, in nanoseconds */ }; void virDomainIOThreadInfoFree(virDomainIOThreadInfoPtr info); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index bef1223..51f59cb 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5776,6 +5776,16 @@ qemuDomainGetIOThreadsLive(virQEMUDriverPtr driver, goto endjob; } virBitmapFree(map); + + if (qemuGetProcessInfo(&(info_ret[i]->cpuTime), + NULL, + NULL, + vm->pid, + iothreads[i]->thread_id) < 0) { + virReportSystemError(errno, "%s", + _("cannot get IO thread cpu time")); + goto endjob; + } } *info = info_ret; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 8bd54e6..9182c33 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2372,6 +2372,7 @@ remoteDomainGetIOThreadInfo(virDomainPtr dom, memcpy(info_ret[i]->cpumap, src->cpumap.cpumap_val, src->cpumap.cpumap_len); info_ret[i]->cpumaplen = src->cpumap.cpumap_len; + info_ret[i]->cpuTime = src->cpu_time; } *info = info_ret; info_ret = NULL; diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index d90e6b5..b1ddf48 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -1193,6 +1193,7 @@ struct remote_domain_get_max_vcpus_ret { struct remote_domain_iothread_info { unsigned int iothread_id; opaque cpumap<REMOTE_CPUMAP_MAX>; + unsigned hyper cpu_time; }; struct remote_domain_get_iothread_info_args { diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index e614f77..5645d8c 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -813,6 +813,7 @@ struct remote_domain_iothread_info { u_int cpumap_len; char * cpumap_val; } cpumap; + uint64_t cpu_time; }; struct remote_domain_get_iothread_info_args { remote_nonnull_domain dom; diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index cb9cb9d..929cceb 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6860,8 +6860,8 @@ cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd) goto cleanup; } - vshPrintExtra(ctl, " %-15s %-15s\n", - _("IOThread ID"), _("CPU Affinity")); + vshPrintExtra(ctl, " %-15s %-15s %-15s\n", + _("IOThread ID"), _("CPU Affinity"), _("CPU time")); vshPrintExtra(ctl, "---------------------------------------------------\n"); for (i = 0; i < niothreads; i++) { char *mapstr = NULL; @@ -6875,12 +6875,22 @@ cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd) vshPrint(ctl, " %-15u ", info[i]->iothread_id); vshPrint(ctl, " %-15s ", mapstr); + if (info[i]->cpuTime != 0) { + double cpuUsed = info[i]->cpuTime; + + cpuUsed /= 1000000000.0; + + vshPrint(ctl, " %.1lf s ", cpuUsed); + } vshPrint(ctl, "\n"); - virDomainIOThreadInfoFree(info[i]); } - VIR_FREE(info); cleanup: + if (niothreads > 0) { + for (i = 0; i < niothreads; i++) + virDomainIOThreadInfoFree(info[i]); + VIR_FREE(info); + } virBitmapFree(map); virDomainFree(dom); return niothreads >= 0; -- 2.0.5
On Wed, Mar 25, 2015 at 19:39:12 +0100, Ján Tomko wrote:
Add cpuTime to virDomainIOThreadInfo, fill it out in the qemu driver and print it in virsh. --- daemon/remote.c | 1 + include/libvirt/libvirt-domain.h | 1 + src/qemu/qemu_driver.c | 10 ++++++++++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 1 + src/remote_protocol-structs | 1 + tools/virsh-domain.c | 18 ++++++++++++++---- 7 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c index 2f4df48..1f44ed5 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -2317,6 +2317,7 @@ remoteDispatchDomainGetIOThreadInfo(virNetServerPtr server ATTRIBUTE_UNUSED, */ dst->cpumap.cpumap_len = info[i]->cpumaplen; dst->cpumap.cpumap_val = (char *)info[i]->cpumap; + dst->cpu_time = info[i]->cpuTime; info[i]->cpumap = NULL; } } else { diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 7be4219..cca08ca 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1603,6 +1603,7 @@ struct _virDomainIOThreadInfo { unsigned char *cpumap; /* CPU map for thread. A pointer to an */ /* array of real CPUs (in 8-bit bytes) */ int cpumaplen; /* cpumap size */ + unsigned long long cpuTime; /* CPU time used, in nanoseconds */
Perhaps add a note that 0 is returned for offline VMs?
};
void virDomainIOThreadInfoFree(virDomainIOThreadInfoPtr info);
...
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index cb9cb9d..929cceb 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c
...
@@ -6875,12 +6875,22 @@ cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd)
vshPrint(ctl, " %-15u ", info[i]->iothread_id); vshPrint(ctl, " %-15s ", mapstr); + if (info[i]->cpuTime != 0) { + double cpuUsed = info[i]->cpuTime; + + cpuUsed /= 1000000000.0; + + vshPrint(ctl, " %.1lf s ", cpuUsed); + }
Should we print a dash or something when the data is not present?
vshPrint(ctl, "\n"); - virDomainIOThreadInfoFree(info[i]); } - VIR_FREE(info);
cleanup: + if (niothreads > 0) { + for (i = 0; i < niothreads; i++) + virDomainIOThreadInfoFree(info[i]); + VIR_FREE(info);
If you initialize info to NULL when it's defined you can avoid having the if (niothreads > 0) condition.
+ } virBitmapFree(map); virDomainFree(dom); return niothreads >= 0;
ACK. I'll not insisting on fixing all the comments, but I'd prefer to have a dash in the output if CPU time doesn't make sense. Peter
On 03/25/2015 02:39 PM, Ján Tomko wrote:
Looking at the proposed SetIOThread API, I noticed that the virsh command for printing the info is named 'iothreadsinfo'. This seemed counter-intuitive for me.
Since the API has not been released yet, I propose a rename of the command to: iothreadinfo (patch 5) and the API for freeing one struct to: virDomainIOThreadInfoFree (patch 1)
I don't feel as strongly about renaming the virDomainGetIOThreadsInfo API (patches 3, 4) or the internal APIs (patch 2).
Looking at virVcpuInfoPtr (which might not be the best inspiration), I realized including the cpu time might be a good idea.
Ján Tomko (7): Rename virDomainIOThreadsInfoFree to virDomainIOThreadInfoFree Rename qemuMonitorIOThreadsInfo* to qemuMonitorIOThreadInfo* Rename DomainGetIOThreadsInfo to DomainGetIOThreadInfo gendispatch: remove IOThreads from name fixups virsh: rename iothreadsinfo to iothreadinfo Do not use vshPrintPinInfo in iothreadinfo add cpu time to iothreadinfo
daemon/remote.c | 15 +++++++------ include/libvirt/libvirt-domain.h | 5 +++-- src/driver-hypervisor.h | 4 ++-- src/libvirt-domain.c | 20 ++++++++--------- src/libvirt_public.syms | 4 ++-- src/qemu/qemu_driver.c | 24 ++++++++++++++------ src/qemu/qemu_monitor.c | 4 ++-- src/qemu/qemu_monitor.h | 10 ++++----- src/qemu/qemu_monitor_json.c | 8 +++---- src/qemu/qemu_monitor_json.h | 2 +- src/qemu/qemu_process.c | 4 ++-- src/remote/remote_driver.c | 23 ++++++++++---------- src/remote/remote_protocol.x | 11 +++++----- src/remote_protocol-structs | 7 +++--- src/rpc/gendispatch.pl | 1 - tests/qemumonitorjsontest.c | 4 ++-- tools/virsh-domain.c | 47 ++++++++++++++++++++++++++++------------ 17 files changed, 113 insertions(+), 80 deletions(-)
Didn't dig into all the details, but use of IOThreads v IOThread was mostly an artifact of the technology name. I'm ambivalent to the naming issue. You'll have to have follow-ups in libvirt-python and libvirt-perl though. Sometimes the *s* is relevant though - as in we're returning all IOThreads found vs returning just one IOThread w/r/t virsh iothreadsinfo vs. iothreadinfo - I see the latter as displaying just one rather than multiple. w/r/t patch 6 - does the output change? IOW: The current way displays "1", "3", "0-3", etc. Does the method you're proposing change to the "y---", "--y-", "yyyy" etc. method? I prefer the former.... The commit message would need to list the change if there is one. w/r/t patch 7 - while it's a "nice to have" I think its far more relevant to list the Resource(s) associated with the thread than the CPU time used. Listing the Resource was rejected in a much earlier patch review, so I don't see why listing the CPU time is important and failing in qemuDomainGetIOThreadsLive because we cannot get the that time, but yet deciding later on to not print it if it doesn't exist doesn't make total sense. John
On Wed, Mar 25, 2015 at 15:00:58 -0400, John Ferlan wrote:
On 03/25/2015 02:39 PM, Ján Tomko wrote:
Looking at the proposed SetIOThread API, I noticed that the virsh command for printing the info is named 'iothreadsinfo'. This seemed counter-intuitive for me.
Since the API has not been released yet, I propose a rename of the command to: iothreadinfo (patch 5) and the API for freeing one struct to: virDomainIOThreadInfoFree (patch 1)
I don't feel as strongly about renaming the virDomainGetIOThreadsInfo API (patches 3, 4) or the internal APIs (patch 2).
Looking at virVcpuInfoPtr (which might not be the best inspiration), I realized including the cpu time might be a good idea.
Ján Tomko (7): Rename virDomainIOThreadsInfoFree to virDomainIOThreadInfoFree Rename qemuMonitorIOThreadsInfo* to qemuMonitorIOThreadInfo* Rename DomainGetIOThreadsInfo to DomainGetIOThreadInfo gendispatch: remove IOThreads from name fixups virsh: rename iothreadsinfo to iothreadinfo Do not use vshPrintPinInfo in iothreadinfo add cpu time to iothreadinfo
daemon/remote.c | 15 +++++++------ include/libvirt/libvirt-domain.h | 5 +++-- src/driver-hypervisor.h | 4 ++-- src/libvirt-domain.c | 20 ++++++++--------- src/libvirt_public.syms | 4 ++-- src/qemu/qemu_driver.c | 24 ++++++++++++++------ src/qemu/qemu_monitor.c | 4 ++-- src/qemu/qemu_monitor.h | 10 ++++----- src/qemu/qemu_monitor_json.c | 8 +++---- src/qemu/qemu_monitor_json.h | 2 +- src/qemu/qemu_process.c | 4 ++-- src/remote/remote_driver.c | 23 ++++++++++---------- src/remote/remote_protocol.x | 11 +++++----- src/remote_protocol-structs | 7 +++--- src/rpc/gendispatch.pl | 1 - tests/qemumonitorjsontest.c | 4 ++-- tools/virsh-domain.c | 47 ++++++++++++++++++++++++++++------------ 17 files changed, 113 insertions(+), 80 deletions(-)
Didn't dig into all the details, but use of IOThreads v IOThread was mostly an artifact of the technology name. I'm ambivalent to the naming issue. You'll have to have follow-ups in libvirt-python and libvirt-perl though. Sometimes the *s* is relevant though - as in we're returning all IOThreads found vs returning just one IOThread
w/r/t virsh iothreadsinfo vs. iothreadinfo - I see the latter as displaying just one rather than multiple.
w/r/t patch 6 - does the output change? IOW: The current way displays "1", "3", "0-3", etc. Does the method you're proposing change to the "y---", "--y-", "yyyy" etc. method? I prefer the former.... The commit message would need to list the change if there is one.
The output does not change. In fact I think the virsh reimpl of virBitmapFormat should be killed and replaced with the function we already have.
w/r/t patch 7 - while it's a "nice to have" I think its far more relevant to list the Resource(s) associated with the thread than the CPU time used. Listing the Resource was rejected in a much earlier patch
Actually, the "resources" is a configruation option and I don't see a way to change it unless you unplug the disk while CPU time is a statistics function that changes a lot.
review, so I don't see why listing the CPU time is important and failing in qemuDomainGetIOThreadsLive because we cannot get the that time, but yet deciding later on to not print it if it doesn't exist doesn't make total sense.
If the VM is offline, then the CPU time is obviously not filled, while when it's live we should return it. Peter
w/r/t patch 7 - while it's a "nice to have" I think its far more relevant to list the Resource(s) associated with the thread than the CPU time used. Listing the Resource was rejected in a much earlier patch
Actually, the "resources" is a configruation option and I don't see a way to change it unless you unplug the disk while CPU time is a statistics function that changes a lot.
Correct, but I also think it's more useful to display resources in IOThreads output. If I'm a user I would rather know which resources are assigned to which IOThreads rather than how much CPU Time they're using. Since CPU Time is a statistic, perhaps it belongs in the stats output rather than the IOThreads information output. As it stands, I have to make my own "map" by dumping the xml, looking for the iothreads attribute in a disk element and then do the correlation. Again - don't forget to update the libvirt-python and libvirt-perl code.
review, so I don't see why listing the CPU time is important and failing in qemuDomainGetIOThreadsLive because we cannot get the that time, but yet deciding later on to not print it if it doesn't exist doesn't make total sense.
If the VM is offline, then the CPU time is obviously not filled, while when it's live we should return it.
You missed my point - if for some reason we fail to get the CPU Time from the QEMU call - we fail the entire call now. If the time wasn't available we don't print it. So why fail the call just because the time isn't available (for whatever reason). It's not all that important, but once done I would hope the bz associated will also get updated to list the new data column so that whomever tests and documents this can do so properly. John
participants (3)
-
John Ferlan -
Ján Tomko -
Peter Krempa