qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 09/18] nbd: Share common reply-sending code in s


From: Alex Bligh
Subject: Re: [Qemu-devel] [PATCH 09/18] nbd: Share common reply-sending code in server
Date: Sat, 9 Apr 2016 11:38:09 +0100

On 8 Apr 2016, at 23:05, Eric Blake <address@hidden> wrote:

> Rather than open-coding NBD_REP_SERVER, reuse the code we
> already have by adding a length parameter.  The code gets
> longer because of added comments, but the refactoring will
> make adding NBD_OPT_GO in a later patch easier.
> 
> Signed-off-by: Eric Blake <address@hidden>


Reviewed-by: Alex Bligh <address@hidden>

> ---
> nbd/server.c | 59 +++++++++++++++++++++++++++++------------------------------
> 1 file changed, 29 insertions(+), 30 deletions(-)
> 
> diff --git a/nbd/server.c b/nbd/server.c
> index c8666ab..69724c9 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -192,12 +192,15 @@ static ssize_t nbd_negotiate_drop_sync(QIOChannel *ioc, 
> size_t size)
> 
> */
> 
> -static int nbd_negotiate_send_rep(QIOChannel *ioc, uint32_t type, uint32_t 
> opt)
> +/* Send a reply header, including length, but no payload.
> + * Return -errno to kill connection, 0 to continue negotiation */
> +static int nbd_negotiate_send_rep_len(QIOChannel *ioc, uint32_t type,
> +                                      uint32_t opt, uint32_t len)
> {
>     uint64_t magic;
> -    uint32_t len;
> 
> -    TRACE("Reply opt=%" PRIx32 " type=%" PRIx32, type, opt);
> +    TRACE("Reply opt=%" PRIx32 " type=%" PRIx32 " len=%" PRIu32,
> +          type, opt, len);
> 
>     magic = cpu_to_be64(NBD_REP_MAGIC);
>     if (nbd_negotiate_write(ioc, &magic, sizeof(magic)) != sizeof(magic)) {
> @@ -214,7 +217,7 @@ static int nbd_negotiate_send_rep(QIOChannel *ioc, 
> uint32_t type, uint32_t opt)
>         LOG("write failed (rep type)");
>         return -EINVAL;
>     }
> -    len = cpu_to_be32(0);
> +    len = cpu_to_be32(len);
>     if (nbd_negotiate_write(ioc, &len, sizeof(len)) != sizeof(len)) {
>         LOG("write failed (rep data length)");
>         return -EINVAL;
> @@ -222,39 +225,35 @@ static int nbd_negotiate_send_rep(QIOChannel *ioc, 
> uint32_t type, uint32_t opt)
>     return 0;
> }
> 
> +/* Send a reply header with default 0 length.
> + * Return -errno to kill connection, 0 to continue negotiation */
> +static int nbd_negotiate_send_rep(QIOChannel *ioc, uint32_t type, uint32_t 
> opt)
> +{
> +    return nbd_negotiate_send_rep_len(ioc, type, opt, 0);
> +}
> +
> +/* Send an NBD_REP_SERVER reply to NBD_OPT_LIST, including payload.
> + * Return -errno to kill connection, 0 to continue negotiation */
> static int nbd_negotiate_send_rep_list(QIOChannel *ioc, NBDExport *exp)
> {
> -    uint64_t magic, name_len;
> -    uint32_t opt, type, len;
> +    uint32_t len;
> +    int rc;
> 
>     TRACE("Advertising export name '%s'", exp->name ? exp->name : "");
> -    name_len = strlen(exp->name);
> -    magic = cpu_to_be64(NBD_REP_MAGIC);
> -    if (nbd_negotiate_write(ioc, &magic, sizeof(magic)) != sizeof(magic)) {
> -        LOG("write failed (magic)");
> -        return -EINVAL;
> -     }
> -    opt = cpu_to_be32(NBD_OPT_LIST);
> -    if (nbd_negotiate_write(ioc, &opt, sizeof(opt)) != sizeof(opt)) {
> -        LOG("write failed (opt)");
> -        return -EINVAL;
> -    }
> -    type = cpu_to_be32(NBD_REP_SERVER);
> -    if (nbd_negotiate_write(ioc, &type, sizeof(type)) != sizeof(type)) {
> -        LOG("write failed (reply type)");
> -        return -EINVAL;
> -    }
> -    len = cpu_to_be32(name_len + sizeof(len));
> -    if (nbd_negotiate_write(ioc, &len, sizeof(len)) != sizeof(len)) {
> -        LOG("write failed (length)");
> -        return -EINVAL;
> -    }
> -    len = cpu_to_be32(name_len);
> +    len = strlen(exp->name);
> +    rc = nbd_negotiate_send_rep_len(ioc, NBD_REP_SERVER, NBD_OPT_LIST,
> +                                    len + sizeof(len));
> +    if (rc < 0) {
> +        return rc;
> +    }
> +
> +    len = cpu_to_be32(len);
>     if (nbd_negotiate_write(ioc, &len, sizeof(len)) != sizeof(len)) {
> -        LOG("write failed (length)");
> +        LOG("write failed (name length)");
>         return -EINVAL;
>     }
> -    if (nbd_negotiate_write(ioc, exp->name, name_len) != name_len) {
> +    len = be32_to_cpu(len);
> +    if (nbd_negotiate_write(ioc, exp->name, len) != len) {
>         LOG("write failed (buffer)");
>         return -EINVAL;
>     }
> -- 
> 2.5.5
> 
> 

-- 
Alex Bligh







reply via email to

[Prev in Thread] Current Thread [Next in Thread]