Use correct constant name.

This alters the meaning of NBD_MAX_SIZE to be the actual max request size
we'll accept over nbd.  Previously it was *nearly* the max size we'd
accept depending on the size of the struct.

.gitlab-ci.yml

@@ -1,10 +1,27 @@
-image: "ruby:2.1"
-
-before_script:
-  - apt-get update; apt-get install -y check libev-dev net-tools dpkg-dev
-
-unit_test:
+stages:
+  - package
+  - publish
+ 
+package:jessie: &package
+  stage: package
+  image: $CI_REGISTRY/docker-images/layers:$DISTRO-deb
+  variables:
+    DISTRO: jessie
   script:
-  - make clean
-  - make build 
-  - make test
+    - package
+  artifacts:
+    paths:
+      - pkg/
+
+package:stretch:
+  <<: *package
+  variables:
+    DISTRO: stretch
+
+publish:
+  stage: publish
+  tags: 
+    - shell
+  script:
+    - publish
+

Makefile

@@ -11,28 +11,9 @@ ifdef DEBUG
 else
 	CFLAGS_EXTRA=-O2
 endif
+
 CFLAGS_EXTRA  += -fPIC --std=gnu99
-LDFLAGS_EXTRA += -Wl,--relax,--gc-sections
-
-TOOLCHAIN		:= $(shell $(CC) --version|awk '/Debian/ {print "debian";exit;}')
-#
-# This bit adds extra flags depending of the distro, and the
-# architecture. To make sure debian packages have the right
-# set of 'native' flags on them
-#
-ifeq ($(TOOLCHAIN),debian)
-DEBARCH			:= $(shell dpkg-architecture -qDEB_BUILD_ARCH)
-ifeq ($(DEBARCH),$(filter $(DEBARCH),amd64 i386))
-CFLAGS_EXTRA	+= -march=native
-endif
-ifeq ($(DEBARCH),armhf)
-CFLAGS_EXTRA	+=  -march=armv7-a -mtune=cortex-a8 -mfpu=neon
-endif
-LDFLAGS_EXTRA	+= -L$(LIB) -Wl,-rpath,${shell readlink -f ${LIB}}
-else
-LDFLAGS_EXTRA	+= -L$(LIB) -Wl,-rpath-link,$(LIB)
-endif
-
+LDFLAGS_EXTRA += -Wl,--relax,--gc-sections -L$(LIB) -Wl,-rpath-link,$(LIB)
 
 # The -Wunreachable-code warning is only implemented in clang, but it
 # doesn't break anything for gcc to see it.
@@ -42,10 +23,10 @@ WARNINGS=-Wall \
 				 -Wstrict-prototypes \
 				 -Wno-missing-field-initializers \
 				 -Wunreachable-code
+
 CCFLAGS=-D_GNU_SOURCE=1 $(WARNINGS) $(CFLAGS_EXTRA) $(CFLAGS)
 LLDFLAGS=-lm -lrt -lev $(LDFLAGS_EXTRA) $(LDFLAGS)
 
-
 CC?=gcc
 
 LIBS=-lpthread
@@ -94,7 +75,7 @@ CHECK_OBJ := $(CHECK_SRC:tests/unit/%.c=build/%.o)
 CHECK_BINS := $(CHECK_SRC:tests/unit/%.c=build/%)
 
 build/check_%: build/check_%.o
-	$(LINK) $^ -o $@ $(COMMON_OBJ) $(SERVER_OBJ) -lcheck
+	$(LINK) $^ -o $@ $(COMMON_OBJ) $(SERVER_OBJ) -lcheck -lsubunit
 
 check_objs: $(CHECK_OBJ)
 

debian/changelog

