10:25 a.m.
A short dump of patches that are separate enough which can be reviewed while
I'm working on other things.
Peter Krempa (6):
virsh: cmdVcpuPin: Simplify handling of API flags
util: Use virBitmapIsBitSet in freebsd impl of virProcessSetAffinity
qemu: vcpupin: Don't overwrite errors from functions setting pinning
qemu: vcpupin: Always set affinity even when cgroups are supported
qemu: qemuDomainGetStatsVcpu: Fix output for possible sparse vCPU
settings
virsh: cpupin: Extract getter code into a separate function
src/qemu/qemu_driver.c | 23 +++------
src/util/virprocess.c | 5 +-
tools/virsh-domain.c | 134 +++++++++++++++++++++++++++----------------------
3 files changed, 82 insertions(+), 80 deletions(-)
--
2.6.2
10:25 a.m.
New subject: [libvirt] [PATCH 1/6] virsh: cmdVcpuPin: Simplify handling of API flags
Rather than setting flags to -1 if none were specified, move the logic
to use the old API to the place where we need to decide. It simplifies
the logic a bit.
---
tools/virsh-domain.c | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 6dd75e2..028df1f 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -6432,9 +6432,6 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
flags |= VIR_DOMAIN_AFFECT_CONFIG;
if (live)
flags |= VIR_DOMAIN_AFFECT_LIVE;
- /* none of the options were specified */
- if (!current && !live && !config)
- flags = -1;
if (vshCommandOptStringReq(ctl, cmd, "cpulist", &cpulist) < 0)
return false;
@@ -6459,11 +6456,6 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
/* Query mode: show CPU affinity information then exit.*/
if (!cpulist) {
- /* 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;
-
if ((ncpus = virshCPUCountCollect(ctl, dom, flags, true)) < 0) {
if (ncpus == -1) {
if (flags & VIR_DOMAIN_AFFECT_LIVE)
@@ -6511,7 +6503,8 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
if (!(cpumap = virshParseCPUList(ctl, &cpumaplen, cpulist, maxcpu)))
goto cleanup;
- if (flags == -1) {
+ /* use old API without any explicit flags */
+ if (flags == 0 && !current) {
if (virDomainPinVcpu(dom, vcpu, cpumap, cpumaplen) != 0)
goto cleanup;
} else {
--
2.6.2
6:19 a.m.
New subject: [libvirt] [PATCH 1/6] virsh: cmdVcpuPin: Simplify handling of API flags
On 02/17/2016 11:25 AM, Peter Krempa wrote:
Rather than setting flags to -1 if none were specified, move the
logic
to use the old API to the place where we need to decide. It simplifies
the logic a bit.
---
tools/virsh-domain.c | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 6dd75e2..028df1f 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -6432,9 +6432,6 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
flags |= VIR_DOMAIN_AFFECT_CONFIG;
if (live)
flags |= VIR_DOMAIN_AFFECT_LIVE;
- /* none of the options were specified */
- if (!current && !live && !config)
- flags = -1;
if (vshCommandOptStringReq(ctl, cmd, "cpulist", &cpulist) < 0)
return false;
@@ -6459,11 +6456,6 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
/* Query mode: show CPU affinity information then exit.*/
if (!cpulist) {
- /* 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;
-
if ((ncpus = virshCPUCountCollect(ctl, dom, flags, true)) < 0) {
if (ncpus == -1) {
if (flags & VIR_DOMAIN_AFFECT_LIVE)
@@ -6511,7 +6503,8 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
if (!(cpumap = virshParseCPUList(ctl, &cpumaplen, cpulist, maxcpu)))
goto cleanup;
- if (flags == -1) {
+ /* use old API without any explicit flags */
+ if (flags == 0 && !current) {
s/0/VIR_DOMAIN_AFFECT_CURRENT
Just to be painfully obvious...
ACK -
John
if (virDomainPinVcpu(dom, vcpu, cpumap, cpumaplen) !=
0)
goto cleanup;
} else {
10:25 a.m.
New subject: [libvirt] [PATCH 2/6] util: Use virBitmapIsBitSet in freebsd impl of virProcessSetAffinity
Use the helper that does not return errors to fix spuriously looking
dead return of -1.
---
src/util/virprocess.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index c7ffa42..bf6a6df 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -535,13 +535,10 @@ int virProcessSetAffinity(pid_t pid,
{
size_t i;
cpuset_t mask;
- bool set = false;
CPU_ZERO(&mask);
for (i = 0; i < virBitmapSize(map); i++) {
- if (virBitmapGetBit(map, i, &set) < 0)
- return -1;
- if (set)
+ if (virBitmapIsBitSet(map, i))
CPU_SET(i, &mask);
}
--
2.6.2
10:25 a.m.
New subject: [libvirt] [PATCH 3/6] qemu: vcpupin: Don't overwrite errors from functions setting pinning
Both errors from the cgroups code and from the affinity code would be
overwritten by the API. Report the more specific error.
---
src/qemu/qemu_driver.c | 12 ++----------
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a762521..24c1ca4 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5066,20 +5066,12 @@ qemuDomainPinVcpuFlags(virDomainPtr dom,
if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_VCPU, vcpu,
false, &cgroup_vcpu) < 0)
goto endjob;
- if (qemuSetupCgroupCpusetCpus(cgroup_vcpu, pcpumap) < 0) {
- virReportError(VIR_ERR_OPERATION_INVALID,
- _("failed to set cpuset.cpus in cgroup"
- " for vcpu %d"), vcpu);
+ if (qemuSetupCgroupCpusetCpus(cgroup_vcpu, pcpumap) < 0)
goto endjob;
- }
} else {
if (virProcessSetAffinity(qemuDomainGetVcpuPid(vm, vcpu),
- pcpumap) < 0) {
- virReportError(VIR_ERR_SYSTEM_ERROR,
- _("failed to set cpu affinity for vcpu %d"),
- vcpu);
+ pcpumap) < 0)
goto endjob;
- }
}
virBitmapFree(vcpuinfolive->cpumask);
--
2.6.2
10:25 a.m.
New subject: [libvirt] [PATCH 4/6] qemu: vcpupin: Always set affinity even when cgroups are supported
VM startup and CPU hotplug always set the affinity regardless of cgroups
support. Use the same approach for the pinning API.
---
src/qemu/qemu_driver.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 24c1ca4..d1e5188 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5068,12 +5068,11 @@ qemuDomainPinVcpuFlags(virDomainPtr dom,
goto endjob;
if (qemuSetupCgroupCpusetCpus(cgroup_vcpu, pcpumap) < 0)
goto endjob;
- } else {
- if (virProcessSetAffinity(qemuDomainGetVcpuPid(vm, vcpu),
- pcpumap) < 0)
- goto endjob;
}
+ if (virProcessSetAffinity(qemuDomainGetVcpuPid(vm, vcpu), pcpumap) < 0)
+ goto endjob;
+
virBitmapFree(vcpuinfolive->cpumask);
vcpuinfolive->cpumask = pcpumaplive;
pcpumaplive = NULL;
--
2.6.2
6:19 a.m.
New subject: [libvirt] [PATCH 4/6] qemu: vcpupin: Always set affinity even when cgroups are supported
On 02/17/2016 11:25 AM, Peter Krempa wrote:
VM startup and CPU hotplug always set the affinity regardless of
cgroups
support. Use the same approach for the pinning API.
---
src/qemu/qemu_driver.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 24c1ca4..d1e5188 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5068,12 +5068,11 @@ qemuDomainPinVcpuFlags(virDomainPtr dom,
goto endjob;
if (qemuSetupCgroupCpusetCpus(cgroup_vcpu, pcpumap) < 0)
goto endjob;
- } else {
- if (virProcessSetAffinity(qemuDomainGetVcpuPid(vm, vcpu),
- pcpumap) < 0)
- goto endjob;
}
+ if (virProcessSetAffinity(qemuDomainGetVcpuPid(vm, vcpu), pcpumap) < 0)
+ goto endjob;
+
virBitmapFree(vcpuinfolive->cpumask);
vcpuinfolive->cpumask = pcpumaplive;
pcpumaplive = NULL;
Why not the same for qemuDomainPinEmulator and qemuDomainPinIOThread?
Since qemuProcessSetEmulatorAffinity and qemuProcessSetupIOThread do the
same as qemuProcessSetupVcpu.
John
7:09 a.m.
New subject: [libvirt] [PATCH 4/6] qemu: vcpupin: Always set affinity even when cgroups are supported
On Fri, Feb 19, 2016 at 07:19:38 -0500, John Ferlan wrote:
On 02/17/2016 11:25 AM, Peter Krempa wrote:
> VM startup and CPU hotplug always set the affinity regardless of cgroups
> support. Use the same approach for the pinning API.
> ---
> src/qemu/qemu_driver.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 24c1ca4..d1e5188 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -5068,12 +5068,11 @@ qemuDomainPinVcpuFlags(virDomainPtr dom,
> goto endjob;
> if (qemuSetupCgroupCpusetCpus(cgroup_vcpu, pcpumap) < 0)
> goto endjob;
> - } else {
> - if (virProcessSetAffinity(qemuDomainGetVcpuPid(vm, vcpu),
> - pcpumap) < 0)
> - goto endjob;
> }
>
> + if (virProcessSetAffinity(qemuDomainGetVcpuPid(vm, vcpu), pcpumap) < 0)
> + goto endjob;
> +
> virBitmapFree(vcpuinfolive->cpumask);
> vcpuinfolive->cpumask = pcpumaplive;
> pcpumaplive = NULL;
>
Why not the same for qemuDomainPinEmulator and qemuDomainPinIOThread?
I'm too focused on the cpus in this series apparently ...
Since qemuProcessSetEmulatorAffinity and qemuProcessSetupIOThread do
the
same as qemuProcessSetupVcpu.
Are you okay with separate patches for this?
9:09 a.m.
New subject: [libvirt] [PATCH 4/6] qemu: vcpupin: Always set affinity even when cgroups are supported
On 02/19/2016 08:09 AM, Peter Krempa wrote:
On Fri, Feb 19, 2016 at 07:19:38 -0500, John Ferlan wrote:
>
>
> On 02/17/2016 11:25 AM, Peter Krempa wrote:
>> VM startup and CPU hotplug always set the affinity regardless of cgroups
>> support. Use the same approach for the pinning API.
>> ---
>> src/qemu/qemu_driver.c | 7 +++----
>> 1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 24c1ca4..d1e5188 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -5068,12 +5068,11 @@ qemuDomainPinVcpuFlags(virDomainPtr dom,
>> goto endjob;
>> if (qemuSetupCgroupCpusetCpus(cgroup_vcpu, pcpumap) < 0)
>> goto endjob;
>> - } else {
>> - if (virProcessSetAffinity(qemuDomainGetVcpuPid(vm, vcpu),
>> - pcpumap) < 0)
>> - goto endjob;
>> }
>>
>> + if (virProcessSetAffinity(qemuDomainGetVcpuPid(vm, vcpu), pcpumap) <
0)
>> + goto endjob;
>> +
>> virBitmapFree(vcpuinfolive->cpumask);
>> vcpuinfolive->cpumask = pcpumaplive;
>> pcpumaplive = NULL;
>>
>
> Why not the same for qemuDomainPinEmulator and qemuDomainPinIOThread?
I'm too focused on the cpus in this series apparently ...
> Since qemuProcessSetEmulatorAffinity and qemuProcessSetupIOThread do the
> same as qemuProcessSetupVcpu.
Are you okay with separate patches for this?
Yes that's fine
John
10:25 a.m.
New subject: [libvirt] [PATCH 5/6] qemu: qemuDomainGetStatsVcpu: Fix output for possible sparse vCPU settings
qemuDomainHelperGetVcpus would correctly return an array of
virVcpuInfoPtr structs for online vcpus even for sparse topologies, but
the loop that fills the returned typed parameters would number the vcpus
incorrectly. Fortunately sparse topologies aren't supported yet.
---
src/qemu/qemu_driver.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d1e5188..b1f94bd 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18965,7 +18965,7 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
for (i = 0; i < virDomainDefGetVcpus(dom->def); i++) {
snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
- "vcpu.%zu.state", i);
+ "vcpu.%u.state", cpuinfo[i].number);
if (virTypedParamsAddInt(&record->params,
&record->nparams,
maxparams,
@@ -18978,7 +18978,7 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
continue;
snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
- "vcpu.%zu.time", i);
+ "vcpu.%u.time", cpuinfo[i].number);
if (virTypedParamsAddULLong(&record->params,
&record->nparams,
maxparams,
@@ -18986,7 +18986,7 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
cpuinfo[i].cpuTime) < 0)
goto cleanup;
snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
- "vcpu.%zu.wait", i);
+ "vcpu.%u.wait", cpuinfo[i].number);
if (virTypedParamsAddULLong(&record->params,
&record->nparams,
maxparams,
--
2.6.2
10:25 a.m.
New subject: [libvirt] [PATCH 6/6] virsh: cpupin: Extract getter code into a separate function
---
tools/virsh-domain.c | 129 ++++++++++++++++++++++++++++++---------------------
1 file changed, 75 insertions(+), 54 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 028df1f..1546826 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -6374,6 +6374,68 @@ virshPrintPinInfo(vshControl *ctl,
return true;
}
+
+static bool
+virshVcpuPinQuery(vshControl *ctl,
+ virDomainPtr dom,
+ unsigned int vcpu,
+ bool got_vcpu,
+ int maxcpu,
+ unsigned int flags)
+{
+ unsigned char *cpumap = NULL;
+ int cpumaplen;
+ size_t i;
+ int ncpus;
+ bool ret = false;
+
+ if ((ncpus = virshCPUCountCollect(ctl, dom, flags, true)) < 0) {
+ if (ncpus == -1) {
+ if (flags & VIR_DOMAIN_AFFECT_LIVE)
+ vshError(ctl, "%s", _("cannot get vcpupin for offline
domain"));
+ else
+ vshError(ctl, "%s", _("cannot get vcpupin for transient
domain"));
+ }
+ return false;
+ }
+
+ if (got_vcpu && vcpu >= ncpus) {
+ if (flags & VIR_DOMAIN_AFFECT_LIVE ||
+ (!(flags & VIR_DOMAIN_AFFECT_CONFIG) &&
+ virDomainIsActive(dom) == 1))
+ vshError(ctl,
+ _("vcpu %d is out of range of live cpu count %d"),
+ vcpu, ncpus);
+ else
+ vshError(ctl,
+ _("vcpu %d is out of range of persistent cpu count %d"),
+ vcpu, ncpus);
+ return false;
+ }
+
+ cpumaplen = VIR_CPU_MAPLEN(maxcpu);
+ cpumap = vshMalloc(ctl, ncpus * cpumaplen);
+ if ((ncpus = virDomainGetVcpuPinInfo(dom, ncpus, cpumap,
+ cpumaplen, flags)) >= 0) {
+ vshPrintExtra(ctl, "%s %s\n", _("VCPU:"), _("CPU
Affinity"));
+ vshPrintExtra(ctl, "----------------------------------\n");
+ for (i = 0; i < ncpus; i++) {
+ if (got_vcpu && i != vcpu)
+ continue;
+
+ vshPrint(ctl, "%4zu: ", i);
+ ret = virshPrintPinInfo(ctl, VIR_GET_CPUMAP(cpumap, cpumaplen, i),
+ cpumaplen);
+ vshPrint(ctl, "\n");
+ if (!ret)
+ break;
+ }
+ }
+
+ return ret;
+}
+
+
static unsigned char *
virshParseCPUList(vshControl *ctl, int *cpumaplen,
const char *cpulist, int maxcpu)
@@ -6416,8 +6478,7 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
bool ret = false;
unsigned char *cpumap = NULL;
int cpumaplen;
- int maxcpu, ncpus;
- size_t i;
+ int maxcpu;
bool config = vshCommandOptBool(cmd, "config");
bool live = vshCommandOptBool(cmd, "live");
bool current = vshCommandOptBool(cmd, "current");
@@ -6456,63 +6517,23 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
/* Query mode: show CPU affinity information then exit.*/
if (!cpulist) {
- if ((ncpus = virshCPUCountCollect(ctl, dom, flags, true)) < 0) {
- if (ncpus == -1) {
- if (flags & VIR_DOMAIN_AFFECT_LIVE)
- vshError(ctl, "%s", _("cannot get vcpupin for offline
domain"));
- else
- vshError(ctl, "%s", _("cannot get vcpupin for
transient domain"));
- }
- goto cleanup;
- }
-
- if (got_vcpu && vcpu >= ncpus) {
- if (flags & VIR_DOMAIN_AFFECT_LIVE ||
- (!(flags & VIR_DOMAIN_AFFECT_CONFIG) &&
- virDomainIsActive(dom) == 1))
- vshError(ctl,
- _("vcpu %d is out of range of live cpu count %d"),
- vcpu, ncpus);
- else
- vshError(ctl,
- _("vcpu %d is out of range of persistent cpu count
%d"),
- vcpu, ncpus);
- goto cleanup;
- }
+ ret = virshVcpuPinQuery(ctl, dom, vcpu, got_vcpu, maxcpu, flags);
+ goto cleanup;
+ }
- cpumaplen = VIR_CPU_MAPLEN(maxcpu);
- cpumap = vshMalloc(ctl, ncpus * cpumaplen);
- if ((ncpus = virDomainGetVcpuPinInfo(dom, ncpus, cpumap,
- cpumaplen, flags)) >= 0) {
- vshPrintExtra(ctl, "%s %s\n", _("VCPU:"), _("CPU
Affinity"));
- vshPrintExtra(ctl, "----------------------------------\n");
- for (i = 0; i < ncpus; i++) {
- if (got_vcpu && i != vcpu)
- continue;
+ /* Pin mode: pinning specified vcpu to specified physical cpus*/
+ if (!(cpumap = virshParseCPUList(ctl, &cpumaplen, cpulist, maxcpu)))
+ goto cleanup;
- vshPrint(ctl, "%4zu: ", i);
- ret = virshPrintPinInfo(ctl, VIR_GET_CPUMAP(cpumap, cpumaplen, i),
- cpumaplen);
- vshPrint(ctl, "\n");
- if (!ret)
- break;
- }
- }
+ /* use old API without any explicit flags */
+ if (flags == 0 && !current) {
+ if (virDomainPinVcpu(dom, vcpu, cpumap, cpumaplen) != 0)
+ goto cleanup;
} else {
- /* Pin mode: pinning specified vcpu to specified physical cpus*/
- if (!(cpumap = virshParseCPUList(ctl, &cpumaplen, cpulist, maxcpu)))
+ if (virDomainPinVcpuFlags(dom, vcpu, cpumap, cpumaplen, flags) != 0)
goto cleanup;
-
- /* use old API without any explicit flags */
- if (flags == 0 && !current) {
- if (virDomainPinVcpu(dom, vcpu, cpumap, cpumaplen) != 0)
- goto cleanup;
- } else {
- if (virDomainPinVcpuFlags(dom, vcpu, cpumap, cpumaplen, flags) != 0)
- goto cleanup;
- }
- ret = true;
}
+ ret = true;
cleanup:
VIR_FREE(cpumap);
--
2.6.2
6:19 a.m.
On 02/17/2016 11:25 AM, Peter Krempa wrote:
A short dump of patches that are separate enough which can be
reviewed while
I'm working on other things.
Peter Krempa (6):
virsh: cmdVcpuPin: Simplify handling of API flags
util: Use virBitmapIsBitSet in freebsd impl of virProcessSetAffinity
qemu: vcpupin: Don't overwrite errors from functions setting pinning
qemu: vcpupin: Always set affinity even when cgroups are supported
qemu: qemuDomainGetStatsVcpu: Fix output for possible sparse vCPU
settings
virsh: cpupin: Extract getter code into a separate function
src/qemu/qemu_driver.c | 23 +++------
src/util/virprocess.c | 5 +-
tools/virsh-domain.c | 134 +++++++++++++++++++++++++++----------------------
3 files changed, 82 insertions(+), 80 deletions(-)
ACK series, but check comments specifically made for patch 1 and 4
John
3253
days inactive
3255
days old
11 comments
2 participants
participants (2)
-
John Ferlan -
Peter Krempa