From 4790912750a46774bc951d870f308074842c06f3 Mon Sep 17 00:00:00 2001 From: Alex Young Date: Mon, 23 Jul 2012 09:48:50 +0100 Subject: [PATCH] Remove listen mode Changing behaviour so that instead of rebinding after a successful migration and continuing as an ordinary server, we simply quit with a 0 exit code and let our caller restart us as a server if they want to. This means that everything in listen.c, listen.h, and anything making reference to a rebind address is unneeded. --- Rakefile | 26 +-- src/control.c | 107 +++++------- src/control.h | 6 +- src/flexnbd.c | 164 ++---------------- src/flexnbd.h | 37 +--- src/listen.c | 123 ------------- src/listen.h | 28 --- src/mode.c | 51 ++---- tests/acceptance/environment.rb | 10 +- .../fakes/dest/break_after_hello.rb | 2 +- .../fakes/source/close_after_entrust.rb | 12 +- .../fakes/source/close_after_entrust_reply.rb | 16 +- .../fakes/source/close_after_write.rb | 5 +- .../fakes/source/close_after_write_data.rb | 4 +- .../fakes/source/successful_transfer.rb | 9 +- tests/acceptance/flexnbd.rb | 8 +- tests/acceptance/test_dest_error_handling.rb | 6 +- .../acceptance/test_source_error_handling.rb | 2 +- tests/unit/check_listen.c | 57 ------ 19 files changed, 122 insertions(+), 551 deletions(-) delete mode 100644 src/listen.c delete mode 100644 src/listen.h delete mode 100644 tests/unit/check_listen.c diff --git a/Rakefile b/Rakefile index 43586f6..594f839 100644 --- a/Rakefile +++ b/Rakefile @@ -115,7 +115,6 @@ file check("client") => %w{build/tests/check_client.o build/self_pipe.o build/nbdtypes.o - build/listen.o build/flexnbd.o build/flexthread.o build/control.o @@ -160,7 +159,6 @@ file check("serve") => build/flexnbd.o build/mirror.o build/status.o - build/listen.o build/acl.o build/mbox.o build/ioutil.o @@ -181,7 +179,6 @@ file check("readwrite") => build/flexnbd.o build/mirror.o build/status.o - build/listen.o build/nbdtypes.o build/mbox.o build/ioutil.o @@ -189,26 +186,6 @@ file check("readwrite") => gcc_link t.name, t.prerequisites + [LIBCHECK] end -file check("listen") => -%w{build/tests/check_listen.o - build/listen.o - build/flexnbd.o - build/status.o - build/flexthread.o - build/mbox.o - build/mirror.o - build/self_pipe.o - build/nbdtypes.o - build/control.o - build/readwrite.o - build/parse.o - build/client.o - build/serve.o - build/acl.o - build/ioutil.o - build/util.o} do |t| - gcc_link t.name, t.prerequisites + [LIBCHECK] -end file check("flexnbd") => %w{build/tests/check_flexnbd.o @@ -216,7 +193,6 @@ file check("flexnbd") => build/ioutil.o build/util.o build/control.o - build/listen.o build/mbox.o build/flexthread.o build/status.o @@ -236,7 +212,7 @@ file check("control") => gcc_link t.name, t.prerequisites + [LIBCHECK] end -(TEST_MODULES- %w{control flexnbd acl client serve readwrite listen util}).each do |m| +(TEST_MODULES- %w{control flexnbd acl client serve readwrite util}).each do |m| tgt = "build/tests/check_#{m}.o" maybe_obj_name = "build/#{m}.o" # Take it out in case we're testing util.o or ioutil.o diff --git a/src/control.c b/src/control.c index d36a1dc..b22a5f0 100644 --- a/src/control.c +++ b/src/control.c @@ -353,48 +353,38 @@ int control_mirror(struct control_client* client, int linesc, char** lines) return -1; } - /* In theory, we should never have to worry about the switch - * lock here, since we should never be able to start more than - * one mirror at a time. This is enforced by only accepting a - * single client at a time on the control socket. - */ - flexnbd_lock_switch( flexnbd ); - { - struct server * serve = flexnbd_server(flexnbd); + struct server * serve = flexnbd_server(flexnbd); - if ( serve->mirror_super ) { - warn( "Tried to start a second mirror run" ); - write_socket( "1: mirror already running" ); - } else { - serve->mirror_super = mirror_super_create( - serve->filename, - connect_to, - connect_from, - max_Bps , - action_at_finish, - client->mirror_state_mbox ); - serve->mirror = serve->mirror_super->mirror; + if ( serve->mirror_super ) { + warn( "Tried to start a second mirror run" ); + write_socket( "1: mirror already running" ); + } else { + serve->mirror_super = mirror_super_create( + serve->filename, + connect_to, + connect_from, + max_Bps , + action_at_finish, + client->mirror_state_mbox ); + serve->mirror = serve->mirror_super->mirror; - FATAL_IF( 0 != pthread_create( - &serve->mirror_super->thread, - NULL, - mirror_super_runner, - serve - ), - "Failed to create mirror thread" - ); + FATAL_IF( 0 != pthread_create( + &serve->mirror_super->thread, + NULL, + mirror_super_runner, + serve + ), + "Failed to create mirror thread" + ); - debug("Control thread mirror super waiting"); - enum mirror_state state = - control_client_mirror_wait( client ); - debug("Control thread writing response"); - control_write_mirror_response( state, client->socket ); - } + debug("Control thread mirror super waiting"); + enum mirror_state state = + control_client_mirror_wait( client ); + debug("Control thread writing response"); + control_write_mirror_response( state, client->socket ); } - debug( "Control thread unlocking switch" ); - flexnbd_unlock_switch( flexnbd ); debug( "Control thread going away." ); - + return 0; } @@ -441,33 +431,29 @@ int control_break( int result = 0; struct flexnbd* flexnbd = client->flexnbd; - flexnbd_lock_switch( flexnbd ); - { - struct server * serve = flexnbd_server( flexnbd ); - if ( server_is_mirroring( serve ) ) { + struct server * serve = flexnbd_server( flexnbd ); + if ( server_is_mirroring( serve ) ) { - info( "Signaling to abandon mirror" ); - server_abandon_mirror( serve ); - debug( "Abandon signaled" ); + info( "Signaling to abandon mirror" ); + server_abandon_mirror( serve ); + debug( "Abandon signaled" ); - if ( server_is_closed( serve ) ) { - info( "Mirror completed while canceling" ); - write( client->socket, - "1: mirror completed\n", 20 ); - } - else { - info( "Mirror successfully stopped." ); - write( client->socket, - "0: mirror stopped\n", 18 ); - result = 1; - } - - } else { - warn( "Not mirroring." ); - write( client->socket, "1: not mirroring\n", 17 ); + if ( server_is_closed( serve ) ) { + info( "Mirror completed while canceling" ); + write( client->socket, + "1: mirror completed\n", 20 ); } + else { + info( "Mirror successfully stopped." ); + write( client->socket, + "0: mirror stopped\n", 18 ); + result = 1; + } + + } else { + warn( "Not mirroring." ); + write( client->socket, "1: not mirroring\n", 17 ); } - flexnbd_unlock_switch( flexnbd ); return result; } @@ -499,7 +485,6 @@ void control_client_cleanup(struct control_client* client, /* This is wrongness */ if ( server_io_locked( client->flexnbd->serve ) ) { server_unlock_io( client->flexnbd->serve ); } if ( server_acl_locked( client->flexnbd->serve ) ) { server_unlock_acl( client->flexnbd->serve ); } - if ( flexnbd_switch_locked( client->flexnbd ) ) { flexnbd_unlock_switch( client->flexnbd ); } control_client_destroy( client ); } diff --git a/src/control.h b/src/control.h index dd40ba2..cfd301f 100644 --- a/src/control.h +++ b/src/control.h @@ -1,10 +1,14 @@ #ifndef CONTROL_H #define CONTROL_H +/* We need this to avoid a complaint about struct server * in + * void accept_control_connection + */ +struct server; #include "parse.h" #include "mirror.h" -#include "control.h" +#include "serve.h" #include "flexnbd.h" #include "mbox.h" diff --git a/src/flexnbd.c b/src/flexnbd.c index 9ae15ea..5124694 100644 --- a/src/flexnbd.c +++ b/src/flexnbd.c @@ -21,7 +21,6 @@ #include "flexnbd.h" #include "serve.h" -#include "listen.h" #include "util.h" #include "control.h" #include "status.h" @@ -76,8 +75,6 @@ void flexnbd_create_shared( } flexnbd->signal_fd = flexnbd_build_signal_fd(); - - flexnbd->switch_mutex = flexthread_mutex_create(); } @@ -102,36 +99,31 @@ struct flexnbd * flexnbd_create_serving( s_acl_entries, max_nbd_clients, 1); - flexnbd_create_shared( flexnbd, s_ctrl_sock ); + flexnbd_create_shared( flexnbd, + s_ctrl_sock ); return flexnbd; } struct flexnbd * flexnbd_create_listening( char* s_ip_address, - char* s_rebind_ip_address, char* s_port, - char* s_rebind_port, char* s_file, char *s_ctrl_sock, int default_deny, int acl_entries, - char** s_acl_entries, - int max_nbd_clients ) + char** s_acl_entries ) { struct flexnbd * flexnbd = xmalloc( sizeof( struct flexnbd ) ); - flexnbd->listen = listen_create( + flexnbd->serve = server_create( flexnbd, s_ip_address, - s_rebind_ip_address, s_port, - s_rebind_port, - s_file, + s_file, default_deny, acl_entries, s_acl_entries, - max_nbd_clients); - flexnbd->serve = flexnbd->listen->init_serve; + 1, 0); flexnbd_create_shared( flexnbd, s_ctrl_sock ); return flexnbd; } @@ -175,37 +167,12 @@ void flexnbd_destroy( struct flexnbd * flexnbd ) if ( flexnbd->control ) { control_destroy( flexnbd->control ); } - if ( flexnbd->listen ) { - listen_destroy( flexnbd->listen ); - } - - flexthread_mutex_destroy( flexnbd->switch_mutex ); close( flexnbd->signal_fd ); free( flexnbd ); } -/* THOU SHALT NOT DEREFERENCE flexnbd->serve OUTSIDE A SWITCH LOCK - */ -void flexnbd_lock_switch( struct flexnbd * flexnbd ) -{ - NULLCHECK( flexnbd ); - flexthread_mutex_lock( flexnbd->switch_mutex ); -} - -void flexnbd_unlock_switch( struct flexnbd * flexnbd ) -{ - NULLCHECK( flexnbd ); - flexthread_mutex_unlock( flexnbd->switch_mutex ); -} - -int flexnbd_switch_locked( struct flexnbd * flexnbd ) -{ - NULLCHECK( flexnbd ); - return flexthread_mutex_held( flexnbd->switch_mutex ); -} - struct server * flexnbd_server( struct flexnbd * flexnbd ) { NULLCHECK( flexnbd ); @@ -216,11 +183,7 @@ struct server * flexnbd_server( struct flexnbd * flexnbd ) void flexnbd_replace_acl( struct flexnbd * flexnbd, struct acl * acl ) { NULLCHECK( flexnbd ); - flexnbd_lock_switch( flexnbd ); - { - server_replace_acl( flexnbd_server(flexnbd), acl ); - } - flexnbd_unlock_switch( flexnbd ); + server_replace_acl( flexnbd_server(flexnbd), acl ); } @@ -229,16 +192,10 @@ struct status * flexnbd_status_create( struct flexnbd * flexnbd ) NULLCHECK( flexnbd ); struct status * status; - flexnbd_lock_switch( flexnbd ); - { - status = status_create( flexnbd_server( flexnbd ) ); - } - flexnbd_unlock_switch( flexnbd ); + status = status_create( flexnbd_server( flexnbd ) ); return status; } -/** THOU SHALT *ONLY* CALL THIS FROM INSIDE A SWITCH LOCK - */ void flexnbd_set_server( struct flexnbd * flexnbd, struct server * serve ) { NULLCHECK( flexnbd ); @@ -246,40 +203,11 @@ void flexnbd_set_server( struct flexnbd * flexnbd, struct server * serve ) } -/* Calls the given callback to exchange server objects, then sets - * flexnbd->server so everything else can see it. */ -void flexnbd_switch( struct flexnbd * flexnbd, struct server *(listen_cb)(struct listen *) ) -{ - NULLCHECK( flexnbd ); - NULLCHECK( flexnbd->listen ); - - flexnbd_lock_switch( flexnbd ); - { - struct server * new_server = listen_cb( flexnbd->listen ); - NULLCHECK( new_server ); - flexnbd_set_server( flexnbd, new_server ); - } - flexnbd_unlock_switch( flexnbd ); - -} - -/* Get the default_deny of the current server object. This takes the - * switch_lock to avoid nastiness if the server switches and gets freed - * in the dereference chain. - * This means that this function must not be called if the switch lock - * is already held. - */ +/* Get the default_deny of the current server object. */ int flexnbd_default_deny( struct flexnbd * flexnbd ) { - int result; - NULLCHECK( flexnbd ); - flexnbd_lock_switch( flexnbd ); - { - result = server_default_deny( flexnbd->serve ); - } - flexnbd_unlock_switch( flexnbd ); - return result; + return server_default_deny( flexnbd->serve ); } @@ -300,67 +228,6 @@ void make_writable( const char * filename ) strerror( errno ) ); } -/** Drops a marker file on the filesystem to show that the image we're - * serving hasn't yet finished its migration yet - */ -void flexnbd_mark_incomplete( struct flexnbd * flexnbd ) -{ - char * filename = - flexnbd_incomplete_filename( flexnbd ); - int fd; - - NULLCHECK( filename ); - - /* It's OK if the file already exists - it's perfectly possible - * that a previous process died part-way through and left it - * behind. However, we might have left the file mode in a bad - * state. - */ - - struct stat ignored; - int exists = stat( filename, &ignored ) == 0; - if ( exists ) { - /* definitely there, need to chmod */ - debug( "%s exists, making it writable", filename ); - make_writable( filename ); - } - else if ( ENOENT != errno ) { - /* Can't tell if it's there or not, weirdness. */ - fatal( "Unable to stat %s: %s", filename, strerror( errno ) ); - } - else { /* definitely not there. NOP. */ } - - - fd = open( filename, O_CREAT|O_WRONLY, 0 ); - FATAL_IF_NEGATIVE( fd, - "Couldn't open %s: %s", - filename, - strerror( errno ) ); - - /* Minor race here - in principle we could see the file - * disappear before the chmod */ - close( fd ); -} - - -/** Removes the .INCOMPLETE marker file from the filesystem. Call this - * only when you know the migration has completed successfully. - */ -void flexnbd_mark_complete( struct flexnbd * flexnbd ) -{ - char * filename = - flexnbd_incomplete_filename( flexnbd ); - - NULLCHECK( filename ); - - make_writable( filename ); - - FATAL_IF_NEGATIVE( unlink( filename ), - "Couldn't unlink %s: %s", - filename, - strerror( errno ) ); -} - int flexnbd_serve( struct flexnbd * flexnbd ) { @@ -372,16 +239,7 @@ int flexnbd_serve( struct flexnbd * flexnbd ) flexnbd_spawn_control( flexnbd ); } - if ( flexnbd->listen ){ - success = do_listen( flexnbd->listen ); - } - else { - do_serve( flexnbd->serve ); - /* We can't tell here what the intent was. We can - * legitimately exit either in control or not. - */ - success = 1; - } + success = do_serve( flexnbd->serve ); if ( flexnbd->control ) { debug( "Stopping control thread" ); diff --git a/src/flexnbd.h b/src/flexnbd.h index 25a05b5..6feb077 100644 --- a/src/flexnbd.h +++ b/src/flexnbd.h @@ -4,7 +4,6 @@ #include "acl.h" #include "mirror.h" #include "serve.h" -#include "listen.h" #include "self_pipe.h" #include "mbox.h" #include "control.h" @@ -12,29 +11,21 @@ /* Carries the "globals". */ struct flexnbd { - /* We always have a serve pointer, but it should never be * dereferenced outside a flexnbd_switch_lock/unlock pair. */ struct server * serve; - /* We only have a listen object if the process was started in - * listen mode. - */ - struct listen * listen; + /* We only have a control object if a control socket name was * passed on the command line. */ struct control * control; - /* switch_mutex is the lock around dereferencing the serve - * pointer. - */ - struct flexthread_mutex * switch_mutex; - /* File descriptor for a signalfd(2) signal stream. */ int signal_fd; }; + struct flexnbd * flexnbd_create(void); struct flexnbd * flexnbd_create_serving( char* s_ip_address, @@ -47,31 +38,21 @@ struct flexnbd * flexnbd_create_serving( int max_nbd_clients); struct flexnbd * flexnbd_create_listening( - char* s_ip_address, - char* s_rebind_ip_address, - char* s_port, - char* s_rebind_port, - char* s_file, - char *s_ctrl_sock, - int default_deny, - int acl_entries, - char** s_acl_entries, - int max_nbd_clients ); + char* s_ip_address, + char* s_port, + char* s_file, + char *s_ctrl_sock, + int default_deny, + int acl_entries, + char** s_acl_entries ); void flexnbd_destroy( struct flexnbd * ); enum mirror_state; enum mirror_state flexnbd_get_mirror_state( struct flexnbd * ); -void flexnbd_lock_switch( struct flexnbd * ); -void flexnbd_unlock_switch( struct flexnbd * ); -int flexnbd_switch_locked( struct flexnbd * ); int flexnbd_default_deny( struct flexnbd * ); void flexnbd_set_server( struct flexnbd * flexnbd, struct server * serve ); -void flexnbd_switch( struct flexnbd * flexnbd, struct server *(listen_cb)(struct listen *) ); int flexnbd_signal_fd( struct flexnbd * flexnbd ); -void flexnbd_mark_incomplete( struct flexnbd * flexnbd ); -void flexnbd_mark_complete( struct flexnbd * flexnbd ); - int flexnbd_serve( struct flexnbd * flexnbd ); struct server * flexnbd_server( struct flexnbd * flexnbd ); diff --git a/src/listen.c b/src/listen.c deleted file mode 100644 index de46e57..0000000 --- a/src/listen.c +++ /dev/null @@ -1,123 +0,0 @@ -#include "listen.h" -#include "serve.h" -#include "util.h" -#include "flexnbd.h" - -#include - -struct listen * listen_create( - struct flexnbd * flexnbd, - char* s_ip_address, - char* s_rebind_ip_address, - char* s_port, - char* s_rebind_port, - char* s_file, - int default_deny, - int acl_entries, - char** s_acl_entries, - int max_nbd_clients ) -{ - NULLCHECK( flexnbd ); - struct listen * listen; - - listen = (struct listen *)xmalloc( sizeof( struct listen ) ); - listen->flexnbd = flexnbd; - listen->init_serve = server_create( - flexnbd, - s_ip_address, - s_port, - s_file, - default_deny, - acl_entries, - s_acl_entries, - 1, 0); - listen->main_serve = server_create( - flexnbd, - s_rebind_ip_address ? s_rebind_ip_address : s_ip_address, - s_rebind_port ? s_rebind_port : s_port, - s_file, - default_deny, - acl_entries, - s_acl_entries, - max_nbd_clients, 1); - return listen; -} - - -void listen_destroy( struct listen * listen ) -{ - NULLCHECK( listen ); - free( listen ); -} - - -struct server *listen_switch( struct listen * listen ) -{ - NULLCHECK( listen ); - - /* TODO: Copy acl from init_serve to main_serve */ - /* TODO: rename underlying file from foo.INCOMPLETE to foo */ - - server_destroy( listen->init_serve ); - listen->init_serve = NULL; - info( "Switched to the main server, serving." ); - return listen->main_serve; -} - - -void listen_cleanup( struct listen * listen ) -{ - NULLCHECK( listen ); - - if ( flexnbd_switch_locked( listen->flexnbd ) ) { - flexnbd_unlock_switch( listen->flexnbd ); - } -} - -int do_listen( struct listen * listen ) -{ - NULLCHECK( listen ); - - int have_control = 0; - - flexnbd_lock_switch( listen->flexnbd ); - { - flexnbd_set_server( listen->flexnbd, listen->init_serve ); - flexnbd_mark_incomplete( listen->flexnbd ); - } - flexnbd_unlock_switch( listen->flexnbd ); - - /* WATCH FOR RACES HERE: flexnbd->serve is set, but the server - * isn't running yet and the switch lock is released. - */ - have_control = do_serve( listen->init_serve ); - - - if( have_control ) { - flexnbd_mark_complete( listen->flexnbd ); - - info( "Taking control."); - flexnbd_switch( listen->flexnbd, listen_switch ); - /* WATCH FOR RACES HERE: the server hasn't been - * restarted before we release the flexnbd switch lock. - * do_serve doesn't return, so there's not a lot of - * choice about that. - */ - do_serve( listen->main_serve ); - } - else { - warn("Failed to take control, giving up."); - server_destroy( listen->init_serve ); - listen->init_serve = NULL; - } - /* TODO: here we must signal the control thread to stop before - * it tries to */ - server_destroy( listen->main_serve ); - listen->main_serve = NULL; - - debug("Listen done, cleaning up"); - listen_cleanup( listen ); - - return have_control; -} - diff --git a/src/listen.h b/src/listen.h deleted file mode 100644 index 55a2bd4..0000000 --- a/src/listen.h +++ /dev/null @@ -1,28 +0,0 @@ -#ifndef LISTEN_H -#define LISTEN_H - -#include "flexnbd.h" -#include "serve.h" - -struct listen { - struct flexnbd * flexnbd; - struct server * init_serve; - struct server * main_serve; -}; - -struct listen * listen_create( - struct flexnbd * flexnbd, - char* s_ip_address, - char* s_rebind_ip_address, - char* s_port, - char* s_rebind_port, - char* s_file, - int default_deny, - int acl_entries, - char** s_acl_entries, - int max_nbd_clients ); -void listen_destroy( struct listen* ); - -int do_listen( struct listen * ); - -#endif diff --git a/src/mode.c b/src/mode.c index 7220548..382720b 100644 --- a/src/mode.c +++ b/src/mode.c @@ -32,31 +32,15 @@ static char serve_help_text[] = QUIET_LINE; -static struct option listen_options[] = { - GETOPT_HELP, - GETOPT_ADDR, - GETOPT_REBIND_ADDR, - GETOPT_PORT, - GETOPT_REBIND_PORT, - GETOPT_FILE, - GETOPT_SOCK, - GETOPT_DENY, - GETOPT_QUIET, - GETOPT_VERBOSE, - {0} -}; -static char listen_short_options[] = "hl:L:p:P:f:s:d" SOPT_QUIET SOPT_VERBOSE; +static struct option * listen_options = serve_options; +static char * listen_short_options = serve_short_options; static char listen_help_text[] = "Usage: flexnbd " CMD_LISTEN " [*]\n\n" - "Listen for an incoming migration on ADDR:PORT, " - "then switch to REBIND_ADDR:REBIND_PORT on completion " - "to serve FILE.\n\n" + "Listen for an incoming migration on ADDR:PORT.\n\n" HELP_LINE "\t--" OPT_ADDR ",-l \tThe address to listen on.\n" - "\t--" OPT_REBIND_ADDR ",-L \tThe address to switch to, if given.\n" "\t--" OPT_PORT ",-p \tThe port to listen on.\n" - "\t--" OPT_REBIND_PORT ",-P \tThe port to switch to, if given..\n" - "\t--" OPT_FILE ",-f \tThe file to serve.\n" + "\t--" OPT_FILE ",-f \tThe file to write to.\n" "\t--" OPT_DENY ",-d\tDeny connections by default unless in ACL.\n" SOCK_LINE VERBOSE_LINE @@ -234,9 +218,7 @@ void read_serve_param( int c, char **ip_addr, char **ip_port, char **file, char void read_listen_param( int c, char **ip_addr, - char **rebind_ip_addr, char **ip_port, - char **rebind_ip_port, char **file, char **sock, int *default_deny ) @@ -249,15 +231,9 @@ void read_listen_param( int c, case 'l': *ip_addr = optarg; break; - case 'L': - *rebind_ip_addr = optarg; - break; case 'p': *ip_port = optarg; break; - case 'P': - *rebind_ip_port = optarg; - break; case 'f': *file = optarg; break; @@ -428,7 +404,15 @@ int mode_serve( int argc, char *argv[] ) } if ( err ) { exit_err( serve_help_text ); } - flexnbd = flexnbd_create_serving( ip_addr, ip_port, file, sock, default_deny, argc - optind, argv + optind, MAX_NBD_CLIENTS ); + flexnbd = flexnbd_create_serving( + ip_addr, + ip_port, + file, + sock, + default_deny, + argc - optind, + argv + optind, + MAX_NBD_CLIENTS ); flexnbd_serve( flexnbd ); flexnbd_destroy( flexnbd ); @@ -440,9 +424,7 @@ int mode_listen( int argc, char *argv[] ) { int c; char *ip_addr = NULL; - char *rebind_ip_addr = NULL; char *ip_port = NULL; - char *rebind_ip_port = NULL; char *file = NULL; char *sock = NULL; int default_deny = 0; // not on by default @@ -456,7 +438,7 @@ int mode_listen( int argc, char *argv[] ) c = getopt_long(argc, argv, listen_short_options, listen_options, NULL); if ( c == -1 ) { break; } - read_listen_param( c, &ip_addr, &rebind_ip_addr, &ip_port, &rebind_ip_port, + read_listen_param( c, &ip_addr, &ip_port, &file, &sock, &default_deny ); } @@ -472,15 +454,12 @@ int mode_listen( int argc, char *argv[] ) flexnbd = flexnbd_create_listening( ip_addr, - rebind_ip_addr, ip_port, - rebind_ip_port, file, sock, default_deny, argc - optind, - argv + optind, - MAX_NBD_CLIENTS ); + argv + optind ); success = flexnbd_serve( flexnbd ); flexnbd_destroy( flexnbd ); diff --git a/tests/acceptance/environment.rb b/tests/acceptance/environment.rb index b71c31b..b2f7a01 100644 --- a/tests/acceptance/environment.rb +++ b/tests/acceptance/environment.rb @@ -17,8 +17,8 @@ class Environment @rebind_port1 = @available_ports.shift @port2 = @available_ports.shift @rebind_port2 = @available_ports.shift - @nbd1 = FlexNBD.new("../../build/flexnbd", @ip, @port1, @ip, @rebind_port1) - @nbd2 = FlexNBD.new("../../build/flexnbd", @ip, @port2, @ip, @rebind_port2) + @nbd1 = FlexNBD.new("../../build/flexnbd", @ip, @port1) + @nbd2 = FlexNBD.new("../../build/flexnbd", @ip, @port2) @fake_pid = nil end @@ -115,7 +115,7 @@ class Environment end - def run_fake( name, addr, port, rebind_addr = addr, rebind_port = port, sock=nil ) + def run_fake( name, addr, port, sock=nil ) fakedir = File.join( File.dirname( __FILE__ ), "fakes" ) fake = Dir[File.join( fakedir, name ) + "*"].sort.find { |fn| File.executable?( fn ) @@ -124,11 +124,9 @@ class Environment raise "no fake executable" unless fake raise "no addr" unless addr raise "no port" unless port - raise "no rebind_addr" unless rebind_addr - raise "no rebind_port" unless rebind_port @fake_pid = fork do - exec [fake, addr, port, @nbd1.pid, rebind_addr, rebind_port, sock].map{|x| x.to_s}.join(" ") + exec [fake, addr, port, @nbd1.pid, sock].map{|x| x.to_s}.join(" ") end sleep(0.5) end diff --git a/tests/acceptance/fakes/dest/break_after_hello.rb b/tests/acceptance/fakes/dest/break_after_hello.rb index 5bbe4a5..8a27cb7 100755 --- a/tests/acceptance/fakes/dest/break_after_hello.rb +++ b/tests/acceptance/fakes/dest/break_after_hello.rb @@ -7,7 +7,7 @@ require 'flexnbd/fake_dest' include FlexNBD -addr, port, src_pid, _, _, sock = *ARGV +addr, port, src_pid, sock = *ARGV server = FakeDest.new( addr, port ) client = server.accept diff --git a/tests/acceptance/fakes/source/close_after_entrust.rb b/tests/acceptance/fakes/source/close_after_entrust.rb index 325e8ef..3c351e6 100755 --- a/tests/acceptance/fakes/source/close_after_entrust.rb +++ b/tests/acceptance/fakes/source/close_after_entrust.rb @@ -3,8 +3,8 @@ # 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. +# In this situation we expect the destination to quit with an error +# status. require 'flexnbd/fake_source' include FlexNBD @@ -28,7 +28,11 @@ system "kill -CONT #{srv_pid}" sleep(0.25) -client2 = FakeSource.new( addr, port, "Timed out reconnecting" ) -client2.close +begin + client2 = FakeSource.new( addr, port, "Expected timeout" ) + fail "Unexpected reconnection" +rescue Timeout::Error + # expected +end exit(0) diff --git a/tests/acceptance/fakes/source/close_after_entrust_reply.rb b/tests/acceptance/fakes/source/close_after_entrust_reply.rb index 0ec149d..f519c2c 100755 --- a/tests/acceptance/fakes/source/close_after_entrust_reply.rb +++ b/tests/acceptance/fakes/source/close_after_entrust_reply.rb @@ -1,10 +1,9 @@ #!/usr/bin/env ruby -# Connect, send a migration, entrust then *immediately* disconnect. +# Connect, send a migration, entrust, read the reply, then 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. +# We expect the destination to quit with an error status. require 'flexnbd/fake_source' include FlexNBD @@ -22,11 +21,12 @@ client.close sleep(0.25) -client2 = FakeSource.new( addr, port, "Timed out reconnecting to mirror" ) -client2.send_mirror -sleep(1) -client3 = FakeSource.new( rebind_addr, rebind_port, "Timed out reconnecting to read" ) -client3.close +begin + client2 = FakeSource.new( addr, port, "Expected timeout" ) + fail "Unexpected reconnection" +rescue Timeout::Error + # expected +end exit(0) diff --git a/tests/acceptance/fakes/source/close_after_write.rb b/tests/acceptance/fakes/source/close_after_write.rb index 0f48dc6..250a462 100755 --- a/tests/acceptance/fakes/source/close_after_write.rb +++ b/tests/acceptance/fakes/source/close_after_write.rb @@ -12,10 +12,11 @@ addr, port, srv_pid = *ARGV client = FakeSource.new( addr, port, "Timed out connecting" ) client.read_hello -Process.kill( "STOP", srv_pid.to_i ) + +system "kill -STOP #{srv_pid}" client.write_write_request( 0, 8 ) client.close -Process.kill( "CONT", srv_pid.to_i ) +system "kill -CONT #{srv_pid}" # This sleep ensures that we don't return control to the test runner # too soon, giving the flexnbd process time to fall over if it's going diff --git a/tests/acceptance/fakes/source/close_after_write_data.rb b/tests/acceptance/fakes/source/close_after_write_data.rb index ca21711..8c23555 100755 --- a/tests/acceptance/fakes/source/close_after_write_data.rb +++ b/tests/acceptance/fakes/source/close_after_write_data.rb @@ -13,13 +13,13 @@ addr, port, srv_pid = *ARGV client = FakeSource.new( addr, port, "Timed out connecting" ) client.read_hello -Process.kill( "STOP", srv_pid.to_i ) +system "kill -STOP #{srv_pid}" client.write_write_request( 0, 8 ) client.write_data( "12345678" ) client.close -Process.kill( "CONT", srv_pid.to_i ) +system "kill -CONT #{srv_pid}" # This sleep ensures that we don't return control to the test runner # too soon, giving the flexnbd process time to fall over if it's going diff --git a/tests/acceptance/fakes/source/successful_transfer.rb b/tests/acceptance/fakes/source/successful_transfer.rb index 47e8ada..4b06155 100755 --- a/tests/acceptance/fakes/source/successful_transfer.rb +++ b/tests/acceptance/fakes/source/successful_transfer.rb @@ -1,17 +1,14 @@ #!/usr/bin/env ruby -# Successfully send a migration, but squat on the IP and port which -# the destination wants to rebind to. The destination should retry -# every second, so we give it up then attempt to connect to the new -# server. +# Successfully send a migration. This test just makes sure that the +# happy path is covered. We expect the destination to quit with a +# success status. require 'flexnbd/fake_source' include FlexNBD addr, port, srv_pid, newaddr, newport = *ARGV -squatter = TCPServer.open( newaddr, newport.to_i ) - client = FakeSource.new( addr, port, "Timed out connecting" ) client.send_mirror() diff --git a/tests/acceptance/flexnbd.rb b/tests/acceptance/flexnbd.rb index 19ec882..0e8d7d7 100644 --- a/tests/acceptance/flexnbd.rb +++ b/tests/acceptance/flexnbd.rb @@ -166,7 +166,7 @@ end # class ValgrindExecutor # Noddy test class to exercise FlexNBD from the outside for testing. # class FlexNBD - attr_reader :bin, :ctrl, :pid, :ip, :port, :rebind_ip, :rebind_port + attr_reader :bin, :ctrl, :pid, :ip, :port class << self def counter @@ -195,7 +195,7 @@ class FlexNBD end end - def initialize(bin, ip, port, rebind_ip = ip, rebind_port = port) + def initialize(bin, ip, port) @bin = bin @do_debug = ENV['DEBUG'] @debug = build_debug_opt @@ -204,8 +204,6 @@ class FlexNBD @ctrl = "/tmp/.flexnbd.ctrl.#{Time.now.to_i}.#{rand}" @ip = ip @port = port - @rebind_ip = rebind_ip - @rebind_port = rebind_port @kill = [] end @@ -235,8 +233,6 @@ class FlexNBD "--addr #{ip} "\ "--port #{port} "\ "--file #{file} "\ - "--rebind-addr #{rebind_ip} " \ - "--rebind-port #{rebind_port} " \ "--sock #{ctrl} "\ "#{@debug} "\ "#{acl.join(' ')}" diff --git a/tests/acceptance/test_dest_error_handling.rb b/tests/acceptance/test_dest_error_handling.rb index 27ef197..1118dca 100644 --- a/tests/acceptance/test_dest_error_handling.rb +++ b/tests/acceptance/test_dest_error_handling.rb @@ -80,15 +80,15 @@ class TestDestErrorHandling < Test::Unit::TestCase end - def test_cant_rebind_dies - @env.nbd1.can_die(6) + def test_straight_migration + @env.nbd1.can_die(0) run_fake( "source/successful_transfer" ) end private def run_fake( name ) - @env.run_fake( name, @env.ip, @env.port1, @env.ip, @env.rebind_port1 ) + @env.run_fake( name, @env.ip, @env.port1 ) assert @env.fake_reports_success, "#{name} failed." end diff --git a/tests/acceptance/test_source_error_handling.rb b/tests/acceptance/test_source_error_handling.rb index 688f4e0..043eb18 100644 --- a/tests/acceptance/test_source_error_handling.rb +++ b/tests/acceptance/test_source_error_handling.rb @@ -97,7 +97,7 @@ class TestSourceErrorHandling < Test::Unit::TestCase private def run_fake(name, opts = {}) - @env.run_fake( name, @env.ip, @env.port2, @env.ip, @env.port2, @env.nbd1.ctrl ) + @env.run_fake( name, @env.ip, @env.port2, @env.nbd1.ctrl ) stdout, stderr = @env.mirror12_unchecked assert_success assert_match( opts[:err], stderr ) if opts[:err] diff --git a/tests/unit/check_listen.c b/tests/unit/check_listen.c deleted file mode 100644 index 90a54d0..0000000 --- a/tests/unit/check_listen.c +++ /dev/null @@ -1,57 +0,0 @@ -#include "serve.h" -#include "listen.h" -#include "util.h" -#include "flexnbd.h" - -#include -#include - -START_TEST( test_defaults_main_serve_opts ) -{ - struct flexnbd flexnbd; - struct listen * listen = listen_create( &flexnbd, "127.0.0.1", NULL, "4777", NULL, - "foo", 0, 0, NULL, 1 ); - NULLCHECK( listen ); - struct server *init_serve = listen->init_serve; - struct server *main_serve = listen->main_serve; - NULLCHECK( init_serve ); - NULLCHECK( main_serve ); - - fail_unless( 0 == memcmp(&init_serve->bind_to, - &main_serve->bind_to, - sizeof( union mysockaddr )), - "Main serve bind_to was not set" ); -} -END_TEST - - -Suite* listen_suite(void) -{ - Suite *s = suite_create("listen"); - TCase *tc_create = tcase_create("create"); - - tcase_add_exit_test(tc_create, test_defaults_main_serve_opts, 0); - - suite_add_tcase(s, tc_create); - - return s; -} - -#ifdef DEBUG -# define LOG_LEVEL 0 -#else -# define LOG_LEVEL 2 -#endif - -int main(void) -{ - log_level = LOG_LEVEL; - int number_failed; - Suite *s = listen_suite(); - SRunner *sr = srunner_create(s); - srunner_run_all(sr, CK_NORMAL); - number_failed = srunner_ntests_failed(sr); - srunner_free(sr); - return (number_failed == 0) ? 0 : 1; -} -