@@ -1,3 +1,37 @@
+flexnbd (0.1.8) UNRELEASED; urgency=medium
+
+  * Set TCP keepalive on sockets so broken connections are reaped (#33, !33)
+  * Add a context to logs to make debugging problems easier (#34, !34)
+
+ -- James Carter <james.carter@bytemark.co.uk>  Thu, 11 Jan 2018 10:05:35 +0000
+
+flexnbd (0.1.7) stable; urgency=medium
+
+  * Return bytes_left in migration statistics.
+
+ -- Chris Elsworth <chris.elsworth@bytemark.co.uk>  Fri, 14 Jul 2017 17:00:38 +0100
+
+flexnbd (0.1.6) stable; urgency=medium
+
+  * Remove lots of per-cpu compiler flags, notably march=native.
+
+ -- Patrick J Cherry <patrick@bytemark.co.uk>  Thu, 13 Apr 2017 12:48:23 +0100
+
+flexnbd (0.1.5) stable; urgency=medium
+
+  * Switched to native packaging.
+
+ -- Patrick J Cherry <patrick@bytemark.co.uk>  Mon, 23 Jan 2017 13:51:58 +0000
+
+flexnbd (0.1.5-1) stable; urgency=medium
+
+  * This fixes the compiler warning pointer-to-int-cast in serve.c.
+  * Fix up "wrong" handle type from char* to uint64_t in check_readwrite test.
+  * fix check_bitset test on 32-bit platforms.
+  * Skip large file test on 32-bit platforms.
+
+ -- Patrick J Cherry <patrick@bytemark.co.uk>  Fri, 07 Oct 2016 12:25:36 +0100
+
 flexnbd (0.1.4-1) stable; urgency=medium
 
   [ Michel Pollet ]

debian/control

@@ -2,7 +2,7 @@ Source: flexnbd
 Section: web
 Priority: extra
 Maintainer: Patrick J Cherry <patrick@bytemark.co.uk> 
-Build-Depends: debhelper (>= 7.0.50), ruby, gcc, libev-dev, txt2man, check, net-tools
+Build-Depends: debhelper (>= 7.0.50), ruby, gcc, libev-dev, txt2man, check, net-tools, libsubunit-dev, ruby-test-unit
 Standards-Version: 3.8.1
 Homepage: https://github.com/BytemarkHosting/flexnbd-c
 

debian/source/format

@@ -1 +1 @@
-3.0 (quilt)
+3.0 (native)

src/common/nbdtypes.h

@@ -16,7 +16,7 @@
 
 
 /* 1MiB is the de-facto standard for maximum size of header + data */
-#define NBD_MAX_SIZE ( 1024 * 1024 )
+#define NBD_MAX_SIZE ( 32 * 1024 * 1024 )
 
 #define NBD_REQUEST_SIZE ( sizeof( struct nbd_request_raw ) )
 #define NBD_REPLY_SIZE   ( sizeof( struct nbd_reply_raw ) )

src/common/sockutil.c

@@ -68,6 +68,37 @@ int sock_set_reuseaddr( int fd, int optval )
 	return setsockopt( fd, SOL_SOCKET, SO_REUSEADDR, &optval, sizeof(optval) );
 }
 
+int sock_set_keepalive_params( int fd, int time, int intvl, int probes)
+{
+	if (sock_set_keepalive(fd, 1) ||
+		sock_set_tcp_keepidle(fd, time) ||
+		sock_set_tcp_keepintvl(fd, intvl) ||
+		sock_set_tcp_keepcnt(fd, probes)) {
+		return -1;
+	}
+	return 0;
+}
+
+int sock_set_keepalive( int fd, int optval )
+{
+	return setsockopt( fd, SOL_SOCKET, SO_KEEPALIVE, &optval, sizeof(optval) );
+}
+
+int sock_set_tcp_keepidle( int fd, int optval )
+{
+	return setsockopt( fd, IPPROTO_TCP, TCP_KEEPIDLE, &optval, sizeof(optval) );
+}
+
+int sock_set_tcp_keepintvl( int fd, int optval )
+{
+	return setsockopt( fd, IPPROTO_TCP, TCP_KEEPINTVL, &optval, sizeof(optval) );
+}
+
+int sock_set_tcp_keepcnt( int fd, int optval )
+{
+	return setsockopt( fd, IPPROTO_TCP, TCP_KEEPCNT, &optval, sizeof(optval) );
+}
+
 /* Set the tcp_nodelay option */
 int sock_set_tcp_nodelay( int fd, int optval )
 {

src/common/sockutil.h

@@ -14,9 +14,24 @@ size_t sockaddr_size(const struct sockaddr* sa);
  */
 const char* sockaddr_address_string(const struct sockaddr* sa, char* dest, size_t len);
 
+/* Configure TCP keepalive on a socket */
+int sock_set_keepalive_params( int fd, int time, int intvl, int probes);
+
+/* Set the SOL_KEEPALIVE otion */
+int sock_set_keepalive(int fd, int optval);
+
 /* Set the SOL_REUSEADDR otion */
 int sock_set_reuseaddr(int fd, int optval);
 
+/* Set the tcp_keepidle option */
+int sock_set_tcp_keepidle(int fd, int optval);
+
+/* Set the tcp_keepintvl option */
+int sock_set_tcp_keepintvl(int fd, int optval);
+
+/* Set the tcp_keepcnt option */
+int sock_set_tcp_keepcnt(int fd, int optval);
+
 /* Set the tcp_nodelay option */
 int sock_set_tcp_nodelay(int fd, int optval);
 

src/common/util.c

@@ -13,6 +13,7 @@
 pthread_key_t cleanup_handler_key;
 
 int log_level = 2;
+char *log_context = "";
 
 void error_init(void)
 {

src/common/util.h

@@ -21,6 +21,9 @@ extern int log_level;
 /* set up the error globals */
 void error_init(void);
 
+/* some context for the overall process that appears on each log line */
+extern char *log_context;
+
 
 void exit_err( const char * );
 
@@ -92,7 +95,7 @@ uint64_t monotonic_time_ms(void);
 
 #define levstr(i) (i==0?'D':(i==1?'I':(i==2?'W':(i==3?'E':'F'))))
 
-#define myloglev(level, msg, ...) mylog( level, "%"PRIu64":%c:%d %p %s:%d: "msg"\n", monotonic_time_ms(), levstr(level), getpid(),pthread_self(), __FILE__, __LINE__, ##__VA_ARGS__ )
+#define myloglev(level, msg, ...) mylog( level, "%"PRIu64":%c:%d %p %s %s:%d: "msg"\n", monotonic_time_ms(), levstr(level), getpid(),pthread_self(), log_context, __FILE__, __LINE__, ##__VA_ARGS__ )
 
 #ifdef DEBUG
 #  define debug(msg, ...) myloglev(0, msg, ##__VA_ARGS__)

src/proxy/proxy.c

@@ -76,8 +76,15 @@ struct proxier* proxy_create(
 	}
 
 	out->init.buf = xmalloc( sizeof( struct nbd_init_raw ) );
-	out->req.buf  = xmalloc( NBD_MAX_SIZE );
-	out->rsp.buf  = xmalloc( NBD_MAX_SIZE );
+
+	/* Add on the request / reply size to our malloc to accommodate both
+	 * the struct and the data
+	 */
+	out->req.buf  = xmalloc( NBD_MAX_SIZE + NBD_REQUEST_SIZE );
+	out->rsp.buf  = xmalloc( NBD_MAX_SIZE + NBD_REPLY_SIZE );
+
+	log_context = xmalloc( strlen(s_upstream_address) + strlen(s_upstream_port) + 2 );
+	sprintf(log_context, "%s:%s", s_upstream_address, s_upstream_port);
 
 	return out;
 }
@@ -437,15 +444,18 @@ int proxy_read_from_downstream( struct proxier *proxy, int state )
 			return EXIT;
 		}
 
-		/* Simple validations */
+		/* Simple validations -- the request / reply size have already
+		 * been taken into account in the xmalloc, so no need to worry
+		 * about them here
+		 */
 		if ( ( request->type & REQUEST_MASK ) == REQUEST_READ ) {
-			if (request->len > ( NBD_MAX_SIZE - NBD_REPLY_SIZE ) ) {
+			if ( request->len > NBD_MAX_SIZE ) {
 				warn( "NBD read request size %"PRIu32" too large", request->len );
 				return EXIT;
 			}
 		}
 		if ( (request->type & REQUEST_MASK ) == REQUEST_WRITE ) {
-			if (request->len > ( NBD_MAX_SIZE - NBD_REQUEST_SIZE ) ) {
+			if ( request->len > NBD_MAX_SIZE ) {
 				warn( "NBD write request size %"PRIu32" too large", request->len );
 				return EXIT;
 			}

src/server/serve.c

@@ -78,6 +78,8 @@ struct server * server_create (
 	NULLCHECK( out->close_signal );
 	NULLCHECK( out->acl_updated_signal );
 
+	log_context = s_file;
+
 	return out;
 }
 
@@ -255,7 +257,7 @@ int tryjoin_client_thread( struct client_tbl_entry *entry, int (*joinfunc)(pthre
 			debug("nbd thread %016x exited (%s) with status %ld",
 					entry->thread,
 					s_client_address,
-					(uint64_t)status);
+					(uintptr_t)status);
 			client_destroy( entry->client );
 			entry->client = NULL;
 			entry->thread = 0;
@@ -422,6 +424,9 @@ void accept_nbd_client(
 	int slot;
 	char s_client_address[64] = {0};
 
+	FATAL_IF_NEGATIVE( sock_set_keepalive_params( client_fd, CLIENT_KEEPALIVE_TIME, CLIENT_KEEPALIVE_INTVL, CLIENT_KEEPALIVE_PROBES),
+		"Error setting keepalive parameters on client socket fd %d", client_fd );
+
 
 	if ( !server_should_accept_client( params, client_address, s_client_address, 64 ) ) {
 		FATAL_IF_NEGATIVE( close( client_fd ),

src/server/serve.h

@@ -21,6 +21,9 @@ struct client_tbl_entry {
 
 
 #define MAX_NBD_CLIENTS 16
+#define CLIENT_KEEPALIVE_TIME 30
+#define CLIENT_KEEPALIVE_INTVL 10
+#define CLIENT_KEEPALIVE_PROBES 3
 struct server {
 	/* The flexnbd wrapper this server is attached to */
 	struct flexnbd * flexnbd;

src/server/status.c

@@ -31,6 +31,7 @@ struct status * status_create( struct server * serve )
 		status->migration_speed_limit = serve->mirror->max_bytes_per_second;
 
 		status->migration_seconds_left = server_mirror_eta( serve );
+		status->migration_bytes_left = server_mirror_bytes_remaining( serve );
 	}
 
 	server_unlock_start_mirror( serve );
@@ -60,6 +61,7 @@ int status_write( struct status * status, int fd )
 		PRINT_UINT64( migration_speed );
 		PRINT_UINT64( migration_duration );
 		PRINT_UINT64( migration_seconds_left );
+		PRINT_UINT64( migration_bytes_left );
 		if ( status->migration_speed_limit < UINT64_MAX ) {
 			PRINT_UINT64( migration_speed_limit );
 		};

src/server/status.h

@@ -64,6 +64,8 @@
  *   Our current best estimate of how many seconds are left before the migration
  *   migration is finished.
  *
+ * migration_bytes_left:
+ *   The number of bytes remaining to migrate.
  */
 
 
@@ -84,6 +86,7 @@ struct status {
 	uint64_t migration_speed;
 	uint64_t migration_speed_limit;
 	uint64_t migration_seconds_left;
+	uint64_t migration_bytes_left;
 };
 
 /** Create a status object for the given server. */

tests/acceptance/test_happy_path.rb

@@ -115,6 +115,11 @@ class TestHappyPath < Test::Unit::TestCase
 
 
   def test_write_to_high_block
+		# 
+		# This test does not work on 32 bit platforms.
+		#
+		skip("Not relevant on 32-bit platforms") if ( ["a"].pack("p").size < 8 )
+
     # Create a large file, then try to write to somewhere after the 2G boundary
     @env.truncate1 "4G"
     @env.serve1

tests/unit/check_bitset.c

@@ -59,7 +59,7 @@ END_TEST
 START_TEST(test_bit_ranges)
 {
 	bitfield_word_t buffer[BIT_WORDS_FOR_SIZE(4160)];
-	uint64_t  *longs = (unsigned long*) buffer;
+	uint64_t  *longs = (uint64_t *) buffer;
 	uint64_t i;
 
 	memset(buffer, 0, 4160);
@@ -67,9 +67,9 @@ START_TEST(test_bit_ranges)
 	for (i=0; i<64; i++) {
 		bit_set_range(buffer, i*64, i);
 		fail_unless(
-			longs[i] == (1UL<<i)-1,
+			longs[i] == (1ULL<<i)-1,
 			"longs[%ld] = %lx SHOULD BE %lx",
-			i, longs[i], (1L<<i)-1
+			i, longs[i], (1ULL<<i)-1
 		);
 
 		fail_unless(longs[i+1] == 0, "bit_set_range overshot at i=%d", i);

tests/unit/check_readwrite.c

@@ -46,8 +46,7 @@ void * responder( void *respond_uncast )
 	struct respond * resp = (struct respond *) respond_uncast;
 	int sock_fd = resp->sock_fds[1];
 	struct nbd_request_raw request_raw;
-	char wrong_handle[] = "WHOOPSIE";
-
+	uint64_t wrong_handle = 0x80;
 
 	if( fd_read_request( sock_fd, &request_raw ) == -1){
 		fprintf(stderr, "Problem with fd_read_request\n");

tests/unit/check_status.c

@@ -308,6 +308,7 @@ START_TEST( test_renders_migration_statistics )
 	status.migration_speed = 40000000;
 	status.migration_speed_limit = 40000001;
 	status.migration_seconds_left = 1;
+	status.migration_bytes_left = 5000;
 
 	status_write( &status, fds[1] );
 	fail_if_rendered( fds[0], "migration_duration" );
@@ -335,6 +336,9 @@ START_TEST( test_renders_migration_statistics )
 	status_write( &status, fds[1] );
 	fail_unless_rendered( fds[0], "migration_seconds_left=1" );
 
+	status_write( &status, fds[1] );
+	fail_unless_rendered( fds[0], "migration_bytes_left=5000" );
+
 	status.migration_speed_limit = UINT64_MAX;
 
 	status_write( &status, fds[1] );