| Message ID | 20170707203049.534-3-eblake@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
07.07.2017 23:30, Eric Blake wrote: > We really don't care if our spec-compliant reply to NBD_OPT_ABORT > was received, so shave off some lines of code by not even tracing it. > > Signed-off-by: Eric Blake <eblake@redhat.com> > > --- > v5: new patch > --- > nbd/server.c | 15 ++++----------- > 1 file changed, 4 insertions(+), 11 deletions(-) > > diff --git a/nbd/server.c b/nbd/server.c > index 9b0c588..e15385b 100644 > --- a/nbd/server.c > +++ b/nbd/server.c > @@ -376,7 +376,6 @@ static int nbd_negotiate_options(NBDClient *client, Error **errp) > { > uint32_t flags; > bool fixedNewstyle = false; > - Error *local_err = NULL; > > /* Client sends: > [ 0 .. 3] client flags > @@ -479,7 +478,9 @@ static int nbd_negotiate_options(NBDClient *client, Error **errp) > if (ret < 0) { > return ret; > } > - /* Let the client keep trying, unless they asked to quit */ > + /* Let the client keep trying, unless they asked to > + * quit. In this mode, we've already sent an error, so > + * we can't ack the abort. */ > if (option == NBD_OPT_ABORT) { > return 1; > } > @@ -498,15 +499,7 @@ static int nbd_negotiate_options(NBDClient *client, Error **errp) > /* NBD spec says we must try to reply before > * disconnecting, but that we must also tolerate > * guests that don't wait for our reply. */ > - nbd_negotiate_send_rep(client->ioc, NBD_REP_ACK, option, > - &local_err); > - > - if (local_err != NULL) { > - const char *error = error_get_pretty(local_err); > - trace_nbd_opt_abort_reply_failed(error); Looks like you forgot to drop this trace from nbd/trace-events > - error_free(local_err); > - } > - > + nbd_negotiate_send_rep(client->ioc, NBD_REP_ACK, option, NULL); > return 1; > > case NBD_OPT_EXPORT_NAME:
13.07.2017 15:12, Vladimir Sementsov-Ogievskiy wrote: > 07.07.2017 23:30, Eric Blake wrote: >> We really don't care if our spec-compliant reply to NBD_OPT_ABORT >> was received, so shave off some lines of code by not even tracing it. >> >> Signed-off-by: Eric Blake <eblake@redhat.com> >> >> --- >> v5: new patch >> --- >> nbd/server.c | 15 ++++----------- >> 1 file changed, 4 insertions(+), 11 deletions(-) >> >> diff --git a/nbd/server.c b/nbd/server.c >> index 9b0c588..e15385b 100644 >> --- a/nbd/server.c >> +++ b/nbd/server.c >> @@ -376,7 +376,6 @@ static int nbd_negotiate_options(NBDClient >> *client, Error **errp) >> { >> uint32_t flags; >> bool fixedNewstyle = false; >> - Error *local_err = NULL; >> >> /* Client sends: >> [ 0 .. 3] client flags >> @@ -479,7 +478,9 @@ static int nbd_negotiate_options(NBDClient >> *client, Error **errp) >> if (ret < 0) { >> return ret; >> } >> - /* Let the client keep trying, unless they asked to >> quit */ >> + /* Let the client keep trying, unless they asked to >> + * quit. In this mode, we've already sent an error, so >> + * we can't ack the abort. */ >> if (option == NBD_OPT_ABORT) { >> return 1; >> } >> @@ -498,15 +499,7 @@ static int nbd_negotiate_options(NBDClient >> *client, Error **errp) >> /* NBD spec says we must try to reply before >> * disconnecting, but that we must also tolerate >> * guests that don't wait for our reply. */ >> - nbd_negotiate_send_rep(client->ioc, NBD_REP_ACK, >> option, >> - &local_err); >> - >> - if (local_err != NULL) { >> - const char *error = error_get_pretty(local_err); >> - trace_nbd_opt_abort_reply_failed(error); > > Looks like you forgot to drop this trace from nbd/trace-events > >> - error_free(local_err); >> - } >> - >> + nbd_negotiate_send_rep(client->ioc, NBD_REP_ACK, >> option, NULL); >> return 1; >> >> case NBD_OPT_EXPORT_NAME: > > with fixed: Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
diff --git a/nbd/server.c b/nbd/server.c index 9b0c588..e15385b 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -376,7 +376,6 @@ static int nbd_negotiate_options(NBDClient *client, Error **errp) { uint32_t flags; bool fixedNewstyle = false; - Error *local_err = NULL; /* Client sends: [ 0 .. 3] client flags @@ -479,7 +478,9 @@ static int nbd_negotiate_options(NBDClient *client, Error **errp) if (ret < 0) { return ret; } - /* Let the client keep trying, unless they asked to quit */ + /* Let the client keep trying, unless they asked to + * quit. In this mode, we've already sent an error, so + * we can't ack the abort. */ if (option == NBD_OPT_ABORT) { return 1; } @@ -498,15 +499,7 @@ static int nbd_negotiate_options(NBDClient *client, Error **errp) /* NBD spec says we must try to reply before * disconnecting, but that we must also tolerate * guests that don't wait for our reply. */ - nbd_negotiate_send_rep(client->ioc, NBD_REP_ACK, option, - &local_err); - - if (local_err != NULL) { - const char *error = error_get_pretty(local_err); - trace_nbd_opt_abort_reply_failed(error); - error_free(local_err); - } - + nbd_negotiate_send_rep(client->ioc, NBD_REP_ACK, option, NULL); return 1; case NBD_OPT_EXPORT_NAME:
We really don't care if our spec-compliant reply to NBD_OPT_ABORT was received, so shave off some lines of code by not even tracing it. Signed-off-by: Eric Blake <eblake@redhat.com> --- v5: new patch --- nbd/server.c | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-)