From e77234c6b1d93f1121f81892536f06a37d14b83f Mon Sep 17 00:00:00 2001 From: Alex Young Date: Sun, 15 Jul 2012 18:30:20 +0100 Subject: [PATCH] Close the mirror client socket on rejection If the mirror attempt connects ok, but is rejected (say, for reporting the wrong size), the client socket needs to be closed. The destination end can't close its socket and accept another connection attempt unless it does. --- src/mirror.c | 11 ++++++++--- tests/acceptance/fakes/dest/hello_wrong_size.rb | 5 +++++ tests/acceptance/flexnbd/fake_dest.rb | 9 +++++++++ 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/mirror.c b/src/mirror.c index 9fb5ca9..b77c8f9 100644 --- a/src/mirror.c +++ b/src/mirror.c @@ -312,6 +312,8 @@ void mirror_cleanup( struct server * serve, int mirror_connect( struct mirror * mirror, off64_t local_size ) { struct sockaddr * connect_from = NULL; + int connected = 0; + if ( mirror->connect_from ) { connect_from = &mirror->connect_from->generic; } @@ -332,6 +334,7 @@ int mirror_connect( struct mirror * mirror, off64_t local_size ) off64_t remote_size; if ( socket_nbd_read_hello( mirror->client, &remote_size ) ) { if( remote_size == local_size ){ + connected = 1; mirror_set_state( mirror, MS_GO ); } else { @@ -349,13 +352,15 @@ int mirror_connect( struct mirror * mirror, off64_t local_size ) warn( "No NBD Hello received." ); mirror_set_state( mirror, MS_FAIL_NO_HELLO ); } + + if ( !connected ) { close( mirror->client ); } } else { warn( "Mirror failed to connect."); mirror_set_state( mirror, MS_FAIL_CONNECT ); } - return MS_GO == mirror_get_state(mirror); + return connected; } @@ -574,9 +579,9 @@ void * mirror_super_runner( void * serve_uncast ) should_retry = 0; } else if (should_retry){ - warn( "Mirror failed, retrying" ); + info( "Mirror failed, retrying" ); } - else { warn( "Mirror failed before commit, giving up" ); } + else { info( "Mirror failed before commit, giving up" ); } } while ( should_retry && !success ); diff --git a/tests/acceptance/fakes/dest/hello_wrong_size.rb b/tests/acceptance/fakes/dest/hello_wrong_size.rb index 6b90d39..ba4bbee 100755 --- a/tests/acceptance/fakes/dest/hello_wrong_size.rb +++ b/tests/acceptance/fakes/dest/hello_wrong_size.rb @@ -21,4 +21,9 @@ client.write_hello( :size => :wrong ) t.join +# Now check that the source closed the first socket (yes, this was an +# actual bug) + +fail "Didn't close socket" unless client.disconnected? + exit 0 diff --git a/tests/acceptance/flexnbd/fake_dest.rb b/tests/acceptance/flexnbd/fake_dest.rb index 3a0de7c..a22f128 100644 --- a/tests/acceptance/flexnbd/fake_dest.rb +++ b/tests/acceptance/flexnbd/fake_dest.rb @@ -62,6 +62,15 @@ module FlexNBD write_reply( handle, 1 ) end + def disconnected? + begin + Timeout.timeout(2) do + @sock.read(1) == nil + end + rescue Timeout::Error + return false + end + end def write_reply( handle, err=0, opts={} ) if opts[:magic] == :wrong