Re: [PATCH 3/5] block/nbd.c: Add yank feature

[Dr. David Alan Gilbert]

* Lukas Straub (address@hidden) wrote:
> On Mon, 11 May 2020 17:19:09 +0100
> "Dr. David Alan Gilbert" <address@hidden> wrote:
> 
> > * Lukas Straub (address@hidden) wrote:
> > > Add yank option, pass it to the socket-channel and register a yank
> > > function which sets s->state = NBD_CLIENT_QUIT. This is the same
> > > behaviour as if an error occured.
> > > 
> > > Signed-off-by: Lukas Straub <address@hidden>  
> > 
> > > +static void nbd_yank(void *opaque)
> > > +{
> > > +    BlockDriverState *bs = opaque;
> > > +    BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
> > > +
> > > +    atomic_set(&s->state, NBD_CLIENT_QUIT);  
> > 
> > I think I was expecting a shutdown on the socket here - why doesn't it
> > have one?
> 
> For nbd, we register two yank functions: This one and we enable the yank 
> feature on the qio channel (see function nbd_establish_connection below).

Oh I see; yeh that still surprises me a little; I'd expected one yank
per item.

Dave

> Regards,
> Lukas Straub
> 
> > Dave
> > 
> > > +}
> > > +
> > >  static void nbd_client_close(BlockDriverState *bs)
> > >  {
> > >      BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
> > > @@ -1407,14 +1421,17 @@ static void nbd_client_close(BlockDriverState *bs)
> > >      nbd_teardown_connection(bs);
> > >  }
> > >  
> > > -static QIOChannelSocket *nbd_establish_connection(SocketAddress *saddr,
> > > +static QIOChannelSocket *nbd_establish_connection(BlockDriverState *bs,
> > > +                                                  SocketAddress *saddr,
> > >                                                    Error **errp)
> > >  {
> > > +    BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
> > >      QIOChannelSocket *sioc;
> > >      Error *local_err = NULL;
> > >  
> > >      sioc = qio_channel_socket_new();
> > >      qio_channel_set_name(QIO_CHANNEL(sioc), "nbd-client");
> > > +    qio_channel_set_yank(QIO_CHANNEL(sioc), s->yank);
> > >  
> > >      qio_channel_socket_connect_sync(sioc, saddr, &local_err);
> > >      if (local_err) {
> > > @@ -1438,7 +1455,7 @@ static int nbd_client_connect(BlockDriverState *bs, 
> > > Error **errp)
> > >       * establish TCP connection, return error if it fails
> > >       * TODO: Configurable retry-until-timeout behaviour.
> > >       */
> > > -    QIOChannelSocket *sioc = nbd_establish_connection(s->saddr, errp);
> > > +    QIOChannelSocket *sioc = nbd_establish_connection(bs, s->saddr, 
> > > errp);
> > >  
> > >      if (!sioc) {
> > >          return -ECONNREFUSED;
> > > @@ -1829,6 +1846,12 @@ static QemuOptsList nbd_runtime_opts = {
> > >                      "future requests before a successful reconnect will "
> > >                      "immediately fail. Default 0",
> > >          },
> > > +        {
> > > +            .name = "yank",
> > > +            .type = QEMU_OPT_BOOL,
> > > +            .help = "Forcibly close the connection and don't attempt to "
> > > +                    "reconnect when the 'yank' qmp command is executed.",
> > > +        },
> > >          { /* end of list */ }
> > >      },
> > >  };
> > > @@ -1888,6 +1911,8 @@ static int nbd_process_options(BlockDriverState 
> > > *bs, QDict *options,
> > >  
> > >      s->reconnect_delay = qemu_opt_get_number(opts, "reconnect-delay", 0);
> > >  
> > > +    s->yank = qemu_opt_get_bool(opts, "yank", false);
> > > +
> > >      ret = 0;
> > >  
> > >   error:
> > > @@ -1921,6 +1946,10 @@ static int nbd_open(BlockDriverState *bs, QDict 
> > > *options, int flags,
> > >      /* successfully connected */
> > >      s->state = NBD_CLIENT_CONNECTED;
> > >  
> > > +    if (s->yank) {
> > > +        yank_register_function(nbd_yank, bs);
> > > +    }
> > > +
> > >      s->connection_co = qemu_coroutine_create(nbd_connection_entry, s);
> > >      bdrv_inc_in_flight(bs);
> > >      aio_co_schedule(bdrv_get_aio_context(bs), s->connection_co);
> > > @@ -1972,6 +2001,11 @@ static void nbd_close(BlockDriverState *bs)
> > >      BDRVNBDState *s = bs->opaque;
> > >  
> > >      nbd_client_close(bs);
> > > +
> > > +    if (s->yank) {
> > > +        yank_unregister_function(nbd_yank, bs);
> > > +    }
> > > +
> > >      nbd_clear_bdrvstate(s);
> > >  }
> > >  
> > > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > > index 943df1926a..1c1578160e 100644
> > > --- a/qapi/block-core.json
> > > +++ b/qapi/block-core.json
> > > @@ -3862,6 +3862,8 @@
> > >  #                   reconnect. After that time, any delayed requests and 
> > > all
> > >  #                   future requests before a successful reconnect will
> > >  #                   immediately fail. Default 0 (Since 4.2)
> > > +# @yank: Forcibly close the connection and don't attempt to reconnect 
> > > when
> > > +#        the 'yank' qmp command is executed. (Since: 5.1)
> > >  #
> > >  # Since: 2.9
> > >  ##
> > > @@ -3870,7 +3872,8 @@
> > >              '*export': 'str',
> > >              '*tls-creds': 'str',
> > >              '*x-dirty-bitmap': 'str',
> > > -            '*reconnect-delay': 'uint32' } }
> > > +            '*reconnect-delay': 'uint32',
> > > +     'yank': 'bool' } }
> > >  
> > >  ##
> > >  # @BlockdevOptionsRaw:
> > > -- 
> > > 2.20.1
> > >   
> > 
> > 
> > --
> > Dr. David Alan Gilbert / address@hidden / Manchester, UK
> > 
> 


--
Dr. David Alan Gilbert / address@hidden / Manchester, UK