[libvirt] [PATCH 0/5] RFC: vcpuinfo, vcpupin: the feature to get CPU affinity information

Hi all, This patchset improves the "virsh vcpupin" and "virsh vcpuinfo" command. This is based on the following Eric's comment: http://www.redhat.com/archives/libvir-list/2011-June/msg00500.html We can use virDomainGetVcpus API to get it, but can't use this API against inactive domains (at least in case of KVM). There is the companion API of this, whose name is virDomainGetVcpusFlags. However its signature is different from what everyone expect, so we can't use this. This patchset introduces the new libvirt API to retrieve CPU affinity information and adds the code to use new API. To be specific, the following are changed. (i) "virsh vcpuinfo" comes to succeed in case of the target domain is inactive on KVM. # virsh domstate VM shut off # virsh vcpuinfo VM VCPU: 0 CPU: N/A State: N/A CPU time N/A CPU Affinity: -yyyyyy--yyyyyyyyyyyy------------------- VCPU: 1 CPU: N/A State: N/A CPU time N/A CPU Affinity: ----------y----------------------------- VCPU: 2 CPU: N/A State: N/A CPU time N/A CPU Affinity: -----y---yyy---yyyyyy------------------- VCPU: 3 CPU: N/A State: N/A CPU time N/A CPU Affinity: -y-y-y-y-y-y-y-y------------------------ (ii) The --query option is added to virsh vcpupin command. # virsh vcpupin VM --query --config VCPU: CPU Affinity ---------------------------------- 0: 1-6,9-20 1: 10 2: 5,9-11,15-20 3: 1,3,5,7,9,11,13,15 --- *[PATCH 1/5] vcpupin: introduce the new libvirt API (virDomainGetVcpupinInfo) *[PATCH 2/5] vcpupin: implement the code to support new API for the qemu driver *[PATCH 3/5] vcpupin: implement the remote protocol to address the new API *[PATCH 4/5] vcpuinfo: add the code to fallback to try new API *[PATCH 5/5] vcpupin: add query option to virsh vcpupin command Best regards, Taku Izumi

