Re: [Qemu-devel] A question about this commit 9894dc0cdcc397ee5b26370bc53da6d360a363c2

[Daniel P. Berrange]

On Tue, Aug 23, 2016 at 08:57:44AM +0000, Gaohaifeng (A) wrote:
> Hi Daniel & Paolo,
> 
> Commit 9894dc0c "char: convert from GIOChannel to QIOChannel", about
> 
> the below code segment:
> 
> -static gboolean tcp_chr_read(GIOChannel *chan, GIOCondition cond, void 
> *opaque)
> +static gboolean tcp_chr_read(QIOChannel *chan, GIOCondition cond, void 
> *opaque)
> {
>      CharDriverState *chr = opaque;
>      TCPCharDriver *s = chr->opaque;
> @@ -2938,9 +2801,7 @@ static gboolean tcp_chr_read(GIOChannel *chan, 
> GIOCondition cond, void *opaque)
>      if (len > s->max_size)
>          len = s->max_size;
>      size = tcp_chr_recv(chr, (void *)buf, len);
> -    if (size == 0 ||
> -        (size < 0 &&
> -         socket_error() != EAGAIN && socket_error() != EWOULDBLOCK)) {
> +    if (size == 0 || size == -1) {
>          /* connection closed */
>          tcp_chr_disconnect(chr);
>      } else if (size > 0) {
> 
> The old version will not call tcp_chr_disconnect when error is not
> EAGAIN.
> The new version will call tcp_chr_disconnect when size==-1. From the
> tcp_chr_recv function we see EAGIN will return -1, so EAGIN will call
> tcp_chr_disconnect.
> 
> We meet an issue when Guest VM use DPDK(1.6.0) l2fwd, it may exit since
> link status is not up. The link is down because tcp_chr_disconnect will
> set it.
> 
> Can you explain it why EAGIN here changes the behavior ?

tcp_chr_read is used in a call to io_add_watch_poll() which sets up an
event loop watch so that tcp_chr_read is called when there is incoming
data to be read.  IOW, when tcp_chr_read is invoked, I would always
expect that at least 1 byte is available, and so EAGAIN should not
occurr.

So I'm puzzled why you would get EAGAIN at all, unless there is some
kind of race with other code also consuming bytes from the same
socket.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|