[Spice-commits] 5 commits - client/display_channel.cpp server/red_worker.c server/spice.h

[Hans de Goede]

Hi,

On 09/07/2012 06:09 PM, Søren Sandmann Pedersen wrote:
>   client/display_channel.cpp |    1 +
>   server/red_worker.c        |   31 +++++++++++++++++++++++++++++++
>   server/spice.h             |    5 ++++-
>   3 files changed, 36 insertions(+), 1 deletion(-)
>
> New commits:
> commit 88283023a89b4ee212ac08ede797d1ce1c3a0906
> Author: Søren Sandmann Pedersen <ssp at redhat.com>
> Date:   Sat Sep 1 11:42:56 2012 -0400
>
>      Bump spice.h version number to 0.11.4
>
>      No new symbols are added, but there is an addition to QXLInterface:
>
>          void (*set_client_capabilities)(QXLInstance *qin,
>                                          uint8_t client_present,
>                                          uint8_t caps[58]);
>

I must admit I've never studied how we do qemu <-> spice ABI
compatibility too closely. But I believe only bumping the server
version is not enough.

This bump will allow qemu to detect compile time it is dealing with
a newer server, but what if we've an old qemu running with the new
server with the above addition to QXLInterface ?

Then the new code will do:

<snip>

> diff --git a/server/red_worker.c b/server/red_worker.c
> index eee9915..1e301c4 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -10344,6 +10344,23 @@ static void handle_new_display_channel(RedWorker *worker, RedClient *client, Red
>       spice_info("jpeg %s", display_channel->enable_jpeg ? "enabled" : "disabled");
>       spice_info("zlib-over-glz %s", display_channel->enable_zlib_glz_wrap ? "enabled" : "disabled");
>
> +    if (worker->qxl->st->qif->set_client_capabilities) {
> +        RedChannelClient *rcc = (RedChannelClient *)dcc;
> +        uint8_t caps[58] = { 0 };
> +
> +#define SET_CAP(a,c)                                                    \
> +        ((a)[(c) / 8] |= (1 << ((c) % 8)))
> +
> +        if (red_channel_client_test_remote_cap(rcc, SPICE_DISPLAY_CAP_SIZED_STREAM))
> +            SET_CAP(caps, SPICE_DISPLAY_CAP_SIZED_STREAM);
> +        if (red_channel_client_test_remote_cap(rcc, SPICE_DISPLAY_CAP_MONITORS_CONFIG))
> +            SET_CAP(caps, SPICE_DISPLAY_CAP_MONITORS_CONFIG);
> +        if (red_channel_client_test_remote_cap(rcc, SPICE_DISPLAY_CAP_COMPOSITE))
> +            SET_CAP(caps, SPICE_DISPLAY_CAP_COMPOSITE);
> +
> +        worker->qxl->st->qif->set_client_capabilities(worker->qxl, TRUE, caps);
> +    }
> +

But this checks for qif->set_client_capabilities, which was not there in the old
QXLInterface struct definition which the old qemu used while compiling, so the check will
point to memory *beyond* the end of the QXLInterface struct (as seen / defined by the older
qemu binary)...

Regards,

Hans