This patch introduces a new libvirt API (virDomainGetVcpupinInfo). We can use virDomainGetVcpus API to retrieve CPU affinity information, but can't use this API against inactive domains (at least in case of KVM). There is the companion API of this, whose name is virDomainGetVcpusFlags. However its signature is different from what everyone expect, so we can't use this to retrieve it either. The virDomainGetVcpupinInfo is the new API to retrieve CPU affinity information of active and inactive domains. Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com> --- include/libvirt/libvirt.h.in | 6 +++ src/driver.h | 8 ++++ src/libvirt.c | 69 +++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 4 files changed, 84 insertions(+) Index: libvirt/include/libvirt/libvirt.h.in =================================================================== --- libvirt.orig/include/libvirt/libvirt.h.in +++ libvirt/include/libvirt/libvirt.h.in @@ -1265,6 +1265,12 @@ int virDomainPinVcpu int maplen, unsigned int flags); +int virDomainGetVcpupinInfo (virDomainPtr domain, + int maxinfo, + unsigned char *cpumaps, + int maplen, + unsigned int flags); + /** * VIR_USE_CPU: * @cpumap: pointer to a bit map of real CPUs (in 8-bit bytes) (IN/OUT) Index: libvirt/src/driver.h =================================================================== --- libvirt.orig/src/driver.h +++ libvirt/src/driver.h @@ -240,6 +240,13 @@ typedef int int maplen, unsigned int flags); typedef int + (*virDrvDomainGetVcpupinInfo) (virDomainPtr domain, + int maxinfo, + unsigned char *cpumaps, + int maplen, + unsigned int flags); + +typedef int (*virDrvDomainGetVcpus) (virDomainPtr domain, virVcpuInfoPtr info, int maxinfo, @@ -724,6 +731,7 @@ struct _virDriver { virDrvDomainGetVcpusFlags domainGetVcpusFlags; virDrvDomainPinVcpu domainPinVcpu; virDrvDomainPinVcpuFlags domainPinVcpuFlags; + virDrvDomainGetVcpupinInfo domainGetVcpupinInfo; virDrvDomainGetVcpus domainGetVcpus; virDrvDomainGetMaxVcpus domainGetMaxVcpus; virDrvDomainGetSecurityLabel domainGetSecurityLabel; Index: libvirt/src/libvirt_public.syms =================================================================== --- libvirt.orig/src/libvirt_public.syms +++ libvirt/src/libvirt_public.syms @@ -467,6 +467,7 @@ LIBVIRT_0.9.3 { virEventUpdateTimeout; virNodeGetCPUStats; virNodeGetMemoryStats; + virDomainGetVcpupinInfo; } LIBVIRT_0.9.2; # .... define new API here using predicted next version number .... Index: libvirt/src/libvirt.c =================================================================== --- libvirt.orig/src/libvirt.c +++ libvirt/src/libvirt.c @@ -7087,6 +7087,75 @@ error: } /** + * virDomainGetVcpupinInfo: + * @domain: pointer to domain object, or NULL for Domain0 + * @maxinfo: the number of cpumap + * @cpumaps: pointer to a bit map of real CPUs for all vcpus of this + * domain (in 8-bit bytes) (OUT) + * It's assumed there is <maxinfo> cpumap in cpumaps array. + * The memory allocated to cpumaps must be (maxinfo * maplen) bytes + * (ie: calloc(maxinfo, maplen)). + * One cpumap inside cpumaps has the format described in + * virDomainPinVcpu() API. + * Must not be NULL. + * @maplen: the number of bytes in one cpumap, from 1 up to size of CPU map. + * Must be positive. + * @flags: an OR'ed set of virDomainModificationImpact + * Must not be specified VIR_DOMAIN_AFFECT_LIVE and + * VIR_DOMAIN_AFFECT_CONFIG concurrently! + * + * Query the CPU affinity setting of all virtual CPUs of domain, store it + * in cpumaps. + * + * Returns the number of virtual CPUs in case of success, + * -1 in case of failure. + */ +int +virDomainGetVcpupinInfo (virDomainPtr domain, int maxinfo, + unsigned char *cpumaps, int maplen, unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "maxinfo=%d, cpumaps=%p, maplen=%d, flags=%u", + maxinfo, cpumaps, maplen, flags); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + if (maxinfo < 1) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + + if ((cpumaps == NULL) || (cpumaps && maplen <= 0)) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + + conn = domain->conn; + + if (conn->driver->domainGetVcpupinInfo) { + int ret; + ret = conn->driver->domainGetVcpupinInfo (domain, maxinfo, + cpumaps, maplen, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(domain->conn); + return -1; +} + +/** * virDomainGetVcpus: * @domain: pointer to domain object, or NULL for Domain0 * @info: pointer to an array of virVcpuInfo structures (OUT)

On Fri, Jun 24, 2011 at 05:56:21PM +0900, Taku Izumi wrote:
This patch introduces a new libvirt API (virDomainGetVcpupinInfo).
We can use virDomainGetVcpus API to retrieve CPU affinity information, but can't use this API against inactive domains (at least in case of KVM). There is the companion API of this, whose name is virDomainGetVcpusFlags. However its signature is different from what everyone expect, so we can't use this to retrieve it either.
The virDomainGetVcpupinInfo is the new API to retrieve CPU affinity information of active and inactive domains.
Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com> --- include/libvirt/libvirt.h.in | 6 +++ src/driver.h | 8 ++++ src/libvirt.c | 69 +++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 4 files changed, 84 insertions(+)
Index: libvirt/include/libvirt/libvirt.h.in =================================================================== --- libvirt.orig/include/libvirt/libvirt.h.in +++ libvirt/include/libvirt/libvirt.h.in @@ -1265,6 +1265,12 @@ int virDomainPinVcpu int maplen, unsigned int flags);
+int virDomainGetVcpupinInfo (virDomainPtr domain, + int maxinfo, + unsigned char *cpumaps, + int maplen, + unsigned int flags); +
API looks fine to me [...]
--- libvirt.orig/src/libvirt.c +++ libvirt/src/libvirt.c @@ -7087,6 +7087,75 @@ error: }
/** + * virDomainGetVcpupinInfo: + * @domain: pointer to domain object, or NULL for Domain0 + * @maxinfo: the number of cpumap + * @cpumaps: pointer to a bit map of real CPUs for all vcpus of this + * domain (in 8-bit bytes) (OUT) + * It's assumed there is <maxinfo> cpumap in cpumaps array. + * The memory allocated to cpumaps must be (maxinfo * maplen) bytes + * (ie: calloc(maxinfo, maplen)). + * One cpumap inside cpumaps has the format described in + * virDomainPinVcpu() API. + * Must not be NULL. + * @maplen: the number of bytes in one cpumap, from 1 up to size of CPU map. + * Must be positive. + * @flags: an OR'ed set of virDomainModificationImpact + * Must not be specified VIR_DOMAIN_AFFECT_LIVE and + * VIR_DOMAIN_AFFECT_CONFIG concurrently! + * + * Query the CPU affinity setting of all virtual CPUs of domain, store it + * in cpumaps. + * + * Returns the number of virtual CPUs in case of success, + * -1 in case of failure. + */ +int +virDomainGetVcpupinInfo (virDomainPtr domain, int maxinfo, + unsigned char *cpumaps, int maplen, unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "maxinfo=%d, cpumaps=%p, maplen=%d, flags=%u", + maxinfo, cpumaps, maplen, flags); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + if (maxinfo < 1) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + }
We don't check for read-only here, but the semantic is purely reading that's normal
+ if ((cpumaps == NULL) || (cpumaps && maplen <= 0)) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + + conn = domain->conn; + + if (conn->driver->domainGetVcpupinInfo) { + int ret; + ret = conn->driver->domainGetVcpupinInfo (domain, maxinfo, + cpumaps, maplen, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(domain->conn); + return -1; +} + +/** * virDomainGetVcpus: * @domain: pointer to domain object, or NULL for Domain0 * @info: pointer to an array of virVcpuInfo structures (OUT)
Okay ACK on the API principle from me, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On 06/24/2011 08:53 AM, Daniel Veillard wrote:
+int virDomainGetVcpupinInfo (virDomainPtr domain, + int maxinfo, + unsigned char *cpumaps, + int maplen, + unsigned int flags); +
API looks fine to me
I'm not quite sure, yet.
[...]
--- libvirt.orig/src/libvirt.c +++ libvirt/src/libvirt.c @@ -7087,6 +7087,75 @@ error: }
/** + * virDomainGetVcpupinInfo: + * @domain: pointer to domain object, or NULL for Domain0 + * @maxinfo: the number of cpumap + * @cpumaps: pointer to a bit map of real CPUs for all vcpus of this + * domain (in 8-bit bytes) (OUT) + * It's assumed there is <maxinfo> cpumap in cpumaps array. + * The memory allocated to cpumaps must be (maxinfo * maplen) bytes + * (ie: calloc(maxinfo, maplen)). + * One cpumap inside cpumaps has the format described in + * virDomainPinVcpu() API. + * Must not be NULL. + * @maplen: the number of bytes in one cpumap, from 1 up to size of CPU map. + * Must be positive. + * @flags: an OR'ed set of virDomainModificationImpact + * Must not be specified VIR_DOMAIN_AFFECT_LIVE and + * VIR_DOMAIN_AFFECT_CONFIG concurrently! + * + * Query the CPU affinity setting of all virtual CPUs of domain, store it + * in cpumaps. + * + * Returns the number of virtual CPUs in case of success, + * -1 in case of failure. + */ +int +virDomainGetVcpupinInfo (virDomainPtr domain, int maxinfo, + unsigned char *cpumaps, int maplen, unsigned int flags)
What is maxinfo for? The counterpart is: virDomainGetVcpus(virDomainPtr domain, virVcpuInfoPtr info, int maxinfo, unsigned char *cpumaps, int maplen) where maxinfo relates to how large the 'info' array is. But virDomainGetVcpupinInfo has no 'info' array.
Okay ACK on the API principle from me,
I'm also reviewing the rest of this series. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On 06/24/2011 09:12 AM, Eric Blake wrote:
On 06/24/2011 08:53 AM, Daniel Veillard wrote:
+int virDomainGetVcpupinInfo (virDomainPtr domain, + int maxinfo, + unsigned char *cpumaps, + int maplen, + unsigned int flags); +
API looks fine to me
I'm not quite sure, yet.
+virDomainGetVcpupinInfo (virDomainPtr domain, int maxinfo, + unsigned char *cpumaps, int maplen, unsigned int flags)
What is maxinfo for? The counterpart is:
virDomainGetVcpus(virDomainPtr domain, virVcpuInfoPtr info, int maxinfo, unsigned char *cpumaps, int maplen)
where maxinfo relates to how large the 'info' array is. But virDomainGetVcpupinInfo has no 'info' array.
On further thought, I guess the issue is that: cpumaps is an array (maxinfo entries), and each entry of that array is a cpumap (maplen bytes). This is the same for virDomainGetVcpus, but that API also had an additional array of virVcpuInfoPtr, where you could then ignore cpumas and maplen. Our normal protocol is to do array/len, which works for info/maxinfo, but doesn't fit for maxinfo/cpumaps/maplen. On the other hand, since we can't change the signature of virDomainGetVcpus, it probably really is better to keep virDomainGetVcpupinInfo consistent. My only suggestion, therefore, is to s/maxinfo/ncpumaps/ to make it obvious that we really are passing array length prior to the array. What a yucky interface we've locked ourselves into, but I can't see any better approach. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On 06/24/2011 02:56 AM, Taku Izumi wrote:
This patch introduces a new libvirt API (virDomainGetVcpupinInfo).
We can use virDomainGetVcpus API to retrieve CPU affinity information, but can't use this API against inactive domains (at least in case of KVM). There is the companion API of this, whose name is virDomainGetVcpusFlags.
Actually not a companion API. virDomainGetVcpusFlags is a counterpart to virDomainGetMaxVcpus. The choice of naming isn't perfect, but we're stuck with it.
However its signature is different from what everyone expect, so we can't use this to retrieve it either.
The virDomainGetVcpupinInfo is the new API to retrieve CPU affinity information of active and inactive domains.
unsigned int flags);
+int virDomainGetVcpupinInfo (virDomainPtr domain, + int maxinfo,
See other email about this naming choice.
--- libvirt.orig/src/libvirt_public.syms +++ libvirt/src/libvirt_public.syms @@ -467,6 +467,7 @@ LIBVIRT_0.9.3 { virEventUpdateTimeout; virNodeGetCPUStats; virNodeGetMemoryStats; + virDomainGetVcpupinInfo;
I'm a fan of sorting, as others on this list will attest :)
+ * @flags: an OR'ed set of virDomainModificationImpact + * Must not be specified VIR_DOMAIN_AFFECT_LIVE and + * VIR_DOMAIN_AFFECT_CONFIG concurrently!
Grammar was a bit awkward, and ! is usually overkill in documentation.
+ * + * Query the CPU affinity setting of all virtual CPUs of domain, store it + * in cpumaps. + * + * Returns the number of virtual CPUs in case of success, + * -1 in case of failure. + */ +int +virDomainGetVcpupinInfo (virDomainPtr domain, int maxinfo, + unsigned char *cpumaps, int maplen, unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "maxinfo=%d, cpumaps=%p, maplen=%d, flags=%u", + maxinfo, cpumaps, maplen, flags); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + if (maxinfo < 1) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + + if ((cpumaps == NULL) || (cpumaps && maplen <= 0)) {
The 'cpumaps &&' check is redundant...
+ virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
...and this error is the same, so I merged some code. Here's what I'm squashing in: diff --git i/include/libvirt/libvirt.h.in w/include/libvirt/libvirt.h.in index 2c78f1f..4f9f158 100644 --- i/include/libvirt/libvirt.h.in +++ w/include/libvirt/libvirt.h.in @@ -1267,7 +1267,7 @@ int virDomainPinVcpuFlags (virDomainPtr domain, unsigned int flags); int virDomainGetVcpupinInfo (virDomainPtr domain, - int maxinfo, + int ncpumaps, unsigned char *cpumaps, int maplen, unsigned int flags); diff --git i/src/libvirt.c w/src/libvirt.c index e289fee..3f9585b 100644 --- i/src/libvirt.c +++ w/src/libvirt.c @@ -7111,20 +7111,20 @@ error: /** * virDomainGetVcpupinInfo: * @domain: pointer to domain object, or NULL for Domain0 - * @maxinfo: the number of cpumap + * @ncpumaps: the number of cpumap (listed first to match virDomainGetVcpus) * @cpumaps: pointer to a bit map of real CPUs for all vcpus of this * domain (in 8-bit bytes) (OUT) - * It's assumed there is <maxinfo> cpumap in cpumaps array. - * The memory allocated to cpumaps must be (maxinfo * maplen) bytes - * (ie: calloc(maxinfo, maplen)). + * It's assumed there is <ncpumaps> cpumap in cpumaps array. + * The memory allocated to cpumaps must be (ncpumaps * maplen) bytes + * (ie: calloc(ncpumaps, maplen)). * One cpumap inside cpumaps has the format described in * virDomainPinVcpu() API. * Must not be NULL. * @maplen: the number of bytes in one cpumap, from 1 up to size of CPU map. * Must be positive. * @flags: an OR'ed set of virDomainModificationImpact - * Must not be specified VIR_DOMAIN_AFFECT_LIVE and - * VIR_DOMAIN_AFFECT_CONFIG concurrently! + * Must not be VIR_DOMAIN_AFFECT_LIVE and + * VIR_DOMAIN_AFFECT_CONFIG concurrently. * * Query the CPU affinity setting of all virtual CPUs of domain, store it * in cpumaps. @@ -7133,13 +7133,13 @@ error: * -1 in case of failure. */ int -virDomainGetVcpupinInfo (virDomainPtr domain, int maxinfo, +virDomainGetVcpupinInfo (virDomainPtr domain, int ncpumaps, unsigned char *cpumaps, int maplen, unsigned int flags) { virConnectPtr conn; - VIR_DOMAIN_DEBUG(domain, "maxinfo=%d, cpumaps=%p, maplen=%d, flags=%u", - maxinfo, cpumaps, maplen, flags); + VIR_DOMAIN_DEBUG(domain, "ncpumaps=%d, cpumaps=%p, maplen=%d, flags=%u", + ncpumaps, cpumaps, maplen, flags); virResetLastError(); @@ -7149,12 +7149,7 @@ virDomainGetVcpupinInfo (virDomainPtr domain, int maxinfo, return -1; } - if (maxinfo < 1) { - virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); - goto error; - } - - if ((cpumaps == NULL) || (cpumaps && maplen <= 0)) { + if (ncpumaps < 1 || !cpumaps || maplen <= 0) { virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); goto error; } @@ -7163,7 +7158,7 @@ virDomainGetVcpupinInfo (virDomainPtr domain, int maxinfo, if (conn->driver->domainGetVcpupinInfo) { int ret; - ret = conn->driver->domainGetVcpupinInfo (domain, maxinfo, + ret = conn->driver->domainGetVcpupinInfo (domain, ncpumaps, cpumaps, maplen, flags); if (ret < 0) goto error; @@ -7197,6 +7192,9 @@ error: * Extract information about virtual CPUs of domain, store it in info array * and also in cpumaps if this pointer isn't NULL. * + * See also virDomainGetVcpupinInfo for querying just cpumaps, including on + * an inactive domain. + * * Returns the number of info filled in case of success, -1 in case of failure. */ int diff --git i/src/libvirt_public.syms w/src/libvirt_public.syms index 38e8446..c7dc3c5 100644 --- i/src/libvirt_public.syms +++ w/src/libvirt_public.syms @@ -453,6 +453,7 @@ LIBVIRT_0.9.2 { LIBVIRT_0.9.3 { global: virDomainGetControlInfo; + virDomainGetVcpupinInfo; virDomainPinVcpuFlags; virDomainSendKey; virEventAddHandle; @@ -463,7 +464,6 @@ LIBVIRT_0.9.3 { virEventUpdateTimeout; virNodeGetCPUStats; virNodeGetMemoryStats; - virDomainGetVcpupinInfo; } LIBVIRT_0.9.2; # .... define new API here using predicted next version number .... -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

2011/6/24 Taku Izumi <izumi.taku@jp.fujitsu.com>:
This patch introduces a new libvirt API (virDomainGetVcpupinInfo).
Just a comment on the name. I'd write pin with a capital P: virDomainGetVcpuPinInfo -- Matthias Bolte http://photron.blogspot.com

On 06/24/2011 02:37 PM, Matthias Bolte wrote:
2011/6/24 Taku Izumi <izumi.taku@jp.fujitsu.com>:
This patch introduces a new libvirt API (virDomainGetVcpupinInfo).
Just a comment on the name. I'd write pin with a capital P: virDomainGetVcpuPinInfo
Sure, I'll refactor that into my amendments to the patch series before pushing. But that also means renaming virDomainVcpupinAdd and friends in domain_conf.h. Patch coming up... -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

This patch implements the code to address the new API (virDomainGetVcpupinInfo) in the qemu driver. Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com> --- src/qemu/qemu_driver.c | 112 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 112 insertions(+) Index: libvirt/src/qemu/qemu_driver.c =================================================================== --- libvirt.orig/src/qemu/qemu_driver.c +++ libvirt/src/qemu/qemu_driver.c @@ -3088,6 +3088,117 @@ qemudDomainPinVcpu(virDomainPtr dom, } static int +qemudDomainGetVcpupinInfo(virDomainPtr dom, + int maxinfo, + unsigned char *cpumaps, + int maplen, + unsigned int flags) { + + struct qemud_driver *driver = dom->conn->privateData; + virDomainObjPtr vm; + virNodeInfo nodeinfo; + virDomainDefPtr targetDef = NULL; + int ret = -1; + bool isActive; + int maxcpu, hostcpus, vcpu, pcpu; + int n; + virDomainVcpupinDefPtr *vcpupin_list; + char *cpumask = NULL; + unsigned char *cpumap; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + + if ((flags & (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG)) == + (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG)) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("cannot get live and persistent info concurrently")); + goto cleanup; + } + + qemuDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemuDriverUnlock(driver); + + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + qemuReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + + isActive = virDomainObjIsActive(vm); + if (flags == VIR_DOMAIN_AFFECT_CURRENT) { + if (isActive) + flags = VIR_DOMAIN_AFFECT_LIVE; + else + flags = VIR_DOMAIN_AFFECT_CONFIG; + } + + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + if (!isActive) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + } + targetDef = vm->def; + } + + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + if (!vm->persistent) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("cannot get persistent config of a transient domain")); + goto cleanup; + } + if (!(targetDef = virDomainObjGetPersistentDef(driver->caps, vm))) + goto cleanup; + } + + if (nodeGetInfo(dom->conn, &nodeinfo) < 0) + goto cleanup; + hostcpus = VIR_NODEINFO_MAXCPUS(nodeinfo); + maxcpu = maplen * 8; + if (maxcpu > hostcpus) + maxcpu = hostcpus; + + /* Clamp to actual number of vcpus */ + if (maxinfo > targetDef->vcpus) + maxinfo = targetDef->vcpus; + + if (maxinfo < 1) { + goto cleanup; + } + + /* initialize cpumaps */ + memset(cpumaps, 0xff, maplen * maxinfo); + if (maxcpu % 8) { + for (vcpu = 0; vcpu < maxinfo; vcpu++) { + cpumap = VIR_GET_CPUMAP(cpumaps, maplen, vcpu); + cpumap[maplen - 1] &= (1 << maxcpu % 8) - 1; + } + } + + /* if vcpupin setting exists, there are unused physical cpus */ + for (n = 0; n < targetDef->cputune.nvcpupin; n++) { + vcpupin_list = targetDef->cputune.vcpupin; + vcpu = vcpupin_list[n]->vcpuid; + cpumask = vcpupin_list[n]->cpumask; + cpumap = VIR_GET_CPUMAP(cpumaps, maplen, vcpu); + for (pcpu = 0; pcpu < maxcpu; pcpu++) { + if (cpumask[pcpu] == 0) + VIR_UNUSE_CPU(cpumap, pcpu); + } + } + ret = maxinfo; + +cleanup: + if (vm) + virDomainObjUnlock(vm); + return ret; +} + +static int qemudDomainGetVcpus(virDomainPtr dom, virVcpuInfoPtr info, int maxinfo, @@ -8358,6 +8469,7 @@ static virDriver qemuDriver = { .domainGetVcpusFlags = qemudDomainGetVcpusFlags, /* 0.8.5 */ .domainPinVcpu = qemudDomainPinVcpu, /* 0.4.4 */ .domainPinVcpuFlags = qemudDomainPinVcpuFlags, /* 0.9.3 */ + .domainGetVcpupinInfo = qemudDomainGetVcpupinInfo, /* 0.9.3 */ .domainGetVcpus = qemudDomainGetVcpus, /* 0.4.4 */ .domainGetMaxVcpus = qemudDomainGetMaxVcpus, /* 0.4.4 */ .domainGetSecurityLabel = qemudDomainGetSecurityLabel, /* 0.6.1 */

On 06/24/2011 02:57 AM, Taku Izumi wrote:
This patch implements the code to address the new API (virDomainGetVcpupinInfo) in the qemu driver.
Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com> --- src/qemu/qemu_driver.c | 112 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 112 insertions(+)
Index: libvirt/src/qemu/qemu_driver.c =================================================================== --- libvirt.orig/src/qemu/qemu_driver.c +++ libvirt/src/qemu/qemu_driver.c @@ -3088,6 +3088,117 @@ qemudDomainPinVcpu(virDomainPtr dom, }
static int +qemudDomainGetVcpupinInfo(virDomainPtr dom, + int maxinfo,
Same name change as in patch 1/5. ACK; here's what I'm squashing in. diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c index c156ea2..d9099d5 100644 --- i/src/qemu/qemu_driver.c +++ w/src/qemu/qemu_driver.c @@ -3167,7 +3167,7 @@ qemudDomainPinVcpu(virDomainPtr dom, static int qemudDomainGetVcpupinInfo(virDomainPtr dom, - int maxinfo, + int ncpumaps, unsigned char *cpumaps, int maplen, unsigned int flags) { @@ -3241,17 +3241,17 @@ qemudDomainGetVcpupinInfo(virDomainPtr dom, maxcpu = hostcpus; /* Clamp to actual number of vcpus */ - if (maxinfo > targetDef->vcpus) - maxinfo = targetDef->vcpus; + if (ncpumaps > targetDef->vcpus) + ncpumaps = targetDef->vcpus; - if (maxinfo < 1) { + if (ncpumaps < 1) { goto cleanup; } /* initialize cpumaps */ - memset(cpumaps, 0xff, maplen * maxinfo); + memset(cpumaps, 0xff, maplen * ncpumaps); if (maxcpu % 8) { - for (vcpu = 0; vcpu < maxinfo; vcpu++) { + for (vcpu = 0; vcpu < ncpumaps; vcpu++) { cpumap = VIR_GET_CPUMAP(cpumaps, maplen, vcpu); cpumap[maplen - 1] &= (1 << maxcpu % 8) - 1; } @@ -3268,7 +3268,7 @@ qemudDomainGetVcpupinInfo(virDomainPtr dom, VIR_UNUSE_CPU(cpumap, pcpu); } } - ret = maxinfo; + ret = ncpumaps; cleanup: if (vm) -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

This patch implements the remote protocol to address the new API (virDomainGetVcpupinInfo). Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com> --- daemon/remote.c | 68 ++++++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 73 +++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 15 ++++++++ src/remote_protocol-structs | 13 +++++++ 4 files changed, 168 insertions(+), 1 deletion(-) Index: libvirt/src/remote/remote_protocol.x =================================================================== --- libvirt.orig/src/remote/remote_protocol.x +++ libvirt/src/remote/remote_protocol.x @@ -903,6 +903,18 @@ struct remote_domain_pin_vcpu_flags_args unsigned int flags; }; +struct remote_domain_get_vcpupin_info_args { + remote_nonnull_domain dom; + int maxinfo; + int maplen; + unsigned int flags; +}; + +struct remote_domain_get_vcpupin_info_ret { + opaque cpumaps<REMOTE_CPUMAPS_MAX>; + int num; +}; + struct remote_domain_get_vcpus_args { remote_nonnull_domain dom; int maxinfo; @@ -2425,7 +2437,8 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_BLOCK_PULL_ABORT = 231, /* autogen autogen */ REMOTE_PROC_DOMAIN_GET_BLOCK_PULL_INFO = 232, /* autogen autogen */ REMOTE_PROC_DOMAIN_EVENT_BLOCK_PULL = 233, /* skipgen skipgen */ - REMOTE_PROC_DOMAIN_GET_CONTROL_INFO = 234 /* autogen autogen */ + REMOTE_PROC_DOMAIN_GET_CONTROL_INFO = 234, /* autogen autogen */ + REMOTE_PROC_DOMAIN_GET_VCPUPIN_INFO = 235 /* skipgen skipgen */ /* * Notice how the entries are grouped in sets of 10 ? Index: libvirt/src/remote/remote_driver.c =================================================================== --- libvirt.orig/src/remote/remote_driver.c +++ libvirt/src/remote/remote_driver.c @@ -2141,6 +2141,78 @@ done: } static int +remoteDomainGetVcpupinInfo (virDomainPtr domain, + int maxinfo, + unsigned char *cpumaps, + int maplen, + unsigned int flags) +{ + int rv = -1; + int i; + remote_domain_get_vcpupin_info_args args; + remote_domain_get_vcpupin_info_ret ret; + struct private_data *priv = domain->conn->privateData; + + remoteDriverLock(priv); + + if (maxinfo > REMOTE_VCPUINFO_MAX) { + remoteError(VIR_ERR_RPC, + _("vCPU count exceeds maximum: %d > %d"), + maxinfo, REMOTE_VCPUINFO_MAX); + goto done; + } + + if (maxinfo * maplen > REMOTE_CPUMAPS_MAX) { + remoteError(VIR_ERR_RPC, + _("vCPU map buffer length exceeds maximum: %d > %d"), + maxinfo * maplen, REMOTE_CPUMAPS_MAX); + goto done; + } + + make_nonnull_domain (&args.dom, domain); + args.maxinfo = maxinfo; + args.maplen = maplen; + args.flags = flags; + + memset (&ret, 0, sizeof ret); + + if (call (domain->conn, priv, 0, REMOTE_PROC_DOMAIN_GET_VCPUPIN_INFO, + (xdrproc_t) xdr_remote_domain_get_vcpupin_info_args, + (char *) &args, + (xdrproc_t) xdr_remote_domain_get_vcpupin_info_ret, + (char *) &ret) == -1) + goto done; + + if (ret.num > maxinfo) { + remoteError(VIR_ERR_RPC, + _("host reports too many vCPUs: %d > %d"), + ret.num, maxinfo); + goto cleanup; + } + + if (ret.cpumaps.cpumaps_len > maxinfo * maplen) { + remoteError(VIR_ERR_RPC, + _("host reports map buffer length exceeds maximum: %d > %d"), + ret.cpumaps.cpumaps_len, maxinfo * maplen); + goto cleanup; + } + + memset (cpumaps, 0, maxinfo * maplen); + + for (i = 0; i < ret.cpumaps.cpumaps_len; ++i) + cpumaps[i] = ret.cpumaps.cpumaps_val[i]; + + rv = ret.num; + +cleanup: + xdr_free ((xdrproc_t) xdr_remote_domain_get_vcpupin_info_ret, (char *) &ret); + +done: + remoteDriverUnlock(priv); + return rv; +} + +static int remoteDomainGetVcpus (virDomainPtr domain, virVcpuInfoPtr info, int maxinfo, @@ -6452,6 +6524,7 @@ static virDriver remote_driver = { .domainGetVcpusFlags = remoteDomainGetVcpusFlags, /* 0.8.5 */ .domainPinVcpu = remoteDomainPinVcpu, /* 0.3.0 */ .domainPinVcpuFlags = remoteDomainPinVcpuFlags, /* 0.9.3 */ + .domainGetVcpupinInfo = remoteDomainGetVcpupinInfo, /* 0.9.3 */ .domainGetVcpus = remoteDomainGetVcpus, /* 0.3.0 */ .domainGetMaxVcpus = remoteDomainGetMaxVcpus, /* 0.3.0 */ .domainGetSecurityLabel = remoteDomainGetSecurityLabel, /* 0.6.1 */ Index: libvirt/src/remote_protocol-structs =================================================================== --- libvirt.orig/src/remote_protocol-structs +++ libvirt/src/remote_protocol-structs @@ -580,6 +580,19 @@ struct remote_domain_pin_vcpu_flags_args } cpumap; u_int flags; }; +struct remote_domain_get_vcpupin_info_args { + remote_nonnull_domain dom; + int maxinfo; + int maplen; + u_int flags; +}; +struct remote_domain_get_vcpupin_info_ret { + struct { + u_int cpumaps_len; + char * cpumaps_val; + } cpumaps; + int num; +}; struct remote_domain_get_vcpus_args { remote_nonnull_domain dom; int maxinfo; Index: libvirt/daemon/remote.c =================================================================== --- libvirt.orig/daemon/remote.c +++ libvirt/daemon/remote.c @@ -1079,6 +1079,74 @@ cleanup: } static int +remoteDispatchDomainGetVcpupinInfo(struct qemud_server *server ATTRIBUTE_UNUSED, + struct qemud_client *client ATTRIBUTE_UNUSED, + virConnectPtr conn, + remote_message_header *hdr ATTRIBUTE_UNUSED, + remote_error *rerr, + remote_domain_get_vcpupin_info_args *args, + remote_domain_get_vcpupin_info_ret *ret) +{ + virDomainPtr dom = NULL; + unsigned char *cpumaps = NULL; + int num; + int rv = -1; + + if (!conn) { + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (!(dom = get_nonnull_domain(conn, args->dom))) + goto cleanup; + + if (args->maxinfo > REMOTE_VCPUINFO_MAX) { + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("maxinfo > REMOTE_VCPUINFO_MAX")); + goto cleanup; + } + + if (args->maxinfo * args->maplen > REMOTE_CPUMAPS_MAX) { + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("maxinfo * maplen > REMOTE_CPUMAPS_MAX")); + goto cleanup; + } + + /* Allocate buffers to take the results. */ + if (args->maplen > 0 && + VIR_ALLOC_N(cpumaps, args->maxinfo * args->maplen) < 0) + goto no_memory; + + if ((num = virDomainGetVcpupinInfo(dom, + args->maxinfo, + cpumaps, + args->maplen, + args->flags)) < 0) + goto cleanup; + + ret->num = num; + /* Don't need to allocate/copy the cpumaps if we make the reasonabl + * assumption that unsigned char and char are the same size. + * Note that remoteDispatchClientRequest will free. + */ + ret->cpumaps.cpumaps_len = args->maxinfo * args->maplen; + ret->cpumaps.cpumaps_val = (char *) cpumaps; + cpumaps = NULL; + + rv = 0; + +cleanup: + if (rv < 0) + remoteDispatchError(rerr); + VIR_FREE(cpumaps); + if (dom) + virDomainFree(dom); + return rv; + +no_memory: + virReportOOMError(); + goto cleanup; +} + +static int remoteDispatchDomainGetVcpus(struct qemud_server *server ATTRIBUTE_UNUSED, struct qemud_client *client ATTRIBUTE_UNUSED, virConnectPtr conn,

On 06/24/2011 03:00 AM, Taku Izumi wrote:
This patch implements the remote protocol to address the new API (virDomainGetVcpupinInfo).
Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com> --- daemon/remote.c | 68 ++++++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 73 +++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 15 ++++++++ src/remote_protocol-structs | 13 +++++++ 4 files changed, 168 insertions(+), 1 deletion(-)
Index: libvirt/src/remote/remote_protocol.x =================================================================== --- libvirt.orig/src/remote/remote_protocol.x +++ libvirt/src/remote/remote_protocol.x @@ -903,6 +903,18 @@ struct remote_domain_pin_vcpu_flags_args unsigned int flags; };
+struct remote_domain_get_vcpupin_info_args { + remote_nonnull_domain dom; + int maxinfo;
Same renaming.
@@ -2425,7 +2437,8 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_BLOCK_PULL_ABORT = 231, /* autogen autogen */ REMOTE_PROC_DOMAIN_GET_BLOCK_PULL_INFO = 232, /* autogen autogen */ REMOTE_PROC_DOMAIN_EVENT_BLOCK_PULL = 233, /* skipgen skipgen */ - REMOTE_PROC_DOMAIN_GET_CONTROL_INFO = 234 /* autogen autogen */ + REMOTE_PROC_DOMAIN_GET_CONTROL_INFO = 234, /* autogen autogen */ + REMOTE_PROC_DOMAIN_GET_VCPUPIN_INFO = 235 /* skipgen skipgen */
Minor merge conflict; easy enough to fix.
/* * Notice how the entries are grouped in sets of 10 ? Index: libvirt/src/remote/remote_driver.c =================================================================== --- libvirt.orig/src/remote/remote_driver.c +++ libvirt/src/remote/remote_driver.c @@ -2141,6 +2141,78 @@ done: }
static int +remoteDomainGetVcpupinInfo (virDomainPtr domain, + int maxinfo,
More renaming.
+ unsigned char *cpumaps, + int maplen, + unsigned int flags) +{ + int rv = -1; + int i; + remote_domain_get_vcpupin_info_args args; + remote_domain_get_vcpupin_info_ret ret; + struct private_data *priv = domain->conn->privateData; + + remoteDriverLock(priv); + + if (maxinfo > REMOTE_VCPUINFO_MAX) { + remoteError(VIR_ERR_RPC, + _("vCPU count exceeds maximum: %d > %d"), + maxinfo, REMOTE_VCPUINFO_MAX); + goto done; + } + + if (maxinfo * maplen > REMOTE_CPUMAPS_MAX) {
Ouch. Potential for integer overflow causing wraparound that we don't detect. But no different than what you copied from, so I'll propose a separate patch.
+ + ret->num = num; + /* Don't need to allocate/copy the cpumaps if we make the reasonabl
typo
+ * assumption that unsigned char and char are the same size.
And in response to the comment, C99 requires that 'char' and 'unsigned char' both be the same size - exactly 1. ACK; here's what I'm squashing in: diff --git i/daemon/remote.c w/daemon/remote.c index 2f3a3e7..27d5dea 100644 --- i/daemon/remote.c +++ w/daemon/remote.c @@ -1068,34 +1068,34 @@ remoteDispatchDomainGetVcpupinInfo(struct qemud_server *server ATTRIBUTE_UNUSED, if (!(dom = get_nonnull_domain(conn, args->dom))) goto cleanup; - if (args->maxinfo > REMOTE_VCPUINFO_MAX) { - virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("maxinfo > REMOTE_VCPUINFO_MAX")); + if (args->ncpumaps > REMOTE_VCPUINFO_MAX) { + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("ncpumaps > REMOTE_VCPUINFO_MAX")); goto cleanup; } - if (args->maxinfo * args->maplen > REMOTE_CPUMAPS_MAX) { + if (args->ncpumaps * args->maplen > REMOTE_CPUMAPS_MAX) { virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("maxinfo * maplen > REMOTE_CPUMAPS_MAX")); goto cleanup; } /* Allocate buffers to take the results. */ if (args->maplen > 0 && - VIR_ALLOC_N(cpumaps, args->maxinfo * args->maplen) < 0) + VIR_ALLOC_N(cpumaps, args->ncpumaps * args->maplen) < 0) goto no_memory; if ((num = virDomainGetVcpupinInfo(dom, - args->maxinfo, + args->ncpumaps, cpumaps, args->maplen, args->flags)) < 0) goto cleanup; ret->num = num; - /* Don't need to allocate/copy the cpumaps if we make the reasonabl + /* Don't need to allocate/copy the cpumaps if we make the reasonable * assumption that unsigned char and char are the same size. * Note that remoteDispatchClientRequest will free. */ - ret->cpumaps.cpumaps_len = args->maxinfo * args->maplen; + ret->cpumaps.cpumaps_len = args->ncpumaps * args->maplen; ret->cpumaps.cpumaps_val = (char *) cpumaps; cpumaps = NULL; diff --git i/src/remote/remote_driver.c w/src/remote/remote_driver.c index d6b5c5d..8ddb2c9 100644 --- i/src/remote/remote_driver.c +++ w/src/remote/remote_driver.c @@ -2142,7 +2142,7 @@ done: static int remoteDomainGetVcpupinInfo (virDomainPtr domain, - int maxinfo, + int ncpumaps, unsigned char *cpumaps, int maplen, unsigned int flags) @@ -2155,22 +2155,22 @@ remoteDomainGetVcpupinInfo (virDomainPtr domain, remoteDriverLock(priv); - if (maxinfo > REMOTE_VCPUINFO_MAX) { + if (ncpumaps > REMOTE_VCPUINFO_MAX) { remoteError(VIR_ERR_RPC, _("vCPU count exceeds maximum: %d > %d"), - maxinfo, REMOTE_VCPUINFO_MAX); + ncpumaps, REMOTE_VCPUINFO_MAX); goto done; } - if (maxinfo * maplen > REMOTE_CPUMAPS_MAX) { + if (ncpumaps * maplen > REMOTE_CPUMAPS_MAX) { remoteError(VIR_ERR_RPC, _("vCPU map buffer length exceeds maximum: %d > %d"), - maxinfo * maplen, REMOTE_CPUMAPS_MAX); + ncpumaps * maplen, REMOTE_CPUMAPS_MAX); goto done; } make_nonnull_domain (&args.dom, domain); - args.maxinfo = maxinfo; + args.ncpumaps = ncpumaps; args.maplen = maplen; args.flags = flags; @@ -2183,21 +2183,21 @@ remoteDomainGetVcpupinInfo (virDomainPtr domain, (char *) &ret) == -1) goto done; - if (ret.num > maxinfo) { + if (ret.num > ncpumaps) { remoteError(VIR_ERR_RPC, _("host reports too many vCPUs: %d > %d"), - ret.num, maxinfo); + ret.num, ncpumaps); goto cleanup; } - if (ret.cpumaps.cpumaps_len > maxinfo * maplen) { + if (ret.cpumaps.cpumaps_len > ncpumaps * maplen) { remoteError(VIR_ERR_RPC, _("host reports map buffer length exceeds maximum: %d > %d"), - ret.cpumaps.cpumaps_len, maxinfo * maplen); + ret.cpumaps.cpumaps_len, ncpumaps * maplen); goto cleanup; } - memset (cpumaps, 0, maxinfo * maplen); + memset (cpumaps, 0, ncpumaps * maplen); for (i = 0; i < ret.cpumaps.cpumaps_len; ++i) cpumaps[i] = ret.cpumaps.cpumaps_val[i]; diff --git i/src/remote/remote_protocol.x w/src/remote/remote_protocol.x index bb1dbe1..ee08b82 100644 --- i/src/remote/remote_protocol.x +++ w/src/remote/remote_protocol.x @@ -905,7 +905,7 @@ struct remote_domain_pin_vcpu_flags_args { struct remote_domain_get_vcpupin_info_args { remote_nonnull_domain dom; - int maxinfo; + int ncpumaps; int maplen; unsigned int flags; }; diff --git i/src/remote_protocol-structs w/src/remote_protocol-structs index aed5c32..be98135 100644 --- i/src/remote_protocol-structs +++ w/src/remote_protocol-structs @@ -582,7 +582,7 @@ struct remote_domain_pin_vcpu_flags_args { }; struct remote_domain_get_vcpupin_info_args { remote_nonnull_domain dom; - int maxinfo; + int ncpumaps; int maplen; u_int flags; }; -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

The "virsh vcpuinfo" command results in failure when the target domain is inactive on KVM. This patch improves this behavior by adding the fallback to invoke virDomainGetVcpupinInfo API in case of virDomainGetVcpus API returns error and the target domain is inactive. Signd-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com> --- tools/virsh.c | 48 ++++++++++++++++++++++++++++++++++-------------- 1 file changed, 34 insertions(+), 14 deletions(-) Index: libvirt/tools/virsh.c =================================================================== --- libvirt.orig/tools/virsh.c +++ libvirt/tools/virsh.c @@ -2903,10 +2903,11 @@ cmdVcpuinfo(vshControl *ctl, const vshCm virDomainPtr dom; virNodeInfo nodeinfo; virVcpuInfoPtr cpuinfo; - unsigned char *cpumap; - int ncpus; + unsigned char *cpumaps; + int ncpus, maxcpu; size_t cpumaplen; bool ret = true; + int n, m; if (!vshConnectionUsability(ctl, ctl->conn)) return false; @@ -2925,15 +2926,14 @@ cmdVcpuinfo(vshControl *ctl, const vshCm } cpuinfo = vshMalloc(ctl, sizeof(virVcpuInfo)*info.nrVirtCpu); - cpumaplen = VIR_CPU_MAPLEN(VIR_NODEINFO_MAXCPUS(nodeinfo)); - cpumap = vshMalloc(ctl, info.nrVirtCpu * cpumaplen); + maxcpu = VIR_NODEINFO_MAXCPUS(nodeinfo); + cpumaplen = VIR_CPU_MAPLEN(maxcpu); + cpumaps = vshMalloc(ctl, info.nrVirtCpu * cpumaplen); if ((ncpus = virDomainGetVcpus(dom, cpuinfo, info.nrVirtCpu, - cpumap, cpumaplen)) >= 0) { - int n; + cpumaps, cpumaplen)) >= 0) { for (n = 0 ; n < ncpus ; n++) { - unsigned int m; vshPrint(ctl, "%-15s %d\n", _("VCPU:"), n); vshPrint(ctl, "%-15s %d\n", _("CPU:"), cpuinfo[n].cpu); vshPrint(ctl, "%-15s %s\n", _("State:"), @@ -2946,8 +2946,8 @@ cmdVcpuinfo(vshControl *ctl, const vshCm vshPrint(ctl, "%-15s %.1lfs\n", _("CPU time:"), cpuUsed); } vshPrint(ctl, "%-15s ", _("CPU Affinity:")); - for (m = 0 ; m < VIR_NODEINFO_MAXCPUS(nodeinfo) ; m++) { - vshPrint(ctl, "%c", VIR_CPU_USABLE(cpumap, cpumaplen, n, m) ? 'y' : '-'); + for (m = 0; m < maxcpu; m++) { + vshPrint(ctl, "%c", VIR_CPU_USABLE(cpumaps, cpumaplen, n, m) ? 'y' : '-'); } vshPrint(ctl, "\n"); if (n < (ncpus - 1)) { @@ -2955,14 +2955,34 @@ cmdVcpuinfo(vshControl *ctl, const vshCm } } } else { - if (info.state == VIR_DOMAIN_SHUTOFF) { - vshError(ctl, "%s", - _("Domain shut off, virtual CPUs not present.")); + if (info.state == VIR_DOMAIN_SHUTOFF && + (ncpus = virDomainGetVcpupinInfo(dom, info.nrVirtCpu, + cpumaps, cpumaplen, + VIR_DOMAIN_AFFECT_CONFIG)) >= 0) { + + /* fallback plan to use virDomainGetVcpupinInfo */ + + for (n = 0; n < ncpus; n++) { + vshPrint(ctl, "%-15s %d\n", _("VCPU:"), n); + vshPrint(ctl, "%-15s %s\n", _("CPU:"), _("N/A")); + vshPrint(ctl, "%-15s %s\n", _("State:"), _("N/A")); + vshPrint(ctl, "%-15s %s\n", _("CPU time"), _("N/A")); + vshPrint(ctl, "%-15s ", _("CPU Affinity:")); + for (m = 0; m < maxcpu; m++) { + vshPrint(ctl, "%c", + VIR_CPU_USABLE(cpumaps, cpumaplen, n, m) ? 'y' : '-'); + } + vshPrint(ctl, "\n"); + if (n < (ncpus - 1)) { + vshPrint(ctl, "\n"); + } + } + } else { + ret = false; } - ret = false; } - VIR_FREE(cpumap); + VIR_FREE(cpumaps); VIR_FREE(cpuinfo); virDomainFree(dom); return ret;

On 06/24/2011 03:01 AM, Taku Izumi wrote:
The "virsh vcpuinfo" command results in failure when the target domain is inactive on KVM. This patch improves this behavior by adding the fallback to invoke virDomainGetVcpupinInfo API in case of virDomainGetVcpus API returns error and the target domain is inactive.
Signd-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
s/Signd/Signed/ [unlike the kernel folks, we aren't sticklers about S-O-B lines on this list, but if you use it, you might as well use it correctly ;)] ACK as-is!
+ if (info.state == VIR_DOMAIN_SHUTOFF && + (ncpus = virDomainGetVcpupinInfo(dom, info.nrVirtCpu,
Well, there's the s/Vcpupin/VcpuPin/ change coming up later, but it has wider-reaching effects, so I'll leave it separate. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

This patch adds --query option to "virsh vcpupin" command. Its feature is to show CPU affnity information in more reader-friendly way. # virsh vcpupin VM --query --config VCPU: CPU Affinity ---------------------------------- 0: 1-6,9-20 1: 10 2: 5,9-11,15-20 3: 1,3,5,7,9,11,13,15 When --query is specified, cpulist is not required and vcpu number is optional. When vcpu number is provided, information of only specified vcpu is displayed. Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com> --- tools/virsh.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++-------- tools/virsh.pod | 11 +++++- 2 files changed, 93 insertions(+), 15 deletions(-) Index: libvirt/tools/virsh.c =================================================================== --- libvirt.orig/tools/virsh.c +++ libvirt/tools/virsh.c @@ -2992,15 +2992,16 @@ cmdVcpuinfo(vshControl *ctl, const vshCm * "vcpupin" command */ static const vshCmdInfo info_vcpupin[] = { - {"help", N_("control domain vcpu affinity")}, + {"help", N_("control or query domain vcpu affinity")}, {"desc", N_("Pin domain VCPUs to host physical CPUs.")}, {NULL, NULL} }; static const vshCmdOptDef opts_vcpupin[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, - {"vcpu", VSH_OT_INT, VSH_OFLAG_REQ, N_("vcpu number")}, - {"cpulist", VSH_OT_DATA, VSH_OFLAG_REQ, N_("host cpu number(s) (comma separated)")}, + {"vcpu", VSH_OT_INT, 0, N_("vcpu number")}, + {"cpulist", VSH_OT_DATA, VSH_OFLAG_EMPTY_OK, N_("host cpu number(s)")}, + {"query", VSH_OT_BOOL, 0, N_("query CPU affinitiy information")}, {"config", VSH_OT_BOOL, 0, N_("affect next boot")}, {"live", VSH_OT_BOOL, 0, N_("affect running domain")}, {"current", VSH_OT_BOOL, 0, N_("affect current domain")}, @@ -3013,14 +3014,18 @@ cmdVcpupin(vshControl *ctl, const vshCmd virDomainInfo info; virDomainPtr dom; virNodeInfo nodeinfo; - int vcpu; + int vcpu = -1; const char *cpulist = NULL; bool ret = true; - unsigned char *cpumap; + unsigned char *cpumap = NULL; + unsigned char *cpumaps = NULL; int cpumaplen; - int i, cpu, lastcpu, maxcpu; + int bit, lastbit; + bool isInvert; + int i, cpu, lastcpu, maxcpu, ncpus; bool unuse = false; const char *cur; + int query = vshCommandOptBool(cmd, "query"); int config = vshCommandOptBool(cmd, "config"); int live = vshCommandOptBool(cmd, "live"); int current = vshCommandOptBool(cmd, "current"); @@ -3049,14 +3054,22 @@ cmdVcpupin(vshControl *ctl, const vshCmd return false; if (vshCommandOptInt(cmd, "vcpu", &vcpu) <= 0) { - vshError(ctl, "%s", _("vcpupin: Invalid or missing vCPU number.")); - virDomainFree(dom); - return false; + /* When query mode, "vcpu" is optional */ + if (!query) { + vshError(ctl, "%s", + _("vcpupin: Invalid or missing vCPU number.")); + virDomainFree(dom); + return false; + } } if (vshCommandOptString(cmd, "cpulist", &cpulist) <= 0) { - virDomainFree(dom); - return false; + /* When query mode, "cpulist" is optional */ + if (!query) { + vshError(ctl, "%s", _("vcpupin: Missing cpulist.")); + virDomainFree(dom); + return false; + } } if (virNodeGetInfo(ctl->conn, &nodeinfo) != 0) { @@ -3078,8 +3091,65 @@ cmdVcpupin(vshControl *ctl, const vshCmd maxcpu = VIR_NODEINFO_MAXCPUS(nodeinfo); cpumaplen = VIR_CPU_MAPLEN(maxcpu); - cpumap = vshCalloc(ctl, 0, cpumaplen); + /* Query mode: show CPU affinity information then exit.*/ + if (query) { + /* When query mode and neither "live", "config" nor "curent" is specified, + * set VIR_DOMAIN_AFFECT_CURRENT as flags */ + if (flags == -1) + flags = VIR_DOMAIN_AFFECT_CURRENT; + + cpumaps = vshMalloc(ctl, info.nrVirtCpu * cpumaplen); + if ((ncpus = virDomainGetVcpupinInfo(dom, info.nrVirtCpu, + cpumaps, cpumaplen, flags)) >= 0) { + + vshPrint(ctl, "%s %s\n", _("VCPU:"), _("CPU Affinity")); + vshPrint(ctl, "----------------------------------\n"); + for (i = 0; i < ncpus; i++) { + + if (vcpu != -1 && i != vcpu) + continue; + + bit = lastbit = 0; + isInvert = false; + lastcpu = -1; + + vshPrint(ctl, "%4d: ", i); + for (cpu = 0; cpu < maxcpu; cpu++) { + + if (VIR_CPU_USABLE(cpumaps, cpumaplen, i, cpu)) + bit = 1; + else + bit = 0; + + isInvert = (bit ^ lastbit) ? true : false; + if (bit && isInvert) { + if (lastcpu == -1) + vshPrint(ctl, "%d", cpu); + else + vshPrint(ctl, ",%d", cpu); + lastcpu = cpu; + } + if (!bit && isInvert && lastcpu != cpu - 1) + vshPrint(ctl, "-%d", cpu - 1); + lastbit = bit; + } + if (bit && !isInvert) { + vshPrint(ctl, "-%d", maxcpu - 1); + } + vshPrint(ctl, "\n"); + } + + } else { + ret = false; + } + VIR_FREE(cpumaps); + goto cleanup; + } + + /* Pin mode: pinning specified vcpu to specified physical cpus*/ + + cpumap = vshCalloc(ctl, 0, cpumaplen); /* Parse cpulist */ cur = cpulist; if (*cur == 0) { @@ -3161,7 +3231,8 @@ cmdVcpupin(vshControl *ctl, const vshCmd } cleanup: - VIR_FREE(cpumap); + if (cpumap) + VIR_FREE(cpumap); virDomainFree(dom); return ret; Index: libvirt/tools/virsh.pod =================================================================== --- libvirt.orig/tools/virsh.pod +++ libvirt/tools/virsh.pod @@ -838,8 +838,15 @@ vCPUs, the running time, the affinity to =item B<vcpupin> I<domain-id> I<vcpu> I<cpulist> optional I<--live> I<--config> I<--current> -Pin domain VCPUs to host physical CPUs. The I<vcpu> number must be provided -and I<cpulist> is a list of physical CPU numbers. Its syntax is a comma +=item B<vcpupin> I<domain-id> I<--query> optional I<vcpu> I<--live> I<--config> +I<--current> + +Pin domain VCPUs to host physical CPUs, or query CPU affinity information +(specify I<--query>). When pinning vCPU, the I<vcpu> number and I<cpulist> must +be provided. When querrying affinity information, I<cpulist> is not required +and I<vcpu> is optional. + +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 vcpupin setting, that is, to pin vcpu all physical cpus,

On 06/24/2011 03:02 AM, Taku Izumi wrote:
This patch adds --query option to "virsh vcpupin" command. Its feature is to show CPU affnity information in more
s/affnity/affinity/
reader-friendly way.
# virsh vcpupin VM --query --config VCPU: CPU Affinity ---------------------------------- 0: 1-6,9-20 1: 10 2: 5,9-11,15-20 3: 1,3,5,7,9,11,13,15
While it would be really cool to see strings like 0-15,^8, I don't think it's worth the complexity of trying to figure that out. Your representation looks nice enough, even if we parse more strings than we generate.
When --query is specified, cpulist is not required and vcpu number is optional. When vcpu number is provided, information of only specified vcpu is displayed.
But why do we need --query? If cpulist and --query are orthogonal, why not just make the absence of a cpulist imply query, without having it be an explicit option?
Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com> --- tools/virsh.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++-------- tools/virsh.pod | 11 +++++- 2 files changed, 93 insertions(+), 15 deletions(-)
Index: libvirt/tools/virsh.c =================================================================== --- libvirt.orig/tools/virsh.c +++ libvirt/tools/virsh.c @@ -2992,15 +2992,16 @@ cmdVcpuinfo(vshControl *ctl, const vshCm * "vcpupin" command */ static const vshCmdInfo info_vcpupin[] = { - {"help", N_("control domain vcpu affinity")}, + {"help", N_("control or query domain vcpu affinity")}, {"desc", N_("Pin domain VCPUs to host physical CPUs.")}, {NULL, NULL} };
static const vshCmdOptDef opts_vcpupin[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, - {"vcpu", VSH_OT_INT, VSH_OFLAG_REQ, N_("vcpu number")}, - {"cpulist", VSH_OT_DATA, VSH_OFLAG_REQ, N_("host cpu number(s) (comma separated)")}, + {"vcpu", VSH_OT_INT, 0, N_("vcpu number")}, + {"cpulist", VSH_OT_DATA, VSH_OFLAG_EMPTY_OK, N_("host cpu number(s)")},
Why is empty okay? That implies that: virsh vcpupin dom 0 '' is valid. Is that shorthand for undoing affinity (basically, restoring that vcpu to use all possible host cpus)? Makes sense... </me reads ahead...> Indeed that's what the code does, but it means a tweak to virsh.pod.
+ {"query", VSH_OT_BOOL, 0, N_("query CPU affinitiy information")},
s/affinitiy/affinity/
@@ -3049,14 +3054,22 @@ cmdVcpupin(vshControl *ctl, const vshCmd return false;
if (vshCommandOptInt(cmd, "vcpu", &vcpu) <= 0) { - vshError(ctl, "%s", _("vcpupin: Invalid or missing vCPU number.")); - virDomainFree(dom); - return false; + /* When query mode, "vcpu" is optional */ + if (!query) {
Not quite the right logic. vshCommandOptInt() < 0 is always an error, and vshCommandOptInt()==0 is an error if !query.
+ vshError(ctl, "%s", + _("vcpupin: Invalid or missing vCPU number.")); + virDomainFree(dom); + return false; + } }
if (vshCommandOptString(cmd, "cpulist", &cpulist) <= 0) { - virDomainFree(dom); - return false; + /* When query mode, "cpulist" is optional */ + if (!query) {
Also not the right logic - --query and --cpulist are mutually exclusive.
@@ -3078,8 +3091,65 @@ cmdVcpupin(vshControl *ctl, const vshCmd
maxcpu = VIR_NODEINFO_MAXCPUS(nodeinfo); cpumaplen = VIR_CPU_MAPLEN(maxcpu); - cpumap = vshCalloc(ctl, 0, cpumaplen);
+ /* Query mode: show CPU affinity information then exit.*/ + if (query) { + /* When query mode and neither "live", "config" nor "curent" is specified,
s/curent/current/, and long line
+ vshPrint(ctl, "%4d: ", i); + for (cpu = 0; cpu < maxcpu; cpu++) { + + if (VIR_CPU_USABLE(cpumaps, cpumaplen, i, cpu))
Indentation.
+ bit = 1; + else + bit = 0;
Can be made shorter by making 'bit' a bool.
+ + isInvert = (bit ^ lastbit) ? true : false;
Likewise.
cleanup: - VIR_FREE(cpumap); + if (cpumap) + VIR_FREE(cpumap);
'make syntax-check' calls you on this one. This change is bogus, since VIR_FREE is a safe no-op on NULL.
-Pin domain VCPUs to host physical CPUs. The I<vcpu> number must be provided -and I<cpulist> is a list of physical CPU numbers. Its syntax is a comma +=item B<vcpupin> I<domain-id> I<--query> optional I<vcpu> I<--live> I<--config> +I<--current>
No need for a dual synopsis form once we get rid of --query.
+ +Pin domain VCPUs to host physical CPUs, or query CPU affinity information +(specify I<--query>). When pinning vCPU, the I<vcpu> number and I<cpulist> must +be provided. When querrying affinity information, I<cpulist> is not required
s/querrying/querying/
+and I<vcpu> is optional. + +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 vcpupin setting, that is, to pin vcpu all physical cpus,
ACK with this squashed in, so I've pushed the series. diff --git i/tools/virsh.c w/tools/virsh.c index 508d97d..31fbeb7 100644 --- i/tools/virsh.c +++ w/tools/virsh.c @@ -3006,8 +3006,8 @@ static const vshCmdInfo info_vcpupin[] = { static const vshCmdOptDef opts_vcpupin[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, {"vcpu", VSH_OT_INT, 0, N_("vcpu number")}, - {"cpulist", VSH_OT_DATA, VSH_OFLAG_EMPTY_OK, N_("host cpu number(s)")}, - {"query", VSH_OT_BOOL, 0, N_("query CPU affinitiy information")}, + {"cpulist", VSH_OT_DATA, VSH_OFLAG_EMPTY_OK, + N_("host cpu number(s) to set, or omit option to query")}, {"config", VSH_OT_BOOL, 0, N_("affect next boot")}, {"live", VSH_OT_BOOL, 0, N_("affect running domain")}, {"current", VSH_OT_BOOL, 0, N_("affect current domain")}, @@ -3026,15 +3026,14 @@ cmdVcpupin(vshControl *ctl, const vshCmd *cmd) unsigned char *cpumap = NULL; unsigned char *cpumaps = NULL; int cpumaplen; - int bit, lastbit; - bool isInvert; + bool bit, lastbit, isInvert; int i, cpu, lastcpu, maxcpu, ncpus; bool unuse = false; const char *cur; - int query = vshCommandOptBool(cmd, "query"); int config = vshCommandOptBool(cmd, "config"); int live = vshCommandOptBool(cmd, "live"); int current = vshCommandOptBool(cmd, "current"); + bool query = false; /* Query mode if no cpulist */ int flags = 0; if (current) { @@ -3059,23 +3058,19 @@ cmdVcpupin(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptInt(cmd, "vcpu", &vcpu) <= 0) { - /* When query mode, "vcpu" is optional */ - if (!query) { - vshError(ctl, "%s", - _("vcpupin: Invalid or missing vCPU number.")); - virDomainFree(dom); - return false; - } + if (vshCommandOptString(cmd, "cpulist", &cpulist) < 0) { + vshError(ctl, "%s", _("vcpupin: Missing cpulist.")); + virDomainFree(dom); + return false; } + query = !cpulist; - if (vshCommandOptString(cmd, "cpulist", &cpulist) <= 0) { - /* When query mode, "cpulist" is optional */ - if (!query) { - vshError(ctl, "%s", _("vcpupin: Missing cpulist.")); - virDomainFree(dom); - return false; - } + /* In query mode, "vcpu" is optional */ + if (vshCommandOptInt(cmd, "vcpu", &vcpu) < !query) { + vshError(ctl, "%s", + _("vcpupin: Invalid or missing vCPU number.")); + virDomainFree(dom); + return false; } if (virNodeGetInfo(ctl->conn, &nodeinfo) != 0) { @@ -3084,7 +3079,7 @@ cmdVcpupin(vshControl *ctl, const vshCmd *cmd) } if (virDomainGetInfo(dom, &info) != 0) { - vshError(ctl, "%s", _("vcpupin: failed to get domain informations.")); + vshError(ctl, "%s", _("vcpupin: failed to get domain information.")); virDomainFree(dom); return false; } @@ -3100,8 +3095,8 @@ cmdVcpupin(vshControl *ctl, const vshCmd *cmd) /* Query mode: show CPU affinity information then exit.*/ if (query) { - /* When query mode and neither "live", "config" nor "curent" is specified, - * set VIR_DOMAIN_AFFECT_CURRENT as flags */ + /* When query mode and neither "live", "config" nor "current" + * is specified, set VIR_DOMAIN_AFFECT_CURRENT as flags */ if (flags == -1) flags = VIR_DOMAIN_AFFECT_CURRENT; @@ -3116,29 +3111,25 @@ cmdVcpupin(vshControl *ctl, const vshCmd *cmd) if (vcpu != -1 && i != vcpu) continue; - bit = lastbit = 0; - isInvert = false; + bit = lastbit = isInvert = false; lastcpu = -1; vshPrint(ctl, "%4d: ", i); for (cpu = 0; cpu < maxcpu; cpu++) { - if (VIR_CPU_USABLE(cpumaps, cpumaplen, i, cpu)) - bit = 1; - else - bit = 0; - - isInvert = (bit ^ lastbit) ? true : false; - if (bit && isInvert) { - if (lastcpu == -1) - vshPrint(ctl, "%d", cpu); - else - vshPrint(ctl, ",%d", cpu); - lastcpu = cpu; - } - if (!bit && isInvert && lastcpu != cpu - 1) - vshPrint(ctl, "-%d", cpu - 1); - lastbit = bit; + bit = VIR_CPU_USABLE(cpumaps, cpumaplen, i, cpu); + + isInvert = (bit ^ lastbit); + if (bit && isInvert) { + if (lastcpu == -1) + vshPrint(ctl, "%d", cpu); + else + vshPrint(ctl, ",%d", cpu); + lastcpu = cpu; + } + if (!bit && isInvert && lastcpu != cpu - 1) + vshPrint(ctl, "-%d", cpu - 1); + lastbit = bit; } if (bit && !isInvert) { vshPrint(ctl, "-%d", maxcpu - 1); @@ -3237,8 +3228,7 @@ cmdVcpupin(vshControl *ctl, const vshCmd *cmd) } cleanup: - if (cpumap) - VIR_FREE(cpumap); + VIR_FREE(cpumap); virDomainFree(dom); return ret; diff --git i/tools/virsh.pod w/tools/virsh.pod index c72a960..fc06075 100644 --- i/tools/virsh.pod +++ w/tools/virsh.pod @@ -827,16 +827,12 @@ values; these two flags cannot both be specified. Returns basic information about the domain virtual CPUs, like the number of vCPUs, the running time, the affinity to physical processors. -=item B<vcpupin> I<domain-id> I<vcpu> I<cpulist> optional I<--live> I<--config> +=item B<vcpupin> I<domain-id> optional I<vcpu> I<cpulist> I<--live> I<--config> I<--current> -=item B<vcpupin> I<domain-id> I<--query> optional I<vcpu> I<--live> I<--config> -I<--current> - -Pin domain VCPUs to host physical CPUs, or query CPU affinity information -(specify I<--query>). When pinning vCPU, the I<vcpu> number and I<cpulist> must -be provided. When querrying affinity information, I<cpulist> is not required -and I<vcpu> is optional. +Query or change the pinning of domain VCPUs to host physical CPUs. To +pin a single I<vcpu>, specify I<cpulist>; otherwise, you can query one +I<vcpu> or omit I<vcpu> to list all at once. 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 @@ -846,11 +842,12 @@ simply specify 'r' as a cpulist. If I<--live> is specified, affect a running guest. If I<--config> is specified, affect the next boot of a persistent guest. If I<--current> is specified, affect the current guest state. -Both I<--live> and I<--config> flags may be given, but I<--current> is exclusive. +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 not identical -to "^8,0-15". +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<vncdisplay> I<domain-id> -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On 06/24/2011 04:56 PM, Eric Blake wrote:
When --query is specified, cpulist is not required and vcpu number is optional. When vcpu number is provided, information of only specified vcpu is displayed.
But why do we need --query? If cpulist and --query are orthogonal, why not just make the absence of a cpulist imply query, without having it be an explicit option?
For the record, I got the idea from 'virsh memtune', which is another command where omitting options performs a query. Since no other command has a --query, we didn't need to add one here. My fault for not realizing that when I first mentioned it in: http://www.redhat.com/archives/libvir-list/2011-June/msg00500.html which is why I didn't mind fixing it :) -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (4)
-
Daniel Veillard
-
Eric Blake
-
Matthias Bolte
-
Taku Izumi