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

[Alon Levy]

> 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 ?

The solution is the SPICE_INTERFACE_QXL_MAJOR & SPICE_INTERFACE_QXL_MINOR
versions which are stored in the compiled qemu binary in hw/qxl.c:qxl_interface

Any change to the QXLInterface must be accompanied by a change to those two constants, in the usual manner - major remains if change is backward compatible. The full version number can be used before accessing fields that have only appeared in a recent version.

> 
> 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
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>