[Spice-devel] [PATCH spice-server 03/14] char_device: Introducing shared flow control code for char devices.

Alon Levy alevy at redhat.com
Mon Jul 2 04:11:02 PDT 2012


On Wed, Jun 27, 2012 at 06:16:41PM +0300, Yonit Halperin wrote:

I didn't review the whole series, but since I see Hans already provided
some comments on this patch I'll add my own.

I also think the patchset looks very good.

For the future, it would be nice to have byte level accounting as well
as token (message) level. For the future! not for now :)

Comments inline.

> SpiceCharDeviceState manages the (1) write-to-device queue
> (2) wakeup and reading from the device (3) client tokens (4)
> sending messages from the device to the client/s, considering the
> available tokens.
> SpiceCharDeviceState can be also stopped and started. When the device
> is stopped, no reading or writing is done from/to the device. Messages
> addressed from the client to the device are being queued.
> Later, an api for stop/start will be added to spice.h and it should
> be called from qemu.
> 
> This patch does not yet remove the wakeup callback from
> SpiceCharDeviceState, but once all the char devices (agent/spicevmc/smartcard)
> code will switch to the new implementation, SpiceCharDeviceState
> will be moved to the c file and its reference to the wakeup callback will be removed.
> ---
>  server/Makefile.am   |    1 +
>  server/char_device.c |  741 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  server/char_device.h |  198 ++++++++++++++
>  3 files changed, 940 insertions(+), 0 deletions(-)
>  create mode 100644 server/char_device.c
> 
> diff --git a/server/Makefile.am b/server/Makefile.am
> index 47b3c10..e7b4977 100644
> --- a/server/Makefile.am
> +++ b/server/Makefile.am
> @@ -43,6 +43,7 @@ libspice_server_la_LIBADD =						\
>  libspice_server_la_SOURCES =			\
>  	agent-msg-filter.c			\
>  	agent-msg-filter.h			\
> +	char_device.c				\
>  	char_device.h				\
>  	demarshallers.h				\
>  	glz_encoder.c				\
> diff --git a/server/char_device.c b/server/char_device.c
> new file mode 100644
> index 0000000..13a3a58
> --- /dev/null
> +++ b/server/char_device.c
> @@ -0,0 +1,741 @@
> +/* spice-server char device flow control code
> +
> +   Copyright (C) 2012 Red Hat, Inc.
> +
> +   Red Hat Authors:
> +   Yonit Halperin <yhalperi at redhat.com>
> +
> +   This library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   This library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with this library; if not, see <http:www.gnu.org/licenses/>.
> +*/
> +
> +
> +#include "char_device.h"
> +#include "red_channel.h"
> +#include "reds.h"
> +
> +#define CHAR_DEVICE_WRITE_TO_TIMEOUT 100
> +#define SPICE_CHAR_DEVICE_WAIT_TOKENS_TIMEOUT 30000
> +
> +typedef struct SpiceCharDeviceClientState SpiceCharDeviceClientState;
> +struct SpiceCharDeviceClientState {
> +    RingItem link;
> +    SpiceCharDeviceState *dev;
> +    RedClient *client;
> +    int do_flow_control;
> +    uint64_t num_client_tokens;
> +    uint64_t num_client_tokens_free; /* client messages that were consumed by the device */
> +    uint64_t num_send_tokens; /* send to client */
> +    SpiceTimer *wait_for_tokens_timer;
> +    int wait_for_tokens_started;
> +    Ring send_queue;
> +    uint32_t send_queue_size;
> +    uint32_t max_send_queue_size;
> +};
> +
> +/* Holding references for avoiding access violation if the char device was
> + * destroyed during a callback */
> +static void spice_char_device_state_ref(SpiceCharDeviceState *char_dev);
> +static void spice_char_device_state_unref(SpiceCharDeviceState *char_dev);
> +
> +static void spice_char_dev_write_retry(void *opaque);
> +
> +typedef struct SpiceCharDeviceMsgToClientItem {
> +    RingItem link;
> +    SpiceCharDeviceMsgToClient *msg;
> +} SpiceCharDeviceMsgToClientItem;
> +
> +static void write_buffers_queue_free(Ring *write_queue)
> +{
> +    while (!ring_is_empty(write_queue)) {
> +        RingItem *item = ring_get_tail(write_queue);
> +        SpiceCharDeviceWriteBuffer *buf;
> +
> +        ring_remove(item);
> +        buf = SPICE_CONTAINEROF(item, SpiceCharDeviceWriteBuffer, link);
> +        free(buf->buf);
> +        free(buf);
> +    }
> +}
> +
> +static void spice_char_device_write_buffer_pool_add(SpiceCharDeviceState *dev,
> +                                                    SpiceCharDeviceWriteBuffer *buf)
> +{
> +    buf->buf_used = 0;
> +    buf->client_origin = NULL;
> +    ring_add(&dev->write_bufs_pool, &buf->link);
> +}
> +
> +static void spice_char_device_client_send_queue_free(SpiceCharDeviceState *dev,
> +                                                     SpiceCharDeviceClientState *dev_client)
> +{
> +    spice_debug("send_queue_empty %d", ring_is_empty(&dev_client->send_queue));
> +    while (!ring_is_empty(&dev_client->send_queue)) {
> +        RingItem *item = ring_get_tail(&dev_client->send_queue);
> +        SpiceCharDeviceMsgToClientItem *msg_item = SPICE_CONTAINEROF(item,
> +                                                                     SpiceCharDeviceMsgToClientItem,
> +                                                                     link);
> +
> +        ring_remove(item);
> +        dev->cbs.unref_msg_to_client(msg_item->msg, dev->opaque);
> +        free(msg_item);
> +    }
> +    dev_client->num_send_tokens += dev_client->send_queue_size;
> +    dev_client->send_queue_size = 0;
> +}
> +
> +static void spice_char_device_client_free(SpiceCharDeviceState *dev,
> +                                          SpiceCharDeviceClientState *dev_client)
> +{
> +    RingItem *item, *next;
> +
> +    if (dev_client->wait_for_tokens_timer) {
> +        core->timer_remove(dev_client->wait_for_tokens_timer);
> +    }
> +
> +    spice_char_device_client_send_queue_free(dev, dev_client);
> +
> +    /* remove write buffers that are associated with the client */
> +    spice_debug("write_queue_is_empty %d", ring_is_empty(&dev->write_queue) && !dev->cur_write_buf);
> +    RING_FOREACH_SAFE(item, next, &dev->write_queue) {
> +        SpiceCharDeviceWriteBuffer *write_buf;
> +
> +        write_buf = SPICE_CONTAINEROF(item, SpiceCharDeviceWriteBuffer, link);
> +        if (write_buf->client_origin == dev_client->client) {
> +            ring_remove(item);
> +            spice_char_device_write_buffer_pool_add(dev, write_buf);
> +        }
> +    }
> +
> +    if (dev->cur_write_buf &&
> +        dev->cur_write_buf->client_origin == dev_client->client) {
> +        dev->cur_write_buf->client_origin = NULL;

By setting client_origin to NULL when we release this buffer via
spice_char_device_write_to_device->spice_char_device_write_buffer_release
we will increment num_self_tokens wrongly. Perhaps use a special -1
value (will never be used by a real pointer that is 4 or 8 bytes
aligned)?

> +    }
> +
> +    dev->num_clients--;
> +    ring_remove(&dev_client->link);
> +    free(dev_client);
> +}
> +
> +static void spice_char_device_handle_client_overflow(SpiceCharDeviceClientState *dev_client)
> +{
> +    SpiceCharDeviceState *dev = dev_client->dev;
> +    spice_printerr("dev %p client %p ", dev, dev_client);
> +    dev->cbs.remove_client(dev_client->client, dev->opaque);
> +}
> +
> +static SpiceCharDeviceClientState *spice_char_device_client_find(SpiceCharDeviceState *dev,
> +                                                                 RedClient *client)
> +{
> +    RingItem *item;
> +
> +    RING_FOREACH(item, &dev->clients) {
> +        SpiceCharDeviceClientState *dev_client;
> +
> +        dev_client = SPICE_CONTAINEROF(item, SpiceCharDeviceClientState, link);
> +        if (dev_client->client == client) {
> +            return dev_client;
> +        }
> +    }
> +    return NULL;
> +}
> +
> +/***************************
> + * Reading from the device *
> + **************************/
> +
> +static void device_client_wait_for_tokens_timeout(void *opaque)
> +{
> +    SpiceCharDeviceClientState *dev_client = opaque;
> +
> +    spice_char_device_handle_client_overflow(dev_client);
> +}
> +
> +static uint64_t spice_char_device_max_send_tokens(SpiceCharDeviceState *dev)
> +{
> +    RingItem *item;
> +    uint64_t max = 0;
> +
> +    RING_FOREACH(item, &dev->clients) {
> +        SpiceCharDeviceClientState *dev_client;
> +
> +        dev_client = SPICE_CONTAINEROF(item, SpiceCharDeviceClientState, link);
> +        if (dev_client->num_send_tokens > max) {
> +            max = dev_client->num_send_tokens;
> +        }
> +    }
> +    return max;
> +}
> +
> +static void spice_char_device_add_msg_to_client_queue(SpiceCharDeviceClientState *dev_client,
> +                                                      SpiceCharDeviceMsgToClient *msg)
> +{
> +    SpiceCharDeviceState *dev = dev_client->dev;
> +    SpiceCharDeviceMsgToClientItem *msg_item;
> +
> +    if (dev_client->send_queue_size >= dev_client->max_send_queue_size) {
> +        spice_char_device_handle_client_overflow(dev_client);
> +        return;
> +    }
> +
> +    msg_item = spice_new0(SpiceCharDeviceMsgToClientItem, 1);
> +    msg_item->msg = dev->cbs.ref_msg_to_client(msg, dev->opaque);
> +    ring_add(&dev_client->send_queue, &msg_item->link);
> +    dev_client->send_queue_size++;
> +    if (!dev_client->wait_for_tokens_started) {
> +        core->timer_start(dev_client->wait_for_tokens_timer,
> +                          SPICE_CHAR_DEVICE_WAIT_TOKENS_TIMEOUT);
> +        dev_client->wait_for_tokens_started = TRUE;
> +    }
> +}
> +
> +static void spice_char_device_send_msg_to_client(SpiceCharDeviceState *dev,

nitpick: add s at the end? i.e.
s/spice_char_device_send_msg_to_client/spice_char_device_send_msg_to_clients/

> +                                                 SpiceCharDeviceMsgToClient *msg)
> +{
> +    RingItem *item, *next;
> +
> +    RING_FOREACH_SAFE(item, next, &dev->clients) {
> +        SpiceCharDeviceClientState *dev_client;
> +
> +        dev_client = SPICE_CONTAINEROF(item, SpiceCharDeviceClientState, link);
> +        if (dev_client->num_send_tokens) {
> +            dev_client->num_send_tokens--;
> +            spice_assert(ring_is_empty(&dev_client->send_queue));
> +            dev->cbs.send_msg_to_client(msg, dev_client->client, dev->opaque);
> +
> +            /* don't refer to dev_client anymore, it may have been released */
> +        } else {
> +            spice_char_device_add_msg_to_client_queue(dev_client, msg);
> +        }
> +    }
> +}
> +
> +static int spice_char_device_read_from_device(SpiceCharDeviceState *dev)
> +{
> +    static int inside_call = 0;

This is a function static, if we have multiple threads (sometime in the
future?) it will break. Not a problem right now though.

> +    uint64_t max_send_tokens;
> +    int did_read = FALSE;
> +
> +    if (!dev->running) {
> +        return FALSE;
> +    }
> +
> +    /* There are 2 scenarios where we can get called recursively:
> +     * 1) spice-vmc vmc_read triggering flush of throttled data, recalling wakeup
> +     * (virtio)
> +     * 2) in case of sending messages to the client, and unreferencing the
> +     * msg, we trigger another read.
> +     */
> +    if (inside_call++ > 0) {
> +        return FALSE;
> +    }
> +
> +    max_send_tokens = spice_char_device_max_send_tokens(dev);
> +    spice_char_device_state_ref(dev);
> +    /*
> +     * Reading from the device only in case at least one of the clients have a free token.
> +     * All messages will be discarded if no client is attached to the device
> +     */
> +    while (max_send_tokens || ring_is_empty(&dev->clients)) {
> +        SpiceCharDeviceMsgToClient *msg;
> +
> +        msg = dev->cbs.read_one_msg_from_device(dev->sin, dev->opaque);
> +        if (!msg) {
> +            if (inside_call > 1) {
> +                inside_call = 1;
> +                continue; /* a wakeup might have been called during the read -
> +                             make sure it doesn't get lost */
> +            }
> +            break;
> +        }
> +        did_read = TRUE;
> +        spice_char_device_send_msg_to_client(dev, msg);
> +        dev->cbs.unref_msg_to_client(msg, dev->opaque);
> +        max_send_tokens--;
> +    }
> +    inside_call = 0;
> +    spice_char_device_state_unref(dev);
> +    return did_read;
> +}
> +
> +static void spice_char_device_client_send_queue_push(SpiceCharDeviceClientState *dev_client)
> +{
> +    RingItem *item;
> +    while ((item = ring_get_tail(&dev_client->send_queue)) && dev_client->num_send_tokens) {
> +        SpiceCharDeviceMsgToClientItem *msg_item;
> +
> +        msg_item = SPICE_CONTAINEROF(item, SpiceCharDeviceMsgToClientItem, link);
> +        ring_remove(item);
> +
> +        dev_client->num_send_tokens--;
> +        dev_client->dev->cbs.send_msg_to_client(msg_item->msg,
> +                                           dev_client->client,
> +                                           dev_client->dev->opaque);
> +        dev_client->dev->cbs.unref_msg_to_client(msg_item->msg, dev_client->dev->opaque);
> +        dev_client->send_queue_size--;
> +        free(msg_item);
> +    }
> +}
> +
> +void spice_char_device_send_to_client_tokens_add(SpiceCharDeviceState *dev,
> +                                                 RedClient *client,
> +                                                 uint32_t tokens)
> +{
> +    SpiceCharDeviceClientState *dev_client;
> +
> +    dev_client = spice_char_device_client_find(dev, client);
> +
> +    if (!dev_client) {
> +        spice_error("client wasn't found dev %p client %p", dev, client);
> +        return;
> +    }
> +
> +    dev_client->num_send_tokens += tokens;
> +
> +    if (dev_client->send_queue_size) {
> +        spice_assert(dev_client->num_send_tokens == tokens);
> +        spice_char_device_client_send_queue_push(dev_client);
> +    }
> +
> +    if (dev_client->num_send_tokens) {
> +        core->timer_cancel(dev_client->wait_for_tokens_timer);
> +        dev_client->wait_for_tokens_started = FALSE;
> +        spice_char_device_read_from_device(dev);
> +    } else if (dev_client->send_queue_size) {
> +        core->timer_start(dev_client->wait_for_tokens_timer,
> +                          SPICE_CHAR_DEVICE_WAIT_TOKENS_TIMEOUT);
> +        dev_client->wait_for_tokens_started = TRUE;
> +    }
> +}
> +
> +void spice_char_device_send_to_client_tokens_set(SpiceCharDeviceState *dev,
> +                                                 RedClient *client,
> +                                                 uint32_t tokens)
> +{
> +    SpiceCharDeviceClientState *dev_client;
> +
> +    dev_client = spice_char_device_client_find(dev, client);
> +
> +    if (!dev_client) {
> +        spice_error("client wasn't found dev %p client %p", dev, client);
> +        return;
> +    }
> +
> +    dev_client->num_send_tokens = tokens;
> +
> +    if (dev_client->send_queue_size) {
> +        spice_assert(dev_client->num_send_tokens == tokens);
> +        spice_char_device_client_send_queue_push(dev_client);
> +    }
> +
> +    if (dev_client->num_send_tokens) {
> +        core->timer_cancel(dev_client->wait_for_tokens_timer);
> +        dev_client->wait_for_tokens_started = FALSE;
> +        spice_char_device_read_from_device(dev);
> +    } else if (dev_client->send_queue_size) {
> +        core->timer_start(dev_client->wait_for_tokens_timer,
> +                          SPICE_CHAR_DEVICE_WAIT_TOKENS_TIMEOUT);
> +        dev_client->wait_for_tokens_started = TRUE;
> +    }
> +}
> +
> +/**************************
> + * Writing to the device  *
> +***************************/
> +
> +static void spice_char_device_client_token_add(SpiceCharDeviceState *dev,
> +                                               SpiceCharDeviceClientState *dev_client)
> +{
> +    if (++dev_client->num_client_tokens_free == dev->client_tokens_interval) {
> +        dev_client->num_client_tokens += dev->client_tokens_interval;
> +        dev_client->num_client_tokens_free = 0;
> +        if (dev_client->do_flow_control) {
> +            dev->cbs.send_tokens_to_client(dev_client->client,
> +                                           dev->client_tokens_interval,
> +                                           dev->opaque);
> +        }
> +    }
> +}
> +
> +static int spice_char_device_write_to_device(SpiceCharDeviceState *dev)
> +{
> +    SpiceCharDeviceInterface *sif;
> +    int total = 0;
> +    int n;
> +
> +    if (!dev->running) {
> +        return 0;
> +    }
> +
> +    spice_char_device_state_ref(dev);
> +    core->timer_cancel(dev->write_to_dev_timer);
> +
> +    sif = SPICE_CONTAINEROF(dev->sin->base.sif, SpiceCharDeviceInterface, base);
> +    while (1) {
> +        uint32_t write_len;
> +
> +        if (!dev->cur_write_buf) {
> +            RingItem *item = ring_get_tail(&dev->write_queue);
> +            if (!item) {
> +                break;
> +            }
> +            dev->cur_write_buf = SPICE_CONTAINEROF(item, SpiceCharDeviceWriteBuffer, link);
> +            dev->cur_write_buf_pos = dev->cur_write_buf->buf;
> +            ring_remove(item);
> +        }
> +
> +        write_len = dev->cur_write_buf->buf + dev->cur_write_buf->buf_used -
> +                    dev->cur_write_buf_pos;
> +        n = sif->write(dev->sin, dev->cur_write_buf_pos, write_len);
> +        if (n <= 0) {
> +            break;
> +        }
> +        total += n;
> +        write_len -= n;
> +        if (!write_len) {
> +            SpiceCharDeviceWriteBuffer *release_buf = dev->cur_write_buf;
> +            dev->cur_write_buf = NULL;
> +            spice_char_device_write_buffer_release(dev, release_buf);
> +            continue;
> +        }
> +        dev->cur_write_buf_pos += n;
> +    }
> +    /* retry writing as long as the write queue is not empty */
> +    if (dev->cur_write_buf) {
> +        core->timer_start(dev->write_to_dev_timer, CHAR_DEVICE_WRITE_TO_TIMEOUT);
> +    } else {
> +        spice_assert(ring_is_empty(&dev->write_queue));
> +    }
> +    spice_char_device_state_unref(dev);
> +    return total;
> +}
> +
> +static void spice_char_dev_write_retry(void *opaque)
> +{
> +    SpiceCharDeviceState *dev = opaque;
> +
> +    core->timer_cancel(dev->write_to_dev_timer);
> +    spice_char_device_write_to_device(dev);
> +}
> +
> +SpiceCharDeviceWriteBuffer *spice_char_device_write_buffer_get(SpiceCharDeviceState *dev,
> +                                                               RedClient *client, int size)
> +{
> +    RingItem *item;
> +    SpiceCharDeviceWriteBuffer *ret;
> +
> +    if (!client && !dev->num_self_tokens) {
> +        spice_printerr("internal buf is not available");
> +        return NULL;
> +    }
> +
> +    if ((item = ring_get_tail(&dev->write_bufs_pool))) {
> +        ret = SPICE_CONTAINEROF(item, SpiceCharDeviceWriteBuffer, link);
> +        ring_remove(item);
> +    } else {
> +        ret = spice_new0(SpiceCharDeviceWriteBuffer, 1);
> +    }
> +
> +    spice_assert(!ret->buf_used);
> +
> +    if (ret->buf_size < size) {
> +        ret->buf = spice_realloc(ret->buf, size);
> +        ret->buf_size = size;
> +    }
> +
> +    if (client) {
> +       SpiceCharDeviceClientState *dev_client = spice_char_device_client_find(dev, client);
> +       if (dev_client) {
> +            if (!dev_client->num_client_tokens) {
> +                spice_printerr("token violation: dev %p client %p", dev, client);
> +                spice_char_device_handle_client_overflow(dev_client);
> +                goto error;
> +            }
> +            ret->client_origin = client;
> +            dev_client->num_client_tokens--;
> +        } else {
> +            /* it is possible that the client was removed due to send tokens underflow, but
> +             * the caller still receive messages from the client */
> +            spice_printerr("client not found: dev %p client %p", dev, client);
> +            goto error;
> +        }
> +    } else {
> +        dev->num_self_tokens--;
> +    }
> +
> +    return ret;
> +error:
> +    ring_add(&dev->write_bufs_pool, &ret->link);
> +    return NULL;
> +}
> +
> +void spice_char_device_write_buffer_add(SpiceCharDeviceState *dev,
> +                                        SpiceCharDeviceWriteBuffer *write_buf)
> +{
> +    spice_assert(dev);
> +    /* caller shouldn't add buffers for client that was removed */
> +    if (write_buf->client_origin &&
> +        !spice_char_device_client_find(dev, write_buf->client_origin)) {
> +        spice_printerr("client not found: dev %p client %p", dev, write_buf->client_origin);
> +        spice_char_device_write_buffer_pool_add(dev, write_buf);
> +        return;
> +    }
> +
> +    ring_add(&dev->write_queue, &write_buf->link);
> +    spice_char_device_write_to_device(dev);
> +}
> +
> +void spice_char_device_write_buffer_release(SpiceCharDeviceState *dev,
> +                                            SpiceCharDeviceWriteBuffer *write_buf)
> +{
> +    RedClient *client = write_buf->client_origin;
> +
> +    spice_assert(!ring_item_is_linked(&write_buf->link));
> +    if (!dev) {
> +        spice_printerr("no device. write buffer is freed");
> +        free(write_buf->buf);
> +        free(write_buf);
> +        return;
> +    }
> +
> +    spice_assert(dev->cur_write_buf != write_buf);
> +
> +    spice_char_device_write_buffer_pool_add(dev, write_buf);
> +    if (client) {
> +        SpiceCharDeviceClientState *dev_client;
> +        dev_client = spice_char_device_client_find(dev, client);
> +        /* when a client is removed, we remove all the buffers that are associated with it */
> +        spice_assert(dev_client);
> +        spice_char_device_client_token_add(dev, dev_client);
> +    } else {
> +        dev->num_self_tokens++;
> +        if (dev->cbs.on_free_self_token) {
> +            dev->cbs.on_free_self_token(dev->opaque);
> +        }
> +    }
> +
> +}
> +
> +
> +/********************************
> + * char_device_state management *
> + ********************************/
> +
> +SpiceCharDeviceState *spice_char_device_state_create(SpiceCharDeviceInstance *sin,
> +                                                     uint32_t client_tokens_interval,
> +                                                     uint32_t self_tokens,
> +                                                     SpiceCharDeviceCallbacks *cbs,
> +                                                     void *opaque)
> +{
> +    SpiceCharDeviceState *char_dev;
> +
> +    spice_assert(sin);
> +    spice_assert(cbs->read_one_msg_from_device && cbs->ref_msg_to_client &&
> +                 cbs->unref_msg_to_client && cbs->send_msg_to_client &&
> +                 cbs->send_tokens_to_client && cbs->remove_client);
> +
> +    char_dev = spice_new0(SpiceCharDeviceState, 1);
> +    char_dev->sin = sin;
> +    char_dev->cbs = *cbs;
> +    char_dev->opaque = opaque;
> +    char_dev->client_tokens_interval = client_tokens_interval;
> +    char_dev->num_self_tokens = self_tokens;
> +
> +    ring_init(&char_dev->write_queue);
> +    ring_init(&char_dev->write_bufs_pool);
> +    ring_init(&char_dev->clients);
> +
> +    char_dev->write_to_dev_timer = core->timer_add(spice_char_dev_write_retry, char_dev);
> +    if (!char_dev->write_to_dev_timer) {
> +        spice_error("failed creating char dev write timer");
> +    }
> +    char_dev->refs = 1;
> +    sin->st = char_dev;
> +    spice_debug("sin %p dev_state %p", sin, char_dev);
> +    return char_dev;
> +}
> +
> +void spice_char_device_state_reset_dev_instance(SpiceCharDeviceState *state,
> +                                                SpiceCharDeviceInstance *sin)
> +{
> +    spice_debug("sin %p dev_state %p", sin, state);
> +    state->sin = sin;
> +    sin->st = state;
> +}
> +
> +void *spice_char_device_state_opaque_get(SpiceCharDeviceState *dev)
> +{
> +    return dev->opaque;
> +}
> +
> +static void spice_char_device_state_ref(SpiceCharDeviceState *char_dev)
> +{
> +    char_dev->refs++;
> +}
> +
> +static void spice_char_device_state_unref(SpiceCharDeviceState *char_dev)
> +{
> +    if (!--char_dev->refs) {

Here refs == 0, but then below..

> +        spice_char_device_state_destroy(char_dev);
> +    }
> +}
> +
> +void spice_char_device_state_destroy(SpiceCharDeviceState *char_dev)
> +{
> +    core->timer_remove(char_dev->write_to_dev_timer);
> +    write_buffers_queue_free(&char_dev->write_queue);
> +    write_buffers_queue_free(&char_dev->write_bufs_pool);
> +
> +    while (!ring_is_empty(&char_dev->clients)) {
> +        RingItem *item = ring_get_tail(&char_dev->clients);
> +        SpiceCharDeviceClientState *dev_client;
> +
> +        dev_client = SPICE_CONTAINEROF(item, SpiceCharDeviceClientState, link);
> +        spice_char_device_client_free(char_dev, dev_client);
> +    }
> +    char_dev->running = FALSE;
> +
> +    if (!--char_dev->refs) {

refs is uint32_t, so it will now be large positive, this will never be
called. Drop the if.

> +        free(char_dev);
> +    }
> +}
> +
> +void spice_char_device_client_add(SpiceCharDeviceState *dev,
> +                                  RedClient *client,
> +                                  int do_flow_control,
> +                                  uint32_t max_send_queue_size,
> +                                  uint32_t num_client_tokens,
> +                                  uint32_t num_send_tokens)
> +{
> +    SpiceCharDeviceClientState *dev_client;
> +
> +    spice_assert(dev);
> +    spice_assert(client);
> +
> +    spice_debug("dev_state %p client %p", dev, client);
> +    dev_client = spice_new0(SpiceCharDeviceClientState, 1);
> +    dev_client->dev = dev;
> +    dev_client->client = client;
> +    ring_init(&dev_client->send_queue);
> +    dev_client->send_queue_size = 0;
> +    dev_client->max_send_queue_size = max_send_queue_size;
> +    dev_client->do_flow_control = do_flow_control;
> +    if (do_flow_control) {
> +        dev_client->wait_for_tokens_timer = core->timer_add(device_client_wait_for_tokens_timeout,
> +                                                            dev_client);
> +        if (!dev_client->wait_for_tokens_timer) {
> +            spice_error("failed to create wait for tokens timer");
> +        }
> +        dev_client->num_client_tokens = num_client_tokens;
> +        dev_client->num_send_tokens = num_send_tokens;
> +    } else {
> +        dev_client->num_client_tokens = ~0;
> +        dev_client->num_send_tokens = ~0;
> +    }
> +    ring_add(&dev->clients, &dev_client->link);
> +    dev->num_clients++;
> +    /* Now that we have a client, forward any pending device data */
> +    spice_char_device_wakeup(dev);
> +}
> +
> +
> +uint32_t spice_char_device_client_num_tokens_get(SpiceCharDeviceState *dev,
> +                                                 RedClient *client)
> +{
> +    SpiceCharDeviceClientState *dev_client;
> +
> +    dev_client = spice_char_device_client_find(dev, client);
> +
> +    if (!dev_client) {
> +        spice_error("client wasn't found");
> +        return 0;
> +    }
> +
> +    return dev_client->num_client_tokens;
> +}
> +
> +void spice_char_device_client_remove(SpiceCharDeviceState *dev,
> +                                     RedClient *client)
> +{
> +    SpiceCharDeviceClientState *dev_client;
> +
> +    spice_debug("dev_state %p client %p", dev, client);
> +    dev_client = spice_char_device_client_find(dev, client);
> +
> +    if (!dev_client) {
> +        spice_error("client wasn't found");
> +        return;
> +    }
> +
> +    spice_char_device_client_free(dev, dev_client);
> +}
> +
> +int spice_char_device_client_exists(SpiceCharDeviceState *dev,
> +                                    RedClient *client)
> +{
> +    return (spice_char_device_client_find(dev, client) != NULL);
> +}
> +
> +void spice_char_device_start(SpiceCharDeviceState *dev)
> +{
> +    spice_debug("dev_state %p", dev);
> +    dev->running = TRUE;
> +    spice_char_device_state_ref(dev);
> +    while (spice_char_device_write_to_device(dev) ||
> +           spice_char_device_read_from_device(dev));
> +    spice_char_device_state_unref(dev);
> +}
> +
> +void spice_char_device_stop(SpiceCharDeviceState *dev)
> +{
> +    spice_debug("dev_state %p", dev);
> +    dev->running = FALSE;
> +    core->timer_cancel(dev->write_to_dev_timer);
> +}
> +
> +void spice_char_device_reset(SpiceCharDeviceState *dev)
> +{
> +    RingItem *client_item;
> +    spice_char_device_stop(dev);
> +
> +    spice_debug("dev_state %p", dev);
> +    while (!ring_is_empty(&dev->write_queue)) {
> +        RingItem *item = ring_get_tail(&dev->write_queue);
> +        SpiceCharDeviceWriteBuffer *buf;
> +
> +        ring_remove(item);
> +        buf = SPICE_CONTAINEROF(item, SpiceCharDeviceWriteBuffer, link);
> +        /* tracking the tokens */
> +        spice_char_device_write_buffer_release(dev, buf);
> +    }
> +    if (dev->cur_write_buf) {
> +        SpiceCharDeviceWriteBuffer *release_buf = dev->cur_write_buf;
> +
> +        dev->cur_write_buf = NULL;
> +        spice_char_device_write_buffer_release(dev, release_buf);
> +    }
> +
> +    RING_FOREACH(client_item, &dev->clients) {
> +        SpiceCharDeviceClientState *dev_client;
> +
> +        dev_client = SPICE_CONTAINEROF(client_item, SpiceCharDeviceClientState, link);
> +        spice_char_device_client_send_queue_free(dev, dev_client);
> +    }
> +    dev->sin = NULL;
> +}
> +
> +void spice_char_device_wakeup(SpiceCharDeviceState *dev)
> +{
> +    spice_char_device_read_from_device(dev);
> +}
> +
> diff --git a/server/char_device.h b/server/char_device.h
> index bdb32ae..e3cd52d 100644
> --- a/server/char_device.h
> +++ b/server/char_device.h
> @@ -2,11 +2,209 @@
>  #define __CHAR_DEVICE_H__
>  
>  #include "spice.h"
> +#include "red_channel.h"
> +
> +/*
> + * Shared code for char devices, mainly for flow control.
> + *
> + * How to use the api:
> + * ==================
> + * device attached: call spice_char_device_state_create
> + * device detached: call spice_char_device_state_destroy/reset
> + *
> + * client connected and assoicated with a device: spice_char_device_client_add
> + * client disconnected: spice_char_device_client_remove
> + *
> + * Writing to the device
> + * ---------------------
> + * Write the data into SpiceCharDeviceWriteBuffer:
> + * call spice_char_device_write_buffer_get in order to get an appropriate buffer.
> + * call spice_char_device_write_buffer_add in order to push the buffer to the write queue.
> + * If you choose not to push the buffer to the device, call
> + * spice_char_device_write_buffer_release
> + *
> + * reading from the device
> + * -----------------------
> + *  The callback read_one_msg_from_device (see below) should be implemented
> + *  (using sif->read).
> + *  When the device is ready, this callback is called, and is expected to
> + *  return one message which is addressed to the client, or NULL if the read
> + *  hasn't completed.
> + *
> + * calls triggered from the device (qemu):
> + * --------------------------------------
> + * spice_char_device_start
> + * spice_char_device_stop
> + * spice_char_device_wakeup (for reading from the device)
> + */
> +
> +/*
> + * Note about multiple-clients:
> + * Multiclients are currently not supported in any of the character devices:
> + * spicevmc does not allow more than one client (and at least for usb, it should stay this way).
> + * smartcard code is not compatible with more than one reader.
> + * The server and guest agent code doesn't distinguish messages from different clients.
> + * In addition, its current flow control code (e.g., tokens handling) is wrong and doesn't
> + * take into account the different clients.
> + *
> + * Nonetheless, the following code introduces some support for multiple-clients:
> + * We track the number of tokens for all the clients, and we read from the device
> + * if one of the clients have enough tokens. For the clients that don't have tokens,
> + * we queue the messages, till they receive tokens, or till a timeout.
> + *
> + * TODO:
> + * At least for the agent, not all the messages from the device will be directed to all
> + * the clients (e.g., copy from guest to a specific client). Thus, support for
> + * client-specific-messages should be added.
> + * In addition, we should have support for clients that are being connected
> + * in the middle of a message transfer from the agent to the clients.
> + *
> + * */
> +
> +/* buffer that is used for writing to the device */
> +typedef struct SpiceCharDeviceWriteBuffer {
> +    RingItem link;
> +    RedClient *client_origin; /* The client that sent the message to the device.
> +                                 NULL if the server created the message */
> +
> +    uint8_t *buf;
> +    uint32_t buf_size;
> +    uint32_t buf_used;
> +} SpiceCharDeviceWriteBuffer;
> +
> +typedef void SpiceCharDeviceMsgToClient;
> +
> +typedef struct SpiceCharDeviceCallbacks {
> +    /*
> +     * Messages that are addressed to the client can be queued in case we have
> +     * multiple clients and some of them don't have enough tokens.
> +     */
> +
> +    /* reads from the device till reaching a msg that should be sent to the client,
> +     * or till the reading fails */
> +    SpiceCharDeviceMsgToClient* (*read_one_msg_from_device)(SpiceCharDeviceInstance *sin,
> +                                                            void *opaque);
> +    SpiceCharDeviceMsgToClient* (*ref_msg_to_client)(SpiceCharDeviceMsgToClient *msg,
> +                                                     void *opaque);
> +    void (*unref_msg_to_client)(SpiceCharDeviceMsgToClient *msg,
> +                                void *opaque);
> +    void (*send_msg_to_client)(SpiceCharDeviceMsgToClient *msg,
> +                               RedClient *client,
> +                               void *opaque); /* after this call, the message is unreferenced */
> +
> +    /* The cb is called when a predefined number of write buffers were consumed by the
> +     * device */
> +    void (*send_tokens_to_client)(RedClient *client, uint32_t tokens, void *opaque);
> +
> +    /* The cb is called when a server (self) message that was addressed to the device,
> +     * has been completely written to it */
> +    void (*on_free_self_token)(void *opaque);
> +
> +    /* This cb is called if it is recommanded that a client will be removed
> +     * due to slow flow or due to some other error.
> +     * The called instance should disconnect the client, or at least the corresponding channel */
> +    void (*remove_client)(RedClient *client, void *opaque);
> +} SpiceCharDeviceCallbacks;
> +
> +typedef struct SpiceCharDeviceState SpiceCharDeviceState;
>  
>  struct SpiceCharDeviceState {
> +    int running;
> +    uint32_t refs;
> +
> +    Ring write_queue;
> +    Ring write_bufs_pool;
> +    SpiceCharDeviceWriteBuffer *cur_write_buf;
> +    uint8_t *cur_write_buf_pos;
> +    SpiceTimer *write_to_dev_timer;
> +    uint64_t num_self_tokens;
> +
> +    Ring clients;
> +    uint32_t num_clients;
> +
> +    uint64_t client_tokens_interval; /* frequency of returning tokens to the client */
> +    SpiceCharDeviceInstance *sin;
> +
> +    SpiceCharDeviceCallbacks cbs;
> +    void *opaque;
> +    /* tmp till all spice char devices will employ the new SpiceCharDeviceState
> +     * implementation. Then, SpiceCharDeviceState will be moved to char_device.c and
> +     * this callback will be removed */
>      void (*wakeup)(SpiceCharDeviceInstance *sin);
>  };
>  
> +
> +SpiceCharDeviceState *spice_char_device_state_create(SpiceCharDeviceInstance *sin,
> +                                                     uint32_t client_tokens_interval,
> +                                                     uint32_t self_tokens,
> +                                                     SpiceCharDeviceCallbacks *cbs,
> +                                                     void *opaque);
> +
> +void spice_char_device_state_reset_dev_instance(SpiceCharDeviceState *dev,
> +                                                SpiceCharDeviceInstance *sin);
> +void spice_char_device_state_destroy(SpiceCharDeviceState *dev);
> +
> +void *spice_char_device_state_opaque_get(SpiceCharDeviceState *dev);
> +
> +
> +/*
> + * Resets write/read queues, and moves that state to being stopped.
> + * This routine is a workaround for a bad tokens management in the vdagent
> + * protocol:
> + *  The client tokens' are set only once, when the main channel is initialized.
> + *  Instead, it would have been more appropriate to reset them upon AGEN_CONNECT.
> + *  The client tokens are tracked as part of the SpiceCharDeviceClientState. Thus,
> + *  in order to be backwartd compatible with the client, we need to track the tokens
> + *  event when the agent is detached. We don't destroy the the char_device state, and
> + *  instead we just reset it.
> + *  In addition, there is a misshandling of AGENT_TOKENS message in spice-gtk: it
> + *  overrides the amount of tokens, instead of adding the given amount.
> + *
> + *  todo: change AGENT_CONNECT msg to contain tokens count.
> + */
> +void spice_char_device_reset(SpiceCharDeviceState *dev);
> +
> +/* max_send_queue_size = how many messages we can read from the device and enqueue for this client,
> + * when we have tokens for other clients and no tokens for this one */
> +void spice_char_device_client_add(SpiceCharDeviceState *dev,
> +                                  RedClient *client,
> +                                  int do_flow_control,
> +                                  uint32_t max_send_queue_size,
> +                                  uint32_t num_client_tokens,
> +                                  uint32_t num_send_tokens);
> +
> +void spice_char_device_client_remove(SpiceCharDeviceState *dev,
> +                                     RedClient *client);
> +int spice_char_device_client_exists(SpiceCharDeviceState *dev,
> +                                    RedClient *client);
> +
> +void spice_char_device_start(SpiceCharDeviceState *dev);
> +void spice_char_device_stop(SpiceCharDeviceState *dev);
> +
> +/** Read from device **/
> +
> +void spice_char_device_wakeup(SpiceCharDeviceState *dev);
> +
> +void spice_char_device_send_to_client_tokens_add(SpiceCharDeviceState *dev,
> +                                                 RedClient *client,
> +                                                 uint32_t tokens);
> +
> +
> +void spice_char_device_send_to_client_tokens_set(SpiceCharDeviceState *dev,
> +                                                 RedClient *client,
> +                                                 uint32_t tokens);
> +/** Write to device **/
> +
> +SpiceCharDeviceWriteBuffer *spice_char_device_write_buffer_get(SpiceCharDeviceState *dev,
> +                                                               RedClient *client, int size);
> +/* Either add the buffer to the write queue or release it */
> +void spice_char_device_write_buffer_add(SpiceCharDeviceState *dev,
> +                                        SpiceCharDeviceWriteBuffer *write_buf);
> +void spice_char_device_write_buffer_release(SpiceCharDeviceState *dev,
> +                                            SpiceCharDeviceWriteBuffer *write_buf);
> +
> +/* api for specific char devices */
> +
>  void spicevmc_device_connect(SpiceCharDeviceInstance *sin,
>                               uint8_t channel_type);
>  void spicevmc_device_disconnect(SpiceCharDeviceInstance *char_device);
> -- 
> 1.7.7.6
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel


More information about the Spice-devel mailing list