qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Orit Wasserman <owasserm@redhat.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: kwolf@redhat.com, aliguori@us.ibm.com, mdroth@linux.vnet.ibm.com,
	quintela@redhat.com, armbru@redhat.com, qemu-devel@nongnu.org,
	lcapitulino@redhat.com, pbonzini@redhat.com, akong@redhat.com
Subject: Re: [Qemu-devel] [PATCH v5 0/4] non-blocking connect address handling cleanup
Date: Mon, 24 Sep 2012 16:37:17 +0200	[thread overview]
Message-ID: <5060701D.7080806@redhat.com> (raw)
In-Reply-To: <20120924143659.GA9971@redhat.com>

On 09/24/2012 04:36 PM, Michael S. Tsirkin wrote:
> On Mon, Sep 24, 2012 at 01:11:06PM +0200, Orit Wasserman wrote:
>> Changes from v4:
>> 	- Rename ConnectHandler to NonBlockingConnectHandler
>> 	- move function comments to functions definitions
>> 	- move connect_state allocation to outside of the loop
>> 	- fix comments text
>>
>> Changes from v3:
>> 	- add missing parenthesis QEMU_SOCKET_RC_INPROGRESS macro
>> 	- remove "block" from dummy_opts
>> 	- remove in_progress from external API (inet_connect_opts and 
>> 	  inet_nonblocking_connect)
>> 	- Allocate ConnectState inside inet_connect_opts, this make the 
>> 	  structure internal to qemu-sockets.c
>> 	- fix migrate_fd_cleanup to handle invalid fd.
>> 	
>> Changes from v2:
>> 	- remove the use of getnameinfo
>> 	- remove errp for inet_connect_addr
>> 	- remove QemuOpt "block"
>> 	- fix errors in wait_for_connect 
>> 	- pass ConnectState as a parameter to allow concurrent connect ops
>>
>> getaddrinfo can give us a list of addresses, but we only try to
>> connect to the first one. If that fails we never proceed to
>> the next one.  This is common on desktop setups that often have ipv6
>> configured but not actually working.
>> A simple way to reproduce the problem is migration:
>> for the destination use -incoming tcp:0:4444, run migrate -d tcp:localhost:4444
>> migration will fail on hosts that have both IPv4 and IPV6 address for localhost.
>>
>> To fix this, refactor address resolution code and make inet_nonblocking_connect
>> retry connection with a different address.
> 
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> 
> More ideas if you feel like touching this code some more:
> - fix listen side: drop ADDRCONF hint and ideally make
>   it address independent
> - enhance error handling so async connect errors propagate to
>   monitor somehow
I working on the second :) 
> 
>> Michael S. Tsirkin (1):
>>   Refactor inet_connect_opts function
>>
>> Orit Wasserman (3):
>>   Separate inet_connect into inet_connect (blocking) and
>>     inet_nonblocking_connect
>>   Fix address handling in inet_nonblocking_connect
>>   Clear handler only for valid fd
>>
>>  migration-tcp.c |   37 ++------
>>  migration.c     |    4 +-
>>  nbd.c           |    2 +-
>>  qemu-char.c     |    2 +-
>>  qemu-sockets.c  |  279 +++++++++++++++++++++++++++++++++++++++++--------------
>>  qemu_socket.h   |   15 +++-
>>  ui/vnc.c        |    2 +-
>>  7 files changed, 237 insertions(+), 104 deletions(-)
>>
>> -- 
>> 1.7.7.6

  reply	other threads:[~2012-09-24 14:38 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-24 11:11 [Qemu-devel] [PATCH v5 0/4] non-blocking connect address handling cleanup Orit Wasserman
2012-09-24 11:11 ` [Qemu-devel] [PATCH v5 1/4] Refactor inet_connect_opts function Orit Wasserman
2012-09-24 11:11 ` [Qemu-devel] [PATCH v5 2/4] Separate inet_connect into inet_connect (blocking) and inet_nonblocking_connect Orit Wasserman
2012-09-24 11:11 ` [Qemu-devel] [PATCH v5 3/4] Fix address handling in inet_nonblocking_connect Orit Wasserman
2012-09-24 11:11 ` [Qemu-devel] [PATCH v5 4/4] Clear handler only for valid fd Orit Wasserman
2012-09-24 14:22 ` [Qemu-devel] [PATCH v5 0/4] non-blocking connect address handling cleanup Markus Armbruster
2012-09-24 14:36 ` Michael S. Tsirkin
2012-09-24 14:37   ` Orit Wasserman [this message]
2012-09-25  1:54 ` Amos Kong
2012-09-26 12:37 ` Anthony Liguori

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=5060701D.7080806@redhat.com \
    --to=owasserm@redhat.com \
    --cc=akong@redhat.com \
    --cc=aliguori@us.ibm.com \
    --cc=armbru@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=lcapitulino@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).