On Tue, Apr 14, 2015 at 14:09:45 +0200, Martin Kletzander wrote:
On Fri, Apr 10, 2015 at 04:45:09PM +0200, Peter Krempa wrote:
Our monitor locking and cleanup code is weird as it allows to remove the monitor object pointer while we are in the monitor. Additionally EVERY api is using the vm private data after @vm was unlocked. This leads to an unpleasant situation, where when qemu crashes during a monitor session while a command is executed (and locks are down), the priv->mon pointer gets cleared (@vm is unlocked).
The next monitor call then crashes on commands that did not check if the monitor is non-NULL.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1209813 and few other possible crashes --- Since this is a workaround rather than a proper fix I'd normally not go this way, but untangling the monitor mess won't be easy so I'll fix the symptoms first.
src/qemu/qemu_monitor.c | 42 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor.h | 12 +++++------- 2 files changed, 47 insertions(+), 7 deletions(-)
...
You could've updated the ArbitraryCommand too. I haven't checked you fixed all of them, but most of them could use a run of attribute clean-ups ;)
@@ -774,7 +772,7 @@ int qemuMonitorBlockJobInfo(qemuMonitorPtr mon, const char *device, virDomainBlockJobInfoPtr info, unsigned long long *bandwidth) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
You removed the attribute but haven't fixed the function itself.
ACK with at least functions qemuMonitorBlockJobInfo() and qemuMonitorArbitraryCommand() fixed as well.
Pity there isn't an easy way of testing all the functions with NULLs.
I've refactored the code and fixed all functions: https://www.redhat.com/archives/libvir-list/2015-April/msg00589.html Peter