Changed msync to actually sync

This copies xnbd's style

.gitlab-ci.yml

@@ -1,10 +1,27 @@
-image: "ruby:2.1"
+stages:
+  - package
+  - publish
  
-before_script:
-  - apt-get update; apt-get install -y check libev-dev net-tools dpkg-dev
-
-unit_test:
+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,14 @@ ifdef DEBUG
 else
 	CFLAGS_EXTRA=-O2
 endif
+
+NO_MSYNC=1
+ifdef NO_MSYNC
+	CFLAGS_EXTRA += -DNO_MSYNC
+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 +28,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 +80,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/compat

@@ -0,0 +1 @@
+7

debian/control

@@ -0,0 +1,25 @@
+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, libsubunit-dev, ruby-test-unit
+Standards-Version: 3.8.1
+Homepage: https://github.com/BytemarkHosting/flexnbd-c
+
+Package: flexnbd
+Architecture: any
+Depends: ${shlibs:Depends}, ${misc:Depends}, libev4 | libev3
+Description: FlexNBD server
+  An NBD server offering push-mirroring and intelligent sparse file handling
+
+Package: flexnbd-dbg
+Architecture: any
+Section: debug
+Priority: extra
+Depends:
+    flexnbd (= ${binary:Version}),
+    ${misc:Depends}
+Description: debugging symbols for flexnbd
+ An NBD server offering push-mirroring and intelligent sparse file handling
+ .
+ This package contains the debugging symbols for flexnbd.

debian/copyright

@@ -0,0 +1,53 @@
+This work was packaged for Debian by:
+
+    Alex Young <alex@bytemark.co.uk> on Wed, 30 May 2012 16:46:58 +0100
+
+It was downloaded from:
+
+    <url://example.com>
+
+Upstream Author(s):
+
+    <put author's name and email here>
+    <likewise for another author>
+
+Copyright:
+
+    <Copyright (C) YYYY Firstname Lastname>
+    <likewise for another author>
+
+License:
+
+### SELECT: ###
+    This package is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 2 of the License, or
+    (at your option) any later version.
+### OR ###
+   This package is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License version 2 as
+   published by the Free Software Foundation.
+##########
+
+    This package is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program. If not, see <http://www.gnu.org/licenses/>
+
+On Debian systems, the complete text of the GNU General
+Public License version 2 can be found in "/usr/share/common-licenses/GPL-2".
+
+The Debian packaging is:
+
+    Copyright (C) 2012 Alex Young <alex@bytemark.co.uk>
+
+you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 2 of the License, or
+(at your option) any later version.
+
+# Please also look if there are files or directories which have a
+# different copyright/license attached and list them here.

debian/flexnbd.install

@@ -0,0 +1,3 @@
+build/flexnbd             usr/bin
+build/flexnbd-proxy       usr/bin
+

debian/flexnbd.manpages

@@ -0,0 +1,2 @@
+build/flexnbd.1.gz
+build/flexnbd-proxy.1.gz

debian/rules

@@ -0,0 +1,19 @@
+#!/usr/bin/make -f
+# -*- makefile -*-
+
+# Uncomment this to turn on verbose mode.
+#export DH_VERBOSE=1
+
+%:
+	dh $@ 
+
+override_dh_strip:
+	dh_strip --dbg-package=flexnbd-dbg
+
+#
+# TODO: The ruby test suites don't work during buiding in a chroot, so leave
+# them out for now.
+#
+#override_dh_auto_test:
+#	rake test:run
+

debian/source/format

@@ -0,0 +1 @@
+3.0 (native)

src/common/ioutil.c

@@ -73,7 +73,6 @@ int build_allocation_map(struct bitset * allocation_map, int fd)
 	return 1;
 }
 
