Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 nbd/server.c | 40 +++++++++++++---------------------------
 1 file changed, 13 insertions(+), 27 deletions(-)

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.
-- 
2.11.1

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').

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

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.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org