[Spice-devel] Purpose of interface version checks in server/reds.c

[Christophe Fergeau]

Hi,

On Mon, Jul 04, 2011 at 01:27:16PM +0200, Gerd Hoffmann wrote:
> On 07/01/11 15:58, Christophe Fergeau wrote:
> >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) {
> >         red_printf("unsupported record interface");
> >         return -1;
> >     }
> >     snd_attach_record(SPICE_CONTAINEROF(sin, SpiceRecordInstance, base));
> >}
> 
> That looks bogous.

Given your explanation below, I agree with you. I'm wondering if what was
originally intended was the opposite check, ie fail to create the channel
if QEMU supports a *newer* version of the interface than the installed
spice-server. Otherwise I'll make a patch to 
- fail the channel creation if interface->major_version !=
  SPICE_INTERFACE_RECORD_MAJOR
- only log a message if interface->minor_version<
  SPICE_INTERFACE_RECORD_MINOR) (but create the channel anyway)

> >I'm a bit surprised by this mechanism, if we break these interfaces in such
> >a way that qemu needs to be recompiled, shouldn't we use different library
> >sonames to make sure things are recompiled?
> 
> No.  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.

Ah ok, if things are done this way, this sounds good and useful to me.

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

I see, thanks. In this case, the change adds new messages and new methods
for the Playback and Record channels, so nothing should change unless these
new methods are explicitly called from qemu, which means raising the minor
version number was probably not strictly needed (but not harmful either).

Thanks for the clarifications,

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20110704/b46382e1/attachment.pgp>