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

[Hou Pu <houpu.main@gmail.com>]

On Tue, Feb 25, 2020 at 2:32 PM Mike Christie <mchristi@redhat.com> 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@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.

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;
> >       }
> >
> >
>