Re: [PATCH 1/2] nbd: enable replace socket if only one connection is configured

[Mike Christie <mchristi@redhat.com>]

On 02/19/2020 12:31 AM, Hou Pu wrote:
> Nbd server with multiple connections could be upgraded since
> 560bc4b (nbd: handle dead connections). But if only one conncection
> is configured, after we take down nbd server, all inflight IO
> would finally timeout and return error. We could requeue them
> like what we do with multiple connections and wait for new socket
> in submit path.
> 
> Signed-off-by: Hou Pu <houpu@bytedance.com>
> ---
>  drivers/block/nbd.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> index 78181908f0df..8e348c9c49a4 100644
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c
> @@ -395,16 +395,19 @@ static enum blk_eh_timer_return nbd_xmit_timeout(struct request *req,
>  	}
>  	config = nbd->config;
>  
> -	if (config->num_connections > 1) {
> +	if (config->num_connections > 1 ||
> +	    (config->num_connections == 1 && nbd->tag_set.timeout)) {
>  		dev_err_ratelimited(nbd_to_dev(nbd),
>  				    "Connection timed out, retrying (%d/%d alive)\n",
>  				    atomic_read(&config->live_connections),
>  				    config->num_connections);
>  		/*
>  		 * Hooray we have more connections, requeue this IO, the submit
> -		 * path will put it on a real connection.
> +		 * path will put it on a real connection. Or if only one
> +		 * connection is configured, the submit path will wait util
> +		 * a new connection is reconfigured or util dead timeout.
>  		 */
> -		if (config->socks && config->num_connections > 1) {
> +		if (config->socks) {
>  			if (cmd->index < config->num_connections) {
>  				struct nbd_sock *nsock =
>  					config->socks[cmd->index];
> @@ -747,8 +750,7 @@ static struct nbd_cmd *nbd_read_stat(struct nbd_device *nbd, int index)
>  				 * and let the timeout stuff handle resubmitting
>  				 * this request onto another connection.
>  				 */
> -				if (nbd_disconnected(config) ||
> -				    config->num_connections <= 1) {
> +				if (nbd_disconnected(config)) {

I think you need to update the comment right above this chunk. It still
mentions num_connections=1 working differently.


>  					cmd->status = BLK_STS_IOERR;
>  					goto out;
>  				}
> @@ -825,7 +827,7 @@ static int find_fallback(struct nbd_device *nbd, int index)
>  
>  	if (config->num_connections <= 1) {
>  		dev_err_ratelimited(disk_to_dev(nbd->disk),
> -				    "Attempted send on invalid socket\n");
> +				    "Dead connection, failed to find a fallback\n");
>  		return new_index;
>  	}
>  
>