Re: [Qemu-devel] [PATCH 11/18] nbd: Let client skip portions of server reply

[Alex Bligh]

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

> The server has a nice helper function nbd_negotiate_drop_sync()
> which lets it easily ignore fluff from the client (such as the
> payload to an unknown option request).  We can't quite make it
> common, since it depends on nbd_negotiate_read() which handles
> coroutine magic, but we can copy the idea into the client where
> we have places where we want to ignore data (such as the
> description tacked on the end of NBD_REP_SERVER).
> 
> Signed-off-by: Eric Blake <address@hidden>


Reviewed-by: Alex Bligh <address@hidden>

> ---
> nbd/client.c | 45 ++++++++++++++++++++++++++++++++-------------
> 1 file changed, 32 insertions(+), 13 deletions(-)
> 
> diff --git a/nbd/client.c b/nbd/client.c
> index 07b8d2e..b2dfc11 100644
> --- a/nbd/client.c
> +++ b/nbd/client.c
> @@ -75,6 +75,32 @@ static QTAILQ_HEAD(, NBDExport) exports = 
> QTAILQ_HEAD_INITIALIZER(exports);
> 
> */
> 
> +/* Discard length bytes from channel.  Return -errno on failure, or
> + * the amount of bytes consumed. */
> +static ssize_t drop_sync(QIOChannel *ioc, size_t size)
> +{
> +    ssize_t ret, dropped = size;
> +    char small[1024];
> +    char *buffer;
> +
> +    buffer = sizeof(small) < size ? small : g_malloc(MIN(65536, size));
> +    while (size > 0) {
> +        ret = read_sync(ioc, buffer, MIN(65536, size));
> +        if (ret < 0) {
> +            goto cleanup;
> +        }
> +        assert(ret <= size);
> +        size -= ret;
> +    }
> +    ret = dropped;
> +
> + cleanup:
> +    if (buffer != small) {
> +        g_free(buffer);
> +    }
> +    return ret;
> +}
> +
> /* Send an option request. Return 0 if successful, -1 with errp set if
>  * it is impossible to continue. */
> static int nbd_send_option_request(QIOChannel *ioc, uint32_t opt,
> @@ -255,18 +281,11 @@ static int nbd_receive_list(QIOChannel *ioc, char 
> **name, Error **errp)
>         }
>         (*name)[namelen] = '\0';
>         len -= namelen;
> -        if (len) {
> -            char *buf = g_malloc(len + 1);
> -            if (read_sync(ioc, buf, len) != len) {
> -                error_setg(errp, "failed to read export description");
> -                g_free(*name);
> -                g_free(buf);
> -                *name = NULL;
> -                return -1;
> -            }
> -            buf[len] = '\0';
> -            TRACE("Ignoring export description: %s", buf);
> -            g_free(buf);
> +        if (drop_sync(ioc, len) != len) {
> +            error_setg(errp, "failed to read export description");
> +            g_free(*name);
> +            *name = NULL;
> +            return -1;
>         }
>     } else {
>         error_setg(errp, "Unexpected reply type %" PRIx32 " expected %x",
> @@ -539,7 +558,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char 
> *name, uint32_t *flags,
>         goto fail;
>     }
> 
> -    if (read_sync(ioc, &buf, 124) != 124) {
> +    if (drop_sync(ioc, 124) != 124) {
>         error_setg(errp, "Failed to read reserved block");
>         goto fail;
>     }
> -- 
> 2.5.5
> 
> 

-- 
Alex Bligh