| Message ID | 20180501211336.986372-3-eblake@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Support larger NBD_CMD_WRITE_ZEROES | expand |
02.05.2018 00:13, Eric Blake wrote: > The next patch will ask the server for more items of NBD_INFO. > However, the server is free to respond with INFO items in a > different order than what we request, so performing any sanity > checks about constraints that occur between multiple INFO items > must be done after all items have been received. Make it easier > to see that such checks are last by sinking the final processing > after the while loop, rather than embedded in the NBD_REP_ACK > processing that occurs textually within the loop before other > INFO processing. > > Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > nbd/client.c | 19 +++++++++++-------- > 1 file changed, 11 insertions(+), 8 deletions(-) > > diff --git a/nbd/client.c b/nbd/client.c > index 232ff4f46da..0abb195b856 100644 > --- a/nbd/client.c > +++ b/nbd/client.c > @@ -1,5 +1,5 @@ > /* > - * Copyright (C) 2016-2017 Red Hat, Inc. > + * Copyright (C) 2016-2018 Red Hat, Inc. > * Copyright (C) 2005 Anthony Liguori <anthony@codemonkey.ws> > * > * Network Block Device Client Side > @@ -365,17 +365,12 @@ static int nbd_opt_go(QIOChannel *ioc, const char *wantname, > > if (reply.type == NBD_REP_ACK) { > /* Server is done sending info and moved into transmission > - phase, but make sure it sent flags */ > + phase */ > if (len) { > error_setg(errp, "server sent invalid NBD_REP_ACK"); > return -1; > } > - if (!info->flags) { > - error_setg(errp, "broken server omitted NBD_INFO_EXPORT"); > - return -1; > - } > - trace_nbd_opt_go_success(); > - return 1; > + break; > } > if (reply.type != NBD_REP_INFO) { > error_setg(errp, "unexpected reply type %" PRIu32 > @@ -482,6 +477,14 @@ static int nbd_opt_go(QIOChannel *ioc, const char *wantname, > break; > } > } > + > + /* Sanity check that server's responses make sense */ > + if (!info->flags) { > + error_setg(errp, "broken server omitted NBD_INFO_EXPORT"); > + return -1; > + } > + trace_nbd_opt_go_success(); > + return 1; > } > > /* Return -1 on failure, 0 if wantname is an available export. */
diff --git a/nbd/client.c b/nbd/client.c index 232ff4f46da..0abb195b856 100644 --- a/nbd/client.c +++ b/nbd/client.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2016-2017 Red Hat, Inc. + * Copyright (C) 2016-2018 Red Hat, Inc. * Copyright (C) 2005 Anthony Liguori <anthony@codemonkey.ws> * * Network Block Device Client Side @@ -365,17 +365,12 @@ static int nbd_opt_go(QIOChannel *ioc, const char *wantname, if (reply.type == NBD_REP_ACK) { /* Server is done sending info and moved into transmission - phase, but make sure it sent flags */ + phase */ if (len) { error_setg(errp, "server sent invalid NBD_REP_ACK"); return -1; } - if (!info->flags) { - error_setg(errp, "broken server omitted NBD_INFO_EXPORT"); - return -1; - } - trace_nbd_opt_go_success(); - return 1; + break; } if (reply.type != NBD_REP_INFO) { error_setg(errp, "unexpected reply type %" PRIu32 @@ -482,6 +477,14 @@ static int nbd_opt_go(QIOChannel *ioc, const char *wantname, break; } } + + /* Sanity check that server's responses make sense */ + if (!info->flags) { + error_setg(errp, "broken server omitted NBD_INFO_EXPORT"); + return -1; + } + trace_nbd_opt_go_success(); + return 1; } /* Return -1 on failure, 0 if wantname is an available export. */
The next patch will ask the server for more items of NBD_INFO. However, the server is free to respond with INFO items in a different order than what we request, so performing any sanity checks about constraints that occur between multiple INFO items must be done after all items have been received. Make it easier to see that such checks are last by sinking the final processing after the while loop, rather than embedded in the NBD_REP_ACK processing that occurs textually within the loop before other INFO processing. Signed-off-by: Eric Blake <eblake@redhat.com> --- nbd/client.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-)