Allow proxy to pass NBD protocol errors downstream; server returns EINVAL/ENOSPC appropriately
Previously the proxy would just disconnect when it saw an NBD protocol error, and retry the operation it was in the middle of. Additionally, the server needs to return the correct error types when this happens.
This commit is contained in:
@@ -618,11 +618,6 @@ int proxy_read_from_upstream( struct proxier* proxy, int state )
|
|||||||
goto disconnect;
|
goto disconnect;
|
||||||
}
|
}
|
||||||
|
|
||||||
if ( reply->error != 0 ) {
|
|
||||||
warn( "NBD error returned from upstream: %"PRIu32, reply->error );
|
|
||||||
goto disconnect;
|
|
||||||
}
|
|
||||||
|
|
||||||
if ( proxy->req_hdr.type == REQUEST_READ ) {
|
if ( proxy->req_hdr.type == REQUEST_READ ) {
|
||||||
/* Get the read reply data too. */
|
/* Get the read reply data too. */
|
||||||
proxy->rsp.size += proxy->req_hdr.len;
|
proxy->rsp.size += proxy->req_hdr.len;
|
||||||
|
@@ -391,7 +391,9 @@ int client_request_needs_reply( struct client * client,
|
|||||||
request.type, request.flags, request.from, request.len, request.handle
|
request.type, request.flags, request.from, request.len, request.handle
|
||||||
);
|
);
|
||||||
|
|
||||||
/* check it's not out of range */
|
/* check it's not out of range. NBD protocol requires ENOSPC to be
|
||||||
|
* returned in this instance
|
||||||
|
*/
|
||||||
if ( request.from+request.len > client->serve->size) {
|
if ( request.from+request.len > client->serve->size) {
|
||||||
warn("write request %"PRIu64"+%"PRIu32" out of range",
|
warn("write request %"PRIu64"+%"PRIu32" out of range",
|
||||||
request.from, request.len
|
request.from, request.len
|
||||||
@@ -399,7 +401,7 @@ int client_request_needs_reply( struct client * client,
|
|||||||
if ( request.type == REQUEST_WRITE ) {
|
if ( request.type == REQUEST_WRITE ) {
|
||||||
client_flush( client, request.len );
|
client_flush( client, request.len );
|
||||||
}
|
}
|
||||||
client_write_reply( client, &request, EPERM ); /* TODO: Change to ERANGE ? */
|
client_write_reply( client, &request, ENOSPC );
|
||||||
client->disconnect = 0;
|
client->disconnect = 0;
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
@@ -418,7 +420,12 @@ int client_request_needs_reply( struct client * client,
|
|||||||
case REQUEST_FLUSH:
|
case REQUEST_FLUSH:
|
||||||
break;
|
break;
|
||||||
default:
|
default:
|
||||||
fatal("Unknown request 0x%08X", request.type);
|
/* NBD prototcol says servers SHOULD return EINVAL to unknown
|
||||||
|
* commands */
|
||||||
|
warn("Unknown request 0x%08X", request.type);
|
||||||
|
client_write_reply( client, &request, EINVAL );
|
||||||
|
client->disconnect = 0;
|
||||||
|
return 0;
|
||||||
}
|
}
|
||||||
return 1;
|
return 1;
|
||||||
}
|
}
|
||||||
|
@@ -72,6 +72,20 @@ module ProxyTests
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def test_write_request_past_end_of_disc_returns_to_client
|
||||||
|
with_proxied_client do |client|
|
||||||
|
n = 1000
|
||||||
|
offset = n * 4096
|
||||||
|
client.write(offset, b * 4096)
|
||||||
|
rsp = client.read_response
|
||||||
|
|
||||||
|
assert_equal FlexNBD::REPLY_MAGIC, rsp[:magic]
|
||||||
|
assert_equal 'myhandle', rsp[:handle]
|
||||||
|
# NBD protocol say ENOSPC (28) in this situation
|
||||||
|
assert_equal 28, rsp[:error]
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
def make_fake_server
|
def make_fake_server
|
||||||
server = FlexNBD::FakeDest.new(@env.ip, @env.port1)
|
server = FlexNBD::FakeDest.new(@env.ip, @env.port1)
|
||||||
@server_up = true
|
@server_up = true
|
||||||
|
@@ -112,7 +112,8 @@ class TestServeMode < Test::Unit::TestCase
|
|||||||
|
|
||||||
assert_equal FlexNBD::REPLY_MAGIC, rsp[:magic]
|
assert_equal FlexNBD::REPLY_MAGIC, rsp[:magic]
|
||||||
assert_equal 'myhandle', rsp[:handle]
|
assert_equal 'myhandle', rsp[:handle]
|
||||||
assert rsp[:error] != 0, "Server sent success reply back: #{rsp[:error]}"
|
# NBD protocol suggests ENOSPC (28) is returned
|
||||||
|
assert_equal 28, rsp[:error], 'Server sent incorrect response'
|
||||||
|
|
||||||
# Ensure we're not disconnected by sending a request. We don't care about
|
# Ensure we're not disconnected by sending a request. We don't care about
|
||||||
# whether the reply is good or not, here.
|
# whether the reply is good or not, here.
|
||||||
@@ -129,7 +130,26 @@ class TestServeMode < Test::Unit::TestCase
|
|||||||
|
|
||||||
assert_equal FlexNBD::REPLY_MAGIC, rsp[:magic]
|
assert_equal FlexNBD::REPLY_MAGIC, rsp[:magic]
|
||||||
assert_equal 'myhandle', rsp[:handle]
|
assert_equal 'myhandle', rsp[:handle]
|
||||||
assert rsp[:error] != 0, "Server sent success reply back: #{rsp[:error]}"
|
# NBD protocol suggests ENOSPC (28) is returned
|
||||||
|
assert_equal 28, rsp[:error], 'Server sent incorrect response'
|
||||||
|
|
||||||
|
# Ensure we're not disconnected by sending a request. We don't care about
|
||||||
|
# whether the reply is good or not, here.
|
||||||
|
client.write(0, "\x00" * @env.file1.size)
|
||||||
|
rsp = client.read_response
|
||||||
|
assert_equal FlexNBD::REPLY_MAGIC, rsp[:magic]
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
def test_unknown_command_receives_error_response
|
||||||
|
connect_to_server do |client|
|
||||||
|
client.send_request(123)
|
||||||
|
rsp = client.read_response
|
||||||
|
|
||||||
|
assert_equal FlexNBD::REPLY_MAGIC, rsp[:magic]
|
||||||
|
assert_equal 'myhandle', rsp[:handle]
|
||||||
|
# NBD protocol suggests EINVAL (22) is returned
|
||||||
|
assert_equal 22, rsp[:error], 'Server sent incorrect response'
|
||||||
|
|
||||||
# Ensure we're not disconnected by sending a request. We don't care about
|
# Ensure we're not disconnected by sending a request. We don't care about
|
||||||
# whether the reply is good or not, here.
|
# whether the reply is good or not, here.
|
||||||
|
Reference in New Issue
Block a user