| Message ID | 20171122101958.17065-6-vsementsov@virtuozzo.com |
|---|---|
| State | New |
| Headers | show |
| Series | NBD server refactoring before BLOCK_STATUS | expand |
On 11/22/2017 04:19 AM, Vladimir Sementsov-Ogievskiy wrote: > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > nbd/server.c | 40 +++++++++++++--------------------------- > 1 file changed, 13 insertions(+), 27 deletions(-) > Nice diffstat; shows that this one probably makes sense (even if it will need rebasing on top of the planned churn for the rest of the series). > diff --git a/nbd/server.c b/nbd/server.c > index 79b937d88f..975fe8efe9 100644 > --- a/nbd/server.c > +++ b/nbd/server.c > @@ -152,43 +152,29 @@ static inline int nbd_opt_drop(NBDClient *client, size_t size, Error **errp) > return nbd_drop(client->ioc, size, errp); > } > > +static inline void set_be_option_rep(NBDOptionReply *rep, uint32_t option, > + uint32_t type, uint32_t length) 'static inline' is probably pointless; the compiler already knows what static functions are ideal to inline, without us having to name it (I realize you've already done this in other patches, so I probably ought to post a cleanup patch that removes 'inline').
On 11/22/2017 04:02 PM, Eric Blake wrote: > On 11/22/2017 04:19 AM, Vladimir Sementsov-Ogievskiy wrote: >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >> --- >> nbd/server.c | 40 +++++++++++++--------------------------- >> 1 file changed, 13 insertions(+), 27 deletions(-) >> > > Nice diffstat; shows that this one probably makes sense (even if it will > need rebasing on top of the planned churn for the rest of the series). > >> diff --git a/nbd/server.c b/nbd/server.c >> index 79b937d88f..975fe8efe9 100644 >> --- a/nbd/server.c >> +++ b/nbd/server.c >> @@ -152,43 +152,29 @@ static inline int nbd_opt_drop(NBDClient *client, size_t size, Error **errp) >> return nbd_drop(client->ioc, size, errp); >> } >> >> +static inline void set_be_option_rep(NBDOptionReply *rep, uint32_t option, >> + uint32_t type, uint32_t length) > > 'static inline' is probably pointless; the compiler already knows what > static functions are ideal to inline, without us having to name it (I > realize you've already done this in other patches, so I probably ought > to post a cleanup patch that removes 'inline'). Then again, we're using it in other .c files, so there's nothing particularly wrong in keeping it as is.
diff --git a/nbd/server.c b/nbd/server.c index 79b937d88f..975fe8efe9 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -152,43 +152,29 @@ static inline int nbd_opt_drop(NBDClient *client, size_t size, Error **errp) return nbd_drop(client->ioc, size, errp); } +static inline void set_be_option_rep(NBDOptionReply *rep, uint32_t option, + uint32_t type, uint32_t length) +{ + stq_be_p(&rep->magic, NBD_REP_MAGIC); + stl_be_p(&rep->option, option); + stl_be_p(&rep->type, type); + stl_be_p(&rep->length, length); +} + /* Send a reply header, including length, but no payload. * Return -errno on error, 0 on success. */ static int nbd_negotiate_send_rep_len(NBDClient *client, uint32_t type, uint32_t len, Error **errp) { - uint64_t magic; - QIOChannel *ioc = client->ioc; - uint32_t opt = client->opt; + NBDOptionReply rep; - trace_nbd_negotiate_send_rep_len(opt, nbd_opt_lookup(opt), + trace_nbd_negotiate_send_rep_len(client->opt, nbd_opt_lookup(client->opt), type, nbd_rep_lookup(type), len); assert(len < NBD_MAX_BUFFER_SIZE); - magic = cpu_to_be64(NBD_REP_MAGIC); - if (nbd_write(ioc, &magic, sizeof(magic), errp) < 0) { - error_prepend(errp, "write failed (rep magic): "); - return -EINVAL; - } - - opt = cpu_to_be32(opt); - if (nbd_write(ioc, &opt, sizeof(opt), errp) < 0) { - error_prepend(errp, "write failed (rep opt): "); - return -EINVAL; - } - - type = cpu_to_be32(type); - if (nbd_write(ioc, &type, sizeof(type), errp) < 0) { - error_prepend(errp, "write failed (rep type): "); - return -EINVAL; - } - len = cpu_to_be32(len); - if (nbd_write(ioc, &len, sizeof(len), errp) < 0) { - error_prepend(errp, "write failed (rep data length): "); - return -EINVAL; - } - return 0; + set_be_option_rep(&rep, client->opt, type, len); + return nbd_write(client->ioc, &rep, sizeof(rep), errp); } /* Send a reply header with default 0 length.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- nbd/server.c | 40 +++++++++++++--------------------------- 1 file changed, 13 insertions(+), 27 deletions(-)