All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: amit.shah@redhat.com, aliguori@us.ibm.com,
	Gerd Hoffmann <kraxel@redhat.com>,
	qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] qemu-char: another io_add_watch_poll fix
Date: Wed, 17 Apr 2013 17:36:46 +0200	[thread overview]
Message-ID: <516EC18E.7060702@redhat.com> (raw)
In-Reply-To: <516D1B1B.6020603@redhat.com>

Ji,

On 04/16/2013 11:34 AM, Paolo Bonzini wrote:
> Il 16/04/2013 11:15, Gerd Hoffmann ha scritto:
>> On 04/10/13 15:23, Paolo Bonzini wrote:
>>> After attaching the source, we have to remove the reference we hold
>>> to it, because we do not hold anymore a pointer to the source.
>>>
>>> If we do not do this, removing the source will not finalize it and
>>> will not drop the "real" I/O watch source.
>>>
>>> This showed up when backporting the new flow control patches to older
>>> versions of QEMU that still used select.  The whole select then failed
>>> with EBADF (poll instead will reporting POLLNVAL on a single pollfd)
>>> and QEMU froze.
>>
>> I get freezes now in master, bisecting points to this patch.
>>
>> Reproducer: "qemu -serial pty".
>>
>> qemu is pretty much unusable with libvirt now as libvirt uses pty
>> chardevs by default for serial & monitor ...
>
> I'm not sure why all users of qemu_chr_fe_add_watch believe that the
> watch will be one-shot.  This is definitely not what g_io_create_watch
> does...

It is supposed to depend on the return value of the callback, if you
return False, then the source should be removed, in essence making
the watch one-shot, see:
http://www.gtk.org/api/2.6/glib/glib-The-Main-Event-Loop.html#GSourceFunc

Which specifically says:

Returns:	it should return FALSE if the source should be removed.

And using sources in a one-shot mode by making the callback return false
is quite normal in the glib world.

Regards,

Hans

  reply	other threads:[~2013-04-17 15:33 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-10 13:23 [Qemu-devel] [PATCH] qemu-char: another io_add_watch_poll fix Paolo Bonzini
2013-04-10 17:59 ` Amit Shah
2013-04-11  8:58   ` Paolo Bonzini
2013-04-12  9:24     ` Amit Shah
2013-04-12 10:31       ` Paolo Bonzini
2013-04-15 21:10 ` Anthony Liguori
2013-04-16  9:15 ` Gerd Hoffmann
2013-04-16  9:34   ` Paolo Bonzini
2013-04-17 15:36     ` Hans de Goede [this message]
2013-04-17 16:54       ` Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=516EC18E.7060702@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=aliguori@us.ibm.com \
    --cc=amit.shah@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.