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

[Gerd Hoffmann]

   Hi,

> -__visible__ void spice_server_vdi_port_wakeup(SpiceVDIPortInstance *sin)
> +__visible__ void spice_server_vdi_port_wakeup(SpiceCharDeviceInstance *sin)

This should be renamed too.  Also please make a separate patch which 
does the pure vdiport -> chardevice renaming.

> +__visible__ int spice_server_char_device_recognized_subtype(const char* subtype)
> +{
> +    return (strcmp(subtype, "vdagent") == 0 || strcmp(subtype, "smartcard") == 0);
> +}
> +
> +__visible__ const char* spice_server_char_device_supported_subtypes(void)
> +{
> +    return "vdagent, smartcard";
> +}

Hmm?  Shouldn't be one function which returns a list be enougth?

> +int spice_server_char_device_add_interface(SpiceServer *s,
> +                                           SpiceBaseInstance *sin)
> +{
> +    SpiceCharDeviceInstance* char_device =
> +            SPICE_CONTAINEROF(sin, SpiceCharDeviceInstance, base);
> +    SpiceCharDeviceInterface* sif;
> +
> +    sif = SPICE_CONTAINEROF(char_device->base.sif, SpiceCharDeviceInterface, base);
> +    const char* subtype = sif->subtype(char_device);
> +    if (strcmp(subtype, "vdagent") == 0) {
> +        if (vdagent) {
> +            red_printf("vdi port already attached");
> +            return -1;
> +        }
> +        char_device->wakeup =&spice_server_vdi_port_wakeup;
> +        attach_to_red_agent(char_device);
> +    } else if (strcmp(subtype, "smartcard") == 0) {
> +        red_printf("smart card not implemented");
> +        return -1;

What is planned here?  How will the smartcard data travel from server to 
client (and back)?

> -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);

No.

> +    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);

When touching this anyway we should take care to fixup the flow control 
issues this interface has too.  When this is done we should be able to 
move it from spice-experimental.h to spice.h

>   };
>
> -struct SpiceVDIPortInstance {
> -    SpiceBaseInstance base;

        const char         *subtype;

> -    SpiceVDIPortState *st;
> +struct SpiceCharDeviceInstance {
> +    SpiceBaseInstance       base;
> +    SpiceCharDeviceState    *st;
> +    /* called by spice embedder */
> +    void (*wakeup)(SpiceCharDeviceInstance *sin);
>   };

cheers,
   Gerd