From 2e4e592c0890ab03eee96ef3716e570b6cad3e6c Mon Sep 17 00:00:00 2001 From: Alex Young Date: Thu, 12 Jul 2012 18:01:10 +0100 Subject: [PATCH] Enable writing after the 2G boundary This patch fixes a bug in readwrite.c which truncated the 'from' field in nbd requests. It was casting them down from an off64_t to an int. --- src/client.c | 10 ++++------ src/ioutil.c | 28 ++++++++++++++++------------ src/readwrite.c | 2 +- tests/acceptance/environment.rb | 4 ++++ tests/acceptance/test_happy_path.rb | 11 +++++++++++ tests/unit/check_nbdtypes.c | 19 +++++++++++++++++++ 6 files changed, 55 insertions(+), 19 deletions(-) diff --git a/src/client.c b/src/client.c index d9a4cca..bf3973e 100644 --- a/src/client.c +++ b/src/client.c @@ -136,7 +136,6 @@ void write_not_zeroes(struct client* client, uint64_t from, int len) */ if (zerobuffer[0] != 0 || memcmp(zerobuffer, zerobuffer + 1, blockrun - 1)) { - debug("non-zero, writing from=%ld, blockrun=%d", from, blockrun); memcpy(client->mapped+from, zerobuffer, blockrun); bitset_set_range(map, from, blockrun); server_dirty(client->serve, from, blockrun); @@ -147,9 +146,7 @@ void write_not_zeroes(struct client* client, uint64_t from, int len) * sparseness as possible. */ } - else { - debug("all zero, skip write"); - } + len -= blockrun; run -= blockrun; from += blockrun; @@ -164,6 +161,7 @@ int fd_read_request( int fd, struct nbd_request_raw *out_request) return readloop(fd, out_request, sizeof(struct nbd_request_raw)); } + /* Returns 1 if *request was filled with a valid request which we should * try to honour. 0 otherwise. */ int client_read_request( struct client * client , struct nbd_request *out_request, int * disconnected ) @@ -318,7 +316,7 @@ int client_request_needs_reply( struct client * client, "after an entrust message."); /* check it's not out of range */ if ( request.from+request.len > client->serve->size) { - debug("request read %ld+%d out of range", + debug("request read %llx+%ld out of range", request.from, request.len ); @@ -495,7 +493,7 @@ void* client_serve(void* client_uncast) NULL, (void**) &client->mapped ), - "Couldn't open/mmap file %s", client->serve->filename + "Couldn't open/mmap file %s: %s", client->serve->filename, strerror( errno ) ); debug("client: sending hello"); client_send_hello(client); diff --git a/src/ioutil.c b/src/ioutil.c index 6aec0be..4519217 100644 --- a/src/ioutil.c +++ b/src/ioutil.c @@ -55,18 +55,22 @@ struct bitset_mapping* build_allocation_map(int fd, uint64_t size, int resolutio ); } - for (i=0; i<(size/resolution); i++) { - debug("map[%d] = %d%d%d%d%d%d%d%d", - i, - (allocation_map->bits[i] & 1) == 1, - (allocation_map->bits[i] & 2) == 2, - (allocation_map->bits[i] & 4) == 4, - (allocation_map->bits[i] & 8) == 8, - (allocation_map->bits[i] & 16) == 16, - (allocation_map->bits[i] & 32) == 32, - (allocation_map->bits[i] & 64) == 64, - (allocation_map->bits[i] & 128) == 128 - ); + /* This is pointlessly verbose for real discs, it's here as a + * reference for pulling data out of the allocation map */ + if ( 0 ) { + for (i=0; i<(size/resolution); i++) { + debug("map[%d] = %d%d%d%d%d%d%d%d", + i, + (allocation_map->bits[i] & 1) == 1, + (allocation_map->bits[i] & 2) == 2, + (allocation_map->bits[i] & 4) == 4, + (allocation_map->bits[i] & 8) == 8, + (allocation_map->bits[i] & 16) == 16, + (allocation_map->bits[i] & 32) == 32, + (allocation_map->bits[i] & 64) == 64, + (allocation_map->bits[i] & 128) == 128 + ); + } } diff --git a/src/readwrite.c b/src/readwrite.c index 400a921..a38b262 100644 --- a/src/readwrite.c +++ b/src/readwrite.c @@ -56,7 +56,7 @@ fail: return 0; } -void fill_request(struct nbd_request *request, int type, int from, int len) +void fill_request(struct nbd_request *request, int type, off64_t from, int len) { request->magic = htobe32(REQUEST_MAGIC); request->type = htobe32(type); diff --git a/tests/acceptance/environment.rb b/tests/acceptance/environment.rb index 69f015b..7675f14 100644 --- a/tests/acceptance/environment.rb +++ b/tests/acceptance/environment.rb @@ -79,6 +79,10 @@ class Environment end + def truncate1( size ) + system "truncate -s #{size} #{@filename1}" + end + def listening_ports `netstat -ltn`. diff --git a/tests/acceptance/test_happy_path.rb b/tests/acceptance/test_happy_path.rb index 8f0c2c7..0eadc9c 100644 --- a/tests/acceptance/test_happy_path.rb +++ b/tests/acceptance/test_happy_path.rb @@ -81,4 +81,15 @@ class TestHappyPath < Test::Unit::TestCase assert @env.status2['has_control'], "destination didn't take control" end + + def test_write_to_high_block + # Create a large file, then try to write to somewhere after the 2G boundary + @env.truncate1 "4G" + @env.serve1 + + @env.nbd1.write( 2**31+2**29, "12345678" ) + sleep(1) + assert_equal "12345678", @env.nbd1.read( 2**31+2**29, 8 ) + end + end diff --git a/tests/unit/check_nbdtypes.c b/tests/unit/check_nbdtypes.c index 81a5418..58e5b40 100644 --- a/tests/unit/check_nbdtypes.c +++ b/tests/unit/check_nbdtypes.c @@ -182,6 +182,24 @@ START_TEST(test_reply_handle) END_TEST +START_TEST( test_convert_from ) +{ + /* Check that we can correctly pull numbers out of an + * nbd_request_raw */ + struct nbd_request_raw request_raw; + struct nbd_request request; + char readbuf[] = {0x80, 0, 0, 0, 0, 0, 0, 0}; + + memcpy( &request_raw.from, readbuf, 8 ); + + nbd_r2h_request( &request_raw, &request ); + + uint64_t target = 1; + target <<= 63; + fail_unless( target == request.from, "from was wrong" ); +} +END_TEST + Suite *nbdtypes_suite(void) { Suite *s = suite_create( "nbdtypes" ); @@ -197,6 +215,7 @@ Suite *nbdtypes_suite(void) tcase_add_test( tc_request, test_request_handle ); tcase_add_test( tc_request, test_request_from ); tcase_add_test( tc_request, test_request_len ); + tcase_add_test( tc_request, test_convert_from ); tcase_add_test( tc_reply, test_reply_magic ); tcase_add_test( tc_reply, test_reply_error ); tcase_add_test( tc_reply, test_reply_handle );