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

[Lukas Straub <lukasstraub2@web.de>]

[-- Attachment #1: Type: text/plain, Size: 4966 bytes --]

On Mon, 11 May 2020 17:19:09 +0100
"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:

> * Lukas Straub (lukasstraub2@web.de) 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 <lukasstraub2@web.de>  
> 
> > +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).

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 / dgilbert@redhat.com / Manchester, UK
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]