[Spice-devel] [RFC 1/2] spice, server: introduce SpiceCharDevice

[Alexander Larsson]

On Wed, 2010-08-25 at 06:53 -0400, Alon Levy wrote:
> index aede4ce..6c02afb 100644
> --- a/server/spice-experimental.h
> +++ b/server/spice-experimental.h
> @@ -1,26 +1,31 @@
> -/* vdi port interface */
> +/* char device interfaces */
>  
> -#define SPICE_INTERFACE_VDI_PORT "vdi_port"
> -#define SPICE_INTERFACE_VDI_PORT_MAJOR 1
> -#define SPICE_INTERFACE_VDI_PORT_MINOR 1
> -typedef struct SpiceVDIPortInterface SpiceVDIPortInterface;
> -typedef struct SpiceVDIPortInstance SpiceVDIPortInstance;
> -typedef struct SpiceVDIPortState SpiceVDIPortState;
> +#define SPICE_INTERFACE_CHAR_DEVICE "char_device"
> +#define SPICE_INTERFACE_CHAR_DEVICE_MAJOR 1
> +#define SPICE_INTERFACE_CHAR_DEVICE_MINOR 1
> +typedef struct SpiceCharDeviceInterface SpiceCharDeviceInterface;
> +typedef struct SpiceCharDeviceInstance SpiceCharDeviceInstance;
> +typedef struct SpiceCharDeviceState SpiceCharDeviceState;
>  
> -struct SpiceVDIPortInterface {
> +struct SpiceCharDeviceInterface {
>      SpiceBaseInterface base;
>  
> -    void (*state)(SpiceVDIPortInstance *sin, int connected);
> -    int (*write)(SpiceVDIPortInstance *sin, const uint8_t *buf, int len);
> -    int (*read)(SpiceVDIPortInstance *sin, uint8_t *buf, int len);
> +    /* called by spice */
> +    const char* (*subtype)(SpiceCharDeviceInstance *sin);

I don't think we need a callback here. Just make this a const char *.

> +    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);
>  };
>  
> -struct SpiceVDIPortInstance {
> -    SpiceBaseInstance base;
> -    SpiceVDIPortState *st;
> +struct SpiceCharDeviceInstance {
> +    SpiceBaseInstance       base;
> +    SpiceCharDeviceState    *st;
> +    /* called by spice embedder */
> +    void (*wakeup)(SpiceCharDeviceInstance *sin);
>  };
>
> -void spice_server_vdi_port_wakeup(SpiceVDIPortInstance *sin);

We've generally avoided having ways to call into spice like this.
Instead have a spice_server_char_device_wakeup() call which internally
could e.g. call via a function pointer in SpiceCharDeviceState.

> +int spice_server_char_device_recognized_subtype(const char* subtype);
> +const char* spice_server_char_device_supported_subtypes(void);

I think its better to have a single call that returns a strv, i.e. a
NULL terminated char *[]. This can be used to handle both of these
functions and is a nicer api to support.

-- 
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
 Alexander Larsson                                            Red Hat, Inc 
       alexl at redhat.com            alexander.larsson at gmail.com 
He's a shy Republican assassin with a robot buddy named Sparky. She's an 
artistic thirtysomething Hell's Angel descended from a line of powerful 
witches. They fight crime!