[libvirt] [PATCH] qemu: monitor: Add check that monitor is non-null to specific commands

[Peter Krempa]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150414/04856bc0/attachment-0001.sig>