8:32 a.m.
More broken stuff ... sigh.
Peter Krempa (6):
qemu: perf: Don't ignore perf setup if allocation fails
util: perf: Use 'error' label in virPerfCmtEnable
util: perf: Adhere to coding style of error checks in
virPerfEventEnable
util: perf: Adhere to coding style of error checks in
qemuDomainSetPerfEvents
qemu: process: Fix failure semantics for perf events
qemu: process: Don't needlesly clear the perf events in
qemuDomainPerfRestart
src/qemu/qemu_driver.c | 4 ++--
src/qemu/qemu_process.c | 29 ++++++++++++-----------------
src/util/virperf.c | 16 ++++++++--------
3 files changed, 22 insertions(+), 27 deletions(-)
--
2.8.1
8:32 a.m.
New subject: [libvirt] [PATCH 1/6] qemu: perf: Don't ignore perf setup if allocation fails
Reject the VM startup if the perf event structure can't be allocated.
---
src/qemu/qemu_process.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 0ccc3ac..f8d13aa 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5410,12 +5410,12 @@ qemuProcessLaunch(virConnectPtr conn,
if (qemuSetupCgroup(driver, vm, nnicindexes, nicindexes) < 0)
goto cleanup;
- priv->perf = virPerfNew();
- if (priv->perf) {
- for (i = 0; i < VIR_PERF_EVENT_LAST; i++) {
- if (vm->def->perf->events[i] == VIR_TRISTATE_BOOL_YES)
- virPerfEventEnable(priv->perf, i, vm->pid);
- }
+ if (!(priv->perf = virPerfNew()))
+ goto cleanup;
+
+ for (i = 0; i < VIR_PERF_EVENT_LAST; i++) {
+ if (vm->def->perf->events[i] == VIR_TRISTATE_BOOL_YES)
+ virPerfEventEnable(priv->perf, i, vm->pid);
}
/* This must be done after cgroup placement to avoid resetting CPU
--
2.8.1
8:32 a.m.
New subject: [libvirt] [PATCH 2/6] util: perf: Use 'error' label in virPerfCmtEnable
The label is used only for the error path, thus rename cleanup.
---
src/util/virperf.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/util/virperf.c b/src/util/virperf.c
index 359a9c3..0ae4744 100644
--- a/src/util/virperf.c
+++ b/src/util/virperf.c
@@ -120,7 +120,7 @@ virPerfCmtEnable(virPerfEventPtr event,
if (virFileReadAll("/sys/devices/intel_cqm/type",
10, &buf) < 0)
- goto cleanup;
+ goto error;
if ((tmp = strchr(buf, '\n')))
*tmp = '\0';
@@ -128,18 +128,18 @@ virPerfCmtEnable(virPerfEventPtr event,
if (virStrToLong_ui(buf, NULL, 10, &event_type) < 0) {
virReportSystemError(errno, "%s",
_("failed to get cmt event type"));
- goto cleanup;
+ goto error;
}
VIR_FREE(buf);
if (virFileReadAll("/sys/devices/intel_cqm/events/llc_occupancy.scale",
10, &buf) < 0)
- goto cleanup;
+ goto error;
if (virStrToLong_ui(buf, NULL, 10, &scale) < 0) {
virReportSystemError(errno, "%s",
_("failed to get cmt scaling factor"));
- goto cleanup;
+ goto error;
}
event->efields.cmt.scale = scale;
@@ -157,19 +157,19 @@ virPerfCmtEnable(virPerfEventPtr event,
virReportSystemError(errno,
_("Unable to open perf type=%d for pid=%d"),
event_type, pid);
- goto cleanup;
+ goto error;
}
if (ioctl(event->fd, PERF_EVENT_IOC_ENABLE) < 0) {
virReportSystemError(errno, "%s",
_("Unable to enable perf event for CMT"));
- goto cleanup;
+ goto error;
}
event->enabled = true;
return 0;
- cleanup:
+ error:
VIR_FORCE_CLOSE(event->fd);
VIR_FREE(buf);
return -1;
--
2.8.1
8:32 a.m.
New subject: [libvirt] [PATCH 3/6] util: perf: Adhere to coding style of error checks in virPerfEventEnable
---
src/util/virperf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virperf.c b/src/util/virperf.c
index 0ae4744..bd65587 100644
--- a/src/util/virperf.c
+++ b/src/util/virperf.c
@@ -186,7 +186,7 @@ virPerfEventEnable(virPerfPtr perf,
switch (type) {
case VIR_PERF_EVENT_CMT:
- if (virPerfCmtEnable(event, pid))
+ if (virPerfCmtEnable(event, pid) < 0)
return -1;
break;
case VIR_PERF_EVENT_LAST:
--
2.8.1
8:32 a.m.
New subject: [libvirt] [PATCH 4/6] util: perf: Adhere to coding style of error checks in qemuDomainSetPerfEvents
---
src/qemu/qemu_driver.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 542d13c..0860822 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10070,9 +10070,9 @@ qemuDomainSetPerfEvents(virDomainPtr dom,
enabled = params->value.b;
type = virPerfEventTypeFromString(param->field);
- if (!enabled && virPerfEventDisable(priv->perf, type))
+ if (!enabled && virPerfEventDisable(priv->perf, type) < 0)
goto endjob;
- if (enabled && virPerfEventEnable(priv->perf, type, vm->pid))
+ if (enabled && virPerfEventEnable(priv->perf, type, vm->pid)
< 0)
goto endjob;
def->perf->events[type] = enabled ?
--
2.8.1
8:32 a.m.
New subject: [libvirt] [PATCH 5/6] qemu: process: Fix failure semantics for perf events
For strange reasons if a perf event type was not supported or failed to
be enabled at VM start libvirt would ignore the failure.
On the other hand on restart if the event could not be re-enabled
libvirt would fail to reconnect to the VM and kill it.
Both don't make really sense. Fix it by failing to start the VM if the
event is not supported and change the event to disabled if it can't be
reconnected (unlikely).
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1329045
---
src/qemu/qemu_process.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index f8d13aa..af06aa6 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3501,17 +3501,14 @@ qemuDomainPerfRestart(virDomainObjPtr vm)
for (i = 0; i < VIR_PERF_EVENT_LAST; i++) {
if (def->perf->events[i] &&
def->perf->events[i] == VIR_TRISTATE_BOOL_YES) {
- if (virPerfEventEnable(priv->perf, i, vm->pid))
- goto cleanup;
+
+ /* Failure to re-enable the perf event should not be fatal */
+ if (virPerfEventEnable(priv->perf, i, vm->pid) < 0)
+ def->perf->events[i] = VIR_TRISTATE_BOOL_NO;
}
}
return 0;
-
- cleanup:
- virPerfFree(priv->perf);
- priv->perf = NULL;
- return -1;
}
struct qemuProcessReconnectData {
@@ -5414,8 +5411,9 @@ qemuProcessLaunch(virConnectPtr conn,
goto cleanup;
for (i = 0; i < VIR_PERF_EVENT_LAST; i++) {
- if (vm->def->perf->events[i] == VIR_TRISTATE_BOOL_YES)
- virPerfEventEnable(priv->perf, i, vm->pid);
+ if (vm->def->perf->events[i] == VIR_TRISTATE_BOOL_YES &&
+ virPerfEventEnable(priv->perf, i, vm->pid) < 0)
+ goto cleanup;
}
/* This must be done after cgroup placement to avoid resetting CPU
--
2.8.1
8:32 a.m.
New subject: [libvirt] [PATCH 6/6] qemu: process: Don't needlesly clear the perf events in qemuDomainPerfRestart
At that point the perf events struct should not be allocated so there's
no use in clearing it.
---
src/qemu/qemu_process.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index af06aa6..41a2b35 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3492,10 +3492,7 @@ qemuDomainPerfRestart(virDomainObjPtr vm)
virDomainDefPtr def = vm->def;
qemuDomainObjPrivatePtr priv = vm->privateData;
- virPerfFree(priv->perf);
-
- priv->perf = virPerfNew();
- if (!priv->perf)
+ if (!(priv->perf = virPerfNew()))
return -1;
for (i = 0; i < VIR_PERF_EVENT_LAST; i++) {
--
2.8.1
3130
days inactive
3132
days old
7 comments
2 participants
participants (2)
-
Pavel Hrdina -
Peter Krempa