[Spice-devel] [PATCH] Fix spice-server/qemu channel version checks

[Alon Levy]

On Mon, Jul 18, 2011 at 04:16:59PM +0200, Christophe Fergeau wrote:
> Ping?
> 
ACK.

> On Fri, Jul 08, 2011 at 03:58:28PM +0200, Christophe Fergeau wrote:
> > When qemu creates a channel, reds.c contains code to check the
> > minor/major channel versions known to QEMU (ie the ones that were
> > current in spice-server when QEMU was compiled) and to compare these
> > versions against the current ones the currently installed spice-server
> > version.
> > 
> > According to kraxel [1], the rules for these interface numbers are:
> > 
> > "The purpose of the versions is exactly to avoid the need for a new
> > soname.  The rules are basically:
> > 
> >    (1) You add stuff to the interface, strictly append-only to not break
> >        binary compatibility.
> >    (2) You bump the minor version of the interface.
> >    (3) You check the minor version at runtime to figure whenever the
> >        added fields contain valid stuff or not.
> > 
> > An example is here (core interface, minor goes from 2 to 3, new
> > channel_event callback):
> > 
> > http://cgit.freedesktop.org/spice/spice/commit/?id=97f33fa86aa6edd25111b173dc0d9599ac29f879
> > "
> > 
> > The code currently refuses to create a channel if QEMU minor version is
> > less than the current spice-server version. This does not correspond
> > to the intended behaviour, this patch changes to fail is qemu was compiled
> > with a spice-server that is *newer* than the one currently installed. This
> > case is something we cannot support nicely.
> > 
> > [1] http://lists.freedesktop.org/archives/spice-devel/2011-July/004440.html
> > ---
> >  server/reds.c |   16 ++++++++--------
> >  1 files changed, 8 insertions(+), 8 deletions(-)
> > 
> > diff --git a/server/reds.c b/server/reds.c
> > index ca6cf4d..ee24e87 100644
> > --- a/server/reds.c
> > +++ b/server/reds.c
> > @@ -3270,7 +3270,7 @@ SPICE_GNUC_VISIBLE int spice_server_add_interface(SpiceServer *s,
> >      if (strcmp(interface->type, SPICE_INTERFACE_KEYBOARD) == 0) {
> >          red_printf("SPICE_INTERFACE_KEYBOARD");
> >          if (interface->major_version != SPICE_INTERFACE_KEYBOARD_MAJOR ||
> > -            interface->minor_version < SPICE_INTERFACE_KEYBOARD_MINOR) {
> > +            interface->minor_version > SPICE_INTERFACE_KEYBOARD_MINOR) {
> >              red_printf("unsupported keyboard interface");
> >              return -1;
> >          }
> > @@ -3280,7 +3280,7 @@ SPICE_GNUC_VISIBLE int spice_server_add_interface(SpiceServer *s,
> >      } else if (strcmp(interface->type, SPICE_INTERFACE_MOUSE) == 0) {
> >          red_printf("SPICE_INTERFACE_MOUSE");
> >          if (interface->major_version != SPICE_INTERFACE_MOUSE_MAJOR ||
> > -            interface->minor_version < SPICE_INTERFACE_MOUSE_MINOR) {
> > +            interface->minor_version > SPICE_INTERFACE_MOUSE_MINOR) {
> >              red_printf("unsupported mouse interface");
> >              return -1;
> >          }
> > @@ -3292,7 +3292,7 @@ SPICE_GNUC_VISIBLE int spice_server_add_interface(SpiceServer *s,
> >  
> >          red_printf("SPICE_INTERFACE_QXL");
> >          if (interface->major_version != SPICE_INTERFACE_QXL_MAJOR ||
> > -            interface->minor_version < SPICE_INTERFACE_QXL_MINOR) {
> > +            interface->minor_version > SPICE_INTERFACE_QXL_MINOR) {
> >              red_printf("unsupported qxl interface");
> >              return -1;
> >          }
> > @@ -3305,7 +3305,7 @@ SPICE_GNUC_VISIBLE int spice_server_add_interface(SpiceServer *s,
> >      } else if (strcmp(interface->type, SPICE_INTERFACE_TABLET) == 0) {
> >          red_printf("SPICE_INTERFACE_TABLET");
> >          if (interface->major_version != SPICE_INTERFACE_TABLET_MAJOR ||
> > -            interface->minor_version < SPICE_INTERFACE_TABLET_MINOR) {
> > +            interface->minor_version > SPICE_INTERFACE_TABLET_MINOR) {
> >              red_printf("unsupported tablet interface");
> >              return -1;
> >          }
> > @@ -3320,7 +3320,7 @@ SPICE_GNUC_VISIBLE int spice_server_add_interface(SpiceServer *s,
> >      } else if (strcmp(interface->type, SPICE_INTERFACE_PLAYBACK) == 0) {
> >          red_printf("SPICE_INTERFACE_PLAYBACK");
> >          if (interface->major_version != SPICE_INTERFACE_PLAYBACK_MAJOR ||
> > -            interface->minor_version < SPICE_INTERFACE_PLAYBACK_MINOR) {
> > +            interface->minor_version > SPICE_INTERFACE_PLAYBACK_MINOR) {
> >              red_printf("unsupported playback interface");
> >              return -1;
> >          }
> > @@ -3329,7 +3329,7 @@ SPICE_GNUC_VISIBLE int spice_server_add_interface(SpiceServer *s,
> >      } else if (strcmp(interface->type, SPICE_INTERFACE_RECORD) == 0) {
> >          red_printf("SPICE_INTERFACE_RECORD");
> >          if (interface->major_version != SPICE_INTERFACE_RECORD_MAJOR ||
> > -            interface->minor_version < SPICE_INTERFACE_RECORD_MINOR) {
> > +            interface->minor_version > SPICE_INTERFACE_RECORD_MINOR) {
> >              red_printf("unsupported record interface");
> >              return -1;
> >          }
> > @@ -3337,7 +3337,7 @@ SPICE_GNUC_VISIBLE int spice_server_add_interface(SpiceServer *s,
> >  
> >      } else if (strcmp(interface->type, SPICE_INTERFACE_CHAR_DEVICE) == 0) {
> >          if (interface->major_version != SPICE_INTERFACE_CHAR_DEVICE_MAJOR ||
> > -            interface->minor_version < SPICE_INTERFACE_CHAR_DEVICE_MINOR) {
> > +            interface->minor_version > SPICE_INTERFACE_CHAR_DEVICE_MINOR) {
> >              red_printf("unsupported char device interface");
> >              return -1;
> >          }
> > @@ -3352,7 +3352,7 @@ SPICE_GNUC_VISIBLE int spice_server_add_interface(SpiceServer *s,
> >              return -1;
> >          }
> >          if (interface->major_version != SPICE_INTERFACE_NET_WIRE_MAJOR ||
> > -            interface->minor_version < SPICE_INTERFACE_NET_WIRE_MINOR) {
> > +            interface->minor_version > SPICE_INTERFACE_NET_WIRE_MINOR) {
> >              red_printf("unsupported net wire interface");
> >              return -1;
> >          }
> > -- 
> > 1.7.6
> > 
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/spice-devel



> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel