On Wed, May 12, 2010 at 12:10:19PM +0200, jdenemar@xxxxxxxxxx wrote:
> From: Jiri Denemark <jdenemar@xxxxxxxxxx>
>
> When closing a monitor using qemuMonitorClose(), we are aware of
> the possibility the monitor is still being used somewhere:
>
> /* NB: ordinarily one might immediately set mon->watch to -1
> * and mon->fd to -1, but there may be a callback active
> * that is still relying on these fields being valid. So
> * we merely close them, but not clear their values and
> * use this explicit 'closed' flag to track this state */
>
> but since we call virEventAddHandle() on that monitor without increasing
> its ref counter, the monitor is still freed which makes possible users
> of it quite unhappy. The unhappiness can lead to a hang if qemuMonitorIO
> tries to lock mutex which no longer exists.
> ---
> src/qemu/qemu_monitor.c | 11 ++++++++++-
> 1 files changed, 10 insertions(+), 1 deletions(-)
>
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index abf1338..7517e39 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -223,6 +223,14 @@ int qemuMonitorUnref(qemuMonitorPtr mon)
> return mon->refs;
> }
>
> +static void
> +qemuMonitorUnwatch(void *monitor)
> +{
> + qemuMonitorPtr mon = monitor;
> +
> + qemuMonitorLock(mon);
> + qemuMonitorUnref(mon);
> +}
>
> static int
> qemuMonitorOpenUnix(const char *monitor)
> @@ -648,11 +656,12 @@ qemuMonitorOpen(virDomainObjPtr vm,
> VIR_EVENT_HANDLE_ERROR |
> VIR_EVENT_HANDLE_READABLE,
> qemuMonitorIO,
> - mon, NULL)) < 0) {
> + mon, qemuMonitorUnwatch)) < 0) {
> qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("unable to register monitor events"));
> goto cleanup;
> }
> + qemuMonitorRef(mon);
>
> VIR_DEBUG("New mon %p fd =%d watch=%d", mon, mon->fd, mon->watch);
> qemuMonitorUnlock(mon);
I was wondering if we should instead qemuMonitorRef() before calling
virEventAddHandle(), but as long as we are in the function there is no
risk I think so that's fine,
ACK,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list