12:49 p.m.
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
12:56 p.m.
New subject: [libvirt] [PATCH 1/5] vcpupin: introduce the new libvirt API (virDomainGetVcpupinInfo)
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(a)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)
6:53 p.m.
New subject: [libvirt] [PATCH 1/5] vcpupin: introduce the new libvirt API (virDomainGetVcpupinInfo)
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(a)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(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
7:12 p.m.
New subject: [libvirt] [PATCH 1/5] vcpupin: introduce the new libvirt API (virDomainGetVcpupinInfo)
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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
7:37 p.m.
New subject: [libvirt] [PATCH 1/5] vcpupin: introduce the new libvirt API (virDomainGetVcpupinInfo)
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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
8:18 p.m.
New subject: [libvirt] [PATCH 1/5] vcpupin: introduce the new libvirt API (virDomainGetVcpupinInfo)
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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
12:37 a.m.
New subject: [libvirt] [PATCH 1/5] vcpupin: introduce the new libvirt API (virDomainGetVcpupinInfo)
2011/6/24 Taku Izumi <izumi.taku(a)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
12:54 a.m.
New subject: [libvirt] [PATCH 1/5] vcpupin: introduce the new libvirt API (virDomainGetVcpupinInfo)
On 06/24/2011 02:37 PM, Matthias Bolte wrote:
2011/6/24 Taku Izumi <izumi.taku(a)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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
12:57 p.m.
New subject: [libvirt] [PATCH 2/5] vcpupin: implement the code to support new API for the qemu driver
This patch implements the code to address the new API (virDomainGetVcpupinInfo)
in the qemu driver.
Signed-off-by: Taku Izumi <izumi.taku(a)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 */
8:38 p.m.
New subject: [libvirt] [PATCH 2/5] vcpupin: implement the code to support new API for the qemu driver
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(a)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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
1 p.m.
New subject: [libvirt] [PATCH 3/5] vcpupin: implement the remote protocol to address the new API
This patch implements the remote protocol to address the new API
(virDomainGetVcpupinInfo).
Signed-off-by: Taku Izumi <izumi.taku(a)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,
9:08 p.m.
New subject: [libvirt] [PATCH 3/5] vcpupin: implement the remote protocol to address the new API
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(a)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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
1:01 p.m.
New subject: [libvirt] [PATCH 4/5] vcpuinfo: add the code to fallback to try new API
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(a)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;
1:59 a.m.
New subject: [libvirt] [PATCH 4/5] vcpuinfo: add the code to fallback to try new API
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(a)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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
1:02 p.m.
New subject: [libvirt] [PATCH 5/5] vcpupin: add query option to virsh vcpupin command
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(a)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,
2:56 a.m.
New subject: [libvirt] [PATCH 5/5] vcpupin: add query option to virsh vcpupin command
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(a)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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
3:03 a.m.
New subject: [libvirt] [PATCH 5/5] vcpupin: add query option to virsh vcpupin command
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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
5048
days inactive
5048
days old
16 comments
4 participants
participants (4)
-
Daniel Veillard -
Eric Blake -
Matthias Bolte -
Taku Izumi