On Tue, Feb 25, 2020 at 2:32 PM Mike Christie <mchristi@xxxxxxxxxx> wrote:
>
> 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@xxxxxxxxxxxxx>
> > ---
> > 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.
Thanks. Will change the comment in v2.
>
>
> > 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;
> > }
> >
> >
>
