[Spice-devel] [PATCH spice 2/2] server: add "port" channel support
Hans de Goede
hdegoede at redhat.com
Sun Dec 2 03:26:06 PST 2012
Hi,
On 11/30/2012 01:51 PM, Marc-André Lureau wrote:
> A Spice port channel carry arbitrary data between the Spice client and
> the Spice server. It may be used to provide additional services on top
> of a Spice connection. For example, a channel can be associated with
> the qemu monitor for the client to interact with it, just like any
> qemu chardev. Or it may be used with various protocols, such as the
> Spice Controller.
>
> A port kind is identified simply by its fqdn, such as org.qemu.monitor,
> org.spice.spicy.test or org.ovirt.controller...
>
> The channel is based on Spicevmc which simply tunnels data between
> client and server, with a few additional messages.
>
> See the description of the channel protocol in spice-common history.
I think that you also need to change spicevmc_red_channel_alloc_msg_rcv_buf,
because now it will allocate a spice_char_device_write_buffer to hold the
event message, which seems the wrong thing to do, and since when handling
the event message you don't set state->recv_from_client_buf = NULL, the
next message received on the channel will trigger this assert:
assert(!state->recv_from_client_buf);
In spicevmc_red_channel_alloc_msg_rcv_buf, I think this does not happen
in your testing since you only close the port once, and then send no
more data.
Regards,
Hans
> ---
> server/reds.c | 11 ++++-
> server/spice-server.syms | 5 ++
> server/spice.h | 5 +-
> server/spicevmc.c | 117 +++++++++++++++++++++++++++++++++++++++++++++--
> 4 files changed, 131 insertions(+), 7 deletions(-)
>
> diff --git a/server/reds.c b/server/reds.c
> index 1cb46f4..084620c 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -3525,6 +3525,7 @@ SPICE_GNUC_VISIBLE void spice_server_char_device_wakeup(SpiceCharDeviceInstance*
> #define SUBTYPE_VDAGENT "vdagent"
> #define SUBTYPE_SMARTCARD "smartcard"
> #define SUBTYPE_USBREDIR "usbredir"
> +#define SUBTYPE_PORT "port"
>
> const char *spice_server_char_device_recognized_subtypes_list[] = {
> SUBTYPE_VDAGENT,
> @@ -3598,6 +3599,10 @@ static int spice_server_char_device_add_interface(SpiceServer *s,
> else if (strcmp(char_device->subtype, SUBTYPE_USBREDIR) == 0) {
> dev_state = spicevmc_device_connect(char_device, SPICE_CHANNEL_USBREDIR);
> }
> + else if (strcmp(char_device->subtype, SUBTYPE_PORT) == 0) {
> + dev_state = spicevmc_device_connect(char_device, SPICE_CHANNEL_PORT);
> + }
> +
> if (dev_state) {
> spice_assert(char_device->st);
> /* setting the char_device state to "started" for backward compatibily with
> @@ -3629,9 +3634,13 @@ static void spice_server_char_device_remove_interface(SpiceBaseInstance *sin)
> smartcard_device_disconnect(char_device);
> }
> #endif
> - else if (strcmp(char_device->subtype, SUBTYPE_USBREDIR) == 0) {
> + else if (strcmp(char_device->subtype, SUBTYPE_USBREDIR) == 0 ||
> + strcmp(char_device->subtype, SUBTYPE_PORT) == 0) {
> spicevmc_device_disconnect(char_device);
> + } else {
> + spice_warning("failed to remove char device %s", char_device->subtype);
> }
> +
> char_device->st = NULL;
> }
>
> diff --git a/server/spice-server.syms b/server/spice-server.syms
> index eadfed8..2091fe0 100644
> --- a/server/spice-server.syms
> +++ b/server/spice-server.syms
> @@ -130,3 +130,8 @@ SPICE_SERVER_0.11.4 {
> global:
> spice_server_set_exit_on_disconnect;
> } SPICE_SERVER_0.11.2;
> +
> +SPICE_SERVER_0.12.2 {
> +global:
> + spice_server_port_event;
> +} SPICE_SERVER_0.11.4;
> diff --git a/server/spice.h b/server/spice.h
> index 22f17d6..4b86f4b 100644
> --- a/server/spice.h
> +++ b/server/spice.h
> @@ -388,7 +388,7 @@ void spice_server_record_set_mute(SpiceRecordInstance *sin, uint8_t mute);
>
> #define SPICE_INTERFACE_CHAR_DEVICE "char_device"
> #define SPICE_INTERFACE_CHAR_DEVICE_MAJOR 1
> -#define SPICE_INTERFACE_CHAR_DEVICE_MINOR 1
> +#define SPICE_INTERFACE_CHAR_DEVICE_MINOR 2
> typedef struct SpiceCharDeviceInterface SpiceCharDeviceInterface;
> typedef struct SpiceCharDeviceInstance SpiceCharDeviceInstance;
> typedef struct SpiceCharDeviceState SpiceCharDeviceState;
> @@ -399,15 +399,18 @@ struct SpiceCharDeviceInterface {
> void (*state)(SpiceCharDeviceInstance *sin, int connected);
> int (*write)(SpiceCharDeviceInstance *sin, const uint8_t *buf, int len);
> int (*read)(SpiceCharDeviceInstance *sin, uint8_t *buf, int len);
> + void (*event)(SpiceCharDeviceInstance *sin, uint8_t event);
> };
>
> struct SpiceCharDeviceInstance {
> SpiceBaseInstance base;
> const char* subtype;
> SpiceCharDeviceState *st;
> + const char* portname;
> };
>
> void spice_server_char_device_wakeup(SpiceCharDeviceInstance *sin);
> +void spice_server_port_event(SpiceCharDeviceInstance *char_device, uint8_t event);
> const char** spice_server_char_device_recognized_subtypes(void);
>
> /* spice server setup */
> diff --git a/server/spicevmc.c b/server/spicevmc.c
> index 058a182..766db73 100644
> --- a/server/spicevmc.c
> +++ b/server/spicevmc.c
> @@ -28,6 +28,8 @@
> #include <netinet/in.h> // IPPROTO_TCP
> #include <netinet/tcp.h> // TCP_NODELAY
>
> +#include "common/generated_server_marshallers.h"
> +
> #include "char_device.h"
> #include "red_channel.h"
> #include "reds.h"
> @@ -56,11 +58,25 @@ typedef struct SpiceVmcState {
> SpiceCharDeviceInstance *chardev_sin;
> SpiceVmcPipeItem *pipe_item;
> SpiceCharDeviceWriteBuffer *recv_from_client_buf;
> + uint8_t port_opened;
> } SpiceVmcState;
>
> +typedef struct PortInitPipeItem {
> + PipeItem base;
> + char* name;
> + uint8_t opened;
> +} PortInitPipeItem;
> +
> +typedef struct PortEventPipeItem {
> + PipeItem base;
> + uint8_t event;
> +} PortEventPipeItem;
> +
> enum {
> PIPE_ITEM_TYPE_SPICEVMC_DATA = PIPE_ITEM_TYPE_CHANNEL_BASE,
> PIPE_ITEM_TYPE_SPICEVMC_MIGRATE_DATA,
> + PIPE_ITEM_TYPE_PORT_INIT,
> + PIPE_ITEM_TYPE_PORT_EVENT,
> };
>
> static SpiceVmcPipeItem *spicevmc_pipe_item_ref(SpiceVmcPipeItem *item)
> @@ -137,6 +153,27 @@ static void spicevmc_chardev_send_msg_to_client(SpiceCharDeviceMsgToClient *msg,
> red_channel_client_pipe_add_push(state->rcc, &vmc_msg->base);
> }
>
> +static void spicevmc_port_send_init(RedChannelClient *rcc)
> +{
> + SpiceVmcState *state = SPICE_CONTAINEROF(rcc->channel, SpiceVmcState, channel);
> + SpiceCharDeviceInstance *sin = state->chardev_sin;
> + PortInitPipeItem *item = spice_malloc(sizeof(PortInitPipeItem));
> +
> + red_channel_pipe_item_init(rcc->channel, &item->base, PIPE_ITEM_TYPE_PORT_INIT);
> + item->name = strdup(sin->portname);
> + item->opened = state->port_opened;
> + red_channel_client_pipe_add_push(rcc, &item->base);
> +}
> +
> +static void spicevmc_port_send_event(RedChannelClient *rcc, uint8_t event)
> +{
> + PortEventPipeItem *item = spice_malloc(sizeof(PortEventPipeItem));
> +
> + red_channel_pipe_item_init(rcc->channel, &item->base, PIPE_ITEM_TYPE_PORT_EVENT);
> + item->event = event;
> + red_channel_client_pipe_add_push(rcc, &item->base);
> +}
> +
> static void spicevmc_char_dev_send_tokens_to_client(RedClient *client,
> uint32_t tokens,
> void *opaque)
> @@ -245,16 +282,32 @@ static int spicevmc_red_channel_client_handle_message(RedChannelClient *rcc,
> uint8_t *msg)
> {
> SpiceVmcState *state;
> + SpiceCharDeviceInstance *sin;
> + SpiceCharDeviceInterface *sif;
>
> state = spicevmc_red_channel_client_get_state(rcc);
> - if (type != SPICE_MSGC_SPICEVMC_DATA) {
> + sin = state->chardev_sin;
> + sif = SPICE_CONTAINEROF(sin->base.sif, SpiceCharDeviceInterface, base);
> +
> + switch (type) {
> + case SPICE_MSGC_SPICEVMC_DATA:
> + spice_assert(state->recv_from_client_buf->buf == msg);
> + state->recv_from_client_buf->buf_used = size;
> + spice_char_device_write_buffer_add(state->chardev_st, state->recv_from_client_buf);
> + state->recv_from_client_buf = NULL;
> + break;
> + case SPICE_MSGC_PORT_EVENT:
> + if (size != sizeof(uint8_t)) {
> + spice_warning("bad port event message size");
> + return FALSE;
> + }
> + if (sif->base.minor_version >= 2 && sif->event != NULL)
> + sif->event(sin, *msg);
> + break;
> + default:
> return red_channel_client_handle_message(rcc, size, type, msg);
> }
>
> - spice_assert(state->recv_from_client_buf->buf == msg);
> - state->recv_from_client_buf->buf_used = size;
> - spice_char_device_write_buffer_add(state->chardev_st, state->recv_from_client_buf);
> - state->recv_from_client_buf = NULL;
> return TRUE;
> }
>
> @@ -323,6 +376,32 @@ static void spicevmc_red_channel_send_migrate_data(RedChannelClient *rcc,
> spice_char_device_state_migrate_data_marshall(state->chardev_st, m);
> }
>
> +static void spicevmc_red_channel_send_port_init(RedChannelClient *rcc,
> + SpiceMarshaller *m,
> + PipeItem *item)
> +{
> + PortInitPipeItem *i = SPICE_CONTAINEROF(item, PortInitPipeItem, base);
> + SpiceMsgPortInit init;
> +
> + red_channel_client_init_send_data(rcc, SPICE_MSG_PORT_INIT, item);
> + init.name = (uint8_t *)i->name;
> + init.name_size = strlen(i->name) + 1;
> + init.opened = i->opened;
> + spice_marshall_msg_port_init(m, &init);
> +}
> +
> +static void spicevmc_red_channel_send_port_event(RedChannelClient *rcc,
> + SpiceMarshaller *m,
> + PipeItem *item)
> +{
> + PortEventPipeItem *i = SPICE_CONTAINEROF(item, PortEventPipeItem, base);
> + SpiceMsgPortEvent event;
> +
> + red_channel_client_init_send_data(rcc, SPICE_MSG_PORT_EVENT, item);
> + event.event = i->event;
> + spice_marshall_msg_port_event(m, &event);
> +}
> +
> static void spicevmc_red_channel_send_item(RedChannelClient *rcc,
> PipeItem *item)
> {
> @@ -335,6 +414,12 @@ static void spicevmc_red_channel_send_item(RedChannelClient *rcc,
> case PIPE_ITEM_TYPE_SPICEVMC_MIGRATE_DATA:
> spicevmc_red_channel_send_migrate_data(rcc, m, item);
> break;
> + case PIPE_ITEM_TYPE_PORT_INIT:
> + spicevmc_red_channel_send_port_init(rcc, m, item);
> + break;
> + case PIPE_ITEM_TYPE_PORT_EVENT:
> + spicevmc_red_channel_send_port_event(rcc, m, item);
> + break;
> default:
> spice_error("bad pipe item %d", item->type);
> free(item);
> @@ -384,6 +469,10 @@ static void spicevmc_connect(RedChannel *channel, RedClient *client,
> state->rcc = rcc;
> red_channel_client_ack_zero_messages_window(rcc);
>
> + if (strcmp(sin->subtype, "port") == 0) {
> + spicevmc_port_send_init(rcc);
> + }
> +
> if (!spice_char_device_client_add(state->chardev_st, client, FALSE, 0, ~0, ~0,
> red_channel_client_waits_for_migrate_data(rcc))) {
> spice_warning("failed to add client to spicevmc");
> @@ -461,3 +550,21 @@ void spicevmc_device_disconnect(SpiceCharDeviceInstance *sin)
> free(state->pipe_item);
> red_channel_destroy(&state->channel);
> }
> +
> +SPICE_GNUC_VISIBLE void spice_server_port_event(SpiceCharDeviceInstance *sin, uint8_t event)
> +{
> + SpiceVmcState *state;
> +
> + state = (SpiceVmcState *)spice_char_device_state_opaque_get(sin->st);
> + if (event == SPICE_PORT_EVENT_OPENED) {
> + state->port_opened = TRUE;
> + } else if (event == SPICE_PORT_EVENT_CLOSED) {
> + state->port_opened = FALSE;
> + }
> +
> + if (state->rcc == NULL) {
> + return;
> + }
> +
> + spicevmc_port_send_event(state->rcc, event);
> +}
>
More information about the Spice-devel
mailing list