-
 int open_and_mmap(const char* filename, int* out_fd, uint64_t *out_size, void **out_map)
 {
 	/* 
@@ -101,7 +100,7 @@ int open_and_mmap(const char* filename, int* out_fd, uint64_t *out_size, void **
 		*out_size = size;
 	}
 
-	if (out_map) {
+	if (0) {
 		*out_map = mmap64(NULL, size, PROT_READ|PROT_WRITE, MAP_SHARED,
 		  *out_fd, 0);
 		if (((long) *out_map) == -1) {
@@ -349,3 +348,36 @@ ssize_t iobuf_write( int fd, struct iobuf *iobuf )
 
 	return count;
 }
+
+struct iommap *iommap_alloc(int fd, off64_t from, uint64_t len) {
+
+	off66_t mmap_from = from & ~((off64_t) getpagesize() - 1);
+        uint64_t mmap_len = len + (from - mmap_from);
+        void *mmap_buf = NULL;
+
+	// TODO: Check the error code from mmap64
+	if(mmap_len)
+		mmap_buf = mmap64(NULL, mmap_len, PROT_READ | PROT_WRITE, MAP_SHARED, fd, mmap_from);
+
+	struct iommap *im = xmalloc(sizeof(struct iommap ));
+
+	im->mmap_buf = mmap_buf;
+	im->mmap_len = mmap_len;
+	im->buf = (char *) mmap_buf + from - mmap_from;
+	debug("mmapped file from %ld:%d", mmap_from, mmap_len);
+	return im;
+}
+
+void iommap_sync(struct iommap *im) {
+        if (im->mmap_len)
+		msync(im->mmap_buf, im->mmap_len, MS_SYNC | MS_INVALIDATE);
+	return;
+}
+
+void iommap_free(struct iommap *im) {
+        if (im->mmap_len)
+		munmap(im->mmap_buf, im->mmap_len);
+
+	free(im);
+}
+

src/common/ioutil.h

@@ -11,7 +11,20 @@ struct iobuf {
 ssize_t iobuf_read( int fd, struct iobuf* iobuf, size_t default_size );
 ssize_t iobuf_write( int fd, struct iobuf* iobuf );
 
+#include <inttypes.h>
+
+struct iommap {
+	char *buf;
+	char *mmap_buf;
+	uint64_t mmap_len;
+};
+
+struct iommap *iommap_alloc(int fd, off64_t from, uint64_t len);
+void   iommap_sync(struct iommap *im);
+void   iommap_free(struct iommap *im);
+
 #include "serve.h"
+
 struct bitset; /* don't need whole of bitset.h here */
 
 /** Scan the file opened in ''fd'', set bits in ''allocation_map''  that

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

@@ -79,6 +79,9 @@ struct proxier* proxy_create(
 	out->req.buf  = xmalloc( NBD_MAX_SIZE );
 	out->rsp.buf  = xmalloc( NBD_MAX_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;
 }
 

src/server/client.c

@@ -118,6 +118,7 @@ void write_not_zeroes(struct client* client, uint64_t from, uint64_t len)
 		 */
 
 		uint64_t run = bitset_run_count(map, from, len);
+		struct iommap *iommap = iommap_alloc(client->fileno, from, len); 
 
 		debug("write_not_zeroes: from=%ld, len=%d, run=%d", from, len, run);
 
@@ -155,7 +156,7 @@ void write_not_zeroes(struct client* client, uint64_t from, uint64_t len)
 		if (bitset_is_set_at(map, from)) {
 			debug("writing the lot: from=%ld, run=%d", from, run);
 			/* already allocated, just write it all */
-			DO_READ(client->mapped + from, run);
+			DO_READ(iommap->buf, run);
 			/* We know from our earlier call to  bitset_run_count that the
 			 * bitset is all-1s at this point, but we need to dirty it for the
 			 * sake of the event stream - the actual bytes have changed, and we
@@ -186,7 +187,7 @@ void write_not_zeroes(struct client* client, uint64_t from, uint64_t len)
 					(0 == memcmp( zerobuffer, zerobuffer+1, blockrun-1 ));
 
 				if ( !all_zeros ) {
-					memcpy(client->mapped+from, zerobuffer, blockrun);
+					memcpy(iommap->buf, zerobuffer, blockrun);
 					bitset_set_range(map, from, blockrun);
 					/* at this point we could choose to
 					 * short-cut the rest of the write for
@@ -205,6 +206,8 @@ void write_not_zeroes(struct client* client, uint64_t from, uint64_t len)
 				from += blockrun;
 			}
 		}
+		iommap_sync(iommap);
+		iommap_free(iommap);
 	}
 }
 
@@ -234,6 +237,10 @@ int client_read_request( struct client * client , struct nbd_request *out_reques
 				debug( "Connection reset while"
 						" reading request" );
 				return 0;
+			case ETIMEDOUT:
+				debug( "Connection timed out while"
+						" reading request" );
+				return 0;
 			default:
 				/* FIXME: I've seen this happen, but I
 				 * couldn't reproduce it so I'm leaving
@@ -445,22 +452,28 @@ void client_reply_to_read( struct client* client, struct nbd_request request )
 void client_reply_to_write( struct client* client, struct nbd_request request )
 {
 	debug("request write from=%"PRIu64", len=%"PRIu32", handle=0x%08X", request.from, request.len, request.handle);
-	if (client->serve->allocation_map_built) {
+	// TODO: Just write directly for now.  Not (yet) convinced my changes later on work.
+	// if (client->serve->allocation_map_built) {
+	if (0) {
 		write_not_zeroes( client, request.from, request.len );
 	}
 	else {
 		debug("No allocation map, writing directly.");
 		/* If we get cut off partway through reading this data:
 		 * */
+		struct iommap *iommap = iommap_alloc(client->fileno, request.from, request.len); 
+
 		ERROR_IF_NEGATIVE(
 			readloop( client->socket,
-				client->mapped + request.from,
+				iommap->buf,
 				request.len),
 			"reading write data failed from=%ld, len=%d",
 			request.from,
 			request.len
 		);
 
+		iommap_sync(iommap);
+		iommap_free(iommap);
 		/* the allocation_map is shared between client threads, and may be
 		 * being built. We need to reflect the write in it, as it may be in
 		 * a position the builder has already gone over.
@@ -468,19 +481,6 @@ void client_reply_to_write( struct client* client, struct nbd_request request )
 		bitset_set_range(client->serve->allocation_map, request.from, request.len);
 	}
 
-	if (1) /* not sure whether this is necessary... */
-	{
-		/* multiple of 4K page size */
-		uint64_t from_rounded = request.from & (!0xfff);
-		uint64_t len_rounded = request.len + (request.from - from_rounded);
-
-		FATAL_IF_NEGATIVE(
-			msync( client->mapped + from_rounded,
-				len_rounded,
-				MS_SYNC | MS_INVALIDATE),
-			"msync failed %ld %ld", request.from, request.len
-		);
-	}
 	client_write_reply( client, &request, 0);
 }
 
@@ -646,9 +646,6 @@ void client_cleanup(struct client* client,
 		debug("Closed client socket fd %d", client->socket);
 		client->socket = -1;
 	}
-	if (client->mapped) {
-		munmap(client->mapped, client->serve->size);
-	}
 	if (client->fileno) {
 		FATAL_IF_NEGATIVE( close(client->fileno),
 			"Error closing file %d",
@@ -664,25 +661,20 @@ void client_cleanup(struct client* client,
 void* client_serve(void* client_uncast)
 {
 	struct client* client = (struct client*) client_uncast;
+	void** a = NULL;
 
 	error_set_handler((cleanup_handler*) client_cleanup, client);
-
 	info("client: mmaping file");
 	FATAL_IF_NEGATIVE(
 		open_and_mmap(
 			client->serve->filename,
 			&client->fileno,
 			NULL,
-			(void**) &client->mapped
+			a
 		),
 		"Couldn't open/mmap file %s: %s", client->serve->filename, strerror( errno )
 	);
 
-	FATAL_IF_NEGATIVE(
-		madvise( client->mapped, client->serve->size, MADV_RANDOM ),
-		SHOW_ERRNO( "Failed to madvise() %s", client->serve->filename )
-	);
-
 	debug( "Opened client file fd %d", client->fileno);
 	debug("client: sending hello");
 	client_send_hello(client);

src/server/client.h

@@ -29,7 +29,6 @@ struct client {
 	int     socket;
 
 	int     fileno;
-	char*   mapped;
 
 	struct self_pipe * stop_signal;
 

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;
 }
 
@@ -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/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] );