From f3cebcdcd5c9ad09136e3ec34de94f86a8381149 Mon Sep 17 00:00:00 2001 From: Alex Young Date: Wed, 11 Jul 2012 15:19:50 +0100 Subject: [PATCH] Test a source crashing after an entrust. This adds a test for destination behaviour, in that if a source crashes after sending an entrust message but before the destination can reply, the destination must allow the source to reconnect and retry the mirror. --- src/mirror.c | 8 +++++ .../fakes/source/close_after_entrust.rb | 34 +++++++++++++++++++ .../fakes/source/close_after_write_data.rb | 2 +- tests/acceptance/flexnbd/fake_source.rb | 16 +++++++-- tests/acceptance/test_dest_error_handling.rb | 4 +++ 5 files changed, 61 insertions(+), 3 deletions(-) create mode 100755 tests/acceptance/fakes/source/close_after_entrust.rb diff --git a/src/mirror.c b/src/mirror.c index 440ea7f..2bf7a3e 100644 --- a/src/mirror.c +++ b/src/mirror.c @@ -239,8 +239,16 @@ void mirror_give_control( struct mirror_status * mirror ) * requests while the response was in flight, and if the * response went astray we'd have two servers claiming * responsibility for the same data. + * + * The meaning of these is as follows: + * The entrust signifies that all the data has been sent, and + * the client is currently paused but not disconnected. + * The disconnect signifies that the client has been + * safely disconnected. + * TODO: Disconnect the client! */ socket_nbd_entrust( mirror->client ); + debug("TODO: The client *should* be disconnected here, but isn't yet"); socket_nbd_disconnect( mirror->client ); } diff --git a/tests/acceptance/fakes/source/close_after_entrust.rb b/tests/acceptance/fakes/source/close_after_entrust.rb new file mode 100755 index 0000000..325e8ef --- /dev/null +++ b/tests/acceptance/fakes/source/close_after_entrust.rb @@ -0,0 +1,34 @@ +#!/usr/bin/env ruby + +# Connect, send a migration, entrust then *immediately* disconnect. +# This simulates a client which fails while the client is blocked. +# +# We attempt to reconnect immediately afterwards to prove that we can +# retry the mirroring. + +require 'flexnbd/fake_source' +include FlexNBD + +addr, port, srv_pid = *ARGV + +client = FakeSource.new( addr, port, "Timed out connecting" ) +client.read_hello +client.write_write_request( 0, 8 ) +client.write_data( "12345678" ) + +# Use system "kill" rather than Process.kill because Process.kill +# doesn't seem to work +system "kill -STOP #{srv_pid}" + +client.write_entrust_request +client.close + +system "kill -CONT #{srv_pid}" + + +sleep(0.25) + +client2 = FakeSource.new( addr, port, "Timed out reconnecting" ) +client2.close + +exit(0) diff --git a/tests/acceptance/fakes/source/close_after_write_data.rb b/tests/acceptance/fakes/source/close_after_write_data.rb index 082f571..ca21711 100755 --- a/tests/acceptance/fakes/source/close_after_write_data.rb +++ b/tests/acceptance/fakes/source/close_after_write_data.rb @@ -27,7 +27,7 @@ Process.kill( "CONT", srv_pid.to_i ) sleep(0.25) # ...and can we reconnect? -client2 = FakeSource.new( addr, port, "Timed out connecting" ) +client2 = FakeSource.new( addr, port, "Timed out reconnecting" ) client2.close exit(0) diff --git a/tests/acceptance/flexnbd/fake_source.rb b/tests/acceptance/flexnbd/fake_source.rb index 49ddb5c..122a5cd 100644 --- a/tests/acceptance/flexnbd/fake_source.rb +++ b/tests/acceptance/flexnbd/fake_source.rb @@ -29,22 +29,34 @@ module FlexNBD end - def write_write_request( from, len, handle="myhandle" ) + def send_request( type, handle="myhandle", from=0, len=0 ) fail "Bad handle" unless handle.length == 8 @sock.write( "\x25\x60\x95\x13" ) - @sock.write( "\x00\x00\x00\x01" ) + @sock.write( [type].pack( 'N' ) ) @sock.write( handle ) @sock.write( "\x0"*4 ) @sock.write( [from].pack( 'N' ) ) @sock.write( [len ].pack( 'N' ) ) end + + def write_write_request( from, len, handle="myhandle" ) + send_request( 1, handle, from, len ) + end + + + def write_entrust_request( handle="myhandle" ) + send_request( 65536, handle ) + end + + def write_data( data ) @sock.write( data ) end + def read_response magic = @sock.read(4) error_s = @sock.read(4) diff --git a/tests/acceptance/test_dest_error_handling.rb b/tests/acceptance/test_dest_error_handling.rb index cc28979..1088550 100644 --- a/tests/acceptance/test_dest_error_handling.rb +++ b/tests/acceptance/test_dest_error_handling.rb @@ -54,6 +54,10 @@ class TestDestErrorHandling < Test::Unit::TestCase run_fake( "source/close_after_write" ) end + def test_disconnect_before_entrust_reply_causes_error + run_fake( "source/close_after_entrust" ) + end + def test_disconnect_before_write_reply_causes_error # Note that this is an odd case: writing the reply doesn't fail.